Remove TypeUpdater in Vacuum and always ReFinalize (#5707)

TypeUpdater::remove must be called after removing a thing from the
tree. If not, then we can get confused by something like this:

(block $b
  (br $b)
)

If we first call TypeUpdater::remove then we see that the block's only
br is going away, so it becomes unreachable. But when we then remove
the br then the block should have type none. Removing the br first
from the IR, and then calling TypeUpdater::remove, is the safe way to do it.

However, changing that order in Vacuum is not trivial. After looking into
this, I realized that it is just simpler to remove TypeUpdater entirely.
Instead, we can ReFinalize at the end unconditionally. This has the downside
that we do not propagate type updates "as we go", but that should be very
rare.

Another downside is that TypeUpdater tracks the number of brs, which
can help remove code like in the one test that regresses here (see comment
there). But I'm not sure that removal was valid - Vacuum should not really be
doing it, and it looks like its related to this bug actually. Instead, we have a
dedicated pass for removing unused brs - RemoveUnusedBrs - so leave
things for it.

This PR's benefit, aside from now handling the fuzz testcase, is that it makes
the code simpler and faster. I see a 10-25% speedup on the Vacuum pass on
various binaries I tested on. (Vacuum is one of our faster passes anyhow,
though, so the runtime of -O1 is not much improved.)

Another minor benefit might be that running ReFinalize more often can
propagate type info more quickly, thanks to #5704 etc. But that is probably
very minor.

1 files changed, 1 insertions, 48 deletions

diff --git a/src/passes/Vacuum.cpp b/src/passes/Vacuum.cpp
index 7c394900d..8d89553b2 100644
--- a/src/passes/Vacuum.cpp
+++ b/src/passes/Vacuum.cpp
@@ -23,7 +23,6 @@
 #include <ir/effects.h>
 #include <ir/iteration.h>
 #include <ir/literal-utils.h>
-#include <ir/type-updating.h>
 #include <ir/utils.h>
 #include <pass.h>
 #include <wasm-builder.h>
@@ -36,32 +35,9 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
 
   std::unique_ptr<Pass> create() override { return std::make_unique<Vacuum>(); }
 
-  TypeUpdater typeUpdater;
-
-  // The TypeUpdater class handles efficient updating of unreachability as we
-  // go, but we may also refine types, which requires refinalization.
-  bool refinalize = false;
-
-  Expression* replaceCurrent(Expression* expression) {
-    auto* old = getCurrent();
-    if (expression->type != old->type &&
-        expression->type != Type::unreachable) {
-      // We are changing this to a new type that is not unreachable, so it is a
-      // refinement that we need to use refinalize to propagate up.
-      refinalize = true;
-    }
-    super::replaceCurrent(expression);
-    // also update the type updater
-    typeUpdater.noteReplacement(old, expression);
-    return expression;
-  }
-
   void doWalkFunction(Function* func) {
-    typeUpdater.walk(func->body);
     walk(func->body);
-    if (refinalize) {
-      ReFinalize().walkFunctionInModule(func, getModule());
-    }
+    ReFinalize().walkFunctionInModule(func, getModule());
   }
 
   // Returns nullptr if curr is dead, curr if it must stay as is, or one of its
@@ -185,11 +161,9 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
         }
       }
       if (!optimized) {
-        typeUpdater.noteRecursiveRemoval(child);
         skip++;
       } else {
         if (optimized != child) {
-          typeUpdater.noteReplacement(child, optimized);
           list[z] = optimized;
         }
         if (skip > 0) {
@@ -198,14 +172,7 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
         }
         // if this is unreachable, the rest is dead code
         if (list[z - skip]->type == Type::unreachable && z < size - 1) {
-          for (Index i = z - skip + 1; i < list.size(); i++) {
-            auto* remove = list[i];
-            if (remove) {
-              typeUpdater.noteRecursiveRemoval(remove);
-            }
-          }
           list.resize(z - skip + 1);
-          typeUpdater.maybeUpdateTypeToUnreachable(curr);
           skip = 0; // nothing more to do on the list
           break;
         }
@@ -213,7 +180,6 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
     }
     if (skip > 0) {
       list.resize(size - skip);
-      typeUpdater.maybeUpdateTypeToUnreachable(curr);
     }
     // the block may now be a trivial one that we can get rid of and just leave
     // its contents
@@ -227,15 +193,10 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
       Expression* child;
       if (value->value.getInteger()) {
         child = curr->ifTrue;
-        if (curr->ifFalse) {
-          typeUpdater.noteRecursiveRemoval(curr->ifFalse);
-        }
       } else {
         if (curr->ifFalse) {
           child = curr->ifFalse;
-          typeUpdater.noteRecursiveRemoval(curr->ifTrue);
         } else {
-          typeUpdater.noteRecursiveRemoval(curr);
           ExpressionManipulator::nop(curr);
           return;
         }
@@ -245,10 +206,6 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
     }
     // if the condition is unreachable, just return it
     if (curr->condition->type == Type::unreachable) {
-      typeUpdater.noteRecursiveRemoval(curr->ifTrue);
-      if (curr->ifFalse) {
-        typeUpdater.noteRecursiveRemoval(curr->ifFalse);
-      }
       replaceCurrent(curr->condition);
       return;
     }
@@ -384,9 +341,6 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
     // the try's body.
     if (!EffectAnalyzer(getPassOptions(), *getModule(), curr->body).throws()) {
       replaceCurrent(curr->body);
-      for (auto* catchBody : curr->catchBodies) {
-        typeUpdater.noteRecursiveRemoval(catchBody);
-      }
       return;
     }
 
@@ -398,7 +352,6 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
     if (curr->type == Type::none && curr->hasCatchAll() &&
         !EffectAnalyzer(getPassOptions(), *getModule(), curr)
            .hasUnremovableSideEffects()) {
-      typeUpdater.noteRecursiveRemoval(curr);
       ExpressionManipulator::nop(curr);
     }
   }