Update binary encoding for block, loop, and if signatures (#711)

Also updates the tests and has a few other changes for binary 0xc:
Update nop/unrechable opcodes
Fix for "name" section

10 files changed, 82 insertions, 38 deletions

diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp
index 363147b98..69dc447a8 100644
--- a/src/passes/Print.cpp
+++ b/src/passes/Print.cpp
@@ -162,6 +162,9 @@ struct PrintSExpression : public Visitor<PrintSExpression> {
     if (curr->name.is()) {
       o << ' ' << curr->name;
     }
+    if (isConcreteWasmType(curr->type)) {
+      o << ' ' << printWasmType(curr->type);
+    }
     incIndent();
     auto block = curr->body->dynCast<Block>();
     if (!full && block && block->name.isNull()) {
diff --git a/src/wasm-binary.h b/src/wasm-binary.h
index 3047d43b7..4a32360a4 100644
--- a/src/wasm-binary.h
+++ b/src/wasm-binary.h
@@ -257,7 +257,7 @@ enum ElementType {
 };
 
 namespace UserSections {
-extern const char* Names;
+extern const char* Name;
 }
 
 enum ASTNodes {
@@ -424,7 +424,7 @@ enum ASTNodes {
   GetGlobal = 0x1a,
   SetGlobal = 0x1b,
 
-  Nop = 0x00,
+  Unreachable = 0x00,
   Block = 0x01,
   Loop = 0x02,
   If = 0x03,
@@ -434,7 +434,7 @@ enum ASTNodes {
   BrIf = 0x07,
   TableSwitch = 0x08,
   Return = 0x09,
-  Unreachable = 0x0a,
+  Nop = 0x0a,
   Drop = 0x0b,
   End = 0x0f
 };
@@ -451,6 +451,20 @@ enum TypeForms {
 
 } // namespace BinaryConsts
 
+
+struct ArityChecker : public PostWalker<ArityChecker, Visitor<ArityChecker>> {
+  std::unordered_map<cashew::IString, bool> arities;
+
+  ArityChecker(Expression* function)  {
+    walk(function);
+  }
+
+  void visitBreak(Break* curr) {
+    // Assume the module has already beeen type-checked, and that all breaks have matching arity.
+    if (curr->value) arities[curr->name] = true;
+  }
+};
+
 inline int8_t binaryWasmType(WasmType type) {
   switch (type) {
     case none: return 0;
@@ -468,6 +482,8 @@ class WasmBinaryWriter : public Visitor<WasmBinaryWriter, void> {
   bool debug;
   bool debugInfo = true;
 
+  std::unordered_map<cashew::IString, bool> brTargetArities;
+
   MixedArena allocator;
 
   void prepare() {
@@ -701,6 +717,10 @@ public:
       if (numLocalsByType[i64]) o << U32LEB(numLocalsByType[i64]) << binaryWasmType(i64);
       if (numLocalsByType[f32]) o << U32LEB(numLocalsByType[f32]) << binaryWasmType(f32);
       if (numLocalsByType[f64]) o << U32LEB(numLocalsByType[f64]) << binaryWasmType(f64);
+
+      ArityChecker ar(function->body);
+      brTargetArities = std::move(ar.arities);
+
       writeExpression(function->body);
       size_t size = o.size() - start;
       assert(size <= std::numeric_limits<uint32_t>::max());
@@ -825,7 +845,7 @@ public:
     if (wasm->functions.size() == 0) return;
     if (debug) std::cerr << "== writeNames" << std::endl;
     auto start = startSection(BinaryConsts::Section::User);
-    writeInlineString("names");
+    writeInlineString(BinaryConsts::UserSections::Name);
     o << U32LEB(wasm->functions.size());
     for (auto& curr : wasm->functions) {
       writeInlineString(curr->name.str);
@@ -898,6 +918,18 @@ public:
   void visitBlock(Block *curr) {
     if (debug) std::cerr << "zz node: Block" << std::endl;
     o << int8_t(BinaryConsts::Block);
+
+    int arity = curr->type != none && curr->type != unreachable;
+    if (brTargetArities.count(curr->name)) {
+      if (curr->type == unreachable) {
+        arity = brTargetArities[curr->name];
+      } else {
+        assert((curr->type != none) == brTargetArities[curr->name]);
+      }
+    }
+    // For blocks with type unreachable but whose breaks have arity 1, encode i32 as their
+    // signature so that the decoder knows to pop a value for the breaks' values.
+    o << binaryWasmType(curr->type != unreachable ? curr->type : arity ? i32 : none);
     breakStack.push_back(curr->name);
     size_t i = 0;
     for (auto* child : curr->list) {
@@ -924,6 +956,7 @@ public:
     if (debug) std::cerr << "zz node: If" << std::endl;
     recurse(curr->condition);
     o << int8_t(BinaryConsts::If);
+    o << binaryWasmType(curr->type != unreachable ? curr->type : none);
     breakStack.push_back(IMPOSSIBLE_CONTINUE); // the binary format requires this; we have a block if we need one; TODO: optimize
     recursePossibleBlockContents(curr->ifTrue); // TODO: emit block contents directly, if possible
     breakStack.pop_back();
@@ -938,6 +971,7 @@ public:
   void visitLoop(Loop *curr) {
     if (debug) std::cerr << "zz node: Loop" << std::endl;
     o << int8_t(BinaryConsts::Loop);
+    o << binaryWasmType(curr->type != unreachable ? curr->type : none);
     breakStack.push_back(curr->name);
     recursePossibleBlockContents(curr->body);
     breakStack.pop_back();
@@ -961,7 +995,7 @@ public:
     }
     if (curr->condition) recurse(curr->condition);
     o << int8_t(curr->condition ? BinaryConsts::BrIf : BinaryConsts::Br)
-      << U32LEB(curr->value ? 1 : 0) << U32LEB(getBreakIndex(curr->name));
+      << U32LEB(getBreakIndex(curr->name));
   }
   void visitSwitch(Switch *curr) {
     if (debug) std::cerr << "zz node: Switch" << std::endl;
@@ -969,11 +1003,11 @@ public:
       recurse(curr->value);
     }
     recurse(curr->condition);
-    o << int8_t(BinaryConsts::TableSwitch) << U32LEB(curr->value ? 1 : 0) << U32LEB(curr->targets.size());
+    o << int8_t(BinaryConsts::TableSwitch) << U32LEB(curr->targets.size());
     for (auto target : curr->targets) {
-      o << uint32_t(getBreakIndex(target));
+      o << U32LEB(getBreakIndex(target));
     }
-    o << uint32_t(getBreakIndex(curr->default_));
+    o << U32LEB(getBreakIndex(curr->default_));
   }
   void visitCall(Call *curr) {
     if (debug) std::cerr << "zz node: Call" << std::endl;
@@ -1340,7 +1374,7 @@ public:
 
   bool readUserSection() {
     Name sectionName = getInlineString();
-    if (sectionName.equals(BinaryConsts::UserSections::Names)) {
+    if (sectionName.equals(BinaryConsts::UserSections::Name)) {
       readNames();
       return true;
     }
@@ -1720,7 +1754,8 @@ public:
     }
   }
 
-  std::vector<Name> breakStack;
+  struct BreakTarget { Name name; int arity;};
+  std::vector<BreakTarget> breakStack;
 
   std::vector<Expression*> expressionStack;
 
@@ -1908,8 +1943,9 @@ public:
     // a common pattern that can be very highly nested.
     std::vector<Block*> stack;
     while (1) {
+      curr->type = getWasmType();
       curr->name = getNextLabel();
-      breakStack.push_back(curr->name);
+      breakStack.push_back({curr->name, curr->type != none});
       stack.push_back(curr);
       if (getInt8() == BinaryConsts::Block) {
         // a recursion
@@ -1960,9 +1996,9 @@ public:
     return block;
   }
 
-  Expression* getBlock() {
+  Expression* getBlock(WasmType ty) {
     Name label = getNextLabel();
-    breakStack.push_back(label);
+    breakStack.push_back({label, ty != none && ty != unreachable});
     auto* block = Builder(wasm).blockify(getMaybeBlock());
     breakStack.pop_back();
     block->cast<Block>()->name = label;
@@ -1971,48 +2007,53 @@ public:
 
   void visitIf(If *curr) {
     if (debug) std::cerr << "zz node: If" << std::endl;
+    curr->type = getWasmType();
     curr->condition = popExpression();
-    curr->ifTrue = getBlock();
+    curr->ifTrue = getBlock(curr->type);
     if (lastSeparator == BinaryConsts::Else) {
-      curr->ifFalse = getBlock();
+      curr->ifFalse = getBlock(curr->type);
       curr->finalize();
     }
     assert(lastSeparator == BinaryConsts::End);
   }
   void visitLoop(Loop *curr) {
     if (debug) std::cerr << "zz node: Loop" << std::endl;
+    curr->type = getWasmType();
     curr->name = getNextLabel();
-    breakStack.push_back(curr->name);
+    breakStack.push_back({curr->name, 0});
     curr->body = getMaybeBlock();
     breakStack.pop_back();
     curr->finalize();
   }
 
-  Name getBreakName(int32_t offset) {
+  BreakTarget getBreakTarget(int32_t offset) {
+    if (debug) std::cerr << "getBreakTarget "<<offset<<std::endl;
     assert(breakStack.size() - 1 - offset < breakStack.size());
+    if (debug) std::cerr <<"breaktarget "<< breakStack[breakStack.size() - 1 - offset].name<< " arity "<<breakStack[breakStack.size() - 1 - offset].arity<< std::endl;
     return breakStack[breakStack.size() - 1 - offset];
   }
 
   void visitBreak(Break *curr, uint8_t code) {
     if (debug) std::cerr << "zz node: Break" << std::endl;
-    auto arity = getU32LEB();
-    assert(arity == 0 || arity == 1);
-    curr->name = getBreakName(getU32LEB());
+    BreakTarget target = getBreakTarget(getU32LEB());
+    curr->name = target.name;
     if (code == BinaryConsts::BrIf) curr->condition = popExpression();
-    if (arity == 1) curr->value = popExpression();
+    if (target.arity) curr->value = popExpression();
     curr->finalize();
   }
   void visitSwitch(Switch *curr) {
     if (debug) std::cerr << "zz node: Switch" << std::endl;
-    auto arity = getU32LEB();
-    assert(arity == 0 || arity == 1);
     curr->condition = popExpression();
-    if (arity == 1) curr->value = popExpression();
+
     auto numTargets = getU32LEB();
+    if (debug) std::cerr << "targets: "<< numTargets<<std::endl;
     for (size_t i = 0; i < numTargets; i++) {
-      curr->targets.push_back(getBreakName(getInt32()));
+      curr->targets.push_back(getBreakTarget(getU32LEB()).name);
     }
-    curr->default_ = getBreakName(getInt32());
+    auto defaultTarget = getBreakTarget(getU32LEB());
+    curr->default_ = defaultTarget.name;
+    if (debug) std::cerr << "default: "<< curr->default_<<std::endl;
+    if (defaultTarget.arity) curr->value = popExpression();
   }
 
   template<typename T>
diff --git a/src/wasm.cpp b/src/wasm.cpp
index f43f4be1e..b5a4314de 100644
--- a/src/wasm.cpp
+++ b/src/wasm.cpp
@@ -27,7 +27,7 @@ Name WASM("wasm"),
 
 namespace BinaryConsts {
 namespace UserSections {
-const char* Names = "names";
+const char* Name = "name";
 }
 }
 
diff --git a/test/example/c-api-kitchen-sink.txt b/test/example/c-api-kitchen-sink.txt
index b66982140..8b4de4402 100644
--- a/test/example/c-api-kitchen-sink.txt
+++ b/test/example/c-api-kitchen-sink.txt
@@ -415,12 +415,12 @@ BinaryenFloat64: 4
               )
             )
             (drop
-              (loop $in
+              (loop $in i32
                 (i32.const 0)
               )
             )
             (drop
-              (loop
+              (loop i32
                 (i32.const 0)
               )
             )
@@ -1958,12 +1958,12 @@ int main() {
               )
             )
             (drop
-              (loop $in
+              (loop $in i32
                 (i32.const 0)
               )
             )
             (drop
-              (loop
+              (loop i32
                 (i32.const 0)
               )
             )
diff --git a/test/example/c-api-kitchen-sink.txt.txt b/test/example/c-api-kitchen-sink.txt.txt
index b42c1ec34..4d166cc2e 100644
--- a/test/example/c-api-kitchen-sink.txt.txt
+++ b/test/example/c-api-kitchen-sink.txt.txt
@@ -410,12 +410,12 @@
               )
             )
             (drop
-              (loop $in
+              (loop $in i32
                 (i32.const 0)
               )
             )
             (drop
-              (loop
+              (loop i32
                 (i32.const 0)
               )
             )
diff --git a/test/passes/nm.txt b/test/passes/nm.txt
index 1b12c5cff..27b12bfda 100644
--- a/test/passes/nm.txt
+++ b/test/passes/nm.txt
@@ -9,7 +9,7 @@
   )
   (func $b (type $0)
     (drop
-      (loop $loop-in1
+      (loop $loop-in1 i32
         (nop)
         (i32.const 1000)
       )
diff --git a/test/passes/simplify-locals.txt b/test/passes/simplify-locals.txt
index f12054887..15cd59140 100644
--- a/test/passes/simplify-locals.txt
+++ b/test/passes/simplify-locals.txt
@@ -323,7 +323,7 @@
         (i32.const 1337)
       )
       (drop
-        (loop $loop-in5
+        (loop $loop-in5 i32
           (drop
             (get_local $a)
           )
diff --git a/test/unit.wast b/test/unit.wast
index fab6705d2..2ba0f6612 100644
--- a/test/unit.wast
+++ b/test/unit.wast
@@ -429,7 +429,7 @@
     (i32.const 0)
   )
   (func $loop-roundtrip (type $7) (param $0 f64) (result f64)
-    (loop $loop-in1
+    (loop $loop-in1 f64
       (drop
         (get_local $0)
       )
diff --git a/test/unit.wast.fromBinary b/test/unit.wast.fromBinary
index 1080d5d20..8516d1775 100644
--- a/test/unit.wast.fromBinary
+++ b/test/unit.wast.fromBinary
@@ -439,7 +439,7 @@
     )
   )
   (func $loop-roundtrip (type $7) (param $var$0 f64) (result f64)
-    (loop $label$0
+    (loop $label$0 f64
       (drop
         (get_local $var$0)
       )
diff --git a/test/unit.wast.fromBinary.noDebugInfo b/test/unit.wast.fromBinary.noDebugInfo
index 4d9578500..d6980b9be 100644
--- a/test/unit.wast.fromBinary.noDebugInfo
+++ b/test/unit.wast.fromBinary.noDebugInfo
@@ -439,7 +439,7 @@
     )
   )
   (func $20 (type $7) (param $var$0 f64) (result f64)
-    (loop $label$0
+    (loop $label$0 f64
       (drop
         (get_local $var$0)
       )