loops no longer have an out label and other upstream loop updates

18 files changed, 86 insertions, 129 deletions

diff --git a/src/asm2wasm.h b/src/asm2wasm.h
index 857145937..2d0d1300e 100644
--- a/src/asm2wasm.h
+++ b/src/asm2wasm.h
@@ -1478,8 +1478,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
         out = getNextId("while-out");
         in = getNextId("while-in");
       }
-      ret->out = out;
-      ret->in = in;
+      ret->name = in;
       breakStack.push_back(out);
       continueStack.push_back(in);
       if (forever) {
@@ -1497,9 +1496,9 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
         ret->body = body;
       }
       // loops do not automatically loop, add a branch back
-      Block* block = blockify(ret->body);
+      Block* block = builder.blockifyWithName(ret->body, out);
       auto continuer = allocator.alloc<Break>();
-      continuer->name = ret->in;
+      continuer->name = ret->name;
       block->list.push_back(continuer);
       ret->body = block;
       continueStack.pop_back();
@@ -1536,13 +1535,12 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
         } else {
           auto loop = allocator.alloc<Loop>();
           loop->body = child;
-          loop->out = stop;
-          loop->in = more;
-          return loop;
+          loop->name = more;
+          return builder.blockifyWithName(loop, stop);
         }
       }
       // general do-while loop
-      auto ret = allocator.alloc<Loop>();
+      auto loop = allocator.alloc<Loop>();
       IString out, in;
       if (!parentLabel.isNull()) {
         out = getBreakLabelName(parentLabel);
@@ -1552,20 +1550,18 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
         out = getNextId("do-out");
         in = getNextId("do-in");
       }
-      ret->out = out;
-      ret->in = in;
+      loop->name = in;
       breakStack.push_back(out);
       continueStack.push_back(in);
-      ret->body = process(ast[2]);
+      loop->body = process(ast[2]);
       continueStack.pop_back();
       breakStack.pop_back();
       Break *continuer = allocator.alloc<Break>();
       continuer->name = in;
       continuer->condition = process(ast[1]);
-      Block *block = blockify(ret->body);
-      block->list.push_back(continuer);
-      ret->body = block;
-      return ret;
+      Block *block = builder.blockifyWithName(loop->body, out, continuer);
+      loop->body = block;
+      return loop;
     } else if (what == FOR) {
       Ref finit = ast[1],
           fcond = ast[2],
@@ -1581,8 +1577,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
         out = getNextId("for-out");
         in = getNextId("for-in");
       }
-      ret->out = out;
-      ret->in = in;
+      ret->name = in;
       breakStack.push_back(out);
       continueStack.push_back(in);
       Break *breakOut = allocator.alloc<Break>();
@@ -1597,10 +1592,9 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
       body->finalize();
       ret->body = body;
       // loops do not automatically loop, add a branch back
-      Block* block = blockify(ret->body);
       auto continuer = allocator.alloc<Break>();
-      continuer->name = ret->in;
-      block->list.push_back(continuer);
+      continuer->name = ret->name;
+      Block* block = builder.blockifyWithName(ret->body, out, continuer);
       ret->body = block;
       continueStack.pop_back();
       breakStack.pop_back();
diff --git a/src/ast_utils.h b/src/ast_utils.h
index 17f5490a9..30e1d9a36 100644
--- a/src/ast_utils.h
+++ b/src/ast_utils.h
@@ -148,8 +148,7 @@ struct EffectAnalyzer : public PostWalker<EffectAnalyzer, Visitor<EffectAnalyzer
     if (curr->name.is()) breakNames.erase(curr->name); // these were internal breaks
   }
   void visitLoop(Loop* curr) {
-    if (curr->in.is()) breakNames.erase(curr->in); // these were internal breaks
-    if (curr->out.is()) breakNames.erase(curr->out); // these were internal breaks
+    if (curr->name.is()) breakNames.erase(curr->name); // these were internal breaks
   }
 
   void visitCall(Call *curr) { calls = true; }
@@ -245,7 +244,7 @@ struct ExpressionManipulator {
         return builder.makeIf(copy(curr->condition), copy(curr->ifTrue), copy(curr->ifFalse));
       }
       Expression* visitLoop(Loop *curr) {
-        return builder.makeLoop(curr->out, curr->in, copy(curr->body));
+        return builder.makeLoop(curr->name, copy(curr->body));
       }
       Expression* visitBreak(Break *curr) {
         return builder.makeBreak(curr->name, copy(curr->value), copy(curr->condition));
@@ -438,8 +437,7 @@ struct ExpressionAnalyzer {
           break;
         }
         case Expression::Id::LoopId: {
-          if (!noteNames(left->cast<Loop>()->out, right->cast<Loop>()->out)) return false;
-          if (!noteNames(left->cast<Loop>()->in, right->cast<Loop>()->in)) return false;
+          if (!noteNames(left->cast<Loop>()->name, right->cast<Loop>()->name)) return false;
           PUSH(Loop, body);
           break;
         }
@@ -655,8 +653,7 @@ struct ExpressionAnalyzer {
           break;
         }
         case Expression::Id::LoopId: {
-          noteName(curr->cast<Loop>()->out);
-          noteName(curr->cast<Loop>()->in);
+          noteName(curr->cast<Loop>()->name);
           PUSH(Loop, body);
           break;
         }
diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp
index 71c6cad80..8703237d5 100644
--- a/src/binaryen-c.cpp
+++ b/src/binaryen-c.cpp
@@ -338,16 +338,13 @@ BinaryenExpressionRef BinaryenIf(BinaryenModuleRef module, BinaryenExpressionRef
 
   return static_cast<Expression*>(ret);
 }
-BinaryenExpressionRef BinaryenLoop(BinaryenModuleRef module, const char* out, const char* in, BinaryenExpressionRef body) {
-  if (out && !in) abort();
-  auto* ret = Builder(*((Module*)module)).makeLoop(out ? Name(out) : Name(), in ? Name(in) : Name(), (Expression*)body);
+BinaryenExpressionRef BinaryenLoop(BinaryenModuleRef module, const char* name, BinaryenExpressionRef body) {
+  auto* ret = Builder(*((Module*)module)).makeLoop(name ? Name(name) : Name(), (Expression*)body);
 
   if (tracing) {
     auto id = noteExpression(ret);
     std::cout << "    expressions[" << id << "] = BinaryenLoop(the_module, ";
-    traceNameOrNULL(out);
-    std::cout << ", ";
-    traceNameOrNULL(in);
+    traceNameOrNULL(name);
     std::cout << ", expressions[" << expressions[body] << "]);\n";
   }
 
diff --git a/src/binaryen-c.h b/src/binaryen-c.h
index 7ca5cd6c7..5d7520a77 100644
--- a/src/binaryen-c.h
+++ b/src/binaryen-c.h
@@ -265,8 +265,7 @@ typedef void* BinaryenExpressionRef;
 BinaryenExpressionRef BinaryenBlock(BinaryenModuleRef module, const char* name, BinaryenExpressionRef* children, BinaryenIndex numChildren);
 // If: ifFalse can be NULL
 BinaryenExpressionRef BinaryenIf(BinaryenModuleRef module, BinaryenExpressionRef condition, BinaryenExpressionRef ifTrue, BinaryenExpressionRef ifFalse);
-// Loop: both out and in can be NULL, or just out can be NULL
-BinaryenExpressionRef BinaryenLoop(BinaryenModuleRef module, const char* out, const char* in, BinaryenExpressionRef body);
+BinaryenExpressionRef BinaryenLoop(BinaryenModuleRef module, const char* in, BinaryenExpressionRef body);
 // Break: value and condition can be NULL
 BinaryenExpressionRef BinaryenBreak(BinaryenModuleRef module, const char* name, BinaryenExpressionRef condition, BinaryenExpressionRef value);
 // Switch: value can be NULL
diff --git a/src/cfg/Relooper.cpp b/src/cfg/Relooper.cpp
index e75adbce5..8a5337ed0 100644
--- a/src/cfg/Relooper.cpp
+++ b/src/cfg/Relooper.cpp
@@ -383,7 +383,7 @@ wasm::Expression* MultipleShape::Render(RelooperBuilder& Builder, bool InLoop) {
 // LoopShape
 
 wasm::Expression* LoopShape::Render(RelooperBuilder& Builder, bool InLoop) {
-  wasm::Expression* Ret = Builder.makeLoop(wasm::Name(), Builder.getShapeContinueName(Id), Inner->Render(Builder, true));
+  wasm::Expression* Ret = Builder.makeLoop(Builder.getShapeContinueName(Id), Inner->Render(Builder, true));
   Ret = HandleFollowupMultiples(Ret, this, Builder, InLoop);
   if (Next) {
     Ret = Builder.makeSequence(Ret, Next->Render(Builder, InLoop));
diff --git a/src/cfg/cfg-traversal.h b/src/cfg/cfg-traversal.h
index d690de4aa..2b96fc67a 100644
--- a/src/cfg/cfg-traversal.h
+++ b/src/cfg/cfg-traversal.h
@@ -130,23 +130,15 @@ struct CFGWalker : public PostWalker<SubType, VisitorType> {
     auto* last = self->currBasicBlock;
     doStartBasicBlock(self, currp);
     self->link(last, self->currBasicBlock); // fallthrough
-    // branches to the new one
     auto* curr = (*currp)->cast<Loop>();
-    if (curr->out.is()) {
-      auto& origins = self->branches[curr->out];
-      for (auto* origin : origins) {
-        self->link(origin, self->currBasicBlock);
-      }
-      self->branches.erase(curr->out);
-    }
     // branches to the top of the loop
-    if (curr->in.is()) {
+    if (curr->name.is()) {
       auto* loopStart = self->loopStack.back();
-      auto& origins = self->branches[curr->in];
+      auto& origins = self->branches[curr->name];
       for (auto* origin : origins) {
         self->link(origin, loopStart);
       }
-      self->branches.erase(curr->in);
+      self->branches.erase(curr->name);
     }
     self->loopStack.pop_back();
   }
diff --git a/src/passes/DeadCodeElimination.cpp b/src/passes/DeadCodeElimination.cpp
index 866b3cb73..b30b8ffbd 100644
--- a/src/passes/DeadCodeElimination.cpp
+++ b/src/passes/DeadCodeElimination.cpp
@@ -132,12 +132,8 @@ struct DeadCodeElimination : public WalkerPass<PostWalker<DeadCodeElimination, V
   }
 
   void visitLoop(Loop* curr) {
-    if (curr->in.is()) {
-      reachableBreaks.erase(curr->in);
-    }
-    if (curr->out.is()) {
-      reachable = reachable || reachableBreaks.count(curr->out);
-      reachableBreaks.erase(curr->out);
+    if (curr->name.is()) {
+      reachableBreaks.erase(curr->name);
     }
     if (isDead(curr->body)) {
       replaceCurrent(curr->body);
diff --git a/src/passes/NameManager.cpp b/src/passes/NameManager.cpp
index df8b34557..9f0198c2f 100644
--- a/src/passes/NameManager.cpp
+++ b/src/passes/NameManager.cpp
@@ -37,8 +37,7 @@ void NameManager::visitBlock(Block* curr) {
   names.insert(curr->name);
 }
 void NameManager::visitLoop(Loop* curr) {
-  names.insert(curr->out);
-  names.insert(curr->in);
+  names.insert(curr->name);
 }
 void NameManager::visitBreak(Break* curr) {
   names.insert(curr->name);
diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp
index e54468c41..4fb9274c7 100644
--- a/src/passes/Print.cpp
+++ b/src/passes/Print.cpp
@@ -64,7 +64,9 @@ struct PrintSExpression : public Visitor<PrintSExpression> {
   }
   void printFullLine(Expression *expression) {
     !minify && doIndent(o, indent);
-    //o << "[" << printWasmType(expression->type) << "] "; // debugging tool
+#ifdef DEBUG_TYPES
+    o << "[" << printWasmType(expression->type) << "] ";
+#endif
     visit(expression);
     o << maybeNewLine;
   }
@@ -152,12 +154,8 @@ struct PrintSExpression : public Visitor<PrintSExpression> {
   }
   void visitLoop(Loop *curr) {
     printOpening(o, "loop");
-    if (curr->out.is()) {
-      o << ' ' << curr->out;
-      assert(curr->in.is()); // if just one is printed, it must be the in
-    }
-    if (curr->in.is()) {
-      o << ' ' << curr->in;
+    if (curr->name.is()) {
+      o << ' ' << curr->name;
     }
     incIndent();
     auto block = curr->body->dynCast<Block>();
@@ -734,7 +732,9 @@ Pass *createFullPrinterPass() {
 std::ostream& WasmPrinter::printExpression(Expression* expression, std::ostream& o, bool minify) {
   PrintSExpression print(o);
   print.setMinify(minify);
-  //o << "[" << printWasmType(expression->type) << "] "; // debugging tool
+#ifdef DEBUG_TYPES
+  o << "[" << printWasmType(expression->type) << "] ";
+#endif
   print.visit(expression);
   return o;
 }
diff --git a/src/passes/RemoveUnusedNames.cpp b/src/passes/RemoveUnusedNames.cpp
index 9c6743479..8e24f9549 100644
--- a/src/passes/RemoveUnusedNames.cpp
+++ b/src/passes/RemoveUnusedNames.cpp
@@ -76,34 +76,12 @@ struct RemoveUnusedNames : public WalkerPass<PostWalker<RemoveUnusedNames, Visit
       }
     }
     handleBreakTarget(curr->name);
-    if (curr->name.is() && curr->list.size() == 1) {
-      auto* child = curr->list[0]->dynCast<Loop>();
-      if (child && !child->out.is()) {
-        // we have just one child, this loop, and it lacks an out label. So this block's name is doing just that!
-        child->out = curr->name;
-        replaceCurrent(child);
-      }
-    }
   }
 
   void visitLoop(Loop *curr) {
-    handleBreakTarget(curr->in);
-    // Loops can have just 'in', but cannot have just 'out'
-    auto out = curr->out;
-    handleBreakTarget(curr->out);
-    if (curr->out.is() && !curr->in.is()) {
-      auto* block = getModule()->allocator.alloc<Block>();
-      block->name = out;
-      block->list.push_back(curr->body);
-      replaceCurrent(block);
-    }
-    if (curr->in.is() && !curr->out.is()) {
-      auto* child = curr->body->dynCast<Block>();
-      if (child && child->name.is()) {
-        // we have just one child, this block, and we lack an out label. So we can take the block's!
-        curr->out = child->name;
-        child->name = Name();
-      }
+    handleBreakTarget(curr->name);
+    if (!curr->name.is()) {
+      replaceCurrent(curr->body);
     }
   }
 
diff --git a/src/s2wasm.h b/src/s2wasm.h
index 5226ceda3..8c04f7891 100644
--- a/src/s2wasm.h
+++ b/src/s2wasm.h
@@ -1064,7 +1064,7 @@ class S2WasmBuilder {
       if (target->is<Block>()) {
         return target->cast<Block>()->name;
       } else {
-        return target->cast<Loop>()->in;
+        return target->cast<Loop>()->name;
       }
     };
     // fixups
@@ -1099,10 +1099,9 @@ class S2WasmBuilder {
       } else if (match("loop")) {
         auto curr = allocator->alloc<Loop>();
         addToBlock(curr);
-        curr->in = getNextLabel();
-        curr->out = getNextLabel();
+        curr->name = getNextLabel();
         auto block = allocator->alloc<Block>();
-        block->name = curr->out; // temporary, fake - this way, on bstack we have the right label at the right offset for a br
+        block->name = getNextLabel();
         curr->body = block;
         loopBlocks.push_back(block);
         bstack.push_back(block);
diff --git a/src/wasm-binary.h b/src/wasm-binary.h
index 791a0459b..373465296 100644
--- a/src/wasm-binary.h
+++ b/src/wasm-binary.h
@@ -875,11 +875,9 @@ public:
   void visitLoop(Loop *curr) {
     if (debug) std::cerr << "zz node: Loop" << std::endl;
     o << int8_t(BinaryConsts::Loop);
-    breakStack.push_back(curr->out);
-    breakStack.push_back(curr->in);
+    breakStack.push_back(curr->name);
     recursePossibleBlockContents(curr->body);
     breakStack.pop_back();
-    breakStack.pop_back();
     o << int8_t(BinaryConsts::End);
   }
 
@@ -1841,13 +1839,10 @@ public:
   }
   void visitLoop(Loop *curr) {
     if (debug) std::cerr << "zz node: Loop" << std::endl;
-    curr->out = getNextLabel();
-    curr->in = getNextLabel();
-    breakStack.push_back(curr->out);
-    breakStack.push_back(curr->in);
+    curr->name = getNextLabel();
+    breakStack.push_back(curr->name);
     curr->body = getMaybeBlock();
     breakStack.pop_back();
-    breakStack.pop_back();
     curr->finalize();
   }
 
diff --git a/src/wasm-builder.h b/src/wasm-builder.h
index 3cba351a2..e68fd5ef2 100644
--- a/src/wasm-builder.h
+++ b/src/wasm-builder.h
@@ -82,9 +82,9 @@ public:
     ret->finalize();
     return ret;
   }
-  Loop* makeLoop(Name out, Name in, Expression* body) {
+  Loop* makeLoop(Name name, Expression* body) {
     auto* ret = allocator.alloc<Loop>();
-    ret->out = out; ret->in = in; ret->body = body;
+    ret->name = name; ret->body = body;
     ret->finalize();
     return ret;
   }
@@ -265,7 +265,21 @@ public:
     if (!block) block = makeBlock(any);
     if (append) {
       block->list.push_back(append);
-      block->finalize();
+      block->finalize(); // TODO: move out of if
+    }
+    return block;
+  }
+
+  // ensure a node is a block, if it isn't already, and optionally append to the block
+  // this variant sets a name for the block, so it will not reuse a block already named
+  Block* blockifyWithName(Expression* any, Name name, Expression* append = nullptr) {
+    Block* block = nullptr;
+    if (any) block = any->dynCast<Block>();
+    if (!block || block->name.is()) block = makeBlock(any);
+    block->name = name;
+    if (append) {
+      block->list.push_back(append);
+      block->finalize(); // TODO: move out of if
     }
     return block;
   }
diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h
index 77cbd8aab..247d34500 100644
--- a/src/wasm-interpreter.h
+++ b/src/wasm-interpreter.h
@@ -170,8 +170,7 @@ public:
     while (1) {
       Flow flow = visit(curr->body);
       if (flow.breaking()) {
-        if (flow.breakTo == curr->in) continue; // lol
-        flow.clearIf(curr->out);
+        if (flow.breakTo == curr->name) continue; // lol
       }
       return flow; // loop does not loop automatically, only continue achieves that
     }
diff --git a/src/wasm-s-parser.h b/src/wasm-s-parser.h
index 49fd52ef0..28975c7e3 100644
--- a/src/wasm-s-parser.h
+++ b/src/wasm-s-parser.h
@@ -1171,24 +1171,28 @@ private:
   Expression* makeLoop(Element& s) {
     auto ret = allocator.alloc<Loop>();
     size_t i = 1;
+    Name out;
     if (s.size() > i + 1 && s[i]->isStr() && s[i + 1]->isStr()) { // out can only be named if both are
-      ret->out = s[i]->str();
+      out = s[i]->str();
       i++;
-    } else {
-      ret->out = getPrefixedName("loop-out");
     }
     if (s.size() > i && s[i]->isStr()) {
-      ret->in = s[i]->str();
+      ret->name = s[i]->str();
       i++;
     } else {
-      ret->in = getPrefixedName("loop-in");
+      ret->name = getPrefixedName("loop-in");
     }
-    labelStack.push_back(ret->out);
-    labelStack.push_back(ret->in);
+    labelStack.push_back(ret->name);
     ret->body = makeMaybeBlock(s, i);
     labelStack.pop_back();
-    labelStack.pop_back();
     ret->finalize();
+    if (out.is()) {
+      auto* block = allocator.alloc<Block>();
+      block->name = out;
+      block->list.push_back(ret);
+      block->finalize();
+      return block;
+    }
     return ret;
   }
 
diff --git a/src/wasm-validator.h b/src/wasm-validator.h
index ca89270e9..5844dafcd 100644
--- a/src/wasm-validator.h
+++ b/src/wasm-validator.h
@@ -89,16 +89,16 @@ public:
 
   static void visitPreLoop(WasmValidator* self, Expression** currp) {
     auto* curr = (*currp)->cast<Loop>();
-    if (curr->in.is()) self->breakTargets[curr->in].push_back(curr);
-    if (curr->out.is()) self->breakTargets[curr->out].push_back(curr);
+    if (curr->name.is()) self->breakTargets[curr->name].push_back(curr);
   }
 
   void visitLoop(Loop *curr) {
-    if (curr->in.is()) {
-      breakTargets[curr->in].pop_back();
-    }
-    if (curr->out.is()) {
-      breakTargets[curr->out].pop_back();
+    if (curr->name.is()) {
+      breakTargets[curr->name].pop_back();
+      if (breakInfos.count(curr) > 0) {
+        auto& info = breakInfos[curr];
+        shouldBeEqual(info.arity, Index(0), curr, "breaks to a loop cannot pass a value");
+      }
     }
   }
 
diff --git a/src/wasm.cpp b/src/wasm.cpp
index 8cfdee759..f6486eb50 100644
--- a/src/wasm.cpp
+++ b/src/wasm.cpp
@@ -120,7 +120,7 @@ struct TypeSeeker : public PostWalker<TypeSeeker, Visitor<TypeSeeker>> {
   void visitLoop(Loop* curr) {
     if (curr == target) {
       types.push_back(curr->body->type);
-    } else if (curr->in == targetName || curr->out == targetName) {
+    } else if (curr->name == targetName) {
       types.clear(); // ignore all breaks til now, they were captured by someone with the same name
     }
   }
@@ -162,13 +162,7 @@ void Block::finalize() {
 }
 
 void Loop::finalize() {
-  if (!out.is()) {
-    type = body->type;
-    return;
-  }
-
-  TypeSeeker seeker(this, this->out);
-  type = mergeTypes(seeker.types);
+  type = body->type;
 }
 
 } // namespace wasm
diff --git a/src/wasm.h b/src/wasm.h
index ee3e12910..de86ebba0 100644
--- a/src/wasm.h
+++ b/src/wasm.h
@@ -1001,7 +1001,7 @@ public:
   Loop() {}
   Loop(MixedArena& allocator) {}
 
-  Name out, in;
+  Name name;
   Expression *body;
 
   // set the type of a loop if you already know it