diff options
| -rw-r--r-- | src/ast/effects.h | 16 | ||||
| -rw-r--r-- | test/passes/simplify-locals-nostructure.txt | 34 | ||||
| -rw-r--r-- | test/passes/simplify-locals-nostructure.wast | 33 |
3 files changed, 77 insertions, 6 deletions
diff --git a/src/ast/effects.h b/src/ast/effects.h index 5392c0e50..2db671b5e 100644 --- a/src/ast/effects.h +++ b/src/ast/effects.h @@ -48,18 +48,18 @@ struct EffectAnalyzer : public PostWalker<EffectAnalyzer> { bool readsMemory = false; bool writesMemory = false; bool implicitTrap = false; // a load or div/rem, which may trap. we ignore trap - // differences, so it is ok to reorder these, and we - // also allow reordering them with other effects - // (so a trap may occur later or earlier, if it is - // going to occur anyhow), but we can't remove them, - // they count as side effects + // differences, so it is ok to reorder these, but we can't + // remove them, as they count as side effects, and we + // can't move them in a way that would cause other noticeable + // (global) side effects bool isAtomic = false; // An atomic load/store/RMW/Cmpxchg or an operator that // has a defined ordering wrt atomics (e.g. grow_memory) bool accessesLocal() { return localsRead.size() + localsWritten.size() > 0; } bool accessesGlobal() { return globalsRead.size() + globalsWritten.size() > 0; } bool accessesMemory() { return calls || readsMemory || writesMemory; } - bool hasSideEffects() { return calls || localsWritten.size() > 0 || writesMemory || branches || globalsWritten.size() > 0 || implicitTrap || isAtomic; } + bool hasGlobalSideEffects() { return calls || globalsWritten.size() > 0 || writesMemory || isAtomic; } + bool hasSideEffects() { return hasGlobalSideEffects() || localsWritten.size() > 0 || branches || implicitTrap; } bool hasAnything() { return branches || calls || accessesLocal() || readsMemory || writesMemory || accessesGlobal() || implicitTrap || isAtomic; } // checks if these effects would invalidate another set (e.g., if we write, we invalidate someone that reads, they can't be moved past us) @@ -98,6 +98,10 @@ struct EffectAnalyzer : public PostWalker<EffectAnalyzer> { if ((implicitTrap && other.branches) || (other.implicitTrap && branches)) { return true; } + // we can't reorder an implicit trap in a way that alters global state + if ((implicitTrap && other.hasGlobalSideEffects()) || (other.implicitTrap && hasGlobalSideEffects())) { + return true; + } return false; } diff --git a/test/passes/simplify-locals-nostructure.txt b/test/passes/simplify-locals-nostructure.txt index 2cb942f43..c48101a92 100644 --- a/test/passes/simplify-locals-nostructure.txt +++ b/test/passes/simplify-locals-nostructure.txt @@ -66,4 +66,38 @@ (local $x i32) (unreachable) ) + (func $implicit-trap-and-global-effects (type $0) + (local $var$0 i32) + (set_local $var$0 + (i32.trunc_u/f64 + (f64.const -nan:0xfffffffffffc3) + ) + ) + (f32.store align=1 + (i32.const 22) + (f32.const 154) + ) + (drop + (get_local $var$0) + ) + ) + (func $implicit-trap-and-local-effects (type $0) + (local $var$0 i32) + (local $other i32) + (nop) + (set_local $other + (i32.const 100) + ) + (drop + (i32.trunc_u/f64 + (f64.const -nan:0xfffffffffffc3) + ) + ) + (if + (i32.const 1) + (drop + (get_local $other) + ) + ) + ) ) diff --git a/test/passes/simplify-locals-nostructure.wast b/test/passes/simplify-locals-nostructure.wast index 548109237..9a4a63a2e 100644 --- a/test/passes/simplify-locals-nostructure.wast +++ b/test/passes/simplify-locals-nostructure.wast @@ -36,5 +36,38 @@ ) ) ) + (func $implicit-trap-and-global-effects + (local $var$0 i32) + (set_local $var$0 + (i32.trunc_u/f64 + (f64.const -nan:0xfffffffffffc3) ;; this implicit trap will actually trap + ) + ) + (f32.store align=1 ;; and if we move it across this store, the store will execute, having global side effects + (i32.const 22) + (f32.const 154) + ) + (drop + (get_local $var$0) + ) + ) + (func $implicit-trap-and-local-effects + (local $var$0 i32) + (local $other i32) + (set_local $var$0 + (i32.trunc_u/f64 + (f64.const -nan:0xfffffffffffc3) ;; this implicit trap will actually trap + ) + ) + (set_local $other (i32.const 100)) ;; but it's fine to move it across a local effect, that vanishes anyhow + (drop + (get_local $var$0) + ) + (if (i32.const 1) + (drop + (get_local $other) + ) + ) + ) ) |