[NFC] Remove a goto (#7804)

Move the code skipped over with the goto into a separate function. The
goto used to break out of multiple loops is now a return from that
function.

Author: tlively

src/wasm/wasm-ir-builder.cpp

@@ -411,27 +411,57 @@ struct IRBuilder::ChildPopper
   Result<> popConstrainedChildren(std::vector<Child>& children) {
     auto& scope = builder.getScope();
 
-    // Two-part indices into the stack of available expressions and the vector
-    // of requirements, allowing them to move independently with the granularity
-    // of a single tuple element.
-    size_t stackIndex = scope.exprStack.size();
-    size_t stackTupleIndex = 0;
-    size_t childIndex = children.size();
-    size_t childTupleIndex = 0;
-
-    // The index of the shallowest unreachable instruction on the stack.
+    // The index of the shallowest unreachable instruction on the stack, found
+    // by checkNeedsUnreachableFallback.
     std::optional<size_t> unreachableIndex;
 
     // Whether popping the children past the unreachable would produce a type
     // mismatch or try to pop from an empty stack.
     bool needUnreachableFallback = false;
 
-    if (!scope.unreachable) {
-      // We only need to check requirements if there is an unreachable.
-      // Otherwise the validator will catch any problems.
-      goto pop;
+    // We only need to check requirements if there is an unreachable.
+    // Otherwise the validator will catch any problems.
+    if (scope.unreachable) {
+      needUnreachableFallback =
+        checkNeedsUnreachableFallback(children, unreachableIndex);
     }
 
+    // We have checked all the constraints, so we are ready to pop children.
+    for (int i = children.size() - 1; i >= 0; --i) {
+      if (needUnreachableFallback &&
+          scope.exprStack.size() == *unreachableIndex + 1 && i > 0) {
+        // The next item on the stack is the unreachable instruction we must
+        // not pop past. We cannot insert unreachables in front of it because
+        // it might be a branch we actually have to execute, so this next item
+        // must be child 0. But we are not ready to pop child 0 yet, so
+        // synthesize an unreachable instead of popping. The deeper
+        // instructions that would otherwise have been popped will remain on
+        // the stack to become prior children of future expressions or to be
+        // implicitly dropped at the end of the scope.
+        *children[i].childp = builder.builder.makeUnreachable();
+        continue;
+      }
+
+      // Pop a child normally.
+      auto val = pop(children[i].constraint.size());
+      CHECK_ERR(val);
+      *children[i].childp = *val;
+    }
+    return Ok{};
+  }
+
+  bool checkNeedsUnreachableFallback(const std::vector<Child>& children,
+                                     std::optional<size_t>& unreachableIndex) {
+    auto& scope = builder.getScope();
+
+    // Two-part indices into the stack of available expressions and the vector
+    // of requirements, allowing them to move independently with the granularity
+    // of a single tuple element.
+    size_t stackIndex = scope.exprStack.size();
+    size_t stackTupleIndex = 0;
+    size_t childIndex = children.size();
+    size_t childTupleIndex = 0;
+
     // Check whether the values on the stack will be able to meet the given
     // requirements.
     while (true) {
@@ -458,8 +488,7 @@ struct IRBuilder::ChildPopper
             // the input unreachable instruction is executed first. If we are
             // not reaching past an unreachable, the error will be caught when
             // we pop.
-            needUnreachableFallback = true;
-            goto pop;
+            return true;
           }
           --stackIndex;
           stackTupleIndex = scope.exprStack[stackIndex]->type.size() - 1;
@@ -483,13 +512,11 @@ struct IRBuilder::ChildPopper
           // Always succeeds.
         } else if (constraint.isAnyReference()) {
           if (!type.isRef() && type != Type::unreachable) {
-            needUnreachableFallback = true;
-            break;
+            return true;
           }
         } else if (auto bound = constraint.getSubtype()) {
           if (!Type::isSubType(type, *bound)) {
-            needUnreachableFallback = true;
-            break;
+            return true;
           }
         } else if (constraint.isAnyI8ArrayReference()) {
           bool isI8Array =
@@ -498,8 +525,7 @@ struct IRBuilder::ChildPopper
           bool isNone =
             type.isRef() && type.getHeapType().isMaybeShared(HeapType::none);
           if (!isI8Array && !isNone && type != Type::unreachable) {
-            needUnreachableFallback = true;
-            break;
+            return true;
           }
         } else if (constraint.isAnyI16ArrayReference()) {
           bool isI16Array =
@@ -508,8 +534,7 @@ struct IRBuilder::ChildPopper
           bool isNone =
             type.isRef() && type.getHeapType().isMaybeShared(HeapType::none);
           if (!isI16Array && !isNone && type != Type::unreachable) {
-            needUnreachableFallback = true;
-            break;
+            return true;
           }
         } else {
           WASM_UNREACHABLE("unexpected constraint");
@@ -518,34 +543,10 @@ struct IRBuilder::ChildPopper
 
       // No problems for children after this unreachable.
       if (type == Type::unreachable) {
-        assert(!needUnreachableFallback);
         unreachableIndex = stackIndex;
       }
     }
-
-  pop:
-    // We have checked all the constraints, so we are ready to pop children.
-    for (int i = children.size() - 1; i >= 0; --i) {
-      if (needUnreachableFallback &&
-          scope.exprStack.size() == *unreachableIndex + 1 && i > 0) {
-        // The next item on the stack is the unreachable instruction we must
-        // not pop past. We cannot insert unreachables in front of it because
-        // it might be a branch we actually have to execute, so this next item
-        // must be child 0. But we are not ready to pop child 0 yet, so
-        // synthesize an unreachable instead of popping. The deeper
-        // instructions that would otherwise have been popped will remain on
-        // the stack to become prior children of future expressions or to be
-        // implicitly dropped at the end of the scope.
-        *children[i].childp = builder.builder.makeUnreachable();
-        continue;
-      }
-
-      // Pop a child normally.
-      auto val = pop(children[i].constraint.size());
-      CHECK_ERR(val);
-      *children[i].childp = *val;
-    }
-    return Ok{};
+    return false;
   }
 
   Result<Expression*> pop(size_t size) {