From 2da1b20451a744daa613e818f71e8f52de3a818e Mon Sep 17 00:00:00 2001
From: Alon Zakai <alonzakai@gmail.com>
Date: Wed, 28 Sep 2016 13:10:59 -0700
Subject: Type check block/loop/if sigs (#717)

* type check using block/loop/if types provided in text and binary formats.
* print if and loop sigs which were missing.
* remove dsl from OptimizeInstructions as after those changes it needs rethinking.
---
 src/passes/OptimizeInstructions.cpp            |  60 +++++++++++
 src/passes/OptimizeInstructions.wast           | 132 +------------------------
 src/passes/OptimizeInstructions.wast.processed | 132 +------------------------
 src/passes/Print.cpp                           |   3 +
 src/passes/RemoveUnusedBrs.cpp                 |   2 +-
 src/wasm-binary.h                              |  52 +++-------
 src/wasm-s-parser.h                            |  28 +++---
 src/wasm-validator.h                           |  23 +++++
 src/wasm.cpp                                   |  43 ++++++++
 src/wasm.h                                     |  42 ++++----
 10 files changed, 179 insertions(+), 338 deletions(-)

(limited to 'src')

diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp
index 48639a341..f467b395e 100644
--- a/src/passes/OptimizeInstructions.cpp
+++ b/src/passes/OptimizeInstructions.cpp
@@ -175,6 +175,7 @@ struct OptimizeInstructions : public WalkerPass<PostWalker<OptimizeInstructions,
       if (handOptimized) {
         curr = handOptimized;
         replaceCurrent(curr);
+        continue;
       }
       auto iter = database->patternMap.find(curr->_id);
       if (iter == database->patternMap.end()) return;
@@ -209,6 +210,56 @@ struct OptimizeInstructions : public WalkerPass<PostWalker<OptimizeInstructions,
             }
           }
         }
