From 99588b8d8560cd5d829137ca039a2c4e50a727ca Mon Sep 17 00:00:00 2001
From: Dan Gohman <sunfish@mozilla.com>
Date: Mon, 8 Feb 2016 15:46:51 -0800
Subject: [PATCH 1/3] Make br_if return the value of its value operand.

---
 ml-proto/spec/check.ml    |  6 +--
 ml-proto/spec/eval.ml     |  2 +-
 ml-proto/test/labels.wast | 87 ++++++++++++++++++++++++++++++++-------
 3 files changed, 76 insertions(+), 19 deletions(-)

diff --git a/ml-proto/spec/check.ml b/ml-proto/spec/check.ml
index d5a4457248..e21c1da5eb 100644
--- a/ml-proto/spec/check.ml
+++ b/ml-proto/spec/check.ml
@@ -134,9 +134,9 @@ let rec check_expr c et e =
     check_expr_opt c (label c x) eo e.at
 
   | Br_if (x, eo, e) ->
-    check_expr_opt c (label c x) eo e.at;
+    check_expr_opt c et eo e.at;
     check_expr c (Some Int32Type) e;
-    check_type None et e.at
+    check_type (label c x) et e.at
 
   | If (e1, e2, e3) ->
     check_expr c (Some Int32Type) e1;
@@ -231,7 +231,7 @@ and check_exprs c ts es =
 
 and check_expr_opt c et eo at =
   match et, eo with
-  | Some t, Some e -> check_expr c et e
+  | _, Some e -> check_expr c et e
   | None, None -> ()
   | _ -> error at "arity mismatch"
 
diff --git a/ml-proto/spec/eval.ml b/ml-proto/spec/eval.ml
index 403b0859e7..511e8e533f 100644
--- a/ml-proto/spec/eval.ml
+++ b/ml-proto/spec/eval.ml
@@ -157,7 +157,7 @@ let rec eval_expr (c : config) (e : expr) =
   | Br_if (x, eo, e) ->
     let v = eval_expr_opt c eo in
     let i = int32 (eval_expr c e) e.at in
-    if i <> 0l then raise (label c x v) else None
+    if i <> 0l then raise (label c x v) else v
 
   | If (e1, e2, e3) ->
     let i = int32 (eval_expr c e1) e1.at in
diff --git a/ml-proto/test/labels.wast b/ml-proto/test/labels.wast
index 8c51f054c6..cac9c699c4 100644
--- a/ml-proto/test/labels.wast
+++ b/ml-proto/test/labels.wast
@@ -130,6 +130,30 @@
              (i32.const 0))
     (get_local $i1))
 
+  (func $br_if4 (result f32)
+    (block $l
+      (br_if $l (f32.const 0) (i32.const 1))))
+
+  (func $br_if5 (param i32) (result f32)
+    (block $l
+      (f32.neg
+        (block $i
+          (br_if $l (f32.const 3) (get_local 0))))))
+
+  (func $br_if6
+    (loop $l0 $l1 (br $l0 (i32.const 0))))
+
+  (func $br_if7
+    (block $l0 (br_if $l0 (nop) (i32.const 1))))
+
+  (func $br_if8 (result i32)
+    (block $l0
+      (if_else (i32.const 1)
+        (br $l0 (block $l1 (br $l1 (i32.const 1))))
+        (block (block $l1 (br $l1 (i32.const 1))) (nop))
+      )
+    (i32.const 1)))
+
   (func $misc1 (result i32)
    (block $l1 (i32.xor (br $l1 (i32.const 1)) (i32.const 2)))
   )
@@ -150,6 +174,11 @@
   (export "br_if1" $br_if1)
   (export "br_if2" $br_if2)
   (export "br_if3" $br_if3)
+  (export "br_if4" $br_if4)
+  (export "br_if5" $br_if5)
+  (export "br_if6" $br_if6)
+  (export "br_if7" $br_if7)
+  (export "br_if8" $br_if8)
   (export "misc1" $misc1)
   (export "misc2" $misc2)
 )
