From 0ece2b9405d81790ff3927d6f211415600f8641f Mon Sep 17 00:00:00 2001
From: Alon Zakai <alonzakai@gmail.com>
Date: Fri, 25 Aug 2017 15:51:24 -0700
Subject: [PATCH 1/7] if a block has a concrete final element (or a break with
 a value), then even if it has an unreachable child, keep it with that
 concrete type. this means we no longe allow the silly case of a block with an
 unreachable in the middle and a concrete as the final element while the block
 is unreachable - after this change, the block would have the type of the
 final element

---
 check.py                                      |  2 +-
 src/ast_utils.h                               |  6 ++--
 src/wasm/wasm-validator.cpp                   |  4 +--
 src/wasm/wasm.cpp                             | 28 ++++++++++++++-----
 test/binaryen.js/call_import_error.js.txt     |  2 +-
 test/binaryen.js/hello-world.js.txt           |  2 +-
 test/passes/precompute.txt                    |  2 +-
 test/passes/remove-unused-brs.txt             |  4 +--
 test/passes/remove-unused-brs.wast            |  2 +-
 .../remove-unused-names_merge-blocks.txt      |  8 ++++--
 .../remove-unused-names_merge-blocks.wast     | 12 ++++----
 test/passes/vacuum.wast                       |  2 +-
 test/unit.wast                                |  4 ++-
 test/unit.wast.from-wast                      |  4 ++-
 14 files changed, 52 insertions(+), 30 deletions(-)

diff --git a/check.py b/check.py
index a95c91655f1..5aee277248b 100755
--- a/check.py
+++ b/check.py
@@ -336,7 +336,7 @@ def do_asm2wasm_test():
     wast = os.path.join(options.binaryen_test, t)
 
     # skip checks for some tests
-    if os.path.basename(wast) in ['linking.wast', 'nop.wast', 'stack.wast', 'typecheck.wast', 'unwind.wast']: # FIXME
+    if os.path.basename(wast) in ['block.wast', 'br.wast', 'linking.wast', 'nop.wast', 'stack.wast', 'typecheck.wast', 'unwind.wast']: # FIXME
       continue
 
     def run_spec_test(wast):
diff --git a/src/ast_utils.h b/src/ast_utils.h
index 1f781b87e50..aad514b59a9 100644
--- a/src/ast_utils.h
+++ b/src/ast_utils.h
@@ -112,6 +112,10 @@ struct ReFinalize : public WalkerPass<PostWalker<ReFinalize>> {
     }
     // nothing branches here
     if (curr->list.size() > 0) {
+      // last element determines type
+      curr->type = curr->list.back()->type;
+      // if concrete, it doesn't matter if we have an unreachable child
+      if (isConcreteWasmType(curr->type)) return;
       // if we have an unreachable child, we are unreachable
       // (we don't need to recurse into children, they can't
       // break to us)
@@ -121,8 +125,6 @@ struct ReFinalize : public WalkerPass<PostWalker<ReFinalize>> {
           return;
         }
       }
-      // children are reachable, so last element determines type
-      curr->type = curr->list.back()->type;
     } else {
       curr->type = none;
     }
diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp
index 819602608db..7ef40529fd9 100644
--- a/src/wasm/wasm-validator.cpp
+++ b/src/wasm/wasm-validator.cpp
@@ -70,9 +70,7 @@ void WasmValidator::visitBlock(Block *curr) {
   if (curr->list.size() > 0) {
     auto backType = curr->list.back()->type;
     if (!isConcreteWasmType(curr->type)) {
-      if (isConcreteWasmType(backType)) {
-        shouldBeTrue(curr->type == unreachable, curr, "block with no value and a last element with a value must be unreachable");
-      }
+      shouldBeFalse(isConcreteWasmType(backType), curr, "if block is not returning a value, final element should not flow out a value");
     } else {
       if (isConcreteWasmType(backType)) {
         shouldBeEqual(curr->type, backType, curr, "block with value and last element with value must match types");
diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp
index ab2db6bd5e3..9806bf4f741 100644
--- a/src/wasm/wasm.cpp
+++ b/src/wasm/wasm.cpp
@@ -174,6 +174,12 @@ static WasmType mergeTypes(std::vector<WasmType>& types) {
 // and there are no branches to it
 static void handleUnreachable(Block* block) {
   if (block->type == unreachable) return; // nothing to do
+  if (block->list.size() == 0) return; // nothing to do
+  // if we are concrete, stop - even an unreachable child
+  // won't change that (since we have a break with a value,
+  // or the final child flows out a value)
+  if (isConcreteWasmType(block->type)) return;
+  // look for an unreachable child
   for (auto* child : block->list) {
     if (child->type == unreachable) {
       // there is an unreachable child, so we are unreachable, unless we have a break
@@ -183,7 +189,7 @@ static void handleUnreachable(Block* block) {
       if (!seeker.found) {
         block->type = unreachable;
       } else {
-        block->type = seeker.valueType;
+        assert(block->type == seeker.valueType);
       }
       return;
     }
@@ -199,19 +205,27 @@ void Block::finalize(WasmType type_) {
 
 void Block::finalize() {
   if (!name.is()) {
-    // nothing branches here, so this is easy
     if (list.size() > 0) {
-      // if we have an unreachable child, we are unreachable
-      // (we don't need to recurse into children, they can't
-      // break to us)
+      // nothing branches here, so this is easy
+      // normally the type is the type of the final child
+      type = list.back()->type;
+      // and even if we have an unreachable child somewhere,
+      // we still mark ourselves as having that type,
+      // (block (result i32)
+      //  (return)
+      //  (i32.const 10)
+      // )
+      if (isConcreteWasmType(type)) return;
+      // if we are unreachable, we are done
+      if (type == unreachable) return;
+      // we may still be unreachable if we have an unreachable
+      // child
       for (auto* child : list) {
         if (child->type == unreachable) {
           type = unreachable;
           return;
         }
       }
-      // children are reachable, so last element determines type
-      type = list.back()->type;
     } else {
       type = none;
     }
diff --git a/test/binaryen.js/call_import_error.js.txt b/test/binaryen.js/call_import_error.js.txt
index 61e3f9339e2..d60d889d259 100644
--- a/test/binaryen.js/call_import_error.js.txt
+++ b/test/binaryen.js/call_import_error.js.txt
@@ -9,7 +9,7 @@
 )
 
 [wasm-validator error in function $main] unexpected false: call target must exist, on 
-(call $fn)
+[none] (call $fn)
 (perhaps it should be a CallImport instead of Call?)
 (module
  (type $v (func))
diff --git a/test/binaryen.js/hello-world.js.txt b/test/binaryen.js/hello-world.js.txt
index 4429180b444..bcc1a88a072 100644
--- a/test/binaryen.js/hello-world.js.txt
+++ b/test/binaryen.js/hello-world.js.txt
@@ -26,7 +26,7 @@ optimized:
  )
 )
 
-binary size: 88
+binary size: 60
 
 [object WebAssembly.Instance]
 
diff --git a/test/passes/precompute.txt b/test/passes/precompute.txt
index 74e4989dcb3..0f521da0b43 100644
--- a/test/passes/precompute.txt
+++ b/test/passes/precompute.txt
@@ -117,7 +117,7 @@
  )
  (func $br_if-condition-is-block-i32-but-unreachable-so-refinalize-tricky (type $2)
   (drop
-   (block $label$1
+   (block $label$1 (result i32)
     (drop
      (br_if $label$1
       (i32.const 100)
diff --git a/test/passes/remove-unused-brs.txt b/test/passes/remove-unused-brs.txt
index 7951cf18f84..ddced21e7e2 100644
--- a/test/passes/remove-unused-brs.txt
+++ b/test/passes/remove-unused-brs.txt
@@ -332,7 +332,7 @@
     )
    )
    (if
-    (block $block6
+    (block $block6 (result i32)
      (block $block15
       (drop
        (i32.const 2)
@@ -1062,7 +1062,7 @@
   )
  )
  (func $unreachable-if-that-could-be-a-br_if (type $7) (result i64)
-  (loop $label$3
+  (loop $label$3 (result i64)
    (if
     (unreachable)
     (f64.const 1)
diff --git a/test/passes/remove-unused-brs.wast b/test/passes/remove-unused-brs.wast
index 20441d9e41e..3a50839d6ea 100644
--- a/test/passes/remove-unused-brs.wast
+++ b/test/passes/remove-unused-brs.wast
@@ -944,7 +944,7 @@
    )
   )
   (func $unreachable-if-that-could-be-a-br_if (result i64)
-   (loop $label$3
+   (loop $label$3 (result i64)
     (if
      (unreachable)
      (f64.const 1)
diff --git a/test/passes/remove-unused-names_merge-blocks.txt b/test/passes/remove-unused-names_merge-blocks.txt
index 19f03c662e2..c170a6c99a4 100644
--- a/test/passes/remove-unused-names_merge-blocks.txt
+++ b/test/passes/remove-unused-names_merge-blocks.txt
@@ -853,9 +853,11 @@
   (unreachable)
  )
  (func $concrete_finale_in_unreachable (type $5) (result f64)
-  (unreachable)
   (drop
-   (f64.const 6.322092475576799e-96)
+   (block (result f64)
+    (unreachable)
+    (f64.const 6.322092475576799e-96)
+   )
   )
   (f64.const -1)
  )
@@ -897,7 +899,7 @@
  )
  (func $drop-unreachable-block-with-concrete-final (type $3)
   (drop
-   (block
+   (block (result i32)
     (drop
      (block
       (drop
diff --git a/test/passes/remove-unused-names_merge-blocks.wast b/test/passes/remove-unused-names_merge-blocks.wast
index 6f6dd92b9e2..67a1f2762d0 100644
--- a/test/passes/remove-unused-names_merge-blocks.wast
+++ b/test/passes/remove-unused-names_merge-blocks.wast
@@ -1007,9 +1007,11 @@
  )
  (func $concrete_finale_in_unreachable (result f64)
   (block $label$0 (result f64)
-   (block ;; this block is unreachable
-    (unreachable)
-    (f64.const 6.322092475576799e-96)
+   (drop
+    (block (result f64)
+     (unreachable)
+     (f64.const 6.322092475576799e-96)
+    )
    )
    (f64.const -1)
   )
@@ -1056,7 +1058,7 @@
  )
  (func $drop-unreachable-block-with-concrete-final
   (drop
-   (block
+   (block (result i32)
     (drop
      (block
       (drop
@@ -1070,7 +1072,7 @@
  )
  (func $merging-with-unreachable-in-middle (result i32)
   (block $label$1 (result i32)
-   (block
+   (block (result i32)
     (return
      (i32.const 21536)
     )
diff --git a/test/passes/vacuum.wast b/test/passes/vacuum.wast
index 3b5b22e9ce2..865e6929e55 100644
--- a/test/passes/vacuum.wast
+++ b/test/passes/vacuum.wast
@@ -502,7 +502,7 @@
    (local $2 i32)
    (block $label$0
     (drop
-     (block
+     (block (result i32)
       (br $label$0)
       (get_local $2)
      )
diff --git a/test/unit.wast b/test/unit.wast
index a429675de44..c28e7b7f84d 100644
--- a/test/unit.wast
+++ b/test/unit.wast
@@ -545,7 +545,9 @@
       )
       (block
         (unreachable)
-        (i32.const 1) ;; ends in a concrete, after an unreachable
+        (drop
+          (i32.const 1)
+        )
       )
     )
   )
diff --git a/test/unit.wast.from-wast b/test/unit.wast.from-wast
index 7fb82d69d29..721b6c3d291 100644
--- a/test/unit.wast.from-wast
+++ b/test/unit.wast.from-wast
@@ -610,7 +610,9 @@
    )
    (block $block12
     (unreachable)
-    (i32.const 1)
+    (drop
+     (i32.const 1)
+    )
    )
   )
  )

From d31d98ea1afd55651f511258f90673fbcc1391b5 Mon Sep 17 00:00:00 2001
From: Alon Zakai <alonzakai@gmail.com>
Date: Fri, 25 Aug 2017 16:55:06 -0700
Subject: [PATCH 2/7] if an if has a concrete element in one arm, make it have
 that type as a result, even if the if condition is unreachable, to parallel
 block

---
 check.py                           |  2 +-
 src/passes/SimplifyLocals.cpp      |  5 +++--
 src/wasm/wasm-validator.cpp        |  8 ++++++++
 src/wasm/wasm.cpp                  | 15 ++++++++++++---
 test/passes/remove-unused-brs.txt  | 22 +++++++++++++---------
 test/passes/remove-unused-brs.wast | 22 +++++++++++++---------
 test/passes/simplify-locals.txt    | 18 +++++++-----------
 test/passes/vacuum.txt             | 14 ++++++++------
 test/passes/vacuum.wast            | 20 +++++++++++---------
 9 files changed, 76 insertions(+), 50 deletions(-)

diff --git a/check.py b/check.py
index 5aee277248b..46b6d8857bc 100755
--- a/check.py
+++ b/check.py
@@ -336,7 +336,7 @@ def do_asm2wasm_test():
     wast = os.path.join(options.binaryen_test, t)
 
     # skip checks for some tests
-    if os.path.basename(wast) in ['block.wast', 'br.wast', 'linking.wast', 'nop.wast', 'stack.wast', 'typecheck.wast', 'unwind.wast']: # FIXME
+    if os.path.basename(wast) in ['block.wast', 'br.wast', 'linking.wast', 'nop.wast', 'return.wast', 'stack.wast', 'typecheck.wast', 'unreachable.wast', 'unwind.wast']: # FIXME
       continue
 
     def run_spec_test(wast):
diff --git a/src/passes/SimplifyLocals.cpp b/src/passes/SimplifyLocals.cpp
index 2d4a02337fd..919784cdf0b 100644
--- a/src/passes/SimplifyLocals.cpp
+++ b/src/passes/SimplifyLocals.cpp
@@ -372,8 +372,9 @@ struct SimplifyLocals : public WalkerPass<LinearExecutionWalker<SimplifyLocals>>
   // optimize set_locals from both sides of an if into a return value
   void optimizeIfReturn(If* iff, Expression** currp, Sinkables& ifTrue) {
     assert(iff->ifFalse);
-    // if this if already has a result, we can't do anything
-    if (isConcreteWasmType(iff->type)) return;
+    // if this if already has a result, or is unreachable code, we have
+    // nothing to do
+    if (iff->type != none) return;
     // We now have the sinkables from both sides of the if.
     Sinkables& ifFalse = sinkables;
     Index sharedIndex = -1;
diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp
index 7ef40529fd9..d3bd3eb5d66 100644
--- a/src/wasm/wasm-validator.cpp
+++ b/src/wasm/wasm-validator.cpp
@@ -116,6 +116,14 @@ void WasmValidator::visitIf(If *curr) {
         shouldBeEqual(curr->ifFalse->type, unreachable, curr, "unreachable if-else must have unreachable false");
       }
     }
+    if (isConcreteWasmType(curr->ifTrue->type)) {
+      shouldBeEqual(curr->type, curr->ifTrue->type, curr, "if type must match concrete ifTrue");
+      shouldBeEqualOrFirstIsUnreachable(curr->ifFalse->type, curr->ifTrue->type, curr, "other arm must match concrete ifTrue");
+    }
+    if (isConcreteWasmType(curr->ifFalse->type)) {
+      shouldBeEqual(curr->type, curr->ifFalse->type, curr, "if type must match concrete ifFalse");
+      shouldBeEqualOrFirstIsUnreachable(curr->ifTrue->type, curr->ifFalse->type, curr, "other arm must match concrete ifFalse");
+    }
   }
 }
 
diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp
index 9806bf4f741..8b0ce6fdcea 100644
--- a/src/wasm/wasm.cpp
+++ b/src/wasm/wasm.cpp
@@ -245,9 +245,7 @@ void If::finalize(WasmType type_) {
 }
 
 void If::finalize() {
-  if (condition->type == unreachable) {
-    type = unreachable;
-  } else if (ifFalse) {
+  if (ifFalse) {
     if (ifTrue->type == ifFalse->type) {
       type = ifTrue->type;
     } else if (isConcreteWasmType(ifTrue->type) && ifFalse->type == unreachable) {
@@ -260,6 +258,17 @@ void If::finalize() {
   } else {
     type = none; // if without else
   }
+  // if the arms return a value, leave it even if the condition
+  // is unreachable, we still mark ourselves as having that type, e.g.
+  // (if (result i32)
+  //  (unreachable)
+  //  (i32.const 10)
+  //  (i32.const 20
+  // )
+  // otherwise, if the condition is unreachable, so is the if
+  if (type == none && condition->type == unreachable) {
+    type = unreachable;
+  }
 }
 
 void Loop::finalize(WasmType type_) {
diff --git a/test/passes/remove-unused-brs.txt b/test/passes/remove-unused-brs.txt
index ddced21e7e2..c1fadcd0530 100644
--- a/test/passes/remove-unused-brs.txt
+++ b/test/passes/remove-unused-brs.txt
@@ -1063,24 +1063,28 @@
  )
  (func $unreachable-if-that-could-be-a-br_if (type $7) (result i64)
   (loop $label$3 (result i64)
-   (if
-    (unreachable)
-    (f64.const 1)
-    (br $label$3)
+   (drop
+    (if (result f64)
+     (unreachable)
+     (f64.const 1)
+     (br $label$3)
+    )
    )
    (i64.const 1)
   )
  )
  (func $nop-br-might-update-type (type $1)
   (block $label$39
-   (if
-    (unreachable)
+   (drop
     (if (result i32)
-     (i32.const 1)
-     (br $label$39)
+     (unreachable)
+     (if (result i32)
+      (i32.const 1)
+      (br $label$39)
+      (i32.const 0)
+     )
      (i32.const 0)
     )
-    (i32.const 0)
    )
   )
  )
diff --git a/test/passes/remove-unused-brs.wast b/test/passes/remove-unused-brs.wast
index 3a50839d6ea..36423554af6 100644
--- a/test/passes/remove-unused-brs.wast
+++ b/test/passes/remove-unused-brs.wast
@@ -945,24 +945,28 @@
   )
   (func $unreachable-if-that-could-be-a-br_if (result i64)
    (loop $label$3 (result i64)
-    (if
-     (unreachable)
-     (f64.const 1)
-     (br $label$3)
+    (drop
+     (if (result f64)
+      (unreachable)
+      (f64.const 1)
+      (br $label$3)
+     )
     )
     (i64.const 1)
    )
   )
   (func $nop-br-might-update-type
    (block $label$39
-    (if
-     (unreachable)
+    (drop
      (if (result i32)
-      (i32.const 1)
-      (br $label$39) ;; if we nop this, then the parent type must change
+      (unreachable)
+      (if (result i32)
+       (i32.const 1)
+       (br $label$39) ;; if we nop this, then the parent type must change
+       (i32.const 0)
+      )
       (i32.const 0)
      )
-     (i32.const 0)
     )
    )
   )
diff --git a/test/passes/simplify-locals.txt b/test/passes/simplify-locals.txt
index 39fc1d97d67..15d8b21e5fc 100644
--- a/test/passes/simplify-locals.txt
+++ b/test/passes/simplify-locals.txt
@@ -877,17 +877,13 @@
   )
  )
  (func $if-return-but-unreachable (type $10) (param $var$0 i64)
-  (tee_local $var$0
-   (if
-    (unreachable)
-    (block (result i64)
-     (nop)
-     (get_local $var$0)
-    )
-    (block (result i64)
-     (nop)
-     (i64.const 1)
-    )
+  (if
+   (unreachable)
+   (set_local $var$0
+    (get_local $var$0)
+   )
+   (set_local $var$0
+    (i64.const 1)
    )
   )
  )
diff --git a/test/passes/vacuum.txt b/test/passes/vacuum.txt
index 011fb99f55f..4caf4adcd73 100644
--- a/test/passes/vacuum.txt
+++ b/test/passes/vacuum.txt
@@ -276,12 +276,14 @@
  (func $unreachable-if-with-nop-arm-that-leaves-a-concrete-value-if-nop-is-removed (type $0)
   (block $label$0
    (loop $label$1
-    (br_if $label$0
-     (i32.load8_s
-      (i32.const 1634541608)
-     )
-     (loop $label$9
-      (br $label$9)
+    (drop
+     (br_if $label$0
+      (i32.load8_s
+       (i32.const 1634541608)
+      )
+      (loop $label$9
+       (br $label$9)
+      )
      )
     )
    )
diff --git a/test/passes/vacuum.wast b/test/passes/vacuum.wast
index 865e6929e55..2079d26eeee 100644
--- a/test/passes/vacuum.wast
+++ b/test/passes/vacuum.wast
@@ -596,17 +596,19 @@
   (func $unreachable-if-with-nop-arm-that-leaves-a-concrete-value-if-nop-is-removed
    (block $label$0
     (loop $label$1
-     (if
-      (br_if $label$0
-       (i32.load8_s
-        (i32.const 1634541608)
-       )
-       (loop $label$9
-        (br $label$9)
+     (drop
+      (if (result i32)
+       (br_if $label$0
+        (i32.load8_s
+         (i32.const 1634541608)
+        )
+        (loop $label$9
+         (br $label$9)
+        )
        )
+       (unreachable)
+       (i32.const 1920103026)
       )
-      (nop)
-      (i32.const 1920103026)
      )
     )
    )

From 2f330464535f9dbf79e1471061c8093e94a7ec83 Mon Sep 17 00:00:00 2001
From: "Alon Zakai (kripken)" <alonzakai@gmail.com>
Date: Sun, 27 Aug 2017 10:45:53 -0700
Subject: [PATCH 3/7] update TypeUpdater

---
 src/ast/type-updating.h | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/ast/type-updating.h b/src/ast/type-updating.h
index dc0ae0f36f6..cf4ebf0c74f 100644
--- a/src/ast/type-updating.h
+++ b/src/ast/type-updating.h
@@ -226,8 +226,12 @@ struct TypeUpdater : public ExpressionStackWalker<TypeUpdater, UnifiedExpression
         return; // already unreachable, stop here
       }
       // most nodes become unreachable if a child is unreachable,
-      // but exceptions exists
+      // but exceptions exist
       if (auto* block = curr->dynCast<Block>()) {
+        // if the block has a fallthrough, it can keep its type
+        if (isConcreteWasmType(block->list.back()->type)) {
+          return; // did not turn
+        }
         // if the block has breaks, it can keep its type
         if (!block->name.is() || blockInfos[block->name].numBreaks == 0) {
           curr->type = unreachable;
@@ -255,7 +259,7 @@ struct TypeUpdater : public ExpressionStackWalker<TypeUpdater, UnifiedExpression
       return; // nothing concrete to change to unreachable
     }
     if (curr->name.is() && blockInfos[curr->name].numBreaks > 0) {
-      return;// has a break, not unreachable
+      return; // has a break, not unreachable
     }
     // look for a fallthrough
     makeBlockUnreachableIfNoFallThrough(curr);
@@ -265,9 +269,13 @@ struct TypeUpdater : public ExpressionStackWalker<TypeUpdater, UnifiedExpression
     if (curr->type == unreachable) {
       return; // no change possible
     }
+    if (!curr->list.empty() &&
+        isConcreteWasmType(curr->list.back()->type)) {
+      return; // should keep type due to fallthrough, even if has an unreachable child
+    }
     for (auto* child : curr->list) {
       if (child->type == unreachable) {
-        // no fallthrough, this block is now unreachable
+        // no fallthrough, and an unreachable, => this block is now unreachable
         changeTypeTo(curr, unreachable);
         return;
       }

From c40a8728861e4517e8609b1e766a0f5bfe013f7e Mon Sep 17 00:00:00 2001
From: "Alon Zakai (kripken)" <alonzakai@gmail.com>
Date: Sun, 27 Aug 2017 11:12:09 -0700
Subject: [PATCH 4/7] spec fixes

---
 check.py          | 2 +-
 src/wasm/wasm.cpp | 2 --
 test/spec         | 2 +-
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/check.py b/check.py
index 46b6d8857bc..a95c91655f1 100755
--- a/check.py
+++ b/check.py
@@ -336,7 +336,7 @@ def do_asm2wasm_test():
     wast = os.path.join(options.binaryen_test, t)
 
     # skip checks for some tests
-    if os.path.basename(wast) in ['block.wast', 'br.wast', 'linking.wast', 'nop.wast', 'return.wast', 'stack.wast', 'typecheck.wast', 'unreachable.wast', 'unwind.wast']: # FIXME
+    if os.path.basename(wast) in ['linking.wast', 'nop.wast', 'stack.wast', 'typecheck.wast', 'unwind.wast']: # FIXME
       continue
 
     def run_spec_test(wast):
diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp
index 8b0ce6fdcea..78c13faaf9f 100644
--- a/src/wasm/wasm.cpp
+++ b/src/wasm/wasm.cpp
@@ -188,8 +188,6 @@ static void handleUnreachable(Block* block) {
       seeker.walk(expr);
       if (!seeker.found) {
         block->type = unreachable;
-      } else {
-        assert(block->type == seeker.valueType);
       }
       return;
     }
diff --git a/test/spec b/test/spec
index 668281b3d7d..8b1077b8a9e 160000
--- a/test/spec
+++ b/test/spec
@@ -1 +1 @@
-Subproject commit 668281b3d7dfe9be6cbc3c4500b537c49d6449b9
+Subproject commit 8b1077b8a9e75054ec373b064a25fd23e9a39719

From 2b73e696afbd8fec58695880913dd8372262bce1 Mon Sep 17 00:00:00 2001
From: Alon Zakai <alonzakai@gmail.com>
Date: Tue, 29 Aug 2017 11:06:00 -0700
Subject: [PATCH 5/7] make type rules for brs and switches simpler, ignore
 whether they are taken or not. whether they are dead code should not affect
 how they influence other types in our IR

disable wasm2wasm tests FIXME
---
 src/ast/branch-utils.h                        | 14 ++---
 src/ast/type-updating.h                       | 29 +--------
 src/ast_utils.h                               | 62 ++++++++++++-------
 src/passes/FlattenControlFlow.cpp             |  7 ++-
 src/passes/RemoveUnusedBrs.cpp                |  4 +-
 src/wasm/wasm-validator.cpp                   |  8 +--
 src/wasm/wasm.cpp                             |  8 +--
 test/passes/flatten-control-flow.txt          |  7 +++
 test/passes/flatten-control-flow.wast         |  7 +++
 test/passes/merge-blocks.txt                  | 14 ++---
 test/passes/merge-blocks.wast                 | 14 ++---
 test/passes/precompute.txt                    | 30 +++++++++
 test/passes/precompute.wast                   | 31 ++++++++++
 test/passes/remove-unused-brs.txt             | 18 +++++-
 test/passes/remove-unused-brs.wast            | 17 ++++-
 test/passes/remove-unused-names.txt           | 10 ++-
 test/passes/remove-unused-names.wast          |  4 +-
 test/passes/vacuum.txt                        |  5 +-
 test/passes/vacuum.wast                       |  3 -
 test/polymorphic_stack.wast                   | 22 ++++++-
 test/polymorphic_stack.wast.from-wast         | 20 +++++-
 test/polymorphic_stack.wast.fromBinary        | 23 ++++++-
 ...ymorphic_stack.wast.fromBinary.noDebugInfo | 23 ++++++-
 test/untaken-br_if.wast                       |  6 +-
 test/untaken-br_if.wast.from-wast             |  6 +-
 test/untaken-br_if.wast.fromBinary            | 10 +--
 .../untaken-br_if.wast.fromBinary.noDebugInfo | 10 +--
 27 files changed, 281 insertions(+), 131 deletions(-)

diff --git a/src/ast/branch-utils.h b/src/ast/branch-utils.h
index 77ec7075326..05ead8571fa 100644
--- a/src/ast/branch-utils.h
+++ b/src/ast/branch-utils.h
@@ -103,11 +103,11 @@ inline std::set<Name> getBranchTargets(Expression* ast) {
 // Finds if there are branches targeting a name. Note that since names are
 // unique in our IR, we just need to look for the name, and do not need
 // to analyze scoping.
-// By default we ignore untaken branches. You can set named to
-// note those as well, then any named branch is noted, even if untaken
+// By default we consider untaken branches (so any named use). You can unset named to
+// avoid that (and only note branches that are not obviously unreachable)
 struct BranchSeeker : public PostWalker<BranchSeeker> {
   Name target;
-  bool named = false;
+  bool named = true;
 
   Index found;
   WasmType valueType;
@@ -144,16 +144,18 @@ struct BranchSeeker : public PostWalker<BranchSeeker> {
     if (curr->default_ == target) noteFound(curr->value);
   }
 
-  static bool has(Expression* tree, Name target) {
+  static bool hasTaken(Expression* tree, Name target) {
     if (!target.is()) return false;
     BranchSeeker seeker(target);
+    seeker.named = false;
     seeker.walk(tree);
     return seeker.found > 0;
   }
 
-  static Index count(Expression* tree, Name target) {
+  static Index countTaken(Expression* tree, Name target) {
     if (!target.is()) return 0;
     BranchSeeker seeker(target);
+    seeker.named = false;
     seeker.walk(tree);
     return seeker.found;
   }
@@ -161,7 +163,6 @@ struct BranchSeeker : public PostWalker<BranchSeeker> {
   static bool hasNamed(Expression* tree, Name target) {
     if (!target.is()) return false;
     BranchSeeker seeker(target);
-    seeker.named = true;
     seeker.walk(tree);
     return seeker.found > 0;
   }
@@ -169,7 +170,6 @@ struct BranchSeeker : public PostWalker<BranchSeeker> {
   static Index countNamed(Expression* tree, Name target) {
     if (!target.is()) return 0;
     BranchSeeker seeker(target);
-    seeker.named = true;
     seeker.walk(tree);
     return seeker.found;
   }
diff --git a/src/ast/type-updating.h b/src/ast/type-updating.h
index cf4ebf0c74f..3cf2f2afe7c 100644
--- a/src/ast/type-updating.h
+++ b/src/ast/type-updating.h
@@ -132,13 +132,9 @@ struct TypeUpdater : public ExpressionStackWalker<TypeUpdater, UnifiedExpression
   // adds (or removes) breaks depending on break/switch contents
   void discoverBreaks(Expression* curr, int change) {
     if (auto* br = curr->dynCast<Break>()) {
-      if (BranchUtils::isBranchTaken(br)) {
-        noteBreakChange(br->name, change, br->value);
-      }
+      noteBreakChange(br->name, change, br->value);
     } else if (auto* sw = curr->dynCast<Switch>()) {
-      if (BranchUtils::isBranchTaken(sw)) {
-        applySwitchChanges(sw, change);
-      }
+      applySwitchChanges(sw, change);
     }
   }
 
@@ -200,27 +196,6 @@ struct TypeUpdater : public ExpressionStackWalker<TypeUpdater, UnifiedExpression
       auto* child = curr;
       curr = parents[child];
       if (!curr) return;
-      // if a child of a break/switch is now unreachable, the
-      // break may no longer be taken. note that if we get here,
-      // this is an actually new unreachable child of the
-      // node, so if there is just 1 such child, it is us, and
-      // we are newly unreachable
-      if (auto* br = curr->dynCast<Break>()) {
-        int unreachableChildren = 0;
-        if (br->value && br->value->type == unreachable) unreachableChildren++;
-        if (br->condition && br->condition->type == unreachable) unreachableChildren++;
-        if (unreachableChildren == 1) {
-          // the break is no longer taken
-          noteBreakChange(br->name, -1, br->value);
-        }
-      } else if (auto* sw = curr->dynCast<Switch>()) {
-        int unreachableChildren = 0;
-        if (sw->value && sw->value->type == unreachable) unreachableChildren++;
-        if (sw->condition->type == unreachable) unreachableChildren++;
-        if (unreachableChildren == 1) {
-          applySwitchChanges(sw, -1);
-        }
-      }
       // get ready to apply unreachability to this node
       if (curr->type == unreachable) {
         return; // already unreachable, stop here
diff --git a/src/ast_utils.h b/src/ast_utils.h
index aad514b59a9..852cf89a341 100644
--- a/src/ast_utils.h
+++ b/src/ast_utils.h
@@ -62,7 +62,7 @@ struct ExpressionAnalyzer {
     if (auto* br = curr->dynCast<Break>()) {
       if (!br->condition) return true;
     } else if (auto* block = curr->dynCast<Block>()) {
-      if (block->list.size() > 0 && obviouslyDoesNotFlowOut(block->list.back()) && !BranchUtils::BranchSeeker::has(block, block->name)) return true;
+      if (block->list.size() > 0 && obviouslyDoesNotFlowOut(block->list.back()) && !BranchUtils::BranchSeeker::hasTaken(block, block->name)) return true;
     }
     return false;
   }
@@ -101,51 +101,59 @@ struct ReFinalize : public WalkerPass<PostWalker<ReFinalize>> {
   std::map<Name, WasmType> breakValues;
 
   void visitBlock(Block *curr) {
+    if (curr->list.size() == 0) {
+      curr->type = none;
+      return;
+    }
     // do this quickly, without any validation
+    auto old = curr->type;
+    // last element determines type
+    curr->type = curr->list.back()->type;
+    // if concrete, it doesn't matter if we have an unreachable child, and we
+    // don't need to look at breaks
+    if (isConcreteWasmType(curr->type)) return;
+    // otherwise, we have no final fallthrough element to determine the type,
+    // could be determined by breaks
     if (curr->name.is()) {
       auto iter = breakValues.find(curr->name);
       if (iter != breakValues.end()) {
         // there is a break to here
-        curr->type = iter->second;
+        auto type = iter->second;
+        if (type == unreachable) {
+          // all we have are breaks with values of type unreachable, and no
+          // concrete fallthrough either. we must have had an existing type, then
+          curr->type = old;
+          assert(isConcreteWasmType(curr->type));
+        } else {
+          curr->type = type;
+        }
         return;
       }
     }
-    // nothing branches here
-    if (curr->list.size() > 0) {
-      // last element determines type
-      curr->type = curr->list.back()->type;
-      // if concrete, it doesn't matter if we have an unreachable child
-      if (isConcreteWasmType(curr->type)) return;
-      // if we have an unreachable child, we are unreachable
-      // (we don't need to recurse into children, they can't
-      // break to us)
+    if (curr->type == unreachable) return;
+    // type is none, but we might be unreachable
+    if (curr->type == none) {
       for (auto* child : curr->list) {
         if (child->type == unreachable) {
           curr->type = unreachable;
-          return;
+          break;
         }
       }
-    } else {
-      curr->type = none;
     }
   }
   void visitIf(If *curr) { curr->finalize(); }
   void visitLoop(Loop *curr) { curr->finalize(); }
   void visitBreak(Break *curr) {
     curr->finalize();
-    if (BranchUtils::isBranchTaken(curr)) {
-      breakValues[curr->name] = getValueType(curr->value);
-    }
+    updateBreakValueType(curr->name, getValueType(curr->value));
   }
   void visitSwitch(Switch *curr) {
     curr->finalize();
-    if (BranchUtils::isBranchTaken(curr)) {
-      auto valueType = getValueType(curr->value);
-      for (auto target : curr->targets) {
-        breakValues[target] = valueType;
-      }
-      breakValues[curr->default_] = valueType;
+    auto valueType = getValueType(curr->value);
+    for (auto target : curr->targets) {
+      updateBreakValueType(target, valueType);
     }
+    updateBreakValueType(curr->default_, valueType);
   }
   void visitCall(Call *curr) { curr->finalize(); }
   void visitCallImport(CallImport *curr) { curr->finalize(); }
@@ -169,7 +177,13 @@ struct ReFinalize : public WalkerPass<PostWalker<ReFinalize>> {
   void visitUnreachable(Unreachable *curr) { curr->finalize(); }
 
   WasmType getValueType(Expression* value) {
-    return value && value->type != unreachable ? value->type : none;
+    return value ? value->type : none;
+  }
+
+  void updateBreakValueType(Name name, WasmType type) {
+    if (type != unreachable || breakValues.count(name) == 0) {
+      breakValues[name] = type;
+    }
   }
 };
 
diff --git a/src/passes/FlattenControlFlow.cpp b/src/passes/FlattenControlFlow.cpp
index 3da5809c3ff..dce8e634580 100644
--- a/src/passes/FlattenControlFlow.cpp
+++ b/src/passes/FlattenControlFlow.cpp
@@ -61,7 +61,7 @@
 #include <wasm.h>
 #include <pass.h>
 #include <wasm-builder.h>
-
+#include <ast_utils.h>
 
 namespace wasm {
 
@@ -461,6 +461,11 @@ struct FlattenControlFlow : public WalkerPass<PostWalker<FlattenControlFlow>> {
       splitter.note(operand);
     }
   }
+
+  void visitFunction(Function* curr) {
+    // removing breaks can alter types
+    ReFinalize().walkFunctionInModule(curr, getModule());
+  }
 };
 
 Pass *createFlattenControlFlowPass() {
diff --git a/src/passes/RemoveUnusedBrs.cpp b/src/passes/RemoveUnusedBrs.cpp
index e627ce13801..1429813d1a4 100644
--- a/src/passes/RemoveUnusedBrs.cpp
+++ b/src/passes/RemoveUnusedBrs.cpp
@@ -394,7 +394,9 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
         if (list.size() == 1 && curr->name.is()) {
           // if this block has just one child, a sub-block, then jumps to the former are jumps to us, really
           if (auto* child = list[0]->dynCast<Block>()) {
-            if (child->name.is() && child->name != curr->name) {
+            // the two blocks must have the same type for us to update the branch, as otherwise
+            // one block may be unreachable and the other concrete, so one might lack a value
+            if (child->name.is() && child->name != curr->name && child->type == curr->type) {
               auto& breaks = breaksToBlock[child];
               for (auto* br : breaks) {
                 newNames[br] = curr->name;
diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp
index d3bd3eb5d66..585f3ce9f10 100644
--- a/src/wasm/wasm-validator.cpp
+++ b/src/wasm/wasm-validator.cpp
@@ -128,11 +128,7 @@ void WasmValidator::visitIf(If *curr) {
 }
 
 void WasmValidator::noteBreak(Name name, Expression* value, Expression* curr) {
-  if (!BranchUtils::isBranchTaken(curr)) {
-    // if not actually taken, just note the name
-    namedBreakTargets.insert(name);
-    return;
-  }
+  namedBreakTargets.insert(name);
   WasmType valueType = none;
   Index arity = 0;
   if (value) {
@@ -554,7 +550,7 @@ void WasmValidator::visitFunction(Function *curr) {
   if (returnType != unreachable) {
     shouldBeEqual(curr->result, returnType, curr->body, "function result must match, if function has returns");
   }
-  if (!shouldBeTrue(namedBreakTargets.empty(), curr->body, "all named break targets must exist (even if not taken)")) {
+  if (!shouldBeTrue(namedBreakTargets.empty(), curr->body, "all named break targets must exist")) {
     std::cerr << "(on label " << *namedBreakTargets.begin() << ")\n";
   }
   returnType = unreachable;
diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp
index 78c13faaf9f..018b1a1f3d3 100644
--- a/src/wasm/wasm.cpp
+++ b/src/wasm/wasm.cpp
@@ -116,13 +116,12 @@ struct TypeSeeker : public PostWalker<TypeSeeker> {
   }
 
   void visitBreak(Break* curr) {
-    if (curr->name == targetName && BranchUtils::isBranchTaken(curr)) {
+    if (curr->name == targetName) {
       types.push_back(curr->value ? curr->value->type : none);
     }
   }
 
   void visitSwitch(Switch* curr) {
-    if (!BranchUtils::isBranchTaken(curr)) return;
     for (auto name : curr->targets) {
       if (name == targetName) types.push_back(curr->value ? curr->value->type : none);
     }
@@ -183,10 +182,7 @@ static void handleUnreachable(Block* block) {
   for (auto* child : block->list) {
     if (child->type == unreachable) {
       // there is an unreachable child, so we are unreachable, unless we have a break
-      BranchUtils::BranchSeeker seeker(block->name);
-      Expression* expr = block;
-      seeker.walk(expr);
-      if (!seeker.found) {
+      if (!BranchUtils::BranchSeeker::hasNamed(block, block->name)) {
         block->type = unreachable;
       }
       return;
diff --git a/test/passes/flatten-control-flow.txt b/test/passes/flatten-control-flow.txt
index d94536712b3..65e7196fb87 100644
--- a/test/passes/flatten-control-flow.txt
+++ b/test/passes/flatten-control-flow.txt
@@ -1174,4 +1174,11 @@
    (get_local $2)
   )
  )
+ (func $switch-unreachable (type $1)
+  (block $label$3
+   (block
+    (unreachable)
+   )
+  )
+ )
 )
diff --git a/test/passes/flatten-control-flow.wast b/test/passes/flatten-control-flow.wast
index 69f3146c037..97ecf74c784 100644
--- a/test/passes/flatten-control-flow.wast
+++ b/test/passes/flatten-control-flow.wast
@@ -773,4 +773,11 @@
     (i32.const 1)
    )
   )
+  (func $switch-unreachable
+   (block $label$3
+    (br_table $label$3
+     (unreachable)
+    )
+   )
+  )
 )
diff --git a/test/passes/merge-blocks.txt b/test/passes/merge-blocks.txt
index f9ca7c3c108..be6906f0ac4 100644
--- a/test/passes/merge-blocks.txt
+++ b/test/passes/merge-blocks.txt
@@ -77,16 +77,12 @@
  )
  (func $drop-unreachable-br_if (type $2) (result i32)
   (block $label$0 (result i32)
-   (drop
-    (block $label$2
-     (drop
-      (br_if $label$2
-       (br $label$0
-        (i32.const 538976371)
-       )
-       (i32.const 1918987552)
-      )
+   (block $label$2 (result i32)
+    (br_if $label$2
+     (br $label$0
+      (i32.const 538976371)
      )
+     (i32.const 1918987552)
     )
    )
   )
diff --git a/test/passes/merge-blocks.wast b/test/passes/merge-blocks.wast
index d7361273538..7cba9bf903e 100644
--- a/test/passes/merge-blocks.wast
+++ b/test/passes/merge-blocks.wast
@@ -55,16 +55,12 @@
   )
   (func $drop-unreachable-br_if (result i32)
     (block $label$0 (result i32)
-     (drop
-      (block $label$2
-       (drop
-        (br_if $label$2
-         (br $label$0
-          (i32.const 538976371)
-         )
-         (i32.const 1918987552)
-        )
+     (block $label$2 (result i32)
+      (br_if $label$2
+       (br $label$0
+        (i32.const 538976371)
        )
+       (i32.const 1918987552)
       )
      )
     )
diff --git a/test/passes/precompute.txt b/test/passes/precompute.txt
index 0f521da0b43..ad5b74e8387 100644
--- a/test/passes/precompute.txt
+++ b/test/passes/precompute.txt
@@ -157,4 +157,34 @@
    (f64.const 4776014875438170098655851e156)
   )
  )
+ (func $refinalize-two-breaks-one-unreachable (type $2)
+  (drop
+   (block $label$0 (result i64)
+    (br_if $label$0
+     (select
+      (i64.const 1)
+      (block $block
+       (set_global $global$0
+        (i32.const 1)
+       )
+       (br $label$0
+        (i64.const -22)
+       )
+      )
+      (i32.const 0)
+     )
+     (i32.const 1)
+    )
+   )
+  )
+ )
+ (func $one-break-value-and-it-is-unreachable (type $3) (result f64)
+  (local $var$0 i32)
+  (block $label$6 (result f64)
+   (br_if $label$6
+    (unreachable)
+    (i32.const 0)
+   )
+  )
+ )
 )
diff --git a/test/passes/precompute.wast b/test/passes/precompute.wast
index 3eb2683bf7b..b339a55d446 100644
--- a/test/passes/precompute.wast
+++ b/test/passes/precompute.wast
@@ -249,4 +249,35 @@
     (f64.const 4776014875438170098655851e156)
    )
   )
+  (func $refinalize-two-breaks-one-unreachable
+   (drop
+    (block $label$0 (result i64)
+     (br_if $label$0
+      (select
+       (i64.const 1)
+       (block (result i64)
+        (set_global $global$0
+         (i32.const 1)
+        )
+        (br_if $label$0
+         (i64.const -22)
+         (i32.const -1)
+        )
+       )
+       (i32.const 0)
+      )
+      (i32.const 1)
+     )
+    )
+   )
+  )
+  (func $one-break-value-and-it-is-unreachable (result f64)
+   (local $var$0 i32)
+   (block $label$6 (result f64)
+    (br_if $label$6
+     (unreachable)
+     (i32.const 0)
+    )
+   )
+  )
 )
diff --git a/test/passes/remove-unused-brs.txt b/test/passes/remove-unused-brs.txt
index c1fadcd0530..6991680f790 100644
--- a/test/passes/remove-unused-brs.txt
+++ b/test/passes/remove-unused-brs.txt
@@ -1037,11 +1037,23 @@
    )
   )
  )
- (func $untaken-br-with-concrete-last-element (type $2) (result i32)
-  (block $label$8 (result i32)
-   (block $label$11 (result i32)
+ (func $untaken-br-with-concrete-last-element (type $1)
+  (block $label$8
+   (block $label$11
     (block $label$14
      (br_if $label$8
+      (br $label$8)
+     )
+    )
+   )
+  )
+ )
+ (func $untaken-br-with-concrete-last-element2 (type $2) (result i32)
+  (block $label$8 (result i32)
+   (block $label$11 (result i32)
+    (block $label$14 (result i32)
+     (br_if $label$14
+      (i32.const 102)
       (br $label$11
        (i32.const 103)
       )
diff --git a/test/passes/remove-unused-brs.wast b/test/passes/remove-unused-brs.wast
index 36423554af6..5c677622685 100644
--- a/test/passes/remove-unused-brs.wast
+++ b/test/passes/remove-unused-brs.wast
@@ -919,11 +919,24 @@
     )
    )
   )
-  (func $untaken-br-with-concrete-last-element (result i32)
+  (func $untaken-br-with-concrete-last-element
+   (block $label$8
+    (block $label$11
+     (block $label$14
+      (br_if $label$14
+       (br $label$11
+       )
+      )
+     )
+    )
+   )
+  )
+  (func $untaken-br-with-concrete-last-element2 (result i32)
    (block $label$8 (result i32)
     (block $label$11 (result i32)
-     (block $label$14
+     (block $label$14 (result i32)
       (br_if $label$14
+       (i32.const 102)
        (br $label$11
         (i32.const 103)
        )
diff --git a/test/passes/remove-unused-names.txt b/test/passes/remove-unused-names.txt
index 894e0716592..6fd0f4e5101 100644
--- a/test/passes/remove-unused-names.txt
+++ b/test/passes/remove-unused-names.txt
@@ -66,15 +66,13 @@
  )
  (func $merge-typed-with-unreachable-child (type $2) (result i32)
   (local $0 f32)
-  (block $label$0 (result i32)
-   (block $label$1
+  (block $label$1 (result i32)
+   (br_if $label$1
+    (i32.const 1)
     (br_if $label$1
      (i32.const 0)
-     (br_if $label$0
+     (br $label$1
       (i32.const 0)
-      (br $label$0
-       (i32.const 0)
-      )
      )
     )
    )
diff --git a/test/passes/remove-unused-names.wast b/test/passes/remove-unused-names.wast
index dc882cf4c27..a4c240f0270 100644
--- a/test/passes/remove-unused-names.wast
+++ b/test/passes/remove-unused-names.wast
@@ -80,9 +80,9 @@
   (func $merge-typed-with-unreachable-child (result i32)
    (local $0 f32)
    (block $label$0 (result i32)
-    (block $label$1
+    (block $label$1 (result i32)
      (br_if $label$1
-      (i32.const 0)
+      (i32.const 1)
       (br_if $label$0
        (i32.const 0)
        (br $label$0
diff --git a/test/passes/vacuum.txt b/test/passes/vacuum.txt
index 4caf4adcd73..b254179cddf 100644
--- a/test/passes/vacuum.txt
+++ b/test/passes/vacuum.txt
@@ -223,7 +223,7 @@
   )
  )
  (func $leave-block-even-if-br-not-taken (type $6) (result f64)
-  (block $label$0
+  (block $label$0 (result f64)
    (f64.store align=1
     (i32.const 879179022)
     (br_if $label$0
@@ -278,9 +278,6 @@
    (loop $label$1
     (drop
      (br_if $label$0
-      (i32.load8_s
-       (i32.const 1634541608)
-      )
       (loop $label$9
        (br $label$9)
       )
diff --git a/test/passes/vacuum.wast b/test/passes/vacuum.wast
index 2079d26eeee..dbbffee41b2 100644
--- a/test/passes/vacuum.wast
+++ b/test/passes/vacuum.wast
@@ -599,9 +599,6 @@
      (drop
       (if (result i32)
        (br_if $label$0
-        (i32.load8_s
-         (i32.const 1634541608)
-        )
         (loop $label$9
          (br $label$9)
         )
diff --git a/test/polymorphic_stack.wast b/test/polymorphic_stack.wast
index 1b245914801..d0b9986baec 100644
--- a/test/polymorphic_stack.wast
+++ b/test/polymorphic_stack.wast
@@ -80,7 +80,8 @@
   (func $untaken-break-should-have-value (result i32)
     (block $x (result i32)
       (block
-        (br_if $x ;; ok to not have a value, since an untaken branch. but must emit valid binary for wasm
+        (br_if $x
+          (i32.const 0)
           (unreachable)
         )
       )
@@ -95,6 +96,7 @@
    )
    (block $label$0 (result i32)
     (br_if $label$0
+     (i32.const 0)
      (return
       (i32.const -32)
      )
@@ -103,7 +105,7 @@
   )
   (func $br_table_unreachable_to_also_unreachable (result i32)
     (block $a (result i32)
-      (block $b
+      (block $b (result i32)
         (br_table $a $b ;; seems to send a value, but is not taken
           (unreachable)
           (unreachable)
@@ -111,5 +113,21 @@
       )
     )
   )
+  (func $untaken-br_if (result i32)
+    (block $label$8 (result i32)
+     (block $label$9
+      (drop
+       (if
+        (i32.const 0)
+        (br_if $label$8
+         (unreachable)
+         (i32.const 0)
+        )
+        (unreachable)
+       )
+      )
+     )
+    )
+  )
 )
 
diff --git a/test/polymorphic_stack.wast.from-wast b/test/polymorphic_stack.wast.from-wast
index 82f88f4af57..8b514d8f4e6 100644
--- a/test/polymorphic_stack.wast.from-wast
+++ b/test/polymorphic_stack.wast.from-wast
@@ -85,6 +85,7 @@
   (block $x (result i32)
    (block $block
     (br_if $x
+     (i32.const 0)
      (unreachable)
     )
    )
@@ -99,6 +100,7 @@
   )
   (block $label$0 (result i32)
    (br_if $label$0
+    (i32.const 0)
     (return
      (i32.const -32)
     )
@@ -107,7 +109,7 @@
  )
  (func $br_table_unreachable_to_also_unreachable (type $1) (result i32)
   (block $a (result i32)
-   (block $b
+   (block $b (result i32)
     (br_table $a $b
      (unreachable)
      (unreachable)
@@ -115,4 +117,20 @@
    )
   )
  )
+ (func $untaken-br_if (type $1) (result i32)
+  (block $label$8 (result i32)
+   (block $label$9
+    (drop
+     (if
+      (i32.const 0)
+      (br_if $label$8
+       (unreachable)
+       (i32.const 0)
+      )
+      (unreachable)
+     )
+    )
+   )
+  )
+ )
 )
diff --git a/test/polymorphic_stack.wast.fromBinary b/test/polymorphic_stack.wast.fromBinary
index 68bb7ecf781..709d131c0e4 100644
--- a/test/polymorphic_stack.wast.fromBinary
+++ b/test/polymorphic_stack.wast.fromBinary
@@ -36,6 +36,9 @@
  (func $untaken-break-should-have-value (type $1) (result i32)
   (block $label$0 (result i32)
    (block $label$1
+    (drop
+     (i32.const 0)
+    )
     (unreachable)
    )
    (unreachable)
@@ -52,6 +55,9 @@
     )
    )
    (block $label$2 (result i32)
+    (drop
+     (i32.const 0)
+    )
     (return
      (i32.const -32)
     )
@@ -60,9 +66,24 @@
  )
  (func $br_table_unreachable_to_also_unreachable (type $1) (result i32)
   (block $label$0 (result i32)
-   (block $label$1
+   (block $label$1 (result i32)
     (unreachable)
    )
+  )
+ )
+ (func $untaken-br_if (type $1) (result i32)
+  (block $label$0 (result i32)
+   (block $label$1
+    (if
+     (i32.const 0)
+     (block $label$2
+      (unreachable)
+     )
+     (block $label$3
+      (unreachable)
+     )
+    )
+   )
    (unreachable)
   )
  )
diff --git a/test/polymorphic_stack.wast.fromBinary.noDebugInfo b/test/polymorphic_stack.wast.fromBinary.noDebugInfo
index 69292952e8d..3c0af42650c 100644
--- a/test/polymorphic_stack.wast.fromBinary.noDebugInfo
+++ b/test/polymorphic_stack.wast.fromBinary.noDebugInfo
@@ -36,6 +36,9 @@
  (func $5 (type $1) (result i32)
   (block $label$0 (result i32)
    (block $label$1
+    (drop
+     (i32.const 0)
+    )
     (unreachable)
    )
    (unreachable)
@@ -52,6 +55,9 @@
     )
    )
    (block $label$2 (result i32)
+    (drop
+     (i32.const 0)
+    )
     (return
      (i32.const -32)
     )
@@ -60,9 +66,24 @@
  )
  (func $7 (type $1) (result i32)
   (block $label$0 (result i32)
-   (block $label$1
+   (block $label$1 (result i32)
     (unreachable)
    )
+  )
+ )
+ (func $8 (type $1) (result i32)
+  (block $label$0 (result i32)
+   (block $label$1
+    (if
+     (i32.const 0)
+     (block $label$2
+      (unreachable)
+     )
+     (block $label$3
+      (unreachable)
+     )
+    )
+   )
    (unreachable)
   )
  )
diff --git a/test/untaken-br_if.wast b/test/untaken-br_if.wast
index a165cf67fff..92ed74c959a 100644
--- a/test/untaken-br_if.wast
+++ b/test/untaken-br_if.wast
@@ -1,11 +1,11 @@
 (module
  (func $binaryify-untaken-br_if (result f32)
-  (if
+  (if (result f32)
    (i32.const 1)
    (unreachable)
-   (block $label$1
+   (block $label$1 (result f32)
     (br_if $label$1
-     (i32.const 1)
+     (f32.const 1)
      (unreachable)
     )
    )
diff --git a/test/untaken-br_if.wast.from-wast b/test/untaken-br_if.wast.from-wast
index 2d6d9dd2d7b..86ce53f41b4 100644
--- a/test/untaken-br_if.wast.from-wast
+++ b/test/untaken-br_if.wast.from-wast
@@ -2,12 +2,12 @@
  (type $0 (func (result f32)))
  (memory $0 0)
  (func $binaryify-untaken-br_if (type $0) (result f32)
-  (if
+  (if (result f32)
    (i32.const 1)
    (unreachable)
-   (block $label$1
+   (block $label$1 (result f32)
     (br_if $label$1
-     (i32.const 1)
+     (f32.const 1)
      (unreachable)
     )
    )
diff --git a/test/untaken-br_if.wast.fromBinary b/test/untaken-br_if.wast.fromBinary
index 815789c351a..fc2e7f215ac 100644
--- a/test/untaken-br_if.wast.fromBinary
+++ b/test/untaken-br_if.wast.fromBinary
@@ -2,15 +2,15 @@
  (type $0 (func (result f32)))
  (memory $0 0)
  (func $binaryify-untaken-br_if (type $0) (result f32)
-  (if
+  (if (result f32)
    (i32.const 1)
-   (block $label$0
+   (block $label$0 (result f32)
     (unreachable)
    )
-   (block $label$1
-    (block $label$2
+   (block $label$1 (result f32)
+    (block $label$2 (result f32)
      (drop
-      (i32.const 1)
+      (f32.const 1)
      )
      (unreachable)
     )
diff --git a/test/untaken-br_if.wast.fromBinary.noDebugInfo b/test/untaken-br_if.wast.fromBinary.noDebugInfo
index 9b31417eb26..b15b0f0b4a0 100644
--- a/test/untaken-br_if.wast.fromBinary.noDebugInfo
+++ b/test/untaken-br_if.wast.fromBinary.noDebugInfo
@@ -2,15 +2,15 @@
  (type $0 (func (result f32)))
  (memory $0 0)
  (func $0 (type $0) (result f32)
-  (if
+  (if (result f32)
    (i32.const 1)
-   (block $label$0
+   (block $label$0 (result f32)
     (unreachable)
    )
-   (block $label$1
-    (block $label$2
+   (block $label$1 (result f32)
+    (block $label$2 (result f32)
      (drop
-      (i32.const 1)
+      (f32.const 1)
      )
      (unreachable)
     )

From 1ae7726692855e7364ea83306554359481166e75 Mon Sep 17 00:00:00 2001
From: Alon Zakai <alonzakai@gmail.com>
Date: Wed, 30 Aug 2017 08:41:37 -0700
Subject: [PATCH 6/7] don't optimize unreachable code in optimize-expressions
 (we don't want to anyhow, and it's more complicated and error prone)

---
 src/passes/OptimizeInstructions.cpp           |  8 ++++
 test/merge/basics.wast.combined.finalized.opt |  2 +-
 test/passes/optimize-instructions.txt         |  9 +++--
 ...optimize-level=2_ignore-implicit-traps.txt | 38 +++++++++++++++++++
 ...ptimize-level=2_ignore-implicit-traps.wast | 38 +++++++++++++++++++
 5 files changed, 91 insertions(+), 4 deletions(-)

diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp
index f458a58b2f4..acbf3944709 100644
--- a/src/passes/OptimizeInstructions.cpp
+++ b/src/passes/OptimizeInstructions.cpp
@@ -397,6 +397,14 @@ struct OptimizeInstructions : public WalkerPass<PostWalker<OptimizeInstructions,
 
   // Optimizations that don't yet fit in the pattern DSL, but could be eventually maybe
   Expression* handOptimize(Expression* curr) {
+    // if this contains dead code, don't bother trying to optimize it, the type
+    // might change (if might not be unreachable if just one arm is, for example).
+    // this optimization pass focuses on actually executing code. the only
+    // exceptions are control flow changes
+    if (curr->type == unreachable &&
+        !curr->is<Break>() && !curr->is<Switch>() && !curr->is<If>()) {
+      return nullptr;
+    }
     if (auto* binary = curr->dynCast<Binary>()) {
       if (Properties::isSymmetric(binary)) {
         // canonicalize a const to the second position
diff --git a/test/merge/basics.wast.combined.finalized.opt b/test/merge/basics.wast.combined.finalized.opt
index 119851fb05b..7f57d67da2c 100644
--- a/test/merge/basics.wast.combined.finalized.opt
+++ b/test/merge/basics.wast.combined.finalized.opt
@@ -68,8 +68,8 @@
   (nop)
   (drop
    (i32.add
-    (unreachable)
     (i32.const 14)
+    (unreachable)
    )
   )
   (nop)
diff --git a/test/passes/optimize-instructions.txt b/test/passes/optimize-instructions.txt
index 55501dfb871..6a99ee10641 100644
--- a/test/passes/optimize-instructions.txt
+++ b/test/passes/optimize-instructions.txt
@@ -355,8 +355,8 @@
   )
   (drop
    (i32.and
-    (unreachable)
     (i32.const 1)
+    (unreachable)
    )
   )
   (drop
@@ -2064,8 +2064,11 @@
  )
  (func $shifts-square-unreachable (type $3) (param $x i32) (result i32)
   (i32.shr_u
-   (unreachable)
-   (i32.const 9)
+   (i32.shr_u
+    (unreachable)
+    (i32.const 1031)
+   )
+   (i32.const 4098)
   )
  )
  (func $mix-shifts (type $2) (result i32)
diff --git a/test/passes/optimize-instructions_optimize-level=2_ignore-implicit-traps.txt b/test/passes/optimize-instructions_optimize-level=2_ignore-implicit-traps.txt
index 922f6400858..85e0dd6d8c9 100644
--- a/test/passes/optimize-instructions_optimize-level=2_ignore-implicit-traps.txt
+++ b/test/passes/optimize-instructions_optimize-level=2_ignore-implicit-traps.txt
@@ -1,5 +1,6 @@
 (module
  (type $0 (func (param i32 i32) (result i32)))
+ (type $1 (func (result f64)))
  (memory $0 0)
  (func $conditionals (type $0) (param $0 i32) (param $1 i32) (result i32)
   (local $2 i32)
@@ -321,4 +322,41 @@
    (get_local $1)
   )
  )
+ (func $conditionalize-if-type-change (type $1) (result f64)
+  (local $0 i32)
+  (drop
+   (loop $label$1 (result f32)
+    (block $label$2 (result f32)
+     (drop
+      (block $label$3 (result f32)
+       (br_if $label$1
+        (i32.or
+         (f32.gt
+          (br_if $label$3
+           (f32.const 1)
+           (get_local $0)
+          )
+          (br $label$2
+           (f32.const 71)
+          )
+         )
+         (i64.eqz
+          (select
+           (i64.const 58)
+           (i64.const -982757)
+           (i64.eqz
+            (i64.const 0)
+           )
+          )
+         )
+        )
+       )
+      )
+     )
+     (f32.const 1)
+    )
+   )
+  )
+  (f64.const -nan:0xfffffffffffff)
+ )
 )
diff --git a/test/passes/optimize-instructions_optimize-level=2_ignore-implicit-traps.wast b/test/passes/optimize-instructions_optimize-level=2_ignore-implicit-traps.wast
index 7e83658126e..6e19255ed92 100644
--- a/test/passes/optimize-instructions_optimize-level=2_ignore-implicit-traps.wast
+++ b/test/passes/optimize-instructions_optimize-level=2_ignore-implicit-traps.wast
@@ -323,5 +323,43 @@
    )
    (return (get_local $1))
   )
+
+ (func $conditionalize-if-type-change (result f64)
+  (local $0 i32)
+  (drop
+   (loop $label$1 (result f32)
+    (block $label$2 (result f32)
+     (drop
+      (block $label$3 (result f32)
+       (br_if $label$1
+        (i32.or ;; this turns into an if, but then the if might not be unreachable
+         (f32.gt
+          (br_if $label$3
+           (f32.const 1)
+           (get_local $0)
+          )
+          (br $label$2
+           (f32.const 71)
+          )
+         )
+         (i64.eqz
+          (select
+           (i64.const 58)
+           (i64.const -982757)
+           (i64.eqz
+            (i64.const 0)
+           )
+          )
+         )
+        )
+       )
+      )
+     )
+     (f32.const 1)
+    )
+   )
+  )
+  (f64.const -nan:0xfffffffffffff)
+ )
 )
 

From 4ee9ef1ea31f2fbf72181dce1ef2645efdca3e00 Mon Sep 17 00:00:00 2001
From: Alon Zakai <alonzakai@gmail.com>
Date: Tue, 5 Sep 2017 14:47:13 -0700
Subject: [PATCH 7/7] update

---
 auto_update_tests.py       | 22 +++++++++++++++++++++-
 test/br_table_temp.2asm.js | 10 +++++-----
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/auto_update_tests.py b/auto_update_tests.py
index 49dce464e48..e62f42f0193 100755
--- a/auto_update_tests.py
+++ b/auto_update_tests.py
@@ -5,8 +5,10 @@
 from scripts.test.support import run_command, split_wast
 from scripts.test.shared import (
     ASM2WASM, MOZJS, S2WASM, WASM_SHELL, WASM_OPT, WASM_AS, WASM_DIS,
-    WASM_CTOR_EVAL, WASM_MERGE, WASM_REDUCE,
+    WASM_CTOR_EVAL, WASM_MERGE, WASM_REDUCE, WASM2ASM,
     BINARYEN_INSTALL_DIR, has_shell_timeout)
+from scripts.test.wasm2asm import tests, spec_tests, extra_tests
+
 
 print '[ processing and updating testcases... ]\n'
 
@@ -261,6 +263,24 @@
     out = t + '.out'
     with open(out, 'w') as o: o.write(actual)
 
+print '\n[ checking wasm2asm ]\n'
+
+for wasm in tests + spec_tests + extra_tests:
+  if not wasm.endswith('.wast'):
+    continue
+
+  asm = os.path.basename(wasm).replace('.wast', '.2asm.js')
+  expected_file = os.path.join('test', asm)
+
+  if not os.path.exists(expected_file):
+    continue
+
+  print '..', wasm
+
+  cmd = WASM2ASM + [os.path.join('test', wasm)]
+  out = run_command(cmd)
+  with open(expected_file, 'w') as o: o.write(out)
+
 if has_shell_timeout():
   print '\n[ checking wasm-reduce ]\n'
 
diff --git a/test/br_table_temp.2asm.js b/test/br_table_temp.2asm.js
index 19fd6736878..eb513b07152 100644
--- a/test/br_table_temp.2asm.js
+++ b/test/br_table_temp.2asm.js
@@ -49531,7 +49531,7 @@ function asmFunc(global, env, buffer) {
  }
  
  function $$20() {
-  var $$0 = 0, $$1 = 0;
+  var $$0 = 0, $$1 = 0, $$2 = 0, $$3 = 0;
   fake_return_waka123 : {
    loop_in : do {
     $$0 = 3;
@@ -49549,7 +49549,7 @@ function asmFunc(global, env, buffer) {
  }
  
  function $$21() {
-  var $$0 = 0, $$1 = 0;
+  var $$0 = 0, $$1 = 0, $$2 = 0, $$3 = 0;
   fake_return_waka123 : {
    loop_in : do {
     dummy();
@@ -49677,7 +49677,7 @@ function asmFunc(global, env, buffer) {
  }
  
  function $$31() {
-  var $$0 = 0, $$1 = 0, $$2 = 0;
+  var $$0 = 0, $$1 = 0, $$2 = 0, $$3 = 0;
   $$if : {
    $$0 = 2;
    $$1 = $$0;
@@ -49686,8 +49686,8 @@ function asmFunc(global, env, buffer) {
     break $$if;
    };
   };
-  $$2 = $$1;
-  return $$2 | 0;
+  $$3 = $$1;
+  return $$3 | 0;
  }
  
  function $$32($$0, $$1) {