Skip to content
This repository has been archived by the owner before Nov 9, 2022. It is now read-only.

cranelift-wasm: Fix reachability tracking for if .. else .. end #1143

Merged
merged 1 commit into from Oct 15, 2019

Conversation

fitzgen
Copy link
Member

@fitzgen fitzgen commented Oct 14, 2019

We weren't previously keeping track of quite the right information for whether an if .. else .. end's following block was reachable or not. It should be reachable if the head is reachable and either the consequent or alternative end reachable (and therefore fall through to the following block) or do an early br_if to it.

This commit rejiggers ControlStackFrame::If to keep track of reachability at the end of the consequent (we don't need to keep track of it at the end of the alternative, since that is simply state.reachable) and adds Wasm tests for every reachability situation we can encounter with if .. else .. end.

Fixes #1132

We weren't previously keeping track of quite the right information for whether
an `if .. else .. end`'s following block was reachable or not. It should be
reachable if the head is reachable and either the consequent or alternative end
reachable (and therefore fall through to the following block) or do an early
`br_if` to it.

This commit rejiggers `ControlStackFrame::If` to keep track of reachability at
the end of the consequent (we don't need to keep track of it at the end of the
alternative, since that is simply `state.reachable`) and adds Wasm tests for
every reachability situation we can encounter with `if .. else .. end`.

Fixes bytecodealliance#1132
@fitzgen fitzgen requested a review from bnjbvr Oct 14, 2019
@fitzgen

This comment has been minimized.

@abrown

This comment has been minimized.

@fitzgen

This comment has been minimized.

@fitzgen

This comment has been minimized.

@bnjbvr
Copy link
Member

bnjbvr commented Oct 15, 2019

Thanks, I confirm this fixes all the pending wasm tests to add, as well as all the Spidermonkey tests that were passing before. I can review it, but @sunfishcode probably has a better preexisting understanding of this code, since he's reviewed the first batch of changes.

@fitzgen fitzgen requested review from sunfishcode and removed request for bnjbvr Oct 15, 2019
@sunfishcode sunfishcode merged commit a1f2a15 into bytecodealliance:master Oct 15, 2019
@fitzgen fitzgen deleted the if-else-reachability branch Oct 15, 2019
@fitzgen
Copy link
Member Author

fitzgen commented Oct 15, 2019

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wasm translation regression in wasm_linpack_float
4 participants