+      } else if (binary->op == EqInt32) {
+        if (auto* c = binary->right->dynCast<Const>()) {
+          if (c->value.geti32() == 0) {
+            // equal 0 => eqz
+            return Builder(*getModule()).makeUnary(EqZInt32, binary->left);
+          }
+        }
+        if (auto* c = binary->left->dynCast<Const>()) {
+          if (c->value.geti32() == 0) {
+            // equal 0 => eqz
+            return Builder(*getModule()).makeUnary(EqZInt32, binary->right);
+          }
+        }
+      }
+    } else if (auto* unary = curr->dynCast<Unary>()) {
+      // de-morgan's laws
+      if (unary->op == EqZInt32) {
+        if (auto* inner = unary->value->dynCast<Binary>()) {
+          switch (inner->op) {
+            case EqInt32:  inner->op = NeInt32;  return inner;
+            case NeInt32:  inner->op = EqInt32;  return inner;
+            case LtSInt32: inner->op = GeSInt32; return inner;
+            case LtUInt32: inner->op = GeUInt32; return inner;
+            case LeSInt32: inner->op = GtSInt32; return inner;
+            case LeUInt32: inner->op = GtUInt32; return inner;
+            case GtSInt32: inner->op = LeSInt32; return inner;
+            case GtUInt32: inner->op = LeUInt32; return inner;
+            case GeSInt32: inner->op = LtSInt32; return inner;
+            case GeUInt32: inner->op = LtUInt32; return inner;
+
+            case EqInt64:  inner->op = NeInt64;  return inner;
+            case NeInt64:  inner->op = EqInt64;  return inner;
+            case LtSInt64: inner->op = GeSInt64; return inner;
+            case LtUInt64: inner->op = GeUInt64; return inner;
+            case LeSInt64: inner->op = GtSInt64; return inner;
+            case LeUInt64: inner->op = GtUInt64; return inner;
+            case GtSInt64: inner->op = LeSInt64; return inner;
+            case GtUInt64: inner->op = LeUInt64; return inner;
+            case GeSInt64: inner->op = LtSInt64; return inner;
+            case GeUInt64: inner->op = LtUInt64; return inner;
+
+            case EqFloat32: inner->op = NeFloat32; return inner;
+            case NeFloat32: inner->op = EqFloat32; return inner;
+
+            case EqFloat64: inner->op = NeFloat64; return inner;
+            case NeFloat64: inner->op = EqFloat64; return inner;
+
+            default: {}
+          }
+        }
       }
     } else if (auto* set = curr->dynCast<SetGlobal>()) {
       // optimize out a set of a get
@@ -218,6 +269,15 @@ struct OptimizeInstructions : public WalkerPass<PostWalker<OptimizeInstructions,
       }
     } else if (auto* iff = curr->dynCast<If>()) {
       iff->condition = optimizeBoolean(iff->condition);
+      if (iff->ifFalse) {
+        if (auto* unary = iff->condition->dynCast<Unary>()) {
+          if (unary->op == EqZInt32) {
+            // flip if-else arms to get rid of an eqz
+            iff->condition = unary->value;
+            std::swap(iff->ifTrue, iff->ifFalse);
+          }
+        }
+      }
     } else if (auto* select = curr->dynCast<Select>()) {
       select->condition = optimizeBoolean(select->condition);
       auto* condition = select->condition->dynCast<Unary>();
diff --git a/src/passes/OptimizeInstructions.wast b/src/passes/OptimizeInstructions.wast
index 48a4e2c9d..5142dc2c6 100644
--- a/src/passes/OptimizeInstructions.wast
+++ b/src/passes/OptimizeInstructions.wast
@@ -17,138 +17,10 @@
   (import $f64.expr "dsl" "f64.expr" (param i32) (result f64))
   (import $any.expr "dsl" "any.expr" (param i32) (result i32)) ;; ignorable return type
 
-  ;; main function. each block here is a pattern pair of input => output
+  ;; TODO for now wasm is not that convenient for a DSL like this. Needs rethinking.
+
   (func $patterns
-    ;; flip if-else arms to get rid of an eqz
-    (block
-      (if
-        (i32.eqz
-          (call_import $i32.expr (i32.const 0))
-        )
-        (call_import $any.expr (i32.const 1))
-        (call_import $any.expr (i32.const 2))
-      )
-      (if
-        (call_import $i32.expr (i32.const 0))
-        (call_import $any.expr (i32.const 2))
-        (call_import $any.expr (i32.const 1))
-      )
-    )
-    ;; equal 0 => eqz
-    (block
-      (i32.eq
-        (call_import $any.expr (i32.const 0))
-        (i32.const 0)
-      )
-      (i32.eqz
-        (call_import $any.expr (i32.const 0))
-      )
-    )
-    (block
-      (i32.eq
-        (i32.const 0)
-        (call_import $any.expr (i32.const 0))
-      )
-      (i32.eqz
-        (call_import $any.expr (i32.const 0))
-      )
-    )
-    ;; De Morgans Laws
-    (block
-      (i32.eqz (i32.eq (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))
-      (i32.ne (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i32.ne (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))
-      (i32.eq (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i32.lt_s (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))
-      (i32.ge_s (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i32.lt_u (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))
-      (i32.ge_u (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i32.le_s (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))
-      (i32.gt_s (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i32.le_u (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))
-      (i32.gt_u (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i32.gt_s (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))
-      (i32.le_s (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i32.gt_u (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))
-      (i32.le_u (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i32.ge_s (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))
-      (i32.lt_s (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i32.ge_u (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))
-      (i32.lt_u (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i64.eq (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))
-      (i64.ne (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i64.ne (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))
-      (i64.eq (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i64.lt_s (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))
-      (i64.ge_s (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i64.lt_u (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))
-      (i64.ge_u (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i64.le_s (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))
-      (i64.gt_s (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i64.le_u (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))
-      (i64.gt_u (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i64.gt_s (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))
-      (i64.le_s (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i64.gt_u (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))
-      (i64.le_u (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i64.ge_s (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))
-      (i64.lt_s (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (i64.ge_u (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))
-      (i64.lt_u (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (f32.eq (call_import $f32.expr (i32.const 0)) (call_import $f32.expr (i32.const 1))))
-      (f32.ne (call_import $f32.expr (i32.const 0)) (call_import $f32.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (f32.ne (call_import $f32.expr (i32.const 0)) (call_import $f32.expr (i32.const 1))))
-      (f32.eq (call_import $f32.expr (i32.const 0)) (call_import $f32.expr (i32.const 1)))
-    )
-    (block
-      (i32.eqz (f64.eq (call_import $f64.expr (i32.const 0)) (call_import $f64.expr (i32.const 1))))
-      (f64.ne (call_import $f64.expr (i32.const 0)) (call_import $f64.expr (i32.const 1)))
-    )
     (block
-      (i32.eqz (f64.ne (call_import $f64.expr (i32.const 0)) (call_import $f64.expr (i32.const 1))))
-      (f64.eq (call_import $f64.expr (i32.const 0)) (call_import $f64.expr (i32.const 1)))
     )
   )
 )
diff --git a/src/passes/OptimizeInstructions.wast.processed b/src/passes/OptimizeInstructions.wast.processed
index 13ccc8241..b69ecdf04 100644
--- a/src/passes/OptimizeInstructions.wast.processed
+++ b/src/passes/OptimizeInstructions.wast.processed
@@ -17,138 +17,10 @@
 "(import $f64.expr \"dsl\" \"f64.expr\" (param i32) (result f64))\n"
 "(import $any.expr \"dsl\" \"any.expr\" (param i32) (result i32)) ;; ignorable return type\n"
 "\n"
-";; main function. each block here is a pattern pair of input => output\n"
+";; TODO for now wasm is not that convenient for a DSL like this. Needs rethinking.\n"
+"\n"
 "(func $patterns\n"
-";; flip if-else arms to get rid of an eqz\n"
-"(block\n"
-"(if\n"
-"(i32.eqz\n"
-"(call_import $i32.expr (i32.const 0))\n"
-")\n"
-"(call_import $any.expr (i32.const 1))\n"
-"(call_import $any.expr (i32.const 2))\n"
-")\n"
-"(if\n"
-"(call_import $i32.expr (i32.const 0))\n"
-"(call_import $any.expr (i32.const 2))\n"
-"(call_import $any.expr (i32.const 1))\n"
-")\n"
-")\n"
-";; equal 0 => eqz\n"
-"(block\n"
-"(i32.eq\n"
-"(call_import $any.expr (i32.const 0))\n"
-"(i32.const 0)\n"
-")\n"
-"(i32.eqz\n"
-"(call_import $any.expr (i32.const 0))\n"
-")\n"
-")\n"
-"(block\n"
-"(i32.eq\n"
-"(i32.const 0)\n"
-"(call_import $any.expr (i32.const 0))\n"
-")\n"
-"(i32.eqz\n"
-"(call_import $any.expr (i32.const 0))\n"
-")\n"
-")\n"
-";; De Morgans Laws\n"
-"(block\n"
-"(i32.eqz (i32.eq (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))\n"
-"(i32.ne (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i32.ne (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))\n"
-"(i32.eq (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i32.lt_s (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))\n"
-"(i32.ge_s (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i32.lt_u (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))\n"
-"(i32.ge_u (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i32.le_s (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))\n"
-"(i32.gt_s (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i32.le_u (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))\n"
-"(i32.gt_u (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i32.gt_s (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))\n"
-"(i32.le_s (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i32.gt_u (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))\n"
-"(i32.le_u (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i32.ge_s (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))\n"
-"(i32.lt_s (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i32.ge_u (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1))))\n"
-"(i32.lt_u (call_import $i32.expr (i32.const 0)) (call_import $i32.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i64.eq (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))\n"
-"(i64.ne (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i64.ne (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))\n"
-"(i64.eq (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i64.lt_s (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))\n"
-"(i64.ge_s (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i64.lt_u (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))\n"
-"(i64.ge_u (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i64.le_s (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))\n"
-"(i64.gt_s (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i64.le_u (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))\n"
-"(i64.gt_u (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i64.gt_s (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))\n"
-"(i64.le_s (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i64.gt_u (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))\n"
-"(i64.le_u (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i64.ge_s (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))\n"
-"(i64.lt_s (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (i64.ge_u (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1))))\n"
-"(i64.lt_u (call_import $i64.expr (i32.const 0)) (call_import $i64.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (f32.eq (call_import $f32.expr (i32.const 0)) (call_import $f32.expr (i32.const 1))))\n"
-"(f32.ne (call_import $f32.expr (i32.const 0)) (call_import $f32.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (f32.ne (call_import $f32.expr (i32.const 0)) (call_import $f32.expr (i32.const 1))))\n"
-"(f32.eq (call_import $f32.expr (i32.const 0)) (call_import $f32.expr (i32.const 1)))\n"
-")\n"
-"(block\n"
-"(i32.eqz (f64.eq (call_import $f64.expr (i32.const 0)) (call_import $f64.expr (i32.const 1))))\n"
-"(f64.ne (call_import $f64.expr (i32.const 0)) (call_import $f64.expr (i32.const 1)))\n"
-")\n"
 "(block\n"
-"(i32.eqz (f64.ne (call_import $f64.expr (i32.const 0)) (call_import $f64.expr (i32.const 1))))\n"
-"(f64.eq (call_import $f64.expr (i32.const 0)) (call_import $f64.expr (i32.const 1)))\n"
 ")\n"
 ")\n"
 ")\n"
diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp
index 69dc447a8..c3ca4021e 100644
--- a/src/passes/Print.cpp
+++ b/src/passes/Print.cpp
@@ -140,6 +140,9 @@ struct PrintSExpression : public Visitor<PrintSExpression> {
   }
   void visitIf(If *curr) {
     printOpening(o, "if");
+    if (isConcreteWasmType(curr->type)) {
+      o << ' ' << printWasmType(curr->type);
+    }
     incIndent();
     printFullLine(curr->condition);
     // ifTrue and False have implict blocks, avoid printing them if possible
diff --git a/src/passes/RemoveUnusedBrs.cpp b/src/passes/RemoveUnusedBrs.cpp
index 15482de25..d1de0086a 100644
--- a/src/passes/RemoveUnusedBrs.cpp
+++ b/src/passes/RemoveUnusedBrs.cpp
@@ -302,7 +302,7 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs, Visitor<R
     } while (anotherCycle);
 
     if (worked) {
-      // Our work may alter block and if types, they may now return
+      // Our work may alter block and if types, they may now return values that we made flow through them
       struct TypeUpdater : public WalkerPass<PostWalker<TypeUpdater, Visitor<TypeUpdater>>> {
         void visitBlock(Block* curr) {
           curr->finalize();
diff --git a/src/wasm-binary.h b/src/wasm-binary.h
index 4a32360a4..70af27856 100644
--- a/src/wasm-binary.h
+++ b/src/wasm-binary.h
@@ -452,19 +452,6 @@ 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;
@@ -482,8 +469,6 @@ class WasmBinaryWriter : public Visitor<WasmBinaryWriter, void> {
   bool debug;
   bool debugInfo = true;
 
-  std::unordered_map<cashew::IString, bool> brTargetArities;
-
   MixedArena allocator;
 
   void prepare() {
@@ -718,9 +703,6 @@ public:
       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());
@@ -918,18 +900,7 @@ 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);
+    o << binaryWasmType(curr->type != unreachable ? curr->type : none);
     breakStack.push_back(curr->name);
     size_t i = 0;
     for (auto* child : curr->list) {
@@ -1703,7 +1674,7 @@ public:
         assert(breakStack.empty());
         assert(expressionStack.empty());
         assert(depth == 0);
-        func->body = getMaybeBlock();
+        func->body = getMaybeBlock(func->result);
         assert(depth == 0);
         assert(breakStack.empty());
         assert(expressionStack.empty());
@@ -1975,12 +1946,12 @@ public:
         curr->list.push_back(expressionStack[i]);
       }
       expressionStack.resize(start);
-      curr->finalize();
+      curr->finalize(curr->type);
       breakStack.pop_back();
     }
   }
 
-  Expression* getMaybeBlock() {
+  Expression* getMaybeBlock(WasmType type) {
     auto start = expressionStack.size();
     processExpressions();
     size_t end = expressionStack.size();
@@ -1991,15 +1962,16 @@ public:
     for (size_t i = start; i < end; i++) {
       block->list.push_back(expressionStack[i]);
     }
-    block->finalize();
+    block->finalize(type);
     expressionStack.resize(start);
     return block;
   }
 
-  Expression* getBlock(WasmType ty) {
+  Expression* getBlock(WasmType type) {
     Name label = getNextLabel();
-    breakStack.push_back({label, ty != none && ty != unreachable});
-    auto* block = Builder(wasm).blockify(getMaybeBlock());
+    breakStack.push_back({label, type != none && type != unreachable});
+    auto* block = Builder(wasm).blockify(getMaybeBlock(type));
+    block->finalize();
     breakStack.pop_back();
     block->cast<Block>()->name = label;
     return block;
@@ -2012,8 +1984,8 @@ public:
     curr->ifTrue = getBlock(curr->type);
     if (lastSeparator == BinaryConsts::Else) {
       curr->ifFalse = getBlock(curr->type);
-      curr->finalize();
     }
+    curr->finalize(curr->type);
     assert(lastSeparator == BinaryConsts::End);
   }
   void visitLoop(Loop *curr) {
@@ -2021,9 +1993,9 @@ public:
     curr->type = getWasmType();
     curr->name = getNextLabel();
     breakStack.push_back({curr->name, 0});
-    curr->body = getMaybeBlock();
+    curr->body = getMaybeBlock(curr->type);
     breakStack.pop_back();
-    curr->finalize();
+    curr->finalize(curr->type);
   }
 
   BreakTarget getBreakTarget(int32_t offset) {
diff --git a/src/wasm-s-parser.h b/src/wasm-s-parser.h
index bf2ab7739..07c2087be 100644
--- a/src/wasm-s-parser.h
+++ b/src/wasm-s-parser.h
@@ -651,7 +651,7 @@ private:
       autoBlock->name = FAKE_RETURN;
     }
     if (autoBlock) {
-      autoBlock->finalize();
+      autoBlock->finalize(result);
     }
     if (!currFunction) {
       makeFunction();
@@ -1100,8 +1100,10 @@ private:
       if (i >= s.size()) break; // empty block
       if (s[i]->isStr()) {
         // block signature
-        i++; // TODO: parse the signature
+        curr->type = stringToWasmType(s[i++]->str());
         if (i >= s.size()) break; // empty block
+      } else {
+        curr->type = none;
       }
       auto& first = *s[i];
       if (first[0]->str() == BLOCK) {
@@ -1133,7 +1135,7 @@ private:
       }
       assert(labelStack.back() == curr->name);
       labelStack.pop_back();
-      curr->finalize();
+      curr->finalize(curr->type);
     }
     return stack[0].second;
   }
@@ -1246,36 +1248,37 @@ private:
       label = getPrefixedName("if");
     }
     labelStack.push_back(label);
+    WasmType type = none;
     if (s[i]->isStr()) {
-      // if type, TODO: parse?
-      i++;
+      type = stringToWasmType(s[i++]->str());
     }
     ret->condition = parseExpression(s[i++]);
     ret->ifTrue = parseExpression(*s[i++]);
     if (i < s.size()) {
       ret->ifFalse = parseExpression(*s[i++]);
     }
-    ret->finalize();
+    ret->finalize(type);
     labelStack.pop_back();
     // create a break target if we must
     if (BreakSeeker::has(ret, label)) {
       auto* block = allocator.alloc<Block>();
       block->name = label;
       block->list.push_back(ret);
-      block->finalize();
+      block->finalize(ret->type);
       return block;
     }
     return ret;
   }
 
-  Expression* makeMaybeBlock(Element& s, size_t i, size_t stopAt=-1) {
+  Expression* makeMaybeBlock(Element& s, size_t i, WasmType type) {
+    Index stopAt = -1;
     if (s.size() == i) return allocator.alloc<Nop>();
     if (s.size() == i+1) return parseExpression(s[i]);
     auto ret = allocator.alloc<Block>();
     for (; i < s.size() && i < stopAt; i++) {
       ret->list.push_back(parseExpression(s[i]));
     }
-    ret->finalize();
+    ret->finalize(type);
     // Note that we do not name these implicit/synthetic blocks. They
     // are the effects of syntactic sugar, and nothing can branch to
     // them anyhow.
@@ -1296,14 +1299,15 @@ private:
     } else {
       ret->name = getPrefixedName("loop-in");
     }
+    ret->type = none;
     if (i < s.size() && s[i]->isStr()) {
       // block signature
-      i++; // TODO: parse the signature
+      ret->type = stringToWasmType(s[i++]->str());
     }
     labelStack.push_back(ret->name);
-    ret->body = makeMaybeBlock(s, i);
+    ret->body = makeMaybeBlock(s, i, ret->type);
     labelStack.pop_back();
-    ret->finalize();
+    ret->finalize(ret->type);
     if (out.is()) {
       auto* block = allocator.alloc<Block>();
       block->name = out;
diff --git a/src/wasm-validator.h b/src/wasm-validator.h
index 988d1104d..3616313d3 100644
--- a/src/wasm-validator.h
+++ b/src/wasm-validator.h
@@ -61,10 +61,18 @@ public:
     if (curr->name.is()) {
       if (breakInfos.count(curr) > 0) {
         auto& info = breakInfos[curr];
+        if (isConcreteWasmType(curr->type)) {
+          shouldBeTrue(info.arity != 0, curr, "break arities must be > 0 if block has a value");
+        } else {
+          shouldBeTrue(info.arity == 0, curr, "break arities must be 0 if block has no value");
+        }
         // none or unreachable means a poison value that we should ignore - if consumed, it will error
         if (isConcreteWasmType(info.type) && isConcreteWasmType(curr->type)) {
           shouldBeEqual(curr->type, info.type, curr, "block+breaks must have right type if breaks return a value");
         }
+        if (isConcreteWasmType(curr->type) && info.arity && info.type != unreachable) {
+          shouldBeEqual(curr->type, info.type, curr, "block+breaks must have right type if breaks have arity");
+        }
         shouldBeTrue(info.arity != Index(-1), curr, "break arities must match");
         if (curr->list.size() > 0) {
           auto last = curr->list.back()->type;
@@ -85,6 +93,9 @@ public:
         }
       }
     }
+    if (!isConcreteWasmType(curr->type) && curr->list.size() > 0) {
+      shouldBeFalse(isConcreteWasmType(curr->list.back()->type), curr, "block with no value cannot have a last element with a value");
+    }
   }
 
   static void visitPreLoop(WasmValidator* self, Expression** currp) {
@@ -100,6 +111,9 @@ public:
         shouldBeEqual(info.arity, Index(0), curr, "breaks to a loop cannot pass a value");
       }
     }
+    if (curr->type == none) {
+      shouldBeFalse(isConcreteWasmType(curr->body->type), curr, "bad body for a loop that has no value");
+    }
   }
 
   void visitIf(If *curr) {
@@ -288,6 +302,15 @@ public:
     shouldBeUnequal(curr->ifFalse->type, none, curr, "select right must be valid");
   }
 
+  void visitDrop(Drop* curr) {
+    // TODO: assert on this, when tests pass
+    if (getenv("BINARYEN_WARN_DROP")) {
+      if (!(isConcreteWasmType(curr->value->type) || curr->value->type == unreachable)) {
+        std::cerr << "warning: bad drop " << curr << " in " << (getFunction() ? getFunction()->name : Name("?")) << '\n';
+      }
+    }
+  }
+
   void visitReturn(Return* curr) {
     if (curr->value) {
       if (returnType == unreachable) {
diff --git a/src/wasm.cpp b/src/wasm.cpp
index b5a4314de..cf58949de 100644
--- a/src/wasm.cpp
+++ b/src/wasm.cpp
@@ -133,6 +133,17 @@ static WasmType mergeTypes(std::vector<WasmType>& types) {
   return type;
 }
 
+void Block::finalize(WasmType type_) {
+  type = type_;
+  if (type == none && list.size() > 0) {
+    if (list.back()->type == unreachable) {
+      if (!BreakSeeker::has(this, name)) {
+        type = unreachable; // the last element is unreachable, and this block truly cannot be exited, so it is unreachable itself
+      }
+    }
+  }
+}
+
 void Block::finalize() {
   if (!name.is()) {
     // nothing branches here, so this is easy
@@ -148,6 +159,38 @@ void Block::finalize() {
   type = mergeTypes(seeker.types);
 }
 
+void If::finalize(WasmType type_) {
+  type = type_;
+  if (type == none && (condition->type == unreachable || (ifTrue->type == unreachable && (!ifFalse || ifFalse->type == unreachable)))) {
+    type = unreachable;
+  }
+}
+
+void If::finalize() {
+  if (condition->type == unreachable) {
+    type = unreachable;
+  } else if (ifFalse) {
+    if (ifTrue->type == ifFalse->type) {
+      type = ifTrue->type;
+    } else if (isConcreteWasmType(ifTrue->type) && ifFalse->type == unreachable) {
+      type = ifTrue->type;
+    } else if (isConcreteWasmType(ifFalse->type) && ifTrue->type == unreachable) {
+      type = ifFalse->type;
+    } else {
+      type = none;
+    }
+  } else {
+    type = none; // if without else
+  }
+}
+
+void Loop::finalize(WasmType type_) {
+  type = type_;
+  if (type == none && body->type == unreachable) {
+    type = unreachable;
+  }
+}
+
 void Loop::finalize() {
   type = body->type;
 }
diff --git a/src/wasm.h b/src/wasm.h
index 53b4e2938..860f419cc 100644
--- a/src/wasm.h
+++ b/src/wasm.h
@@ -964,12 +964,12 @@ public:
   Name name;
   ExpressionList list;
 
-  // set the type of a block if you already know it
-  void finalize(WasmType type_) {
-    type = type_;
-  }
+  // set the type given you know its type, which is the case when parsing
+  // s-expression or binary, as explicit types are given. the only additional work
+  // this does is to set the type to unreachable in the cases that is needed.
+  void finalize(WasmType type_);
 
-  // set the type of a block based on its contents. this scans the block, so it is not fast
+  // set the type purely based on its contents. this scans the block, so it is not fast
   void finalize();
 };
 
@@ -980,21 +980,13 @@ public:
 
   Expression *condition, *ifTrue, *ifFalse;
 
-  void finalize() {
-    if (ifFalse) {
-      if (ifTrue->type == ifFalse->type) {
-        type = ifTrue->type;
-      } else if (isConcreteWasmType(ifTrue->type) && ifFalse->type == unreachable) {
-        type = ifTrue->type;
-      } else if (isConcreteWasmType(ifFalse->type) && ifTrue->type == unreachable) {
-        type = ifFalse->type;
-      } else {
-        type = none;
-      }
-    } else {
-      type = none; // if without else
-    }
-  }
+  // set the type given you know its type, which is the case when parsing
+  // s-expression or binary, as explicit types are given. the only additional work
+  // this does is to set the type to unreachable in the cases that is needed.
+  void finalize(WasmType type_);
+
+  // set the type purely based on its contents.
+  void finalize();
 };
 
 class Loop : public SpecificExpression<Expression::LoopId> {
@@ -1005,12 +997,12 @@ public:
   Name name;
   Expression *body;
 
-  // set the type of a loop if you already know it
-  void finalize(WasmType type_) {
-    type = type_;
-  }
+  // set the type given you know its type, which is the case when parsing
+  // s-expression or binary, as explicit types are given. the only additional work
+  // this does is to set the type to unreachable in the cases that is needed.
+  void finalize(WasmType type_);
 
-  // set the type of a loop based on its contents. this scans the loop, so it is not fast
+  // set the type purely based on its contents.
   void finalize();
 };
 
-- 
cgit v1.2.3