fitzgen opened PR #6586 from fitzgen:call-pop-virtual-sp
to bytecodealliance:main
:
Callees that use the
tail
calling convention will pop stack arguments from the stack for their callers. They do not, however, adjust the caller's virtual SP, so that still needs to happen in our ABI andCallSite
code. This is, however, slightly trickier than just emitting a nominal SP adjustment pseudo-instruction because we cannot let regalloc attempt to spill or reload values between the call and the SP adjustment because the stack offsets will be off by the size of the stack arguments to the call. Therefore, we add the number of bytes that the callee pops to theCallInfo
structures and have emission update the virtual SP atomically with regards to the call itself.Fixes #6581
Fixes #6582<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
fitzgen requested abrown for a review on PR #6586.
fitzgen requested wasmtime-compiler-reviewers for a review on PR #6586.
fitzgen updated PR #6586.
fitzgen requested cfallin for a review on PR #6586.
fitzgen updated PR #6586.
fitzgen updated PR #6586.
cfallin submitted PR review:
Looks right to me! Just one nit below.
cfallin submitted PR review:
Looks right to me! Just one nit below.
cfallin created PR review comment:
Accidentally left this in from debugging?
(Aside: this seems to be really easy to do; we had it slip into a release previously; perhaps we could add a grep or something to a CI check-job to make sure we don't regress again?)
jameysharp created PR review comment:
I objected to this change during pair programming but Nick convinced me that since the cranelift tools are not performance critical it's actually more useful to leave this always-on in that specific case.
fitzgen created PR review comment:
This was intentional because
clif-util
is a debugging tool, so it should enable our debugging-related Cargo features. It isn't changing anything about the default features forcranelift-codegen
itself.
cfallin created PR review comment:
Ah, this is for the CLI crate only; I see. Yeah, I think that's actually a useful change.
fitzgen merged PR #6586.
Last updated: Dec 23 2024 at 12:05 UTC