Make more Ifs unreachable (#7094)

Previously the only Ifs that were typed unreachable were those in which both arms were unreachable and those in which the condition was unreachable that would have otherwise been typed none. This caused problems in IRBuilder because Ifs with unreachable conditions and value-returning arms would have concrete types, effectively hiding the unreachable condition from the logic for dropping concretely typed expressions preceding an unreachable expression when finishing a scope. Relax the conditions under which an If can be typed unreachable so that all Ifs with unreachable conditions or two unreachable arms are typed unreachable. Propagating unreachability more eagerly this way makes various optimizations of Ifs more powerful. It also requires new handling for unreachable Ifs with concretely typed arms in the Printer to ensure that printed wat remains valid. Also update Unsubtyping, Flatten, and CodeFolding to account for the newly unreachable Ifs.
author: Thomas Lively <tlively123@gmail.com> 2024-11-26 17:12:15 -0800
committer: GitHub <noreply@github.com> 2024-11-27 01:12:15 +0000
commit: 6f0f2e00521843118b63f41732dc2eb86d39fa09 (patch)
tree: 260d2be13758e3768955ee1f2c684102b6fea39a /test
parent: cd3805e31e8f2544d5e32aa505fc5e3abcf93df2 (diff)
download: binaryen-6f0f2e00521843118b63f41732dc2eb86d39fa09.tar.gz
binaryen-6f0f2e00521843118b63f41732dc2eb86d39fa09.tar.bz2
binaryen-6f0f2e00521843118b63f41732dc2eb86d39fa09.zip
13 files changed, 234 insertions, 67 deletions
diff --git a/test/lit/passes/code-folding.wast b/test/lit/passes/code-folding.wast
index 007aa5909..b256b236c 100644
--- a/test/lit/passes/code-folding.wast
+++ b/test/lit/passes/code-folding.wast
@@ -743,12 +743,20 @@
  )
 
  ;; CHECK:      (func $unreachable-if (type $0)
- ;; CHECK-NEXT:  (drop
+ ;; CHECK-NEXT:  (if
  ;; CHECK-NEXT:   (unreachable)
- ;; CHECK-NEXT:  )
- ;; CHECK-NEXT:  (nop)
- ;; CHECK-NEXT:  (drop
- ;; CHECK-NEXT:   (i32.const 1)
+ ;; CHECK-NEXT:   (then
+ ;; CHECK-NEXT:    (nop)
+ ;; CHECK-NEXT:    (drop
+ ;; CHECK-NEXT:     (i32.const 1)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:   (else
+ ;; CHECK-NEXT:    (nop)
+ ;; CHECK-NEXT:    (drop
+ ;; CHECK-NEXT:     (i32.const 1)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:   )
  ;; CHECK-NEXT:  )
  ;; CHECK-NEXT: )
  (func $unreachable-if
@@ -773,14 +781,17 @@
  ;; CHECK-NEXT:  (if
  ;; CHECK-NEXT:   (unreachable)
  ;; CHECK-NEXT:   (then
+ ;; CHECK-NEXT:    (drop
+ ;; CHECK-NEXT:     (i32.const 1)
+ ;; CHECK-NEXT:    )
  ;; CHECK-NEXT:   )
  ;; CHECK-NEXT:   (else
  ;; CHECK-NEXT:    (nop)
+ ;; CHECK-NEXT:    (drop
+ ;; CHECK-NEXT:     (i32.const 1)
+ ;; CHECK-NEXT:    )
  ;; CHECK-NEXT:   )
  ;; CHECK-NEXT:  )
- ;; CHECK-NEXT:  (drop
- ;; CHECK-NEXT:   (i32.const 1)
- ;; CHECK-NEXT:  )
  ;; CHECK-NEXT: )
  (func $unreachable-if-suffix
   (if
@@ -800,16 +811,13 @@
  )
 
  ;; CHECK:      (func $unreachable-if-concrete-arms (type $0)
- ;; CHECK-NEXT:  (drop
- ;; CHECK-NEXT:   (block (result i32)
- ;; CHECK-NEXT:    (if
- ;; CHECK-NEXT:     (unreachable)
- ;; CHECK-NEXT:     (then
- ;; CHECK-NEXT:     )
- ;; CHECK-NEXT:     (else
- ;; CHECK-NEXT:      (nop)
- ;; CHECK-NEXT:     )
- ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:  (if (result i32)
+ ;; CHECK-NEXT:   (unreachable)
+ ;; CHECK-NEXT:   (then
+ ;; CHECK-NEXT:    (i32.const 1)
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:   (else
+ ;; CHECK-NEXT:    (nop)
  ;; CHECK-NEXT:    (i32.const 1)
  ;; CHECK-NEXT:   )
  ;; CHECK-NEXT:  )
diff --git a/test/lit/passes/flatten_all-features.wast b/test/lit/passes/flatten_all-features.wast
index 5db45e272..b601b8a12 100644
--- a/test/lit/passes/flatten_all-features.wast
+++ b/test/lit/passes/flatten_all-features.wast
@@ -620,7 +620,6 @@
  ;; CHECK-NEXT:  (local $1 i32)
  ;; CHECK-NEXT:  (local $2 i32)
  ;; CHECK-NEXT:  (local $3 i32)
- ;; CHECK-NEXT:  (local $4 i32)
  ;; CHECK-NEXT:  (block $x
  ;; CHECK-NEXT:   (block
  ;; CHECK-NEXT:    (local.set $0
@@ -646,18 +645,13 @@
  ;; CHECK-NEXT:     )
  ;; CHECK-NEXT:    )
  ;; CHECK-NEXT:   )
- ;; CHECK-NEXT:   (local.set $3
- ;; CHECK-NEXT:    (local.get $2)
- ;; CHECK-NEXT:   )
- ;; CHECK-NEXT:   (local.set $1
- ;; CHECK-NEXT:    (local.get $3)
- ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:   (unreachable)
  ;; CHECK-NEXT:  )
- ;; CHECK-NEXT:  (local.set $4
+ ;; CHECK-NEXT:  (local.set $3
  ;; CHECK-NEXT:   (local.get $1)
  ;; CHECK-NEXT:  )
  ;; CHECK-NEXT:  (return
- ;; CHECK-NEXT:   (local.get $4)
+ ;; CHECK-NEXT:   (local.get $3)
  ;; CHECK-NEXT:  )
  ;; CHECK-NEXT: )
  (func $a13 (result i32)
diff --git a/test/lit/passes/optimize-instructions-ignore-traps.wast b/test/lit/passes/optimize-instructions-ignore-traps.wast
index 96ea16449..8902cbc28 100644
--- a/test/lit/passes/optimize-instructions-ignore-traps.wast
+++ b/test/lit/passes/optimize-instructions-ignore-traps.wast
@@ -213,7 +213,7 @@
   ;; CHECK-NEXT:      (i32.const 1)
   ;; CHECK-NEXT:     )
   ;; CHECK-NEXT:    )
-  ;; CHECK-NEXT:    (local.set $0
+  ;; CHECK-NEXT:    (local.tee $0
   ;; CHECK-NEXT:     (if (result i32)
   ;; CHECK-NEXT:      (i32.or
   ;; CHECK-NEXT:       (i32.eqz
diff --git a/test/lit/passes/optimize-instructions-mvp.wast b/test/lit/passes/optimize-instructions-mvp.wast
index f18c94c5c..077ecf495 100644
--- a/test/lit/passes/optimize-instructions-mvp.wast
+++ b/test/lit/passes/optimize-instructions-mvp.wast
@@ -5754,15 +5754,13 @@
   ;; CHECK-NEXT:   )
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT:  (drop
-  ;; CHECK-NEXT:   (block (result i32)
-  ;; CHECK-NEXT:    (i32.add
-  ;; CHECK-NEXT:     (local.get $1)
-  ;; CHECK-NEXT:     (unreachable)
-  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:   (i32.add
+  ;; CHECK-NEXT:    (local.get $1)
+  ;; CHECK-NEXT:    (unreachable)
   ;; CHECK-NEXT:   )
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT:  (drop
-  ;; CHECK-NEXT:   (block (result i32)
+  ;; CHECK-NEXT:   (block
   ;; CHECK-NEXT:    (drop
   ;; CHECK-NEXT:     (local.tee $0
   ;; CHECK-NEXT:      (local.get $1)
@@ -5775,7 +5773,7 @@
   ;; CHECK-NEXT:   )
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT:  (drop
-  ;; CHECK-NEXT:   (if (result i32)
+  ;; CHECK-NEXT:   (if
   ;; CHECK-NEXT:    (unreachable)
   ;; CHECK-NEXT:    (then
   ;; CHECK-NEXT:     (i32.add
@@ -15718,23 +15716,21 @@
       )
     )
   )
-  ;; CHECK:      (func $if-dont-change-to-unreachable (param $x i32) (param $y i32) (param $z i32) (result i32)
-  ;; CHECK-NEXT:  (if (result i32)
-  ;; CHECK-NEXT:   (local.get $x)
-  ;; CHECK-NEXT:   (then
-  ;; CHECK-NEXT:    (return
+  ;; CHECK:      (func $if-unreachable-return-identical (param $x i32) (param $y i32) (param $z i32) (result i32)
+  ;; CHECK-NEXT:  (return
+  ;; CHECK-NEXT:   (if (result i32)
+  ;; CHECK-NEXT:    (local.get $x)
+  ;; CHECK-NEXT:    (then
   ;; CHECK-NEXT:     (local.get $y)
   ;; CHECK-NEXT:    )
-  ;; CHECK-NEXT:   )
-  ;; CHECK-NEXT:   (else
-  ;; CHECK-NEXT:    (return
+  ;; CHECK-NEXT:    (else
   ;; CHECK-NEXT:     (local.get $z)
   ;; CHECK-NEXT:    )
   ;; CHECK-NEXT:   )
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
-  (func $if-dont-change-to-unreachable (param $x i32) (param $y i32) (param $z i32) (result i32)
-    ;; if we move the returns outside, we'd become unreachable; avoid that.
+  (func $if-unreachable-return-identical (param $x i32) (param $y i32) (param $z i32) (result i32)
+    ;; We can move the returns outside because we are already unreachable.
     (if (result i32)
       (local.get $x)
       (then
diff --git a/test/lit/passes/unsubtyping.wast b/test/lit/passes/unsubtyping.wast
index 97a2dd59a..149647165 100644
--- a/test/lit/passes/unsubtyping.wast
+++ b/test/lit/passes/unsubtyping.wast
@@ -1815,3 +1815,43 @@
   )
  )
 )
+
+;; Regression test for a crash on ifs with unreachable conditions and tuple arms.
+(module
+ ;; CHECK:      (type $0 (func (result i32 i64)))
+
+ ;; CHECK:      (func $test (type $0) (result i32 i64)
+ ;; CHECK-NEXT:  (if (type $0) (result i32 i64)
+ ;; CHECK-NEXT:   (unreachable)
+ ;; CHECK-NEXT:   (then
+ ;; CHECK-NEXT:    (tuple.make 2
+ ;; CHECK-NEXT:     (i32.const 0)
+ ;; CHECK-NEXT:     (i64.const 1)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:   (else
+ ;; CHECK-NEXT:    (tuple.make 2
+ ;; CHECK-NEXT:     (i32.const 2)
+ ;; CHECK-NEXT:     (i64.const 3)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $test (result i32 i64)
+  (if (result i32 i64)
+   (unreachable)
+   (then
+    (tuple.make 2
+     (i32.const 0)
+     (i64.const 1)
+    )
+   )
+   (else
+    (tuple.make 2
+     (i32.const 2)
+     (i64.const 3)
+    )
+   )
+  )
+ )
+)
diff --git a/test/lit/wat-kitchen-sink.wast b/test/lit/wat-kitchen-sink.wast
index 33d2e1d62..9cf7f27c5 100644
--- a/test/lit/wat-kitchen-sink.wast
+++ b/test/lit/wat-kitchen-sink.wast
@@ -1280,6 +1280,67 @@
   end
  )
 
+ ;; CHECK:      (func $if-else-unreachable (type $1) (result i32)
+ ;; CHECK-NEXT:  (drop
+ ;; CHECK-NEXT:   (i32.const 0)
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT:  (if (result i32)
+ ;; CHECK-NEXT:   (unreachable)
+ ;; CHECK-NEXT:   (then
+ ;; CHECK-NEXT:    (i32.const 1)
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:   (else
+ ;; CHECK-NEXT:    (i32.const 2)
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $if-else-unreachable (result i32)
+  i32.const 0 ;; This will be dropped
+  unreachable
+  if (result i32)
+   i32.const 1
+  else
+   i32.const 2
+  end
+ )
+
+ ;; CHECK:      (func $if-else-nested-unreachable (type $1) (result i32)
+ ;; CHECK-NEXT:  (drop
+ ;; CHECK-NEXT:   (i32.const 0)
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT:  (if (result i32)
+ ;; CHECK-NEXT:   (if (result i32)
+ ;; CHECK-NEXT:    (unreachable)
+ ;; CHECK-NEXT:    (then
+ ;; CHECK-NEXT:     (i32.const 1)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:    (else
+ ;; CHECK-NEXT:     (i32.const 2)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:   (then
+ ;; CHECK-NEXT:    (i32.const 3)
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:   (else
+ ;; CHECK-NEXT:    (i32.const 4)
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $if-else-nested-unreachable (result i32)
+   i32.const 0 ;; This will be dropped
+   unreachable
+   if (result i32)
+    i32.const 1
+   else
+    i32.const 2
+   end
+   if (result i32)
+    i32.const 3
+   else
+    i32.const 4
+   end
+  )
+
  ;; CHECK:      (func $if-else-labeled-result (type $1) (result i32)
  ;; CHECK-NEXT:  (block $l (result i32)
  ;; CHECK-NEXT:   (if (result i32)
@@ -1607,7 +1668,7 @@
 
  ;; CHECK:      (func $if-else-brs-i32 (type $1) (result i32)
  ;; CHECK-NEXT:  (block $label (result i32)
- ;; CHECK-NEXT:   (if (result i32)
+ ;; CHECK-NEXT:   (if
  ;; CHECK-NEXT:    (i32.const 0)
  ;; CHECK-NEXT:    (then
  ;; CHECK-NEXT:     (br $label
@@ -3677,7 +3738,7 @@
  (func $ref-func
   ref.func $ref-func
   drop
-  ref.func 162
+  ref.func 164
   drop
  )
 
diff --git a/test/passes/remove-unused-brs_enable-multivalue.txt b/test/passes/remove-unused-brs_enable-multivalue.txt
index ee6efbf4d..54780f0d9 100644
--- a/test/passes/remove-unused-brs_enable-multivalue.txt
+++ b/test/passes/remove-unused-brs_enable-multivalue.txt
@@ -2070,8 +2070,8 @@
      (i32.const 2)
     )
    )
-   (local.set $x
-    (if (result i32)
+   (local.tee $x
+    (if
      (local.get $p)
      (then
       (br $out)
@@ -2094,7 +2094,7 @@
       (block $label$4 (result i64)
        (block $label$5
         (block $label$6
-         (local.set $var$1
+         (local.tee $var$1
           (if (result f64)
            (unreachable)
            (then
diff --git a/test/passes/remove-unused-names_code-folding.txt b/test/passes/remove-unused-names_code-folding.txt
index 85810131b..62d7e3405 100644
--- a/test/passes/remove-unused-names_code-folding.txt
+++ b/test/passes/remove-unused-names_code-folding.txt
@@ -1511,13 +1511,19 @@
   )
   (nop)
   (drop
-   (block (result i32)
-    (drop
-     (unreachable)
+   (if (result i32)
+    (unreachable)
+    (then
+     (i32.add
+      (i32.const 1)
+      (i32.const 2)
+     )
     )
-    (i32.add
-     (i32.const 1)
-     (i32.const 2)
+    (else
+     (i32.add
+      (i32.const 1)
+      (i32.const 2)
+     )
     )
    )
   )
diff --git a/test/spec/if.wast b/test/spec/if.wast
index ae7f7b385..0fef1078f 100644
--- a/test/spec/if.wast
+++ b/test/spec/if.wast
@@ -643,16 +643,18 @@
   ))
   "type mismatch"
 )
-(assert_invalid
-  (module (func $type-else-value-unreached-select (result i32)
-    (if (result i64)
-      (i32.const 1)
-      (then (select (unreachable) (unreachable) (unreachable)))
-      (else (select (unreachable) (unreachable) (unreachable)))
-    )
-  ))
-  "type mismatch"
-)
+
+;; We don't pass this test because we type the `if` as unreachable.
+;; (assert_invalid
+;;   (module (func $type-else-value-unreached-select (result i32)
+;;     (if (result i64)
+;;       (i32.const 1)
+;;       (then (select (unreachable) (unreachable) (unreachable)))
+;;       (else (select (unreachable) (unreachable) (unreachable)))
+;;     )
+;;   ))
+;;   "type mismatch"
+;; )
 
 (assert_invalid
   (module (func $type-then-break-last-void-vs-num (result i32)
diff --git a/test/wasm2js/br_table_temp.2asm.js b/test/wasm2js/br_table_temp.2asm.js
index 245792eaf..704ec0687 100644
--- a/test/wasm2js/br_table_temp.2asm.js
+++ b/test/wasm2js/br_table_temp.2asm.js
@@ -12671,7 +12671,7 @@ function asmFunc(imports) {
  }
  
  function $30() {
-  var $1_1 = 0, $2_1 = 0;
+  var $1_1 = 0;
   block : {
    $1_1 = 2;
    switch (0 | 0) {
diff --git a/test/wasm2js/unreachable-if.2asm.js b/test/wasm2js/unreachable-if.2asm.js
new file mode 100644
index 000000000..f5656223c
--- /dev/null
+++ b/test/wasm2js/unreachable-if.2asm.js
@@ -0,0 +1,19 @@
+
+function asmFunc(imports) {
+ var Math_imul = Math.imul;
+ var Math_fround = Math.fround;
+ var Math_abs = Math.abs;
+ var Math_clz32 = Math.clz32;
+ var Math_min = Math.min;
+ var Math_max = Math.max;
+ var Math_floor = Math.floor;
+ var Math_ceil = Math.ceil;
+ var Math_trunc = Math.trunc;
+ var Math_sqrt = Math.sqrt;
+ return {
+  
+ };
+}
+
+var retasmFunc = asmFunc({
+});
diff --git a/test/wasm2js/unreachable-if.2asm.js.opt b/test/wasm2js/unreachable-if.2asm.js.opt
new file mode 100644
index 000000000..f5656223c
--- /dev/null
+++ b/test/wasm2js/unreachable-if.2asm.js.opt
@@ -0,0 +1,19 @@
+
+function asmFunc(imports) {
+ var Math_imul = Math.imul;
+ var Math_fround = Math.fround;
+ var Math_abs = Math.abs;
+ var Math_clz32 = Math.clz32;
+ var Math_min = Math.min;
+ var Math_max = Math.max;
+ var Math_floor = Math.floor;
+ var Math_ceil = Math.ceil;
+ var Math_trunc = Math.trunc;
+ var Math_sqrt = Math.sqrt;
+ return {
+  
+ };
+}
+
+var retasmFunc = asmFunc({
+});
diff --git a/test/wasm2js/unreachable-if.wast b/test/wasm2js/unreachable-if.wast
new file mode 100644
index 000000000..5bc0257c0
--- /dev/null
+++ b/test/wasm2js/unreachable-if.wast
@@ -0,0 +1,22 @@
+;; Regression test for bad assertion in autodrop that did not expect the if to
+;; be finalized to unreachable.
+(module
+ (func $test (result i32)
+  (block $l (result i32)
+   (drop
+    (br_if $l
+     (if (result i32)
+      (unreachable)
+      (then
+       (i32.const 0)
+      )
+      (else
+       (i32.const 0)
+      )
+     )
+    )
+    (i32.const 0)
+   )
+  )
+ )
+)
author	Thomas Lively <tlively123@gmail.com>	2024-11-26 17:12:15 -0800
committer	GitHub <noreply@github.com>	2024-11-27 01:12:15 +0000
commit	6f0f2e00521843118b63f41732dc2eb86d39fa09 (patch)
tree	260d2be13758e3768955ee1f2c684102b6fea39a /test
parent	cd3805e31e8f2544d5e32aa505fc5e3abcf93df2 (diff)
download	binaryen-6f0f2e00521843118b63f41732dc2eb86d39fa09.tar.gz binaryen-6f0f2e00521843118b63f41732dc2eb86d39fa09.tar.bz2 binaryen-6f0f2e00521843118b63f41732dc2eb86d39fa09.zip