don't remove values from breaks if the values have side effects

Author: kripken

src/passes/MergeBlocks.cpp

@@ -73,14 +73,23 @@ namespace wasm {
 // For example, if there is a switch targeting us, we can't do it - we can't remove the value from other targets
 struct ProblemFinder : public ControlFlowWalker<ProblemFinder> {
   Name origin;
-  bool foundSwitch = false;
+  bool noWay = false;
   // count br_ifs, and dropped br_ifs. if they don't match, then a br_if flow value is used, and we can't drop it
   Index brIfs = 0;
   Index droppedBrIfs = 0;
+  PassOptions& passOptions;
+
+  ProblemFinder(PassOptions& passOptions) : passOptions(passOptions) {}
 
   void visitBreak(Break* curr) {
-    if (curr->name == origin && curr->condition) {
-      brIfs++;
+    if (curr->name == origin) {
+      if (curr->condition) {
+        brIfs++;
+      }
+      // if the value has side effects, we can't remove it
+      if (EffectAnalyzer(passOptions, curr->value).hasSideEffects()) {
+        noWay = true;
+      }
     }
   }
 
@@ -94,27 +103,30 @@ struct ProblemFinder : public ControlFlowWalker<ProblemFinder> {
 
   void visitSwitch(Switch* curr) {
     if (curr->default_ == origin) {
-      foundSwitch = true;
+      noWay = true;
       return;
     }
     for (auto& target : curr->targets) {
       if (target == origin) {
-        foundSwitch = true;
+        noWay = true;
         return;
       }
     }
   }
 
   bool found() {
     assert(brIfs >= droppedBrIfs);
-    return foundSwitch || brIfs > droppedBrIfs;
+    return noWay || brIfs > droppedBrIfs;
   }
 };
 
 // Drops values from breaks to an origin.
 // While doing so it can create new blocks, so optimize blocks as well.
 struct BreakValueDropper : public ControlFlowWalker<BreakValueDropper> {
   Name origin;
+  PassOptions& passOptions;
+
+  BreakValueDropper(PassOptions& passOptions) : passOptions(passOptions) {}
 
   void visitBlock(Block* curr);
 
@@ -143,7 +155,7 @@ struct BreakValueDropper : public ControlFlowWalker<BreakValueDropper> {
 };
 
 // core block optimizer routine
-static void optimizeBlock(Block* curr, Module* module) {
+static void optimizeBlock(Block* curr, Module* module, PassOptions& passOptions) {
   bool more = true;
   bool changed = false;
   while (more) {
@@ -159,14 +171,14 @@ static void optimizeBlock(Block* curr, Module* module) {
             if (child->name.is()) {
               Expression* expression = child;
               // check if it's ok to remove the value from all breaks to us
-              ProblemFinder finder;
+              ProblemFinder finder(passOptions);
               finder.origin = child->name;
               finder.walk(expression);
               if (finder.found()) {
                 child = nullptr;
               } else {
                 // fix up breaks
-                BreakValueDropper fixer;
+                BreakValueDropper fixer(passOptions);
                 fixer.origin = child->name;
                 fixer.setModule(module);
                 fixer.walk(expression);
@@ -217,7 +229,7 @@ static void optimizeBlock(Block* curr, Module* module) {
 }
 
 void BreakValueDropper::visitBlock(Block* curr) {
-  optimizeBlock(curr, getModule());
+  optimizeBlock(curr, getModule(), passOptions);
 }
 
 struct MergeBlocks : public WalkerPass<PostWalker<MergeBlocks>> {
@@ -226,7 +238,7 @@ struct MergeBlocks : public WalkerPass<PostWalker<MergeBlocks>> {
   Pass* create() override { return new MergeBlocks; }
 
   void visitBlock(Block *curr) {
-    optimizeBlock(curr, getModule());
+    optimizeBlock(curr, getModule(), getPassOptions());
   }
 
   Block* optimize(Expression* curr, Expression*& child, Block* outer = nullptr, Expression** dependency1 = nullptr, Expression** dependency2 = nullptr) {

test/passes/merge-blocks.txt

@@ -78,10 +78,15 @@
  )
  (func $drop-unreachable-br_if (type $2) (result i32)
   (block $label$0 (result i32)
-   (block $label$2
-    (drop
-     (br $label$0
-      (i32.const 538976371)
+   (drop
+    (block $label$2
+     (drop
+      (br_if $label$2
+       (br $label$0
+        (i32.const 538976371)
+       )
+       (i32.const 1918987552)
+      )
      )
     )
    )
@@ -100,4 +105,23 @@
    )
   )
  )
+ (func $br-goes-away-label2-becomes-unreachable (type $0)
+  (block $block
+   (drop
+    (block $label$1 (result i32)
+     (block $label$2
+      (drop
+       (br_if $label$1
+        (unreachable)
+        (i32.eqz
+         (br $label$2)
+        )
+       )
+      )
+     )
+     (i32.const 1)
+    )
+   )
+  )
+ )
 )

test/passes/merge-blocks.wast

@@ -82,5 +82,24 @@
     )
    )
   )
+  (func $br-goes-away-label2-becomes-unreachable
+   (block
+    (drop
+     (block $label$1 (result i32)
+      (block $label$2
+       (drop
+        (br_if $label$1
+         (unreachable)
+         (i32.eqz
+          (br $label$2)
+         )
+        )
+       )
+      )
+      (i32.const 1)
+     )
+    )
+   )
+  )
 )