ReFinalize in MergeBlocks so we can optimize unreachable instructions…

… too (#6994)

In #6984 we optimized dropped blocks even if they had unreachable code. In #6988
that part was reverted, and blocks with unreachable code were ignored once more.
However, I realized that the check was not actually for unreachable code, but for
having an unreachable child, so it would miss things like this:

(block
  (block
    ..
    (br $somewhere) ;; unreachable type, but no unreachable code
  )
)

But it is useful to merge such blocks: we don't need the inner block here.

To fix this, just run ReFinalize if we change anything, which will propagate
unreachability as needed. I think MergeBlocks was written before we had
that utility, so it didn't use it...

This is not only useful for itself but will unblock an EH optimization in a
later PR, that has code in this form. It also simplifies the code by removing
the hasUnreachableChild checks.

src/passes/MergeBlocks.cpp

@@ -176,15 +176,6 @@ struct BreakValueDropper : public ControlFlowWalker<BreakValueDropper> {
   }
 };
 
-static bool hasUnreachableChild(Block* block) {
-  for (auto* test : block->list) {
-    if (test->type == Type::unreachable) {
-      return true;
-    }
-  }
-  return false;
-}
-
 // Checks for code after an unreachable element.
 static bool hasDeadCode(Block* block) {
   auto& list = block->list;
@@ -206,11 +197,6 @@ static bool optimizeDroppedBlock(Drop* drop,
                                  PassOptions& options,
                                  BranchUtils::BranchSeekerCache& branchInfo) {
   assert(drop->value == block);
-  if (hasUnreachableChild(block)) {
-    // Don't move around unreachable code, as it can change types (leave it for
-    // DCE).
-    return false;
-  }
   if (block->name.is()) {
     // There may be breaks: see if we can remove their values.
     Expression* expression = block;
@@ -241,8 +227,8 @@ static bool optimizeDroppedBlock(Drop* drop,
   return true;
 }
 
-// Core block optimizer routine.
-static void optimizeBlock(Block* curr,
+// Core block optimizer routine. Returns true when we optimize.
+static bool optimizeBlock(Block* curr,
                           Module* module,
                           PassOptions& passOptions,
                           BranchUtils::BranchSeekerCache& branchInfo) {
@@ -412,6 +398,7 @@ static void optimizeBlock(Block* curr,
   if (changed) {
     curr->finalize(curr->type);
   }
+  return changed;
 }
 
 void BreakValueDropper::visitBlock(Block* curr) {
@@ -427,6 +414,8 @@ struct MergeBlocks
     return std::make_unique<MergeBlocks>();
   }
 
+  bool refinalize = false;
+
   BranchUtils::BranchSeekerCache branchInfo;
 
   void visitBlock(Block* curr) {
@@ -438,6 +427,7 @@ struct MergeBlocks
       if (optimizeDroppedBlock(
             curr, block, *getModule(), getPassOptions(), branchInfo)) {
         replaceCurrent(block);
+        refinalize = true;
       }
     }
   }
@@ -485,13 +475,6 @@ struct MergeBlocks
     }
     if (auto* block = child->dynCast<Block>()) {
       if (!block->name.is() && block->list.size() >= 2) {
-        // if we move around unreachable code, type changes could occur. avoid
-        // that, as anyhow it means we should have run dce before getting here
-        if (curr->type == Type::none && hasUnreachableChild(block)) {
-          // moving the block to the outside would replace a none with an
-          // unreachable
-          return outer;
-        }
         auto* back = block->list.back();
         if (back->type == Type::unreachable) {
           // curr is not reachable, dce could remove it; don't try anything
@@ -510,6 +493,7 @@ struct MergeBlocks
           return outer;
         }
         child = back;
+        refinalize = true;
         if (outer == nullptr) {
           // reuse the block, move it out
           block->list.back() = curr;
@@ -600,8 +584,7 @@ struct MergeBlocks
       // too small for us to remove anything from (we cannot remove the last
       // element), or if it has unreachable code (leave that for dce), then give
       // up.
-      if (!block || block->name.is() || block->list.size() <= 1 ||
-          hasUnreachableChild(block)) {
+      if (!block || block->name.is() || block->list.size() <= 1) {
         continueEarly();
         continue;
       }
@@ -682,6 +665,7 @@ struct MergeBlocks
       outerBlock->list.push_back(curr);
       outerBlock->finalize(curr->type);
       replaceCurrent(outerBlock);
+      refinalize = true;
     }
   }
 
@@ -700,6 +684,12 @@ struct MergeBlocks
       outer = optimize(curr, curr->operands[i], outer);
     }
   }
+
+  void visitFunction(Function* curr) {
+    if (refinalize) {
+      ReFinalize().walkFunctionInModule(curr, getModule());
+    }
+  }
 };
 
 Pass* createMergeBlocksPass() { return new MergeBlocks(); }

test/lit/passes/merge-blocks.wast

@@ -405,12 +405,11 @@
  )
 
  ;; CHECK:      (func $toplevel (type $4)
- ;; CHECK-NEXT:  (drop
- ;; CHECK-NEXT:   (block $label (result i32)
- ;; CHECK-NEXT:    (br $label
- ;; CHECK-NEXT:     (i32.const 42)
- ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:  (block $label
+ ;; CHECK-NEXT:   (drop
+ ;; CHECK-NEXT:    (i32.const 42)
  ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:   (br $label)
  ;; CHECK-NEXT:  )
  ;; CHECK-NEXT: )
  (func $toplevel

test/lit/passes/monomorphize-drop.wast

@@ -672,12 +672,7 @@
 
 ;; CAREFUL:      (func $return-normal_4 (type $1)
 ;; CAREFUL-NEXT:  (drop
-;; CAREFUL-NEXT:   (block
-;; CAREFUL-NEXT:    (drop
-;; CAREFUL-NEXT:     (call $import)
-;; CAREFUL-NEXT:    )
-;; CAREFUL-NEXT:    (return)
-;; CAREFUL-NEXT:   )
+;; CAREFUL-NEXT:   (call $import)
 ;; CAREFUL-NEXT:  )
 ;; CAREFUL-NEXT: )
 

test/passes/Oz_fuzz-exec_all-features.txt

@@ -183,14 +183,10 @@
   (nop)
  )
  (func $br-on_non_null-2 (type $void_func)
-  (drop
-   (block
-    (call $log
-     (i32.const 1)
-    )
-    (unreachable)
-   )
+  (call $log
+   (i32.const 1)
   )
+  (unreachable)
  )
  (func $cast-on-func (type $void_func)
   (call $log

test/passes/merge-blocks.txt

@@ -10,11 +10,12 @@
   )
  )
  (func $drop-block-br
-  (drop
-   (block $x (result i32)
-    (br $x
-     (i32.const 1)
-    )
+  (block $x
+   (drop
+    (i32.const 1)
+   )
+   (br $x)
+   (drop
     (i32.const 0)
    )
   )
@@ -77,15 +78,9 @@
   )
  )
  (func $drop-block-squared-iloop
-  (drop
-   (block $label$0 (result i32)
-    (drop
-     (block $label$1
-      (loop $label$2
-       (br $label$2)
-      )
-     )
-    )
+  (block $label$0
+   (loop $label$2
+    (br $label$2)
    )
   )
  )
@@ -109,11 +104,7 @@
  (func $loop-block-drop-block-return
   (loop $label$4
    (block $label$5
-    (drop
-     (block $label$6 (result i32)
-      (return)
-     )
-    )
+    (return)
    )
   )
  )