@@ -173,25 +202,53 @@
 (assert_return (invoke "br_if1") (i32.const 1))
 (assert_return (invoke "br_if2") (i32.const 1))
 (assert_return (invoke "br_if3") (i32.const 2))
+(assert_return (invoke "br_if4") (f32.const 0))
+(assert_return (invoke "br_if5" (i32.const 0)) (f32.const -3))
+(assert_return (invoke "br_if5" (i32.const 1)) (f32.const 3))
+(assert_return (invoke "br_if6"))
+(assert_return (invoke "br_if7"))
+(assert_return (invoke "br_if8") (i32.const 1))
 (assert_return (invoke "misc1") (i32.const 1))
 (assert_return (invoke "misc2") (i32.const 1))
 
-(assert_invalid (module (func (loop $l (br $l (i32.const 0))))) "arity mismatch")
-(assert_invalid (module (func (block $l (f32.neg (br_if $l (i32.const 1))) (nop)))) "type mismatch")
+(assert_invalid (module (func (block $l (f32.neg (br_if $l (i32.const 1))) (nop)))) "arity mismatch")
 
-(assert_invalid (module (func (result f32) (block $l (br_if $l (f32.const 0) (i32.const 1))))) "type mismatch")
 (assert_invalid (module (func (result i32) (block $l (br_if $l (f32.const 0) (i32.const 1))))) "type mismatch")
-(assert_invalid (module (func (block $l (f32.neg (br_if $l (f32.const 0) (i32.const 1)))))) "arity mismatch")
+(assert_invalid (module (func (block $l (f32.neg (br_if $l (f32.const 0) (i32.const 1)))))) "type mismatch")
 (assert_invalid (module (func (param i32) (result i32) (block $l (f32.neg (br_if $l (f32.const 0) (get_local 0)))))) "type mismatch")
-(assert_invalid (module (func (param i32) (result f32)
-  (block $l (f32.neg (block $i (br_if $l (f32.const 3) (get_local 0)))))))
+
+;; br_if's own result type doesn't match the result type of the block it's in.
+(assert_invalid (module
+  (func (param i32) (result f32)
+    (block $l
+      (f32.convert_s/i32
+        (block $i
+          (br_if $l (f32.const 3) (get_local 0)))))))
+  "type mismatch")
+
+;; br_if's result value's type doesn't match the type of the destination block.
+(assert_invalid (module
+  (func (param i32) (result f32)
+    (block $l
+      (i32.trunc_s/f32
+        (block $i
+          (br_if $l (f32.const 3) (get_local 0)))))))
+  "type mismatch")
+
+;; br_if operand has incorrect type.
+(assert_invalid (module
+  (func (param i32) (result f32)
+    (block $l
+      (f32.neg
+        (block $i
+          (br_if $l (i32.const 3) (get_local 0)))))))
+  "type mismatch")
+
+;; Function return type is wrong.
+(assert_invalid (module
+  (func (param i32) (result i32)
+    (block $l
+      (f32.neg
+        (block $i
+          (br_if $l (f32.const 3) (get_local 0)))))))
   "type mismatch")
-(assert_invalid (module (func (block $l0 (br_if $l0 (nop) (i32.const 1)))))
-  "arity mismatch")
-(assert_invalid (module (func (result i32)
-  (block $l0
-    (if_else (i32.const 1)
-      (br $l0 (block $l1 (br $l1 (i32.const 1))))
-      (block (block $l1 (br $l1 (i32.const 1))) (nop))
-    )
-  (i32.const 1)))) "arity mismatch")

From 8158ca1ab50950b0f9edd0a98dbe4e3b87ac5958 Mon Sep 17 00:00:00 2001
From: Dan Gohman <sunfish@mozilla.com>
Date: Mon, 8 Feb 2016 23:55:32 -0800
Subject: [PATCH 2/3] Type-check the result-value expression against the
 destination's type, not just the br_if's type.

---
 ml-proto/spec/check.ml    |  4 ++--
 ml-proto/test/labels.wast | 21 ++++++++++++++++++---
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/ml-proto/spec/check.ml b/ml-proto/spec/check.ml
index e21c1da5eb..57b21bec9a 100644
--- a/ml-proto/spec/check.ml
+++ b/ml-proto/spec/check.ml
@@ -135,8 +135,8 @@ let rec check_expr c et e =
 
   | Br_if (x, eo, e) ->
     check_expr_opt c et eo e.at;
-    check_expr c (Some Int32Type) e;
-    check_type (label c x) et e.at
+    check_expr_opt c (label c x) eo e.at;
+    check_expr c (Some Int32Type) e
 
   | If (e1, e2, e3) ->
     check_expr c (Some Int32Type) e1;
diff --git a/ml-proto/test/labels.wast b/ml-proto/test/labels.wast
index cac9c699c4..56e77428aa 100644
--- a/ml-proto/test/labels.wast
+++ b/ml-proto/test/labels.wast
@@ -141,10 +141,12 @@
           (br_if $l (f32.const 3) (get_local 0))))))
 
   (func $br_if6
-    (loop $l0 $l1 (br $l0 (i32.const 0))))
+    (loop $l0 $l1
+      (br $l0 (i32.const 0))))
 
   (func $br_if7
-    (block $l0 (br_if $l0 (nop) (i32.const 1))))
+    (block $l0
+      (br_if $l0 (nop) (i32.const 1))))
 
   (func $br_if8 (result i32)
     (block $l0
@@ -154,6 +156,10 @@
       )
     (i32.const 1)))
 
