More refactoring of branch utility code to remove boilerplate. (#3448)

This is almost NFC, but it may emit slightly different IR in cases that
don't matter much. Specifically,

(block (result i32) ;; can also be unreachable
  (unreachable)
  (i32.const 1)
)

That can be finalized to have type unreachable or i32, as both are
valid. After this PR we should consistently do the same thing in all
places. (Either option would be ok - we prefer to keep the type if
there is one.)

In practice, DCE will remove all the dead code anyhow, leaving no
difference to matter. However, the IR is different without DCE, and
that may be noticeable in an unoptimized build - but it should have
no effect on behavior, just on the binary.

2 files changed, 38 insertions, 101 deletions

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index 66715686e..a4d652608 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -3154,8 +3154,9 @@ void WasmBinaryBuilder::visitBlock(Block* curr) {
     }
     pushBlockElements(curr, curr->type, start);
     curr->finalize(curr->type,
-                   breakTargetNames.find(curr->name) !=
-                     breakTargetNames.end() /* hasBreak */);
+                   breakTargetNames.find(curr->name) != breakTargetNames.end()
+                     ? Block::HasBreak
+                     : Block::NoBreak);
     breakStack.pop_back();
     breakTargetNames.erase(curr->name);
   }
diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp
index 851e6a804..d9ca03185 100644
--- a/src/wasm/wasm.cpp
+++ b/src/wasm/wasm.cpp
@@ -267,72 +267,12 @@ Literals getLiteralsFromConstExpression(Expression* curr) {
   }
 }
 
-// core AST type checking
-
-struct TypeSeeker : public PostWalker<TypeSeeker> {
-  Expression* target; // look for this one
-  Name targetName;
-  std::vector<Type> types;
-
-  TypeSeeker(Expression* target, Name targetName)
-    : target(target), targetName(targetName) {
-    Expression* temp = target;
-    walk(temp);
-  }
-
-  void visitBreak(Break* curr) {
-    if (curr->name == targetName) {
-      types.push_back(curr->value ? curr->value->type : Type::none);
-    }
-  }
-
-  void visitSwitch(Switch* curr) {
-    for (auto name : curr->targets) {
-      if (name == targetName) {
-        types.push_back(curr->value ? curr->value->type : Type::none);
-      }
-    }
-    if (curr->default_ == targetName) {
-      types.push_back(curr->value ? curr->value->type : Type::none);
-    }
-  }
-
-  void visitBrOnExn(BrOnExn* curr) {
-    if (curr->name == targetName) {
-      types.push_back(curr->sent);
-    }
-  }
-
-  void visitBlock(Block* curr) {
-    if (curr == target) {
-      if (curr->list.size() > 0) {
-        types.push_back(curr->list.back()->type);
-      } else {
-        types.push_back(Type::none);
-      }
-    } else if (curr->name == targetName) {
-      // ignore all breaks til now, they were captured by someone with the same
-      // name
-      types.clear();
-    }
-  }
-
-  void visitLoop(Loop* curr) {
-    if (curr == target) {
-      types.push_back(curr->body->type);
-    } else if (curr->name == targetName) {
-      // ignore all breaks til now, they were captured by someone with the same
-      // name
-      types.clear();
-    }
-  }
-};
-
 // a block is unreachable if one of its elements is unreachable,
 // and there are no branches to it
-static void handleUnreachable(Block* block,
-                              bool breakabilityKnown = false,
-                              bool hasBreak = false) {
+
+static void
+handleUnreachable(Block* block,
+                  Block::Breakability breakability = Block::Unknown) {
   if (block->type == Type::unreachable) {
     return; // nothing to do
   }
@@ -350,10 +290,12 @@ static void handleUnreachable(Block* block,
     if (child->type == Type::unreachable) {
       // there is an unreachable child, so we are unreachable, unless we have a
       // break
-      if (!breakabilityKnown) {
-        hasBreak = BranchUtils::BranchSeeker::has(block, block->name);
+      if (breakability == Block::Unknown) {
+        breakability = BranchUtils::BranchSeeker::has(block, block->name)
+                         ? Block::HasBreak
+                         : Block::NoBreak;
       }
-      if (!hasBreak) {
+      if (breakability == Block::NoBreak) {
         block->type = Type::unreachable;
       }
       return;
@@ -362,41 +304,35 @@ static void handleUnreachable(Block* block,
 }
 
 void Block::finalize() {
+  if (list.size() == 0) {
+    type = Type::none;
+    return;
+  }
+  // The default type is what is at the end. Next we need to see if breaks and/
+  // or unreachabitily change that.
+  type = list.back()->type;
   if (!name.is()) {
-    if (list.size() > 0) {
-      // nothing branches here, so this is easy
-      // normally the type is the type of the final child
-      type = list.back()->type;
-      // and even if we have an unreachable child somewhere,
-      // we still mark ourselves as having that type,
-      // (block (result i32)
-      //  (return)
-      //  (i32.const 10)
-      // )
-      if (type.isConcrete()) {
-        return;
-      }
-      // if we are unreachable, we are done
-      if (type == Type::unreachable) {
-        return;
-      }
-      // we may still be unreachable if we have an unreachable
-      // child
-      for (auto* child : list) {
-        if (child->type == Type::unreachable) {
-          type = Type::unreachable;
-          return;
-        }
-      }
+    // Nothing branches here, so this is easy.
+    handleUnreachable(this, NoBreak);
+    return;
+  }
+
+  // The default type is according to the value that flows out.
+  BranchUtils::BranchSeeker seeker(this->name);
+  Expression* temp = this;
+  seeker.walk(temp);
+  if (seeker.found) {
+    // Take the branch values into account.
+    if (seeker.valueType != Type::none) {
+      type = Type::getLeastUpperBound(type, seeker.valueType);
     } else {
+      // No value is sent, but as we have a branch we are not unreachable.
       type = Type::none;
     }
-    return;
+  } else {
+    // There are no branches, so this block may be unreachable.
+    handleUnreachable(this, NoBreak);
   }
-
-  TypeSeeker seeker(this, this->name);
-  type = Type::mergeTypes(seeker.types);
-  handleUnreachable(this);
 }
 
 void Block::finalize(Type type_) {
@@ -406,10 +342,10 @@ void Block::finalize(Type type_) {
   }
 }
 
-void Block::finalize(Type type_, bool hasBreak) {
+void Block::finalize(Type type_, Breakability breakability) {
   type = type_;
   if (type == Type::none && list.size() > 0) {
-    handleUnreachable(this, true, hasBreak);
+    handleUnreachable(this, breakability);
   }
 }