Skip to content

Commit 513b756

Browse files
authored
feat: support callback struct arguments on AMD64 (#42)
* fix: resolve issue #41 Add support for callback struct by-value arguments on AMD64 Unix (System V ABI). It's the inverse of the struct type arg support in internal/arch/amd64/call_unix.go. Instead of marshalling from a struct to registers and the stack, data from registers and the stack are marshalled in to a struct. * Address a data race with writing to struct value Writing to valPtr, for example *(*float64)(valPtr) = getFloat() would result in a data race checkptr: converted pointer straddles multiple allocations Using a []byte to represent the struct value, and then copying the slice in to a struct reflect.Value avoids the data race. * Add end-to-end tests for callback struct args * Don't double increment stackIdx for partial memory chunk It was unnecessay to call getInt for the partial chunk, as the chunk is already in the chunk variable. And the getInt function would increment stackIdx an extra time. Fix the tests that test MEMORY class with a final partial chunk. The structs used were all a multiple of 8 bytes rather than the intended size. * Do not write past the end of ephemeral struct storage * Only run callback struct args tests on amd64 * Don't run callback struct arg tests Windows * Simplify classifyEightbyte * Use correct return type descriptor for C void functions * Add support for nested structs in isStructAllFloats * Add comment noting classifyEightbyte's lack of nested struct support
1 parent 05fe0a3 commit 513b756

4 files changed

Lines changed: 677 additions & 35 deletions

File tree

ffi/callback.go

Lines changed: 142 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func validateCallbackSignature(typ reflect.Type) {
103103
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
104104
reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64,
105105
reflect.Uintptr, reflect.Float32, reflect.Float64,
106-
reflect.Pointer, reflect.UnsafePointer, reflect.Bool:
106+
reflect.Pointer, reflect.UnsafePointer, reflect.Bool, reflect.Struct:
107107
// Valid types
108108
default:
109109
panic("ffi: unsupported callback argument type: " + argType.Kind().String())
@@ -192,6 +192,46 @@ func callbackWrap(a *callbackArgs) {
192192
var intIdx int // Current integer register index (0-5)
193193
stackIdx := numFloatRegs + numIntRegs // Stack arguments start after registers
194194

195+
getFloat := func() float64 {
196+
// Float64 comes from XMM register
197+
if floatIdx < numFloatRegs {
198+
bits := frame[floatIdx]
199+
floatIdx++
200+
return *(*float64)(unsafe.Pointer(&bits))
201+
} else {
202+
bits := frame[stackIdx]
203+
stackIdx++
204+
return *(*float64)(unsafe.Pointer(&bits))
205+
}
206+
}
207+
208+
getInt := func() uintptr {
209+
var value uintptr
210+
if intIdx < numIntRegs {
211+
value = frame[numFloatRegs+intIdx]
212+
intIdx++
213+
} else {
214+
// All register slots are used: value is on the stack
215+
value = frame[stackIdx]
216+
stackIdx++
217+
}
218+
return value
219+
}
220+
221+
// Write only some bytes to a struct to avoid overwrite.
222+
writePartial := func(dest unsafe.Pointer, size uintptr, value uintptr) {
223+
switch {
224+
case size == 1:
225+
*(*uint8)(dest) = uint8(value)
226+
case size == 2:
227+
*(*uint16)(dest) = uint16(value)
228+
case size <= 4:
229+
*(*uint32)(dest) = uint32(value)
230+
default:
231+
*(*uintptr)(dest) = value
232+
}
233+
}
234+
195235
// Build argument slice for reflection Call
196236
args := make([]reflect.Value, numArgs)
197237

@@ -201,34 +241,10 @@ func callbackWrap(a *callbackArgs) {
201241

202242
switch argType.Kind() {
203243
case reflect.Float32:
204-
// Float32 comes from XMM register (stored as float64)
205-
if floatIdx < numFloatRegs {
206-
// Read as uintptr, reinterpret as float64 bits
207-
bits := frame[floatIdx]
208-
f64 := *(*float64)(unsafe.Pointer(&bits))
209-
val = reflect.ValueOf(float32(f64))
210-
floatIdx++
211-
} else {
212-
// From stack
213-
bits := frame[stackIdx]
214-
f64 := *(*float64)(unsafe.Pointer(&bits))
215-
val = reflect.ValueOf(float32(f64))
216-
stackIdx++
217-
}
244+
val = reflect.ValueOf(float32(getFloat()))
218245

219246
case reflect.Float64:
220-
// Float64 comes from XMM register
221-
if floatIdx < numFloatRegs {
222-
bits := frame[floatIdx]
223-
f64 := *(*float64)(unsafe.Pointer(&bits))
224-
val = reflect.ValueOf(f64)
225-
floatIdx++
226-
} else {
227-
bits := frame[stackIdx]
228-
f64 := *(*float64)(unsafe.Pointer(&bits))
229-
val = reflect.ValueOf(f64)
230-
stackIdx++
231-
}
247+
val = reflect.ValueOf(getFloat())
232248

233249
case reflect.Bool:
234250
// Bool comes from integer register
@@ -272,16 +288,65 @@ func callbackWrap(a *callbackArgs) {
272288
stackIdx++
273289
}
274290

291+
case reflect.Struct:
292+
sz := argType.Size()
293+
structData := make([]byte, max(sz, 8))
294+
var valPtr unsafe.Pointer
295+
if sz > 0 {
296+
valPtr = unsafe.Pointer(&structData[0])
297+
}
298+
299+
switch {
300+
case sz == 0:
301+
// Zero-size struct: no fields to populate
302+
303+
case sz <= 8:
304+
// Single eightbyte: INTEGER if any member is not float/double, else SSE.
305+
if isStructAllFloats(argType) {
306+
*(*float64)(valPtr) = getFloat()
307+
} else {
308+
writePartial(valPtr, sz, getInt())
309+
}
310+
case sz <= 16:
311+
// Two eightbytes: classify each independently.
312+
// System V ABI §3.2.3: INTEGER wins over SSE within an eightbyte.
313+
if classifyEightbyte(argType, 0, 8) {
314+
*(*float64)(valPtr) = getFloat()
315+
} else {
316+
*(*uintptr)(valPtr) = getInt()
317+
}
318+
remaining := sz - 8
319+
valPtr = unsafe.Add(valPtr, 8)
320+
if classifyEightbyte(argType, 8, sz) {
321+
*(*float64)(valPtr) = getFloat()
322+
} else {
323+
writePartial(valPtr, remaining, getInt())
324+
}
325+
default:
326+
// MEMORY class (> 16 bytes): copy from the stack in 8-byte chunks.
327+
// Per SysV ABI §3.2.3: MEMORY class structs bypass registers entirely.
328+
nChunks := (sz + 7) / 8
329+
for k := range nChunks {
330+
chunkPtr := unsafe.Add(valPtr, k*8)
331+
chunk := frame[stackIdx]
332+
bytesLeft := sz - k*8
333+
if bytesLeft >= 8 {
334+
*(*uintptr)(chunkPtr) = chunk
335+
} else {
336+
writePartial(chunkPtr, bytesLeft, chunk)
337+
}
338+
stackIdx++
339+
}
340+
}
341+
val = reflect.New(argType)
342+
valByteSlice := unsafe.Slice((*byte)(val.UnsafePointer()), sz)
343+
copy(valByteSlice, structData)
344+
val = val.Elem()
345+
275346
default:
276347
// All other integer types (int, uint, int32, etc.)
277-
if intIdx < numIntRegs {
278-
pos := numFloatRegs + intIdx
279-
val = reflect.NewAt(argType, unsafe.Pointer(&frame[pos])).Elem()
280-
intIdx++
281-
} else {
282-
val = reflect.NewAt(argType, unsafe.Pointer(&frame[stackIdx])).Elem()
283-
stackIdx++
284-
}
348+
value := getInt()
349+
val = reflect.NewAt(argType, unsafe.Pointer(&value)).Elem()
285350
}
286351

287352
args[i] = val
@@ -322,3 +387,45 @@ func callbackWrap(a *callbackArgs) {
322387
//go:linkname _callbackTrampoline github.com/go-webgpu/goffi/ffi.callbackTrampoline
323388
var _callbackTrampoline byte
324389
var trampolineBaseAddr = uintptr(unsafe.Pointer(&_callbackTrampoline))
390+
391+
// isStructAllFloats returns true if every member of a flat struct is float or double.
392+
// Per System V AMD64 ABI §3.2.3: if any member in an eightbyte is INTEGER class,
393+
// the entire eightbyte is classified as INTEGER (INTEGER wins over SSE).
394+
func isStructAllFloats(structType reflect.Type) bool {
395+
if structType.NumField() == 0 {
396+
return false
397+
}
398+
399+
for i := range structType.NumField() {
400+
field := structType.Field(i)
401+
if field.Type.Kind() == reflect.Struct {
402+
if !isStructAllFloats(field.Type) {
403+
return false
404+
}
405+
} else if field.Type.Kind() != reflect.Float32 && field.Type.Kind() != reflect.Float64 {
406+
return false
407+
}
408+
}
409+
return true
410+
}
411+
412+
// classifyEightbyte returns true if all struct fields whose offset falls within
413+
// [startOff, endOff) are SSE types (float or double).
414+
// Returns false if any field in the range is INTEGER class, or if no fields lie in the range.
415+
//
416+
// CAUTION: Does not currently support nested structs.
417+
func classifyEightbyte(structType reflect.Type, startOff, endOff uintptr) bool {
418+
allFloat := true
419+
hasField := false
420+
for i := range structType.NumField() {
421+
field := structType.Field(i)
422+
if field.Offset >= startOff && field.Offset < endOff {
423+
hasField = true
424+
if field.Type.Kind() != reflect.Float32 && field.Type.Kind() != reflect.Float64 {
425+
allFloat = false
426+
break
427+
}
428+
}
429+
}
430+
return hasField && allFloat
431+
}

0 commit comments

Comments
 (0)