Stream: git-wasmtime

Topic: wasmtime / PR #6586 Cranelift: Adjust virtual SP after `t...

Wasmtime GitHub notifications bot (Jun 15 2023 at 21:12):

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 and CallSite 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 the CallInfo 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.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Jun 15 2023 at 21:12):

fitzgen requested abrown for a review on PR #6586.

Wasmtime GitHub notifications bot (Jun 15 2023 at 21:12):

fitzgen requested wasmtime-compiler-reviewers for a review on PR #6586.

Wasmtime GitHub notifications bot (Jun 15 2023 at 21:21):

fitzgen updated PR #6586.

Wasmtime GitHub notifications bot (Jun 15 2023 at 21:27):

fitzgen requested cfallin for a review on PR #6586.

Wasmtime GitHub notifications bot (Jun 15 2023 at 21:29):

fitzgen updated PR #6586.

Wasmtime GitHub notifications bot (Jun 15 2023 at 21:53):

fitzgen updated PR #6586.

Wasmtime GitHub notifications bot (Jun 15 2023 at 21:59):

cfallin submitted PR review:

Looks right to me! Just one nit below.

Wasmtime GitHub notifications bot (Jun 15 2023 at 21:59):

cfallin submitted PR review:

Looks right to me! Just one nit below.

Wasmtime GitHub notifications bot (Jun 15 2023 at 21:59):

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?)

Wasmtime GitHub notifications bot (Jun 15 2023 at 22:05):

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.

Wasmtime GitHub notifications bot (Jun 15 2023 at 22:33):

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 for cranelift-codegen itself.

Wasmtime GitHub notifications bot (Jun 15 2023 at 22:52):

cfallin created PR review comment:

Ah, this is for the CLI crate only; I see. Yeah, I think that's actually a useful change.

Wasmtime GitHub notifications bot (Jun 16 2023 at 01:49):

fitzgen merged PR #6586.

Last updated: Nov 22 2024 at 16:03 UTC