Stream: git-wasmtime

Topic: wasmtime / PR #6666 Cranelift: Refactor `return_call[_ind...

Wasmtime GitHub notifications bot (Jun 28 2023 at 20:56):

fitzgen requested cfallin for a review on PR #6666.

Wasmtime GitHub notifications bot (Jun 28 2023 at 20:56):

fitzgen opened PR #6666 from fitzgen:refactor-lower-return-calls to bytecodealliance:main:

Commons up some code paths and sets the stage for other architectures. This should have fewer calls back and forth between architecture specific and independent bits of code, which I have found hard to keep track of. Now, lowering tail calls is done in architecture specific code that can call out to architecture independent helpers as needed. Before it was architecture independent code that would call architecture specific hooks that would call architecture independent helpers. Too much stuff split across too many layers. This new approach removes at least one layer of indirection and unnecessarily confusing abstraction.

<!--
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 28 2023 at 20:56):

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

Wasmtime GitHub notifications bot (Jun 28 2023 at 23:16):

cfallin submitted PR review:

Looks reasonable to me, thanks!

@fitzgen and I talked a bit about shared vs arch-specific ABI code and finding the right balance here. Originally we had copied-and-pasted abi.rs for both backends at the time (x64 and aarch64) and keeping them in-sync was tedious and error-prone; #2128 and #2142 merged the two into a shared common "vanilla ABI" with hooks. While that made sense at the time, we've had a significant combinatorial complexity jump since then with (4 ISAs) x (more calling conventions) x (more features), and empirically the overlap is shrinking as we add more special cases and additional hook points. So instead perhaps it makes sense to have arch-specific code at the top level (called directly from arch-specific lowering rules) and then factor out common helpers where it makes sense. This PR is a first step toward that and we'll hopefully move further that way in the future.

Wasmtime GitHub notifications bot (Jun 29 2023 at 00:24):

fitzgen merged PR #6666.

Last updated: Nov 22 2024 at 17:03 UTC