winch: Add support for br_table#6951
Merged
cfallin merged 4 commits intobytecodealliance:mainfrom Sep 2, 2023
Merged
Conversation
saulecabrera
requested review from
alexcrichton
and removed request for
a team and
alexcrichton
This change adds support for the `br_table` instruction, including several modifications to the existing control flow implementation: * Improved handling of jumps to loops: Previously, the compiler erroneously treated the result of loop blocks as the definitive result of the jump. This change fixes this bug. * Streamlined result handling and stack pointer balancing: In the past, these operations were executed in two distinct steps, complicating the process of ensuring the correct invariants when emitting unconditional jumps. To simplify this, `CodeGenContext::unconditional_jump` is introduced . This function guarantees all necessary invariants are met, encapsulating the entire operation within a single function for easier understanding and maintenance. * Handling of unreachable state at the end of a function: when reaching the end of a function in an unreachable state, clear the stack and ensure that the machine stack pointer is correctly placed according to the expectations of the outermost block. In addition to the above refactoring, the main implementation of the `br_table` instruction involves emitting labels for each target. Within each label, an unconditional jump is emitted to the frame's label, ensuring correct stack pointer balancing when the jump is emitted. While it is possible to optimize this process by avoiding intermediate labels when balancing isn't required, I've opted to maintain the current implementation until such optimization becomes necessary.
saulecabrera
force-pushed
the
winch-br_table
branch
from
9ebe17e to
ae4c431
Compare
saulecabrera
requested review from
elliottt
and removed request for
a team and
elliottt
github-actions
bot
added
fuzzing
Subscribe to Label ActionDetailsThis issue or pull request has been labeled: "fuzzing", "winch"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
cfallin
reviewed
Sep 1, 2023
Member
cfallin
left a comment
cfallin
left a comment
There was a problem hiding this comment.
This looks overall good to me, except for a bit of confusion around one stack-alignment case. With a slightly more detailed description in the comment I think it should be good though!
(also as an fyi, I'm on PTO all of next week; happy to re-review today if it happens, otherwise further reviews will be a bit delayed, sorry)
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Member
Author
|
Hmm the failure seems to be related to WASI sockets, I suspect it's the same failure reported in #6952 (comment) |
Member
|
Now that #6953 is merged let's see if this goes in... |
eduardomourar
pushed a commit
to eduardomourar/wasmtime
that referenced
this pull request
Sep 6, 2023
* winch: Add support for `br_table` This change adds support for the `br_table` instruction, including several modifications to the existing control flow implementation: * Improved handling of jumps to loops: Previously, the compiler erroneously treated the result of loop blocks as the definitive result of the jump. This change fixes this bug. * Streamlined result handling and stack pointer balancing: In the past, these operations were executed in two distinct steps, complicating the process of ensuring the correct invariants when emitting unconditional jumps. To simplify this, `CodeGenContext::unconditional_jump` is introduced . This function guarantees all necessary invariants are met, encapsulating the entire operation within a single function for easier understanding and maintenance. * Handling of unreachable state at the end of a function: when reaching the end of a function in an unreachable state, clear the stack and ensure that the machine stack pointer is correctly placed according to the expectations of the outermost block. In addition to the above refactoring, the main implementation of the `br_table` instruction involves emitting labels for each target. Within each label, an unconditional jump is emitted to the frame's label, ensuring correct stack pointer balancing when the jump is emitted. While it is possible to optimize this process by avoiding intermediate labels when balancing isn't required, I've opted to maintain the current implementation until such optimization becomes necessary. * chore: Rust fmt * fuzzing: Add `BrTable` to list of support instructions * docs: Improve documentation for `unconditional_jump`
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.
Part of #6528
This change adds support for the
br_tableinstruction, including several modifications to the existing control flow implementation:Improved handling of jumps to loops: Previously, the compiler erroneously treated the result of loop blocks as the definitive result of the jump. This change fixes this bug.
Streamlined result handling and stack pointer balancing: In the past, these operations were executed in two distinct steps, complicating the process of ensuring the correct invariants when emitting unconditional jumps. To simplify this,
CodeGenContext::unconditional_jumpis introduced . This function guarantees all necessary invariants are met, encapsulating the entire operation within a single function for easier understanding and maintenance.Handling of unreachable state at the end of a function: when reaching the end of a function in an unreachable state, clear the stack and ensure that the machine stack pointer is correctly placed according to the expectations of the outermost block.
In addition to the above refactoring, the main implementation of the
br_tableinstruction involves emitting labels for each target. Within each label, an unconditional jump is emitted to the frame's label, ensuring correct stack pointer balancing when the jump is emitted.While it is possible to optimize this process by avoiding intermediate labels when balancing isn't required, I've opted to maintain the current implementation until such optimization becomes necessary.