MLton / mlton

The MLton repository
http://mlton.org
Other
960 stars 127 forks source link

Fix bug in X86GenerateTransfers introduced by 286a54cc0. #529

Closed MatthewFluet closed 1 year ago

MatthewFluet commented 1 year ago

Commit 286a54cc0 introduced a dependency on the declared prototype of a C function when generating the assembly for a C function call, in order to properly sign extend 8-bit and 16-bit arguments.

Early in the x86 codegen, Word64 values are converted into pairs of Word32 values, so a C function call with Word64 values will converted to a call with pairs of Word32 values for those arguments (increasing the number of arguments relative to the earlier IRs). But, no such translation was made to the C function's prototype, which continued to use Int64 and Word64 types. This lead to a Fail: fold2 internal-compiler error due to the mismatch in the number of actual arguments and the number of prototype arguments.

A simple solution is to expand the Int64 and Word64 prototype arguments to pairs of Word32 arguments (signedness does not matter).

ii8 commented 1 year ago

So why does the loss of the sign information not matter? I'm just curious. Are Ints and Words not treated different anymore after this point?

MatthewFluet commented 1 year ago

Immediately after type checking, MLton erases the distinction between int (signed) and word (unsigned), except that for foreign C functions, we record in their prototype whether they took a signed vs unsigned integer.

Signedness is really a property of the operation (e.g., signed integer arithmetic vs unsigned integer arithmetic) and how it interprets the bits. One can take the output of a signed integer arithmetic operation and immediately perform an unsigned integer arithmetic operation on it.

As noted in 286a54cc0, when passing an 8-bit or 16-bit value via the x86/amd64 C calling convention, that value needs to be extended to 32 bits and the callee may assume that it was sign- or zero-extended according to the declared signedness of the argument.

Since 32-bit values don't need to be extended to be passed, the perceived sign of the argument doesn't matter.