-
Couldn't load subscription status.
- Fork 13.9k
rustc_codegen: fix musttail returns for cast/indirect ABIs #148240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Some changes occurred in compiler/rustc_codegen_ssa |
|
The best practice I'm usually trying to follow is: add tests in a separate commit (and make them "pass" there, possibly with known bug annotations, etc). That way the second commit with the impl clearly shows how the behavior changed, in the tests' diff. If you have time I'd recommend you do the same here (tip: jj makes splitting/modifying commits easy). And congrats on your first contribution! I'll try to review your PR shortly :) |
| @@ -0,0 +1,19 @@ | |||
| //@ check-pass | |||
| #![allow(incomplete_features)] | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use expect instead, so that we remember to remoce this when the feature stops being incomplete.
| "tail calls to functions with indirect returns cannot store into a destination" | ||
| ), | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason to not merge this into the match kind below that does make_return_dest for the non-tail call case already?
| ReturnDest::Nothing => {} | ||
| _ => bug!( | ||
| "tail calls to functions with indirect returns cannot store into a destination" | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is correct. make_return_dest only returns something other than ReturnDest::Nothing for indirect returns when the destination is an LocalRef::PendingOperand, but _0 should be a LocalRef::Place for indirect returns and tail calls shouldn't use any destination other than _0.
Fixes #148239
(Hopefully!)
Explicit tail calls trigger
bug!for any callee whose ABI returns viaPassMode::Cast, and we forgot to to forward the hiddensretout-pointer when the ABI requested an indirect return. The former causes ICE, the latter produced malformed IR (wrong codegen) if the return value is large enough to needsret.Updated the musttail helper to accept cast-mode returns, made it so that we pass the return pointer through the tail-call path, and added UI tests that cover both cases.
Added two UI tests (should they be elsewhere?)
This is my first time contributing, please do check if I did it right.
r? theemathas