diff options
| -rw-r--r-- | src/passes/OptimizeInstructions.cpp | 22 | ||||
| -rw-r--r-- | test/lit/passes/optimize-instructions-gc-tnh.wast | 26 |
2 files changed, 40 insertions, 8 deletions
diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index c38506d62..5d91ce29f 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -1500,21 +1500,27 @@ struct OptimizeInstructions // We need to see if a child with side effects exists after |input|. // If there is such a child, it is a problem as mentioned above (it // is fine for such a child to appear *before* |input|, as then we - // wouldn't be reordering effects). + // wouldn't be reordering effects). Thus, all we need to do is + // accumulate the effects in children after |input|, as we want to + // move the trap across those. bool seenInput = false; + EffectAnalyzer crossedEffects(options, *getModule()); for (auto* child : ChildIterator(parent)) { if (child == input) { seenInput = true; } else if (seenInput) { - // TODO We could ignore trap effects here (since traps are ok to - // reorder) and also local effects (since a change to a var - // would not be noticeable, unlike say a global). - if (EffectAnalyzer(options, *getModule(), child) - .hasSideEffects()) { - return; - } + crossedEffects.walk(child); } } + + // Check if the effects we cross interfere with the effects of the + // trap we want to move. (We use a shallow effect analyzer since we + // will only move the ref.as_non_null itself.) + ShallowEffectAnalyzer movingEffects(options, *getModule(), input); + if (crossedEffects.invalidates(movingEffects)) { + return; + } + // If we got here, we've checked the siblings and found no problem. checkedSiblings = true; } diff --git a/test/lit/passes/optimize-instructions-gc-tnh.wast b/test/lit/passes/optimize-instructions-gc-tnh.wast index 87795f31d..b7ddc2357 100644 --- a/test/lit/passes/optimize-instructions-gc-tnh.wast +++ b/test/lit/passes/optimize-instructions-gc-tnh.wast @@ -608,6 +608,7 @@ ) ;; TNH: (func $null.cast-other.effects (type $ref?|$struct|_=>_none) (param $x (ref null $struct)) + ;; TNH-NEXT: (local $i i32) ;; TNH-NEXT: (struct.set $struct 0 ;; TNH-NEXT: (local.get $x) ;; TNH-NEXT: (call $import) @@ -616,8 +617,15 @@ ;; TNH-NEXT: (local.get $x) ;; TNH-NEXT: (i32.const 10) ;; TNH-NEXT: ) + ;; TNH-NEXT: (struct.set $struct 0 + ;; TNH-NEXT: (local.get $x) + ;; TNH-NEXT: (local.tee $i + ;; TNH-NEXT: (i32.const 10) + ;; TNH-NEXT: ) + ;; TNH-NEXT: ) ;; TNH-NEXT: ) ;; NO_TNH: (func $null.cast-other.effects (type $ref?|$struct|_=>_none) (param $x (ref null $struct)) + ;; NO_TNH-NEXT: (local $i i32) ;; NO_TNH-NEXT: (struct.set $struct 0 ;; NO_TNH-NEXT: (ref.as_non_null ;; NO_TNH-NEXT: (local.get $x) @@ -628,8 +636,15 @@ ;; NO_TNH-NEXT: (local.get $x) ;; NO_TNH-NEXT: (i32.const 10) ;; NO_TNH-NEXT: ) + ;; NO_TNH-NEXT: (struct.set $struct 0 + ;; NO_TNH-NEXT: (local.get $x) + ;; NO_TNH-NEXT: (local.tee $i + ;; NO_TNH-NEXT: (i32.const 10) + ;; NO_TNH-NEXT: ) + ;; NO_TNH-NEXT: ) ;; NO_TNH-NEXT: ) (func $null.cast-other.effects (param $x (ref null $struct)) + (local $i i32) (struct.set $struct 0 ;; We cannot remove this ref.as_non_null, even though the struct.set will ;; trap if the ref is null, because that would move the trap from before @@ -648,6 +663,17 @@ ) (i32.const 10) ) + (struct.set $struct 0 + ;; This one can be removed even without TNH, even though there are effects + ;; in it. A tee only has local effects, which do not interfere with a + ;; trap. + (ref.as_non_null + (local.get $x) + ) + (local.tee $i + (i32.const 10) + ) + ) ) ;; Helper functions. |