diff options
| author | Alon Zakai <azakai@google.com> | 2020-10-09 19:51:22 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2020-10-09 19:51:22 -0700 |
| commit | caf624d0b1b81cf93cfb66dfdb50f49c5cc51578 (patch) | |
| tree | cdb61ee8ddd3f6008e30ccd2c0f700aa421d9afb | |
| parent | 399cb3df1e2e053fc601ed77744d41fe2378e54c (diff) | |
| download | binaryen-caf624d0b1b81cf93cfb66dfdb50f49c5cc51578.tar.gz binaryen-caf624d0b1b81cf93cfb66dfdb50f49c5cc51578.tar.bz2 binaryen-caf624d0b1b81cf93cfb66dfdb50f49c5cc51578.zip | |
RemoveUnusedBrs fuzz fix for switches with a single target and with a value (#3220)
We turn a br_table with a single target into a br, but we reverse the order of the
condition and the value when doing so, which we forgot to take into account.
| -rw-r--r-- | src/passes/RemoveUnusedBrs.cpp | 17 | ||||
| -rw-r--r-- | test/passes/remove-unused-brs_enable-multivalue.txt | 22 | ||||
| -rw-r--r-- | test/passes/remove-unused-brs_enable-multivalue.wast | 22 |
3 files changed, 56 insertions, 5 deletions
diff --git a/src/passes/RemoveUnusedBrs.cpp b/src/passes/RemoveUnusedBrs.cpp index 2238e9e95..0a3374c4d 100644 --- a/src/passes/RemoveUnusedBrs.cpp +++ b/src/passes/RemoveUnusedBrs.cpp @@ -828,11 +828,18 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> { void visitSwitch(Switch* curr) { if (BranchUtils::getUniqueTargets(curr).size() == 1) { - // This switch has just one target no matter what; replace with a br. - Builder builder(*getModule()); - replaceCurrent(builder.makeSequence( - builder.makeDrop(curr->condition), // might have side effects - builder.makeBreak(curr->default_, curr->value))); + // This switch has just one target no matter what; replace with a br + // if we can (to do so, we must put the condition before a possible + // value). + if (!curr->value || EffectAnalyzer::canReorder(passOptions, + getModule()->features, + curr->condition, + curr->value)) { + Builder builder(*getModule()); + replaceCurrent(builder.makeSequence( + builder.makeDrop(curr->condition), // might have side effects + builder.makeBreak(curr->default_, curr->value))); + } } } diff --git a/test/passes/remove-unused-brs_enable-multivalue.txt b/test/passes/remove-unused-brs_enable-multivalue.txt index ee980b523..6cbfe2373 100644 --- a/test/passes/remove-unused-brs_enable-multivalue.txt +++ b/test/passes/remove-unused-brs_enable-multivalue.txt @@ -2502,4 +2502,26 @@ ) ) ) + (func $unswitch-reordering (param $x i32) (result i32) + (block $label$1 (result i32) + (br_table $label$1 + (block $label$2 (result i32) + (i32.store + (i32.const 1) + (i32.const 2) + ) + (i32.const 3) + ) + (block $block (result i32) + (if + (local.get $x) + (return + (i32.const 5) + ) + ) + (i32.const 6) + ) + ) + ) + ) ) diff --git a/test/passes/remove-unused-brs_enable-multivalue.wast b/test/passes/remove-unused-brs_enable-multivalue.wast index b37b75776..ceefefa40 100644 --- a/test/passes/remove-unused-brs_enable-multivalue.wast +++ b/test/passes/remove-unused-brs_enable-multivalue.wast @@ -2106,4 +2106,26 @@ ) ) ) + (func $unswitch-reordering (param $x i32) (result i32) + (block $label$1 (result i32) + (br_table $label$1 + (block $label$2 (result i32) + (i32.store ;; has a possible side effect + (i32.const 1) + (i32.const 2) + ) + (i32.const 3) + ) + (block (result i32) + (if + (local.get $x) + (return + (i32.const 5) + ) + ) + (i32.const 6) + ) + ) + ) + ) ) |