Stream: git-wasmtime

Topic: wasmtime / PR #8296 cranelift: Simplify spill/reload emis...


view this post on Zulip Wasmtime GitHub notifications bot (Apr 03 2024 at 18:21):

jameysharp requested cfallin for a review on PR #8296.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 03 2024 at 18:21):

jameysharp requested wasmtime-compiler-reviewers for a review on PR #8296.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 03 2024 at 18:21):

jameysharp opened PR #8296 from jameysharp:simplify-spill-reload to bytecodealliance:main:

The gen_spill and gen_reload methods on Callee are used to emit appropriate moves between registers and the stack, as directed by the register allocator.

These moves always apply to a single register at a time, even if that register was originally part of a group of registers. For example, when an I128 is represented using two 64-bit registers, either of those registers may be spilled independently.

As a result, the load_spillslot/store_spillslot helpers were more general than necessary, which in turn required extra complexity in the gen_load_stack_multi/gen_store_stack_multi helpers. None of these helpers were used in any other context, so all that complexity was unnecessary.

Inlining all four helpers and then simplifying eliminates a lot of code without changing the output of the compiler.

These helpers were also the only uses of StackAMode::offset, so I've deleted that. While I was there, I also deleted StackAMode::get_type, which was introduced in #8151 and became unused again in #8246.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 03 2024 at 18:28):

cfallin submitted PR review:

Good catch! I think this is leftover generality from a previous version of multi-reg handling; indeed we don't seem to need it anymore. I'm having a little trouble remembering whether the _multi variants were useful for arm32 previously (e.g., stack args); I suspect the rest of the ABI machinery should work without this though, so let's clean up and cross other bridges later :-)

view this post on Zulip Wasmtime GitHub notifications bot (Apr 03 2024 at 18:34):

jameysharp has enabled auto merge for PR #8296.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 03 2024 at 19:22):

jameysharp merged PR #8296.


Last updated: Oct 23 2024 at 20:03 UTC