+  (func $br_if9
+    (block $l
+      (f32.neg (br_if $l (f32.const 0) (i32.const 1)))))
+
   (func $misc1 (result i32)
    (block $l1 (i32.xor (br $l1 (i32.const 1)) (i32.const 2)))
   )
@@ -179,6 +185,7 @@
   (export "br_if6" $br_if6)
   (export "br_if7" $br_if7)
   (export "br_if8" $br_if8)
+  (export "br_if9" $br_if9)
   (export "misc1" $misc1)
   (export "misc2" $misc2)
 )
@@ -208,13 +215,13 @@
 (assert_return (invoke "br_if6"))
 (assert_return (invoke "br_if7"))
 (assert_return (invoke "br_if8") (i32.const 1))
+(assert_return (invoke "br_if9"))
 (assert_return (invoke "misc1") (i32.const 1))
 (assert_return (invoke "misc2") (i32.const 1))
 
 (assert_invalid (module (func (block $l (f32.neg (br_if $l (i32.const 1))) (nop)))) "arity mismatch")
 
 (assert_invalid (module (func (result i32) (block $l (br_if $l (f32.const 0) (i32.const 1))))) "type mismatch")
-(assert_invalid (module (func (block $l (f32.neg (br_if $l (f32.const 0) (i32.const 1)))))) "type mismatch")
 (assert_invalid (module (func (param i32) (result i32) (block $l (f32.neg (br_if $l (f32.const 0) (get_local 0)))))) "type mismatch")
 
 ;; br_if's own result type doesn't match the result type of the block it's in.
@@ -252,3 +259,11 @@
         (block $i
           (br_if $l (f32.const 3) (get_local 0)))))))
   "type mismatch")
+
+;; br_if result not used, but value's type doesn't match branched-to block.
+(assert_invalid (module
+  (func (param i32) (result i32)
+    (block $l
+      (br_if $l (f32.const 3) (get_local 0))
+      (i32.const 0))))
+  "type mismatch")

From 733ce2de70b3a327b30e665ed553eada878b2987 Mon Sep 17 00:00:00 2001
From: Dan Gohman <sunfish@mozilla.com>
Date: Tue, 9 Feb 2016 04:42:18 -0800
Subject: [PATCH 3/3] If br_if has a value, require it to be used in both
 successors.

---
 ml-proto/spec/check.ml    |   2 +-
 ml-proto/test/labels.wast | 140 ++++++++++++++++++++------------------
 2 files changed, 74 insertions(+), 68 deletions(-)

diff --git a/ml-proto/spec/check.ml b/ml-proto/spec/check.ml
index 57b21bec9a..840daaea86 100644
--- a/ml-proto/spec/check.ml
+++ b/ml-proto/spec/check.ml
@@ -231,7 +231,7 @@ and check_exprs c ts es =
 
 and check_expr_opt c et eo at =
   match et, eo with
