Fix signed fill arguments at the C FFI boundary - #445
Conversation
The setInt8/16/32Array# and setInt8/16/32OffAddr# imports pass signed values to C helpers that expect unsigned arguments. On RISC-V, the calling convention requires different extension for signed and unsigned 16-bit values. Filling four Int16 elements with -2 can therefore produce [-2,-1,-1,-1]. For GHC 9.2 and newer, convert the narrow signed values to their unsigned equivalents and call the existing unsigned imports through inline wrappers. This preserves the element bits while matching the C argument types. This fixes 13 failures in the existing test suite on riscv64: Int16 filling, replicatePrimArray, and ByteArray Set Range for Int16 and its newtype wrappers.
|
What about GHC < 9.2? Is the bug not present there? |
Add C compatibility helpers that accept machine-word arguments and narrow them to the element type inside C. Route older unsigned imports through these helpers and convert signed values with int2Word# for both array and pointer fills. Validated with the existing test suite on x86_64 using GHC 9.0.2 and 9.6.6. No GHC older than 9.2 is available for riscv64, so the older compiler branch was tested on x86_64. The compatibility helpers also passed a separate C-level RISC-V ABI check under QEMU.
I don't really have the environment to test because GHC < 9.2 isn't very usable on riscv64. I just added a commit for that branch as well so you can decide whether it's worth it :) |
|
Nice fix. The previous approach was already on shaky footing to begin with since it used the FFI to wrap a C function with the wrong type (implicitly casting The only thing that I'm not sure of is whether or not this library should still be supporting GHCs earlier than 9.2. It would be nice to just drop all of the shims. I've opened #446 for a discussion on this. Before merging this, I'd like to see if it's possible to quickly gather consensus about dropping support for old GHCs (in the linked issue). If it seems like it will take more time to figure that out, I'll merge this as is. But if there is strong consensus that old GHCs are not worth the effort, we should drop the compatibility paths from this before merging. |
3 participants
The setInt8/16/32Array# and setInt8/16/32OffAddr# imports pass signed values to C helpers that expect unsigned arguments. On RISC-V, the calling convention requires different extension for signed and unsigned 16-bit values. Filling four Int16 elements with -2 can therefore produce [-2,-1,-1,-1].
For GHC 9.2 and newer, convert the narrow signed values to their unsigned equivalents and call the existing unsigned imports through inline wrappers. This preserves the element bits while matching the C argument types.
This fixes 13 failures in the existing test suite on riscv64: Int16 filling, replicatePrimArray, and ByteArray Set Range for Int16 and its newtype wrappers.