Don't reorder an implicit trap with a global side effect (#1133)

1 files changed, 10 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;
   }