Implement passing arguments by ref for win64 ABI #1670
Conversation
github-actions
bot
added
the
cranelift
Subscribe to Label Actioncc @bnjbvr
This issue or pull request has been labeled: "cranelift"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
| @@ -156,6 +156,8 @@ pub struct AbiParam { | |||
| /// ABI-specific location of this argument, or `Unassigned` for arguments that have not yet | |||
| /// been legalized. | |||
| pub location: ArgumentLoc, | |||
| /// Was the argument converted to pointer during legalization? | |||
| pub legalized_to_pointer: bool, | |||
There was a problem hiding this comment.
I think this should instead be a new variant of ArgumentLoc.
There was a problem hiding this comment.
I implemented the change of value into pointer as a new ValueConversion, like splitting or zero-extending, so it's separate from argument location. In case of passing-by-ref, location specifies how pointer (not the original value) should be passed - register or stack. For example legalization of i128 argument for win64 goes as follows:
- We start with
i128 - First pass:
i128is converted to pointer of typei64 - Second pass: Now argument has type
i64, so we assign register or stack location
As far as I can see the signature legalization just changes argument types without saving information how to convert them and it is later recovered during legalization of calls/entry block params (legalize_abi_value). I introduced legalized_to_pointer flag to be able to detect it has been converted to pointer (and not split, for example).
There was a problem hiding this comment.
For (i64, i64, i64, i64, i128) is the i128 still passed as a pointer, and then the pointer is stored on the stack at a fixed offset like a normal argument that doesn't fit in a register anymore?
There was a problem hiding this comment.
teapotd
force-pushed
the
win64-pass-by-ref
branch
2 times, most recently
from
4796bcf to
9962084
Compare
teapotd
force-pushed
the
win64-pass-by-ref
branch
from
9962084 to
aa1ae77
Compare
|
#1510 just got merged, I rebased this PR so it contains 1 commit. |
teapotd
force-pushed
the
win64-pass-by-ref
branch
from
aa1ae77 to
759cc3e
Compare
|
Seems like there were some issues with MacOS CI. |
Re-running the CI jobs, hopefully that will clear them up. |
It looks like the re-run CI jobs are green, but the cancelled ones are still making the whole thing show up as not passing. Pretty sure this is a github actions bug. I'm going to just merge this. |
This PR goes on top of #1510 and fixes legalization of function arguments in win64 ABI - arguments larger than pointer width are passed by reference. Fixes #1651.