winch(x64): Add support for loop, br and br_if
#6603
Merged
Conversation
This file contains 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
saulecabrera
requested review from
alexcrichton,
jameysharp and
cfallin
and removed request for
a team,
alexcrichton and
jameysharp
github-actions
bot
added
fuzzing
Subscribe to Label Action
This 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 |
That's totally fine; we'll have to review it anyway and IMHO when closely coupled it's better to see the pieces together. I'll try to get to this today! |
cfallin
reviewed
Jun 20, 2023
saulecabrera
force-pushed
the
winch-control-flow-loop
branch
from
60e9959
to
348cf72
Compare
|
@cfallin I've addressed all of your comments, I think this is ready for another pass. Regarding the visitor vs match-on-op discussion above, I've updated the approach to rely only on the visitor approach, by extending the existing |
There was a problem hiding this comment.
Nice! Seems good to me but I'd defer to @cfallin still for other technical review bits
cfallin
approved these changes
Jun 20, 2023
This change adds support for the `loop`, `br` and `br_if` instructions as well as unreachable code handling. Whenever an instruction that affects reachability is emitted (`br` in the case of this PR), the compiler will enter into an unreachable code state, essentially ignoring most of the subsequent instructions. When handling the unreachable code state some instructions are still observed, in order to determine if reachability should be restored. This change, particulary the handling of unreachable code, adds all the necessary building blocks to the compiler to emit other instructions that affect reachability (e.g `unreachable`, `return`). Address review feedback * Rename `branch_target` to `is_branch_target` * Use the visitor pattern to handle unreachable code Avoid string comparison and split unreachable handling functions
saulecabrera
force-pushed
the
winch-control-flow-loop
branch
from
74b64d0
to
9e00fd5
Compare
|
Seems that the Fuzz Targets step is having issues trying to fetch OCaml: |
|
Seems to be fixed! |
saulecabrera
added a commit
to saulecabrera/wasmtime
that referenced
this pull request
Jun 21, 2023
This change adds support for the `return` and `unreachable` instructions. This change builds on top of the control flow building blocks introduced in bytecodealliance#6603
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 21, 2023
This change adds support for the `return` and `unreachable` instructions. This change builds on top of the control flow building blocks introduced in #6603
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
loop,brandbr_ifinstructions as well as unreachable code handling. Whenever an instruction that affects reachability is emitted (brin the case of this PR), the compiler will enter into an unreachable code state, essentially ignoring most of the subsequent instructions. When handling the unreachable code state some instructions are still observed, in order to determine if reachability should be restored.This change, particulary the handling of unreachable code, adds all the necessary building blocks to the compiler to emit other instructions that affect reachability (e.g
unreachable,return).I decided to bundle these instructions together to be able to test more complex scenarios with
loop, but I'm happy to split this change into two (or more).