Delay argument location use in CallSite::gen_arg#8151
Merged
elliottt merged 2 commits intobytecodealliance:mainfrom Mar 15, 2024
Merged
Delay argument location use in CallSite::gen_arg#8151elliottt merged 2 commits intobytecodealliance:mainfrom
CallSite::gen_arg#8151elliottt merged 2 commits intobytecodealliance:mainfrom
Conversation
elliottt
force-pushed
the
trevor/gen-arg-locs
branch
from
8f65f36 to
d52660f
Compare
Co-authored-by: Jamey Sharp <jsharp@fastly.com>
elliottt
force-pushed
the
trevor/gen-arg-locs
branch
from
d52660f to
8adc11a
Compare
github-actions
bot
added
cranelift
jameysharp
added a commit
to jameysharp/wasmtime
that referenced
this pull request
Apr 3, 2024
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 bytecodealliance#8151 and became unused again in bytecodealliance#8246.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Apr 3, 2024
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.
jameysharp
added a commit
to jameysharp/wasmtime
that referenced
this pull request
Apr 18, 2024
This reverts the key parts of e3a08d4 (bytecodealliance#8151), because it turns out that we didn't need that abstraction. Several changes in the last month have enabled this: - bytecodealliance#8292 and then bytecodealliance#8316 allow us to refer to either incoming or outgoing argument areas in a (mostly) consistent way - bytecodealliance#8327, bytecodealliance#8377, and bytecodealliance#8383 demonstrate that we never need to delay writing stack arguments directly to their final location
jameysharp
added a commit
to jameysharp/wasmtime
that referenced
this pull request
Apr 18, 2024
This reverts the key parts of e3a08d4 (bytecodealliance#8151), because it turns out that we didn't need that abstraction. Several changes in the last month have enabled this: - bytecodealliance#8292 and then bytecodealliance#8316 allow us to refer to either incoming or outgoing argument areas in a (mostly) consistent way - bytecodealliance#8327, bytecodealliance#8377, and bytecodealliance#8383 demonstrate that we never need to delay writing stack arguments directly to their final location prtest:full
github-merge-queue bot
pushed a commit
that referenced
this pull request
Apr 18, 2024
This reverts the key parts of e3a08d4 (#8151), because it turns out that we didn't need that abstraction. Several changes in the last month have enabled this: - #8292 and then #8316 allow us to refer to either incoming or outgoing argument areas in a (mostly) consistent way - #8327, #8377, and #8383 demonstrate that we never need to delay writing stack arguments directly to their final location prtest:full
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In preparation for handling callee-save registers in the tail calling convention, split the logic for handling argument locations out of
CallSite::gen_arg. This allows us to process the arguments as normal, but delay the actual move to the stack until later.I also took the opportunity to directly emit the instruction vector at the end of
CallSite::gen_arginstead of returning it, as this is what's done at every use ofCallSite::gen_argalready. Unfortunately the intermediate vector is still necessary as all calls to emit would be underneath an immutable borrow ofctx, but this does clean things up a little bit.