-  | _, Some e -> check_expr c et e
+  | Some t, Some e -> check_expr c et e
   | None, None -> ()
   | _ -> error at "arity mismatch"
 
diff --git a/ml-proto/test/labels.wast b/ml-proto/test/labels.wast
index 56e77428aa..c4ec2743e2 100644
--- a/ml-proto/test/labels.wast
+++ b/ml-proto/test/labels.wast
@@ -90,28 +90,6 @@
     )
   )
 
-  (func $br_if0 (result i32)
-    (local $i i32)
-    (set_local $i (i32.const 0))
-    (block $outer
-      (block $inner
-        (br_if $inner (i32.const 0))
-        (set_local $i (i32.or (get_local $i) (i32.const 0x1)))
-        (br_if $inner (i32.const 1))
-        (set_local $i (i32.or (get_local $i) (i32.const 0x2)))
-      )
-      (br_if $outer (set_local $i (i32.or (get_local $i) (i32.const 0x4))) (i32.const 0))
-      (set_local $i (i32.or (get_local $i) (i32.const 0x8)))
-      (br_if $outer (set_local $i (i32.or (get_local $i) (i32.const 0x10))) (i32.const 1))
-      (set_local $i (i32.or (get_local $i) (i32.const 0x20)))
-    )
-  )
-
-  (func $br_if1 (result i32)
-    (block $l0
-      (br_if $l0 (block $l1 (br $l1 (i32.const 1))) (i32.const 1))
-      (i32.const 1)))
-
   (func $br_if2 (result i32)
     (block $l0
       (if (i32.const 1)
@@ -120,16 +98,6 @@
             (br $l1 (i32.const 1)))))
       (i32.const 1)))
 
-  (func $br_if3 (result i32)
-    (local $i1 i32)
-    (i32.add (block $l0
-               (br_if $l0
-                      (set_local $i1 (i32.const 1))
-                      (set_local $i1 (i32.const 2)))
-               (i32.const 0))
-             (i32.const 0))
-    (get_local $i1))
-
   (func $br_if4 (result f32)
     (block $l
       (br_if $l (f32.const 0) (i32.const 1))))
@@ -140,26 +108,6 @@
         (block $i
           (br_if $l (f32.const 3) (get_local 0))))))
 
-  (func $br_if6
-    (loop $l0 $l1
-      (br $l0 (i32.const 0))))
-
-  (func $br_if7
-    (block $l0
-      (br_if $l0 (nop) (i32.const 1))))
-
-  (func $br_if8 (result i32)
-    (block $l0
-      (if_else (i32.const 1)
-        (br $l0 (block $l1 (br $l1 (i32.const 1))))
-        (block (block $l1 (br $l1 (i32.const 1))) (nop))
-      )
-    (i32.const 1)))
-
-  (func $br_if9
-    (block $l
-      (f32.neg (br_if $l (f32.const 0) (i32.const 1)))))
-
   (func $misc1 (result i32)
    (block $l1 (i32.xor (br $l1 (i32.const 1)) (i32.const 2)))
   )
@@ -176,16 +124,9 @@
   (export "loop5" $loop5)
   (export "switch" $switch)
   (export "return" $return)
-  (export "br_if0" $br_if0)
-  (export "br_if1" $br_if1)
   (export "br_if2" $br_if2)
-  (export "br_if3" $br_if3)
   (export "br_if4" $br_if4)
   (export "br_if5" $br_if5)
-  (export "br_if6" $br_if6)
-  (export "br_if7" $br_if7)
-  (export "br_if8" $br_if8)
-  (export "br_if9" $br_if9)
   (export "misc1" $misc1)
   (export "misc2" $misc2)
 )
@@ -205,17 +146,10 @@
 (assert_return (invoke "return" (i32.const 0)) (i32.const 0))
 (assert_return (invoke "return" (i32.const 1)) (i32.const 2))
 (assert_return (invoke "return" (i32.const 2)) (i32.const 2))
-(assert_return (invoke "br_if0") (i32.const 0x1d))
-(assert_return (invoke "br_if1") (i32.const 1))
 (assert_return (invoke "br_if2") (i32.const 1))
-(assert_return (invoke "br_if3") (i32.const 2))
 (assert_return (invoke "br_if4") (f32.const 0))
 (assert_return (invoke "br_if5" (i32.const 0)) (f32.const -3))
 (assert_return (invoke "br_if5" (i32.const 1)) (f32.const 3))
-(assert_return (invoke "br_if6"))
-(assert_return (invoke "br_if7"))
-(assert_return (invoke "br_if8") (i32.const 1))
-(assert_return (invoke "br_if9"))
 (assert_return (invoke "misc1") (i32.const 1))
 (assert_return (invoke "misc2") (i32.const 1))
 
@@ -224,6 +158,78 @@
 (assert_invalid (module (func (result i32) (block $l (br_if $l (f32.const 0) (i32.const 1))))) "type mismatch")
 (assert_invalid (module (func (param i32) (result i32) (block $l (f32.neg (br_if $l (f32.const 0) (get_local 0)))))) "type mismatch")
 
+;; If br_if has a value, it should be used in both successors.
+(assert_invalid (module
+  (func (result i32)
+    (local $i i32)
+    (set_local $i (i32.const 0))
+    (block $outer
+      (block $inner
+        (br_if $inner (i32.const 0))
+        (set_local $i (i32.or (get_local $i) (i32.const 0x1)))
+        (br_if $inner (i32.const 1))
+        (set_local $i (i32.or (get_local $i) (i32.const 0x2)))
+      )
+      (br_if $outer (set_local $i (i32.or (get_local $i) (i32.const 0x4))) (i32.const 0))
+      (set_local $i (i32.or (get_local $i) (i32.const 0x8)))
+      (br_if $outer (set_local $i (i32.or (get_local $i) (i32.const 0x10))) (i32.const 1))
+      (set_local $i (i32.or (get_local $i) (i32.const 0x20)))
+    ))
+) "arity mismatch")
+
+;; If br_if has a value, it should be used in both successors.
+(assert_invalid (module
+  (func (result i32)
+    (block $l0
+      (br_if $l0 (block $l1 (br $l1 (i32.const 1))) (i32.const 1))
+      (i32.const 1)))
+) "arity mismatch")
+
+;; If br_if has a value, it should be used in both successors.
+(assert_invalid (module
+  (func (result i32)
+    (local $i1 i32)
+    (i32.add (block $l0
+               (br_if $l0
+                      (set_local $i1 (i32.const 1))
+                      (set_local $i1 (i32.const 2)))
+               (i32.const 0))
+             (i32.const 0))
+    (get_local $i1))
+) "arity mismatch")
+
+;; If br_if has a value, it should be used in both successors.
+(assert_invalid (module
+  (func
+    (loop $l0 $l1
+      (br $l0 (i32.const 0))))
+) "arity mismatch")
+
+;; If br_if has a value, it should be used in both successors.
+(assert_invalid (module
+  (func
+    (block $l0
+      (br_if $l0 (nop) (i32.const 1))))
+) "arity mismatch")
+
+;; If br_if has a value, it should be used in both successors.
+(assert_invalid (module
+  (func
+    (block $l0
+      (if_else (i32.const 1)
+        (br $l0 (block $l1 (br $l1 (i32.const 1))))
+        (block (block $l1 (br $l1 (i32.const 1))) (nop))
+      )
+    (i32.const 1)))
+) "arity mismatch")
+
+;; If br_if has a value, it should be used in both successors.
+(assert_invalid (module
+  (func
+    (block $l
+      (f32.neg (br_if $l (f32.const 0) (i32.const 1)))))
+) "arity mismatch")
+
 ;; br_if's own result type doesn't match the result type of the block it's in.
 (assert_invalid (module
   (func (param i32) (result f32)
@@ -266,4 +272,4 @@
     (block $l
       (br_if $l (f32.const 3) (get_local 0))
       (i32.const 0))))
-  "type mismatch")
+  "arity mismatch")