Skip to content

docs: record what callbacks accept per platform, and the arm64 aggreg… - #84

Merged
kolkov merged 2 commits into
go-webgpu:mainfrom
unxed:feature/callback-abi-docs
Nov 8, 2026
Merged

kolkov merged 2 commits into
go-webgpu:mainfrom
unxed:feature/callback-abi-docs

Conversation

@unxed

@unxed unxed commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

…ate gap

NewCallback answers an unsupported signature with a panic, and two of the three limits behind those panics are undocumented, so they read as bugs:

panic: ffi: unsupported callback argument type: struct
panic: ffi: Windows callbacks require uintptr-sized return type, got int32

docs/CALLBACK_ABI.md puts the three rows in one table -- amd64 Unix takes struct-by-value arguments, arm64 does not, Windows takes uintptr-sized arguments and one uintptr-sized result and nothing else -- and separates the limit that is permanent from the one that is not. Windows is the platform's own contract: NewCallback delegates to syscall.NewCallback, which accepts nothing more without cgo, and purego documents the same restriction. arm64 is unfinished work.

For that one the page carries what an implementer needs rather than a wish: where amd64 does the equivalent (callbackWrap's eightbyte classification in ffi/callback.go), the four AAPCS64 cases to cover, the two that are easy to get wrong -- a four-float32 HFA occupying S0-S3, i.e. the low halves of four V registers the frame stores as 64-bit slots, and register exhaustion sending the whole aggregate to the stack with no backfill -- and Apple's naturally aligned stack packing, which means any implementation has to be checked against both a Linux and an Apple toolchain. It also points at ffi/callback_struct_args_test.go as the model for testing classification with a hand-built frame, no C toolchain and no real foreign call needed.

Callers get the workaround in one line: take a pointer instead of a value.

The panic site in ffi/callback_arm64.go now points at the page, so the message someone actually sees leads to the explanation.

@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@kolkov kolkov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — a page that tells an unimplemented case from a broken one is exactly what was missing, and the arm64 section is a good starting point for whoever implements AAPCS64 aggregates.

While verifying the table against the code I ran real C→Go callbacks (not the hand-built frames from callback_test.go), and a row is wrong. This is a pre-existing bug, not something your PR introduced, but the page would publish it as supported:

float32 in callbacks is broken on amd64 Unix (linux/amd64, glibc 2.35, current main):

int    call_fi(int (*f)(float, float), float a, float b)     { return f(a, b); }
int    call_di(int (*f)(double, double), double a, double b) { return f(a, b); }
float  call_if(float (*f)(long), long a)                    { return f(a); }
double call_id(double (*f)(long), long a)                   { return f(a); }
float32 args:   Go saw a=0 b=0 (want 2.5 4)
float32 return: C got 0 (want 6)
float64 args:   Go saw a=2.5 b=4            ok
float64 return: C got 6                     ok

Why:

  • Args: the C caller puts a float in the low 32 bits of XMMn as single precision. callbackWrap reads the 64-bit slot as a float64 and converts it with float32(getFloat()), which turns the bit pattern into garbage. It should be math.Float32frombits(uint32(bits)). TestCallback_Float32Args doesn't catch this because it fills the slots with float64 bit patterns, i.e. it mirrors the bug rather than the ABI.
  • Returns: callbackDispatcher only moves the result into AX (R0 on arm64). Nothing writes XMM0 / D0. float64 returns currently come out right, apparently because X0 still happens to hold the value from ret.Float() when the dispatcher returns. That's luck, not a contract. float32 returns come out wrong because the low half of a float64 bit pattern isn't the float32.
  • arm64 has the same code shape (FSTPD saves D0–D7, the arm64 dispatcher returns only R0, and float32 args go through float32(f64)). I couldn't run arm64 here, so for arm64 this comes from reading the code, not from a test run.

We'll track the fix separately (#93). For this PR, could you:

  1. Change the float32 / float64 column so it says what's actually true today. Something like "float64 args: yes; float64 returns: correct today, but not guaranteed by the dispatcher; float32 args and returns: broken, see #93". Then update it when the fix lands.
  2. Add two caveats to the amd64 struct row, since "yes" overstates it. First, register exhaustion isn't handled: SysV passes a 9–16-byte aggregate in memory if its eightbytes don't all fit in the remaining registers. gcc confirms it: a struct { long x, y; } after five longs goes on the stack and R9 stays unused. callbackWrap would still take the first eightbyte from R9. Second, classifyEightbyte doesn't recurse into nested structs (its own comment says so).
  3. Optional: the Apple note applies to scalars too, not only aggregates. Apple's ABI doc says "Function arguments may consume slots on the stack that are not multiples of 8 bytes" (a char takes one byte), while the arm64 callbackWrap reads every stack argument as an 8-byte slot. Going by the code, a callback with more than 8 integer arguments narrower than 8 bytes is misread on darwin/arm64 today. I haven't run that on a Mac.

The callback_arm64.go comment pointing at the page is a nice touch. Happy to approve once the table matches reality.

unxed and others added 2 commits November 8, 2026 09:23
…ate gap

NewCallback answers an unsupported signature with a panic, and two of the
three limits behind those panics are undocumented, so they read as bugs:

    panic: ffi: unsupported callback argument type: struct
    panic: ffi: Windows callbacks require uintptr-sized return type, got int32

docs/CALLBACK_ABI.md puts the three rows in one table -- amd64 Unix takes
struct-by-value arguments, arm64 does not, Windows takes uintptr-sized
arguments and one uintptr-sized result and nothing else -- and separates the
limit that is permanent from the one that is not. Windows is the platform's
own contract: NewCallback delegates to syscall.NewCallback, which accepts
nothing more without cgo, and purego documents the same restriction. arm64 is
unfinished work.

For that one the page carries what an implementer needs rather than a wish:
where amd64 does the equivalent (callbackWrap's eightbyte classification in
ffi/callback.go), the four AAPCS64 cases to cover, the two that are easy to
get wrong -- a four-float32 HFA occupying S0-S3, i.e. the low halves of four V
registers the frame stores as 64-bit slots, and register exhaustion sending
the whole aggregate to the stack with no backfill -- and Apple's naturally
aligned stack packing, which means any implementation has to be checked
against both a Linux and an Apple toolchain. It also points at
ffi/callback_struct_args_test.go as the model for testing classification with
a hand-built frame, no C toolchain and no real foreign call needed.

Callers get the workaround in one line: take a pointer instead of a value.

The panic site in ffi/callback_arm64.go now points at the page, so the message
someone actually sees leads to the explanation.
isStructAllFloats is gone; the amd64 callback path now takes its register and
stack assignment from the call path's System V allocator. Link the page from
README and ARCHITECTURE.md and record it in the CHANGELOG.
@kolkov
kolkov force-pushed the feature/callback-abi-docs branch from 3f9133f to 11156ba Compare November 8, 2026 06:27

@kolkov kolkov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything I asked for here has since been fixed on main rather than in the table: float32 arguments and float results in callbacks (#93), System V register exhaustion and nested structs, and Apple's natural-size stack packing. So the table is now right as you wrote it.

I rebased the branch onto main. Your Android CI commit was already there through #85, so the rebase dropped it. I also added one commit on top: the page referred to isStructAllFloats, which no longer exists, so I fixed the name; it also links the page from the README and ARCHITECTURE.md and adds a CHANGELOG entry with your name. Thanks for writing this up, and especially the arm64 section: it is the spec for whoever implements AAPCS64 struct callbacks.

@kolkov
kolkov merged commit 0812f42 into go-webgpu:main Nov 8, 2026
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch: arm64 ARM64 specific (AAPCS64 ABI) area: callbacks C-to-Go callback trampolines priority: medium Normal priority status: review In code review type: docs Documentation only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants