Fix v8 -O0 take 2

src/asm2wasm.h

@@ -565,6 +565,8 @@ class Asm2WasmBuilder {
   }
 
   Function* processFunction(Ref ast);
+
+  void fixFallthrough(Function* func);
 };
 
 void Asm2WasmBuilder::processAsm(Ref ast) {
@@ -689,7 +691,7 @@ void Asm2WasmBuilder::processAsm(Ref ast) {
         passRunner.setValidateGlobally(false);
       }
       // run autodrop first, before optimizations
-      passRunner.add<AutoDrop>();
+      passRunner.add<AutoDrop>(); // TODO: we can likely remove this, and speed up the build a little
       // optimize relooper label variable usage at the wasm level, where it is easy
       passRunner.add("relooper-jump-threading");
     }, debug, false /* do not validate globally yet */);
@@ -1507,8 +1509,9 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
       abort_on("bad unary", ast);
     } else if (what == IF) {
       auto* condition = process(ast[1]);
-      auto* ifTrue = process(ast[2]);
-      return builder.makeIf(truncateToInt32(condition), ifTrue, !!ast[3] ? process(ast[3]) : nullptr);
+      auto* ifTrue = builder.dropIfConcretelyTyped(process(ast[2]));
+      auto* ifFalse = !!ast[3] ? builder.dropIfConcretelyTyped(process(ast[3])) : nullptr;
+      return builder.makeIf(truncateToInt32(condition), ifTrue, ifFalse);
     } else if (what == CALL) {
       if (ast[1]->isString()) {
         IString name = ast[1]->getIString();
@@ -1830,7 +1833,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
           block = allocator.alloc<Block>();
           block->name = name;
           block->list.push_back(ret);
-          block->finalize();
+          block->finalize(none);
           ret = block;
         }
       }
@@ -1874,16 +1877,16 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
         auto body = allocator.alloc<Block>();
         body->list.push_back(condition);
         body->list.push_back(process(ast[2]));
-        body->finalize();
+        body->finalize(none);
         ret->body = body;
       }
       // loops do not automatically loop, add a branch back
       Block* block = builder.blockifyWithName(ret->body, out);
       auto continuer = allocator.alloc<Break>();
       continuer->name = ret->name;
       block->list.push_back(continuer);
-      block->finalize();
-      ret->body = block;
+      block->finalize(none);
+      ret->body = builder.dropIfConcretelyTyped(block);
       ret->finalize();
       continueStack.pop_back();
       breakStack.pop_back();
@@ -1904,7 +1907,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
         Name more = nameMapper.pushLabelName("unlikely-continue");
         breakStack.push_back(stop);
         continueStack.push_back(more);
-        auto child = process(ast[2]);
+        auto child = builder.dropIfConcretelyTyped(process(ast[2]));
         continueStack.pop_back();
         breakStack.pop_back();
         nameMapper.popLabelName(more);
@@ -1919,13 +1922,13 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
             block->list.push_back(builder.makeNop()); // ensure a nop at the end, so the block has guaranteed none type and no values fall through
           }
           block->name = stop;
-          block->finalize();
+          block->finalize(none);
           return block;
         } else {
           auto loop = allocator.alloc<Loop>();
           loop->body = child;
           loop->name = more;
-          loop->finalize();
+          loop->finalize(none);
           return builder.blockifyWithName(loop, stop);
         }
       }
@@ -1945,7 +1948,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
       loop->name = in;
       breakStack.push_back(out);
       continueStack.push_back(in);
-      loop->body = process(ast[2]);
+      loop->body = builder.dropIfConcretelyTyped(process(ast[2]));
       continueStack.pop_back();
       breakStack.pop_back();
       nameMapper.popLabelName(in);
@@ -1955,7 +1958,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
       continuer->condition = process(ast[1]);
       Block *block = builder.blockifyWithName(loop->body, out, continuer);
       loop->body = block;
-      loop->finalize();
+      loop->finalize(none);
       return loop;
     } else if (what == FOR) {
       Ref finit = ast[1],
@@ -1987,8 +1990,8 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
       body->list.push_back(condition);
       body->list.push_back(process(fbody));
       body->list.push_back(process(finc));
-      body->finalize();
-      ret->body = body;
+      body->finalize(none);
+      ret->body = builder.dropIfConcretelyTyped(body);
       // loops do not automatically loop, add a branch back
       auto continuer = allocator.alloc<Break>();
       continuer->name = ret->name;
@@ -2003,7 +2006,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
       // add an outer block for the init as well
       outer->list.push_back(process(finit));
       outer->list.push_back(ret);
-      outer->finalize();
+      outer->finalize(none);
       return outer;
     } else if (what == LABEL) {
       assert(parentLabel.isNull());
@@ -2139,7 +2142,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
         }
 
         top->list.push_back(br);
-        top->finalize();
+        top->finalize(none);
 
         for (unsigned i = 0; i < cases->size(); i++) {
           Ref curr = cases[i];
@@ -2163,9 +2166,9 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
           }
           auto next = allocator.alloc<Block>();
           top->name = name;
-          next->list.push_back(top);
-          next->list.push_back(case_);
-          next->finalize();
+          next->list.push_back(builder.dropIfConcretelyTyped(top));
+          next->list.push_back(builder.dropIfConcretelyTyped(case_));
+          next->finalize(none);
           top = next;
           nameMapper.popLabelName(name);
         }
@@ -2212,9 +2215,9 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
           }
           auto next = allocator.alloc<Block>();
           top->name = name;
-          next->list.push_back(top);
-          next->list.push_back(case_);
-          next->finalize();
+          next->list.push_back(builder.dropIfConcretelyTyped(top));
+          next->list.push_back(builder.dropIfConcretelyTyped(case_));
+          next->finalize(none);
           top = next;
           nameMapper.popLabelName(name);
         }
@@ -2230,7 +2233,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
         first->ifFalse = builder.makeBreak(br->default_);
 
         brHolder->list.push_back(chain);
-        brHolder->finalize();
+        brHolder->finalize(none);
       }
 
       breakStack.pop_back();
@@ -2268,6 +2271,8 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
     for (unsigned i = from; i < ast->size(); i++) {
       block->list.push_back(process(ast[i]));
     }
+    // if the last element has a value, we must drop it - a list of statements never falls through in asm.js
+    block->list.back() = builder.dropIfConcretelyTyped(block->list.back());
     block->finalize();
     return block;
   };
@@ -2276,9 +2281,30 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
   // cleanups/checks
   assert(breakStack.size() == 0 && continueStack.size() == 0);
   assert(parentLabel.isNull());
+  // fix up the fallthrough return value. asm2wasm output is structured statements,
+  // but the wasm toplevel scope is a fallthrough that must have the same type as
+  // the function return value, if there is one. We must propage that type up
+  // as far as necessary, replacing unreachable types with concrete ones, e.g.
+  //
+  // block
+  //   if
+  //     condition
+  //     return 1
+  //     return 2
+  //
+  // then naively they all have unreachable type, but if the function returns i32,
+  // the block *and* the if must be i32
+  fixFallthrough(function);
   return function;
 }
 
+void Asm2WasmBuilder::fixFallthrough(Function* func) {
+  if (func->result == none) return;
+  Block* block = builder.blockify(func->body);
+  block->finalize(func->result);
+  func->body = block;
+}
+
 } // namespace wasm
 
 #endif // wasm_asm2wasm_h

src/ast_utils.h

@@ -270,6 +270,10 @@ struct ExpressionAnalyzer {
     return !curr->condition && !curr->value;
   }
 
+  static bool isControlFlowStructure(Expression* curr) {
+    return curr->is<Block>() || curr->is<If>() || curr->is<Loop>();
+  }
+
   // Checks if an expression does not flow out in an obvious way.
   // We return true if it cannot flow out. If it can flow out, we
   // might still return true, as the analysis here is simple and fast.
@@ -335,26 +339,6 @@ struct AutoDrop : public WalkerPass<ExpressionStackWalker<AutoDrop, Visitor<Auto
 
   AutoDrop() { name = "autodrop"; }
 
-  bool maybeDrop(Expression*& child) {
-    bool acted = false;
-    if (isConcreteWasmType(child->type)) {
-      expressionStack.push_back(child);
-      if (!ExpressionAnalyzer::isResultUsed(expressionStack, getFunction()) && !ExpressionAnalyzer::isResultDropped(expressionStack)) {
-        child = Builder(*getModule()).makeDrop(child);
-        acted = true;
-      }
-      expressionStack.pop_back();
-    }
-    return acted;
-  }
-
-  void reFinalize() {
-    for (int i = int(expressionStack.size()) - 1; i >= 0; i--) {
-      auto* curr = expressionStack[i];
-      ReFinalize().visit(curr);
-    }
-  }
-
   void visitBlock(Block* curr) {
     if (curr->list.size() == 0) return;
     for (Index i = 0; i < curr->list.size() - 1; i++) {
@@ -363,21 +347,16 @@ struct AutoDrop : public WalkerPass<ExpressionStackWalker<AutoDrop, Visitor<Auto
         curr->list[i] = Builder(*getModule()).makeDrop(child);
       }
     }
-    if (maybeDrop(curr->list.back())) {
-      reFinalize();
-      assert(curr->type == none);
-    }
   }
 
   void visitIf(If* curr) {
-    bool acted = false;
-    if (maybeDrop(curr->ifTrue)) acted = true;
-    if (curr->ifFalse) {
-      if (maybeDrop(curr->ifFalse)) acted = true;
+    if (isConcreteWasmType(curr->type)) return; // ok to return a value, no need to drop
+    // this is an if without an else, or an if-else with no return value - drop any values
+    if (isConcreteWasmType(curr->ifTrue->type)) {
+      curr->ifTrue = Builder(*getModule()).makeDrop(curr->ifTrue);
     }
-    if (acted) {
-      reFinalize();
-      assert(curr->type == none);
+    if (curr->ifFalse && isConcreteWasmType(curr->ifFalse->type)) {
+      curr->ifFalse = Builder(*getModule()).makeDrop(curr->ifFalse);
     }
   }
 

src/binaryen-c.cpp

@@ -299,13 +299,13 @@ BinaryenOp BinaryenCurrentMemory(void) { return CurrentMemory; }
 BinaryenOp BinaryenGrowMemory(void) { return GrowMemory; }
 BinaryenOp BinaryenHasFeature(void) { return HasFeature; }
 
-BinaryenExpressionRef BinaryenBlock(BinaryenModuleRef module, const char* name, BinaryenExpressionRef* children, BinaryenIndex numChildren) {
+BinaryenExpressionRef BinaryenBlock(BinaryenModuleRef module, const char* name, BinaryenExpressionRef* children, BinaryenIndex numChildren, BinaryenType resultType) {
   auto* ret = ((Module*)module)->allocator.alloc<Block>();
   if (name) ret->name = name;
   for (BinaryenIndex i = 0; i < numChildren; i++) {
     ret->list.push_back((Expression*)children[i]);
   }
-  ret->finalize();
+  ret->finalize(WasmType(resultType));
 
   if (tracing) {
     std::cout << "  {\n";
@@ -319,7 +319,7 @@ BinaryenExpressionRef BinaryenBlock(BinaryenModuleRef module, const char* name,
     auto id = noteExpression(ret);
     std::cout << "    expressions[" << id << "] = BinaryenBlock(the_module, ";
     traceNameOrNULL(name);
-    std::cout << ", children, " << numChildren << ");\n";
+    std::cout << ", children, " << numChildren << ", " << resultType << ");\n";
     std::cout << "  }\n";
   }
 

src/binaryen-c.h

@@ -261,8 +261,8 @@ BinaryenOp BinaryenHasFeature(void);
 
 typedef void* BinaryenExpressionRef;
 
-// Block: name can be NULL
-BinaryenExpressionRef BinaryenBlock(BinaryenModuleRef module, const char* name, BinaryenExpressionRef* children, BinaryenIndex numChildren);
+// Block: name can be NULL. type must be explicitly provided, per the wasm type rules
+BinaryenExpressionRef BinaryenBlock(BinaryenModuleRef module, const char* name, BinaryenExpressionRef* children, BinaryenIndex numChildren, BinaryenType resultType);
 // If: ifFalse can be NULL
 BinaryenExpressionRef BinaryenIf(BinaryenModuleRef module, BinaryenExpressionRef condition, BinaryenExpressionRef ifTrue, BinaryenExpressionRef ifFalse);
 BinaryenExpressionRef BinaryenLoop(BinaryenModuleRef module, const char* in, BinaryenExpressionRef body);

src/mixed_arena.h

@@ -171,6 +171,10 @@ class ArenaVectorBase {
     return usedElements;
   }
 
+  bool empty() const {
+    return size() == 0;
+  }
+
   void resize(size_t size) {
     if (size > allocatedElements) {
       reallocate(size);

src/pass.h

@@ -238,8 +238,9 @@ class Printer : public Pass {
   std::ostream& o;
 
 public:
-  Printer() : o(std::cout) {}
-  Printer(std::ostream* o) : o(*o) {}
+  Printer(std::ostream* o = nullptr) : o(o ? *o : std::cout) {
+    name = "printer";
+  }
 
   void run(PassRunner* runner, Module* module) override;
 };

src/passes/CoalesceLocals.cpp

@@ -632,7 +632,7 @@ static void removeIfCopy(Expression** origin, SetLocal* set, If* iff, Expression
     if (!iff->ifTrue) {
       Builder(*module).flip(iff);
     }
-    iff->finalize();
+    iff->finalize(none);
   }
 }
 

src/passes/DeadCodeElimination.cpp

@@ -122,10 +122,11 @@ struct DeadCodeElimination : public WalkerPass<PostWalker<DeadCodeElimination, V
         // see https://github.com/WebAssembly/spec/issues/355
         if (!(isConcreteWasmType(block->type) && block->list[i]->type == none)) {
           block->list.resize(i + 1);
-          // note that we do *not* finalize here. it is incorrect to re-finalize a block
-          // after removing elements, as it may no longer have branches to it that would
-          // determine its type, so re-finalizing would just wipe out an existing type
-          // that it had.
+          if (isConcreteWasmType(block->type)) {
+            // the last element may be unreachable, and we may need to propagate our
+            // concrete type to it
+            block->finalize(block->type);
+          }
         }
       }
     }

src/passes/MergeBlocks.cpp

@@ -170,7 +170,7 @@ static void optimizeBlock(Block* curr, Module* module) {
               // reuse the drop
               drop->value = child->list.back();
               child->list.back() = drop;
-              child->finalize();
+              child->finalize(none);
               curr->list[i] = child;
               more = true;
               changed = true;
@@ -226,7 +226,12 @@ struct MergeBlocks : public WalkerPass<PostWalker<MergeBlocks, Visitor<MergeBloc
         if (outer == nullptr) {
           // reuse the block, move it out
           block->list.back() = curr;
-          block->finalize(); // last block element was our input, and is now our output, which may differ TODO optimize
+          block->finalize(curr->type); // last block element was our input, and is now our output
+          if (block->type == unreachable && curr == getFunction()->body && getFunction()->result != none) {
+            // corner case: if the block isn't typed (e.g. ends in a return now), and is about to
+            // be placed as the function's fallthrough, then it must be typed if the function returns
+            block->finalize(getFunction()->result);
+          }
           replaceCurrent(block);
           return block;
         } else {