don't move code around a drop-block when the block contains unreachables, which can cause type changes in the outside. dce should be run on that anyhow

Author: kripken

src/passes/MergeBlocks.cpp

@@ -168,32 +168,42 @@ static void optimizeBlock(Block* curr, Module* module, PassOptions& passOptions)
         if (drop) {
           child = drop->value->dynCast<Block>();
           if (child) {
-            if (child->name.is()) {
-              Expression* expression = child;
-              // check if it's ok to remove the value from all breaks to us
-              ProblemFinder finder(passOptions);
-              finder.origin = child->name;
-              finder.walk(expression);
-              if (finder.found()) {
+            // if we move around unreachable code, type changes could occur. avoid that, as
+            // anyhow it means we should have run dce before getting here
+            for (auto* test : child->list) {
+              if (test->type == unreachable) {
                 child = nullptr;
-              } else {
-                // fix up breaks
-                BreakValueDropper fixer(passOptions);
-                fixer.origin = child->name;
-                fixer.setModule(module);
-                fixer.walk(expression);
+                break;
               }
             }
             if (child) {
-              // we can do it!
-              // reuse the drop
-              drop->value = child->list.back();
-              drop->finalize();
-              child->list.back() = drop;
-              child->finalize();
-              curr->list[i] = child;
-              more = true;
-              changed = true;
+              if (child->name.is()) {
+                Expression* expression = child;
+                // check if it's ok to remove the value from all breaks to us
+                ProblemFinder finder(passOptions);
+                finder.origin = child->name;
+                finder.walk(expression);
+                if (finder.found()) {
+                  child = nullptr;
+                } else {
+                  // fix up breaks
+                  BreakValueDropper fixer(passOptions);
+                  fixer.origin = child->name;
+                  fixer.setModule(module);
+                  fixer.walk(expression);
+                }
+              }
+              if (child) {
+                // we can do it!
+                // reuse the drop
+                drop->value = child->list.back();
+                drop->finalize();
+                child->list.back() = drop;
+                child->finalize();
+                curr->list[i] = child;
+                more = true;
+                changed = true;
+              }
             }
           }
         }

test/passes/merge-blocks.txt

@@ -14,12 +14,11 @@
  )
  (func $drop-block-br (type $0)
   (block $block
-   (block $x
-    (drop
-     (i32.const 1)
-    )
-    (br $x)
-    (drop
+   (drop
+    (block $x (result i32)
+     (br $x
+      (i32.const 1)
+     )
      (i32.const 0)
     )
    )
@@ -95,8 +94,8 @@
  (func $drop-block-squared-iloop (type $0)
   (drop
    (block $label$0 (result i32)
-    (block $label$1
-     (drop
+    (drop
+     (block $label$1
       (loop $label$2
        (br $label$2)
       )
@@ -124,4 +123,15 @@
    )
   )
  )
+ (func $loop-block-drop-block-return (type $0)
+  (loop $label$4
+   (block $label$5
+    (drop
+     (block $label$6 (result i32)
+      (return)
+     )
+    )
+   )
+  )
+ )
 )

test/passes/merge-blocks.wast

@@ -101,5 +101,16 @@
     )
    )
   )
+  (func $loop-block-drop-block-return
+   (loop $label$4
+    (block $label$5
+     (drop
+      (block $label$6 (result i32)
+       (return)
+      )
+     )
+    )
+   )
+  )
 )
 

test/passes/remove-unused-names_merge-blocks.txt

@@ -840,7 +840,9 @@
  (func $drop-unreachable (type $4) (result i32)
   (local $0 i32)
   (drop
-   (unreachable)
+   (block (result i32)
+    (unreachable)
+   )
   )
   (unreachable)
  )