feat: expose queue timestamp period#23
Conversation
There was a problem hiding this comment.
Thanks for the PR — GetTimestampPeriod is a real gap in our API, and the ABI analysis (separate float-return path via float32Proc) is the right approach. The test coverage is thorough and the example update is a nice touch.
I have one blocking issue and a few smaller items.
Blocking: CallFloat32 returns garbage on Windows
I traced the full call chain on Windows:
windowsProc.CallFloat32()
→ ffi.CallFunction(&cif, fn, &result, argPtrs)
→ executeFunction() → arch.Registry.Caller.Execute()
→ amd64.Implementation.Execute() [call_windows.go]
→ syscall.SyscallN(fn, args...) → ret = RAX
→ handleReturn(cif, rvalue, uint64(ret), 0, 0, 0)
→ case FloatType: *(*float32)(rvalue) = *(*float32)(unsafe.Pointer(&retVal))
The problem: goffi on Windows passes fret=0 to handleReturn (call_windows.go:90). This is because syscall.SyscallN only captures RAX — there is no Windows assembly trampoline to capture XMM0 (compare: Unix has MOVQ X0, 128(DI) in syscall_unix_amd64.s:111 to save XMM0 after the C call).
Then handleReturn for FloatType interprets the lower 32 bits of RAX as float32:
case types.FloatType:
*(*float32)(rvalue) = *(*float32)(unsafe.Pointer(&retVal)) // retVal = RAX bitsPer Windows x64 ABI, float returns go exclusively in XMM0 — RAX is a scratch register with undefined contents. This applies to MSVC, Clang, and Rust/LLVM (x86_64-pc-windows-msvc). So CallFloat32 will interpret whatever garbage is in RAX as a float32.
This is a documented known limitation in goffi (TASK-019 / GAP-7) — purego has the same issue.
Suggested path forward: since you're already deep in goffi internals (goffi#62 Android), would you be up for adding a Windows assembly trampoline to capture XMM0? The Unix path already does this in syscall_unix_amd64.s:111:
MOVQ X0, 128(DI) // f1: float return in XMM0Windows would need a similar trampoline that wraps (or replaces) syscall.SyscallN for float-return calls and saves XMM0 to the syscallArgs struct. Bundling both (goffi fix + this PR rebased on top) would ship a complete, correct solution across all platforms.
If you'd rather keep this PR scoped to webgpu only, a build-tag split with a safe Windows fallback (return 0 or 1.0 with a log warning) would also work as a temporary measure — I can file a separate goffi issue for the trampoline.
Smaller items
mustInit() in GetTimestampPeriod — Other public Queue methods (Submit, WriteBuffer) assume Init() was already called. mustInit() panics on missing library, which is inconsistent. The next procQueueGetTimestampPeriod == nil check already handles this — suggest removing the mustInit() block.
CIF not cached in CallFloat32 — Both Unix and Windows CallFloat32 create a fresh CIF on every call. The existing Call() caches via u.prepared + u.cifMu. Not a perf issue for GetTimestampPeriod, but a // TODO: cache CIF comment would acknowledge the pattern gap.
Example hard-fails on period <= 0 — return fmt.Errorf("timestamp period unavailable") exits the example entirely. A warning + fallback to 1.0 would be more forgiving, especially given the Windows issue.
Summary
The Unix/macOS implementation is correct and well-tested. The Windows path needs either a goffi-level fix (XMM0 trampoline) or a temporary workaround before merge. Happy to discuss either approach — and nice work on the broader effort with Hearth, goffi#62, and gogpu/wgpu#268.
Summary
Queue.GetTimestampPeriodforwgpuQueueGetTimestampPeriodin wgpu-native v29Testing
v29.0.0.0TestQueueGetTimestampPeriodtests passedCGO_ENABLED=0 go vet ./wgpu/...CGO_ENABLED=0 go build ./wgpu/...0 issuesgo mod verifyandgit diff --check