purego: support structs on loong64#473
Open
hajimehoshi wants to merge 7 commits into
Open
Conversation
Struct arguments and return values can now be passed to and received from C functions via RegisterFunc and RegisterLibFunc on linux/loong64, linux/ppc64le, linux/riscv64, and linux/s390x. The per-architecture addStruct and getStruct implementations already existed but were unreachable behind a guard that only permitted amd64 and arm64. The guard is now direction-aware. ensureStructSupported gates the forward call path (RegisterFunc), while the new ensureCallbackStructSupported keeps the callback path (NewCallback) restricted to amd64 and arm64, where getCallbackStruct and setStruct are implemented. Struct callbacks therefore remain unsupported on the newly enabled architectures. The struct tests are un-gated for these architectures, with the callback-based variants skipped where callbacks are unsupported. 32-bit arm and 386 stay unsupported: arm's EABI struct handling is still a placeholder and the tests assume a 64-bit int. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands PureGo’s ability to pass and return C structs by value via RegisterFunc/RegisterLibFunc to additional 64-bit Linux architectures (loong64, ppc64le, riscv64, s390x), while keeping struct support in NewCallback restricted to amd64/arm64.
Changes:
- Broadened
ensureStructSupportedto allow struct args/returns on loong64, ppc64le, riscv64, and s390x for direct C calls. - Introduced
ensureCallbackStructSupportedand applied it to the callback compilation path. - Un-gated struct tests for the newly supported arches, skipping callback-based variants where callbacks don’t support structs; updated README support notes accordingly.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| syscall_unix.go | Switches struct gating in callback signature validation to callback-specific support checks. |
| func.go | Expands direct-call struct support to additional 64-bit Linux architectures; adds callback-specific support guard. |
| struct_test.go | Enables struct tests on newly supported architectures; skips callback variants where unsupported. |
| README.md | Updates platform support notes to reflect direct-call-only struct support on the new architectures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Passing and returning structs on loong64 mishandled floating-point aggregates. getStruct did not mask the NaN-boxed upper bits of a single-precision value read back from an FP register, so a struct of two float32 came back with a corrupted second field. addStruct placed every floating-point field in its own FP register, so a struct with three or more float members was passed in FP registers instead of the integer registers the LoongArch hard-float ABI requires. Replace the isAllSameFloat heuristic with a classifier that flattens nested structs and arrays and applies the LoongArch rule: an aggregate uses FP registers only when it has one or two floating-point members and nothing else, or exactly one floating-point member with one integer member. Every other aggregate uses the integer calling convention. This also fixes aggregates of two differently sized floating-point members such as a float32 and a float64, which the old heuristic misclassified. Also validate struct return types when creating a callback so an unsupported signature fails at NewCallback time instead of panicking later when the callback is invoked. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The LoongArch struct classifier flattened blank (_) padding fields into members, so a struct padded to match the C layout was misclassified. A bool and a float32 separated by byte padding looked like one float and several integers, and a float32 and float64 separated by a blank float32 looked like three floats. Both fell back to the integer calling convention instead of the floating-point registers the ABI requires. Skip blank fields when flattening: they are explicit padding, not members that the C calling convention counts. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
hajimehoshi
changed the title
Only loong64 is enabled and tested here. ppc64le, riscv64, and s390x still use the isAllSameFloat heuristic that mishandles floating-point aggregates (ppc64le currently crashes under test), so each will be enabled separately once its calling convention is implemented correctly. Drop them from the struct-support guard, the test build tag, and the README so the guard once again permits only amd64, arm64, and loong64. Updates ebitengine#474 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
hajimehoshi
force-pushed
the
claude/affectionate-sammet-5282e3
branch
from
d29ea72 to
79850cf
Compare
Member
Author
|
@TotallyGamerJet Please take a look |
The loong-prefixed identifiers in struct_loong64.go read like a misspelling of "long". Prefix them with the full GOARCH name: loongLeaf, loongFlatten, loongClassify, loongStoreInt, and loongLoadInt become loong64Leaf, loong64Flatten, loong64Classify, loong64StoreInt, and loong64LoadInt. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
- Use `for range` instead of a C-style loop in loong64Flatten. - Copy the return-pointer register into a local before taking its address so the reflect.Value cannot alias the pooled syscallArgs. - Invert the float check in getStruct to flatten the integer path. - Drop the redundant amd64/arm64 comments in the callback test skip. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
getStruct receives syscallArgs by value, so taking the address of syscall.a1 does not alias the pooled syscallArgs and the local copy added earlier is unnecessary. Read the return pointer directly. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What issue is this addressing?
Updates #474
What type of issue is this addressing?
feature
What this PR does | solves
Enables passing and returning C structs by value through
RegisterFunc/RegisterLibFuncon linux/loong64.The per-architecture code added in #431 was unreachable behind a guard that
only permitted amd64 and arm64. This PR makes that guard direction-aware and
adds loong64:
ensureStructSupportedgates the forward call path (RegisterFunc) and nowallows loong64 in addition to amd64/arm64.
ensureCallbackStructSupportedkeeps the callback path (NewCallback)restricted to amd64/arm64, where
getCallbackStruct/setStructareimplemented. Struct callbacks remain unsupported on loong64.
unsupported signature fails at
NewCallbacktime instead of panicking later.The loong64
struct_loong64.goimplementation was rewritten to follow theLoongArch hard-float calling convention: an aggregate uses FP registers only
when, after flattening nested structs/arrays and ignoring blank padding fields,
it has one or two floating-point members and nothing else, or exactly one
floating-point member with one integer member; everything else uses the integer
convention.
float32values are NaN-boxed in FP registers. This was validatedagainst the real compiler with
zig cc -target loongarch64-linux-gnu -Sand bythe QEMU test job.
Scope: only loong64 is enabled here. ppc64le, riscv64, and s390x still use
the old
isAllSameFloatheuristic, which mishandles floating-point aggregates(ppc64le currently crashes under test). Each will be enabled in its own PR once
its calling convention is implemented correctly.
README.mdreflects this:loong64 moves to "structs when calling C functions, but not in callbacks", while
the other three stay unsupported.
Authored by Claude (Claude Code) on behalf of @hajimehoshi.
🤖 Generated with Claude Code