Fix a bug with (add (sub 0 X) Y) => (sub Y X) (#1727)

We need to verify that the reordering is valid if there are side effects.

Original bug report: https://groups.google.com/forum/?nomobile=true#!topic/emscripten-discuss/HIlGf8o2Ato

3 files changed, 27 insertions, 3 deletions

diff --git a/src/ir/effects.h b/src/ir/effects.h
index 8919c6da4..8c95f463d 100644
--- a/src/ir/effects.h
+++ b/src/ir/effects.h
@@ -283,6 +283,14 @@ struct EffectAnalyzer : public PostWalker<EffectAnalyzer> {
     isAtomic = true;
   }
   void visitUnreachable(Unreachable *curr) { branches = true; }
+
+  // Helpers
+
+  static bool canReorder(PassOptions& passOptions, Expression* a, Expression* b) {
+    EffectAnalyzer aEffects(passOptions, a);
+    EffectAnalyzer bEffects(passOptions, b);
+    return !aEffects.invalidates(bEffects);
+  }
 };
 
 } // namespace wasm
diff --git a/src/passes/MinifyImportsAndExports.cpp b/src/passes/MinifyImportsAndExports.cpp
index 9cbc3b19a..f32859384 100644
--- a/src/passes/MinifyImportsAndExports.cpp
+++ b/src/passes/MinifyImportsAndExports.cpp
@@ -64,7 +64,6 @@ struct MinifyImportsAndExports : public Pass {
       reserved.insert("enum");
       reserved.insert("void");
       reserved.insert("this");
-      reserved.insert("void");
       reserved.insert("with");
 
       validInitialChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ_$";
diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp
index 255ff4cbd..7cdc048f7 100644
--- a/src/passes/OptimizeInstructions.cpp
+++ b/src/passes/OptimizeInstructions.cpp
@@ -380,16 +380,32 @@ struct OptimizeInstructions : public WalkerPass<PostWalker<OptimizeInstructions,
         //     Y
         //     X
         //   )
+        // Note that this reorders X and Y, so we need to be careful about that.
         if (auto* sub = binary->left->dynCast<Binary>()) {
           if (sub->op == SubInt32) {
             if (auto* subZero = sub->left->dynCast<Const>()) {
               if (subZero->value.geti32() == 0) {
-                sub->left = binary->right;
-                return sub;
+                if (EffectAnalyzer::canReorder(getPassOptions(), sub->right, binary->right)) {
+                  sub->left = binary->right;
+                  return sub;
+                }
               }
             }
           }
         }
+        // The flip case is even easier, as no reordering occurs:
+        //   (i32.add
+        //     Y
+        //     (i32.sub
+        //       (i32.const 0)
+        //       X
+        //     )
+        //   )
+        // =>
+        //   (i32.sub
+        //     Y
+        //     X
+        //   )
         if (auto* sub = binary->right->dynCast<Binary>()) {
           if (sub->op == SubInt32) {
             if (auto* subZero = sub->left->dynCast<Const>()) {
@@ -761,6 +777,7 @@ private:
           constant += value * mul;
           constants.push_back(c);
         }
+        return;
       } else if (auto* binary = curr->dynCast<Binary>()) {
         if (binary->op == AddInt32) {
           seek(binary->left, mul);