[EH] Fix pop enclosed within a block in DCE (#6922)

#6400 fixed this case but that handled only when a `pop` is an
immediate child of the current expression, but a `pop` can be nested
deeper down.

We conservatively run the EH fixup whenever we have a `pop` and create
`block`s in the optimization. We considered using `FindAll<Pop>` to make
it precise, but we decided the quadratic time plexity was not worth it.

Fixes #6918.

1 files changed, 13 insertions, 12 deletions

diff --git a/src/passes/DeadCodeElimination.cpp b/src/passes/DeadCodeElimination.cpp
index a3667376f..c82c73998 100644
--- a/src/passes/DeadCodeElimination.cpp
+++ b/src/passes/DeadCodeElimination.cpp
@@ -56,6 +56,10 @@ struct DeadCodeElimination
   // as we remove code, we must keep the types of other nodes valid
   TypeUpdater typeUpdater;
 
+  // Information used to decide whether we need EH fixups at the end
+  bool hasPop = false;     // Do we have a 'pop' in this function?
+  bool addedBlock = false; // Have we added blocks in this function?
+
   Expression* replaceCurrent(Expression* expression) {
     auto* old = getCurrent();
     if (old == expression) {
@@ -73,6 +77,10 @@ struct DeadCodeElimination
   }
 
   void visitExpression(Expression* curr) {
+    if (curr->is<Pop>()) {
+      hasPop = true;
+    }
+
     if (!Properties::isControlFlowStructure(curr)) {
       // Control flow structures require special handling, but others are
       // simple.
@@ -90,11 +98,7 @@ struct DeadCodeElimination
           Builder builder(*getModule());
           std::vector<Expression*> remainingChildren;
           bool afterUnreachable = false;
-          bool hasPop = false;
           for (auto* child : ChildIterator(curr)) {
-            if (child->is<Pop>()) {
-              hasPop = true;
-            }
             if (afterUnreachable) {
               typeUpdater.noteRecursiveRemoval(child);
               continue;
@@ -109,12 +113,8 @@ struct DeadCodeElimination
           if (remainingChildren.size() == 1) {
             replaceCurrent(remainingChildren[0]);
           } else {
+            addedBlock = true;
             replaceCurrent(builder.makeBlock(remainingChildren));
-            if (hasPop) {
-              // We are moving a pop into a new block we just created, which
-              // means we may need to fix things up here.
-              needEHFixups = true;
-            }
           }
         }
       }
@@ -196,10 +196,11 @@ struct DeadCodeElimination
     }
   }
 
-  bool needEHFixups = false;
-
   void visitFunction(Function* curr) {
-    if (needEHFixups) {
+    // We conservatively run the EH pop fixup if this function has a 'pop' and
+    // if we have ever added blocks in the optimization, which may have moved
+    // pops into the blocks.
+    if (hasPop && addedBlock) {
       EHUtils::handleBlockNestedPops(curr, *getModule());
     }
   }