diff options
| -rw-r--r-- | src/ir/properties.h | 16 | ||||
| -rw-r--r-- | test/lit/passes/simplify-locals_rse_fallthrough.wast | 47 |
2 files changed, 62 insertions, 1 deletions
diff --git a/src/ir/properties.h b/src/ir/properties.h index a32a737ef..bd0487d8f 100644 --- a/src/ir/properties.h +++ b/src/ir/properties.h @@ -286,8 +286,22 @@ inline Expression* getImmediateFallthrough( } } } else if (auto* br = curr->dynCast<Break>()) { + // Note that we must check for the ability to reorder the condition and the + // value, as the value is first, which would be a problem here: + // + // (br_if .. + // (local.get $x) ;; value + // (tee_local $x ..) ;; condition + // ) + // + // We must not say that the fallthrough value is $x, since it is the + // *earlier* value of $x before the tee that is passed out. But, if we can + // reorder then that means that the value could have been last and so we do + // know the fallthrough in that case. if (br->condition && br->value && - behavior == FallthroughBehavior::AllowTeeBrIf) { + behavior == FallthroughBehavior::AllowTeeBrIf && + EffectAnalyzer::canReorder( + passOptions, module, br->condition, br->value)) { return br->value; } } else if (auto* tryy = curr->dynCast<Try>()) { diff --git a/test/lit/passes/simplify-locals_rse_fallthrough.wast b/test/lit/passes/simplify-locals_rse_fallthrough.wast new file mode 100644 index 000000000..a9161a853 --- /dev/null +++ b/test/lit/passes/simplify-locals_rse_fallthrough.wast @@ -0,0 +1,47 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. +;; RUN: wasm-opt %s --simplify-locals --rse -S -o - \ +;; RUN: | filecheck %s + +(module + ;; CHECK: (func $no (param $x i32) (result i32) + ;; CHECK-NEXT: (i32.add + ;; CHECK-NEXT: (block $block (result i32) + ;; CHECK-NEXT: (local.tee $x + ;; CHECK-NEXT: (br_if $block + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (local.tee $x + ;; CHECK-NEXT: (i32.const 42) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $no (param $x i32) (result i32) + (i32.add + (block $block (result i32) + ;; This local.tee is necessary. It might seem that the br_if flows out + ;; $x, so we don't need to write that same value to $x once more. + ;; SimplifyLocals and RedundantSetElimination try to find such sets and + ;; remove them. But this set is necessary, since a tee of $x executes + ;; after that local.get. That is, the br_if flows out the *old* value of + ;; $x. What happens in practice is that we branch (since the condition + ;; is 42), and flow out the old $x, which gets tee'd into $x once more. + ;; As a result, the block flows out the original value of $x, and the + ;; local.get reads that same value, so the final add returns 2 times the + ;; original value of $x. For all that to happen, we need those + ;; optimization passes to *not* remove the outer local.tee. + (local.tee $x + (br_if $block + (local.get $x) + (local.tee $x + (i32.const 42) + ) + ) + ) + ) + (local.get $x) + ) + ) +) |