-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Implement support for become
and explicit tail call codegen for the LLVM backend
#144232
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
Conversation
Some changes occurred in compiler/rustc_codegen_gcc Some changes occurred in compiler/rustc_codegen_ssa |
6a6d3c2
to
f912c90
Compare
This comment has been minimized.
This comment has been minimized.
b36cf15
to
ac87434
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
138b2a7
to
3fac279
Compare
This comment has been minimized.
This comment has been minimized.
I thought LLVM already supported the Wasm tail-call instructions — what's the missing piece that needs bikeshedding? |
@rustbot author it seems a tailcall test is still hitting a stack overflow, do you know why? |
Reminder, once the PR becomes ready for a review, use |
This was me mistakenly marking the rest |
Ah, my knowledge is outdated then. I've edited my comment, though point still stands for backends that aren't llvm/gcc. |
@rustbot ready |
@WaffleLapkin I'm going to go ahead and guess this is something you want to have a look at. |
@bors r+ Thanks! Glad we finally support it! |
4028c67
to
58c6633
Compare
@rustbot ready Alright, that makes sense. It's been removed. |
if tail && matches!(fn_abi.args[i].mode, PassMode::Indirect { .. }) { | ||
span_bug!( | ||
fn_span, | ||
"arguments using PassMode::Indirect are currently not supported for tail calls" |
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.
q: Why?
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.
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.
Also #144293
Could you also clean up the git history, to avoid the back-and-forths and simple fixups of later commits? Thanks. |
r? WaffleLapkin |
|
12c0f0d
to
ea59690
Compare
Done. |
ea59690
to
3a0a2da
Compare
…nd the LLVM codegen backend.
3a0a2da
to
a448837
Compare
Thanks! |
…fleLapkin Implement support for `become` and explicit tail call codegen for the LLVM backend This PR implements codegen of explicit tail calls via `become` in `rustc_codegen_ssa` and support within the LLVM backend. Completes a task on (rust-lang#112788). This PR implements all the necessary bits to make explicit tail calls usable, other backends have received stubs for now and will ICE if you use `become` on them. I suspect there is some bikeshedding to be done on how we should go about implementing this for other backends, but it should be relatively straightforward for GCC after this is merged. During development I also put together a POC bytecode VM based on tail call dispatch to test these changes out and analyze the codegen to make sure it generates expected assembly. That is available [here](https://github.com/xacrimon/tcvm).
Rollup of 6 pull requests Successful merges: - #135975 (Implement `push_mut`) - #143672 (Fix Box allocator drop elaboration) - #144232 (Implement support for `become` and explicit tail call codegen for the LLVM backend) - #144663 (coverage: Re-land "Enlarge empty spans during MIR instrumentation") - #144683 (Simplify library dependencies on `compiler-builtins`) - #144685 (Only extract lang items once in codegen_fn_attrs) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 6 pull requests Successful merges: - #135975 (Implement `push_mut`) - #143672 (Fix Box allocator drop elaboration) - #144232 (Implement support for `become` and explicit tail call codegen for the LLVM backend) - #144663 (coverage: Re-land "Enlarge empty spans during MIR instrumentation") - #144685 (Only extract lang items once in codegen_fn_attrs) - #144717 (Move `rustc_middle::parameterized`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #144232 - xacrimon:explicit-tail-call, r=WaffleLapkin Implement support for `become` and explicit tail call codegen for the LLVM backend This PR implements codegen of explicit tail calls via `become` in `rustc_codegen_ssa` and support within the LLVM backend. Completes a task on (#112788). This PR implements all the necessary bits to make explicit tail calls usable, other backends have received stubs for now and will ICE if you use `become` on them. I suspect there is some bikeshedding to be done on how we should go about implementing this for other backends, but it should be relatively straightforward for GCC after this is merged. During development I also put together a POC bytecode VM based on tail call dispatch to test these changes out and analyze the codegen to make sure it generates expected assembly. That is available [here](https://github.com/xacrimon/tcvm).
This PR implements codegen of explicit tail calls via
become
inrustc_codegen_ssa
and support within the LLVM backend. Completes a task on (#112788). This PR implements all the necessary bits to make explicit tail calls usable, other backends have received stubs for now and will ICE if you usebecome
on them. I suspect there is some bikeshedding to be done on how we should go about implementing this for other backends, but it should be relatively straightforward for GCC after this is merged.During development I also put together a POC bytecode VM based on tail call dispatch to test these changes out and analyze the codegen to make sure it generates expected assembly. That is available here.