cfallin requested elliottt for a review on PR #5029.
cfallin opened PR #5029 from cherrypick-2.0.0-pr5023
to release-2.0.0
:
- Fix StructReturn handling: properly mark the clobber, and offset actual rets.
The legalization of
StructReturn
was causing issues in the new call-handling code: theStructReturn
ret was included in theSigData
as if it were an actual CLIF-level return value, but it is not.Prior to using regalloc constraints for return values, we unconditionally included rax (or the architecture's usual return register) as a def, so it would be properly handled as "clobbered" by the regalloc. With the new scheme, we include defs on the call only for CLIF-level outputs. Callees with
StructReturn
args were thus not known to clobber the return-value register, and values might be corrupted.This PR updates the code to include a
StructReturn
ret as a clobber rather than a returned value in the relevant spots. I observed it causing saves/restores of rax in some CLIF that @bjorn3 provided me, but I was having difficulty minimizing this into a test-case that I would be comfortable including as a precise-output case (including the whole thing verbatim would lock down a bunch of other irrelevant details and cause test-update noise later). If we can find a more minimized example I'm happy to include it as a filetest.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
cfallin edited PR #5029 from cherrypick-2.0.0-pr5023
to release-2.0.0
:
The legalization of
StructReturn
was causing issues in the new call-handling code: theStructReturn
ret was included in theSigData
as if it were an actual CLIF-level return value, but it is not.Prior to using regalloc constraints for return values, we unconditionally included rax (or the architecture's usual return register) as a def, so it would be properly handled as "clobbered" by the regalloc. With the new scheme, we include defs on the call only for CLIF-level outputs. Callees with
StructReturn
args were thus not known to clobber the return-value register, and values might be corrupted.This PR updates the code to include a
StructReturn
ret as a clobber rather than a returned value in the relevant spots. I observed it causing saves/restores of rax in some CLIF that @bjorn3 provided me, but I was having difficulty minimizing this into a test-case that I would be comfortable including as a precise-output case (including the whole thing verbatim would lock down a bunch of other irrelevant details and cause test-update noise later). If we can find a more minimized example I'm happy to include it as a filetest.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
elliottt submitted PR review.
cfallin updated PR #5029 from cherrypick-2.0.0-pr5023
to release-2.0.0
.
cfallin merged PR #5029.
Last updated: Dec 23 2024 at 12:05 UTC