Skip to content

fix(windows): capture scalar float returns - #65

Merged
kolkov merged 1 commit into
go-webgpu:mainfrom
besmpl:besmpl/windows-float-returns
Jul 22, 2026
Merged

fix(windows): capture scalar float returns#65
kolkov merged 1 commit into
go-webgpu:mainfrom
besmpl:besmpl/windows-float-returns

Conversation

@besmpl

@besmpl besmpl commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • recover Windows AMD64 scalar float32/float64 returns from the raw XMM0 bits exposed as syscall.SyscallN's second result
  • add an actual Windows DLL end-to-end regression test for both return widths, using the pointer-argument call shape needed by native APIs
  • remove the now-obsolete README limitation and record the fix in the changelog

Why

This is the bounded goffi-side fix for the Windows correctness blocker @kolkov found while reviewing go-webgpu/webgpu#23.

It does not require another custom assembly trampoline. Go's Windows AMD64 asmstdcall deliberately copies XMM0 into SyscallN's second return slot; goffi was already receiving that value and discarding it. Selecting that slot only for scalar float return types keeps integer/pointer behavior unchanged.

Verification

  • go test -count=1 ./ffi ./types
  • GOOS=windows GOARCH=amd64 CGO_ENABLED=0 go test -count=1 -run '^$' -exec=true ./...
  • GOOS=windows GOARCH=amd64 CGO_ENABLED=1 go test -count=1 -run '^$' -exec=true ./...
  • the repository's Windows CI executes TestWindowsAMD64ScalarFloatReturns against the generated DLL, rather than only cross-compiling it

This should let webgpu consume the next canonical goffi release and keep Queue.GetTimestampPeriod correct without a fork or Windows-only fallback.

@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.

Good find. We had this documented as a known limitation ("Go's syscall package on Windows only exposes RAX") — turns out syscall.SyscallN was already giving us XMM0 in the second return slot, we just weren't reading it.

The fix is minimal and correct:

  • floatRet captured from syscall.SyscallN second return
  • Selected only for FloatType/DoubleType — integer/pointer paths unchanged
  • Old limitation comment and README entry removed (no longer applies)

The end-to-end test with real Windows DLL (structtest.dll) covers both float32 (0.125) and float64 (0.625) with a pointer argument — same call shape as native APIs like wgpuQueueGetTimestampPeriod.

LGTM.

@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.

Clean fix for a long-standing known issue (TASK-019). The XMM0 bits were already in SyscallN's second return — we just weren't reading them. E2E test on real DLL, no regressions. LGTM.

@kolkov
kolkov merged commit 23345c8 into go-webgpu:main Jul 22, 2026
14 checks passed
@kolkov

kolkov commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Released as goffi v0.6.2. This closes a limitation that was open since v0.4.1 — nice catch that syscall.SyscallN was already giving us XMM0 all along.

webgpu#23 can now bump to v0.6.2.

Side note: you're now in goffi CODEOWNERS for Android paths (v0.6.1). We plan the same for wgpu hal/vulkan/*android* paths once #268 lands — same pattern, same ownership scope. Just confirming you're on board with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants