RemoveUnusedBrs: Avoid an error on loops with unreachable ifs (#7156)

We normally like to move brs after ifs into the if, when in a loop:

(loop $loop
  (if
    ..
    (unreachable)
    (code)
  )
  (br $loop)
)

=>

(loop $loop
  (if
    ..
    (unreachable)
    (block
      (code)
      (br $loop) ;; moved in
    )
  )
)

However this may be invalid to do if the if condition is unreachable, as
then one arm may be concrete (`code` in the example could be an `i32`,
for example). As this is dead code anyhow, leave it for DCE.

2 files changed, 57 insertions, 19 deletions

diff --git a/src/passes/RemoveUnusedBrs.cpp b/src/passes/RemoveUnusedBrs.cpp
index 32f7bd48a..47a1f1c65 100644
--- a/src/passes/RemoveUnusedBrs.cpp
+++ b/src/passes/RemoveUnusedBrs.cpp
@@ -621,9 +621,13 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
             block->finalize();
             return true;
           }
-        } else {
-          // this is already an if-else. if one side is a dead end, we can
-          // append to the other, if there is no returned value to concern us
+        } else if (iff->condition->type != Type::unreachable) {
+          // This is already an if-else. If one side is a dead end, we can
+          // append to the other, if there is no returned value to concern us.
+          // Note that we skip ifs with unreachable conditions, as they are dead
+          // code that DCE can remove, and modifying them can lead to errors
+          // (one of the arms may still be concrete, in which case appending to
+          // it would be invalid).
 
           // can't be, since in the middle of a block
           assert(!iff->type.isConcrete());
diff --git a/test/lit/passes/remove-unused-brs.wast b/test/lit/passes/remove-unused-brs.wast
index 2019d37a4..c61320602 100644
--- a/test/lit/passes/remove-unused-brs.wast
+++ b/test/lit/passes/remove-unused-brs.wast
@@ -30,7 +30,7 @@
     )
   )
 
-  ;; CHECK:      (func $selectify-simple (type $0) (param $0 i32) (result i32)
+  ;; CHECK:      (func $selectify-simple (type $1) (param $0 i32) (result i32)
   ;; CHECK-NEXT:  (select
   ;; CHECK-NEXT:   (i32.const 1)
   ;; CHECK-NEXT:   (i32.lt_u
@@ -73,7 +73,7 @@
     )
   )
 
-  ;; CHECK:      (func $restructure-br_if (type $0) (param $x i32) (result i32)
+  ;; CHECK:      (func $restructure-br_if (type $1) (param $x i32) (result i32)
   ;; CHECK-NEXT:  (if (result i32)
   ;; CHECK-NEXT:   (local.get $x)
   ;; CHECK-NEXT:   (then
@@ -104,12 +104,12 @@
     )
   )
 
-  ;; CHECK:      (func $nothing (type $1)
+  ;; CHECK:      (func $nothing (type $0)
   ;; CHECK-NEXT: )
   (func $nothing)
 
 
-  ;; CHECK:      (func $restructure-br_if-condition-reorderable (type $0) (param $x i32) (result i32)
+  ;; CHECK:      (func $restructure-br_if-condition-reorderable (type $1) (param $x i32) (result i32)
   ;; CHECK-NEXT:  (if (result i32)
   ;; CHECK-NEXT:   (block (result i32)
   ;; CHECK-NEXT:    (call $nothing)
@@ -146,7 +146,7 @@
     )
   )
 
-  ;; CHECK:      (func $restructure-br_if-value-effectful (type $0) (param $x i32) (result i32)
+  ;; CHECK:      (func $restructure-br_if-value-effectful (type $1) (param $x i32) (result i32)
   ;; CHECK-NEXT:  (select
   ;; CHECK-NEXT:   (block (result i32)
   ;; CHECK-NEXT:    (call $nothing)
@@ -188,7 +188,7 @@
     )
   )
 
-  ;; CHECK:      (func $restructure-br_if-value-effectful-corner-case-1 (type $0) (param $x i32) (result i32)
+  ;; CHECK:      (func $restructure-br_if-value-effectful-corner-case-1 (type $1) (param $x i32) (result i32)
   ;; CHECK-NEXT:  (block $x (result i32)
   ;; CHECK-NEXT:   (drop
   ;; CHECK-NEXT:    (br_if $x
@@ -233,7 +233,7 @@
     (i32.const 400)
   )
 
-  ;; CHECK:      (func $restructure-br_if-value-effectful-corner-case-2 (type $0) (param $x i32) (result i32)
+  ;; CHECK:      (func $restructure-br_if-value-effectful-corner-case-2 (type $1) (param $x i32) (result i32)
   ;; CHECK-NEXT:  (block $x (result i32)
   ;; CHECK-NEXT:   (drop
   ;; CHECK-NEXT:    (br_if $x
@@ -272,7 +272,7 @@
       (call $get-i32)
     )
   )
-  ;; CHECK:      (func $restructure-br_if-value-effectful-corner-case-3 (type $0) (param $x i32) (result i32)
+  ;; CHECK:      (func $restructure-br_if-value-effectful-corner-case-3 (type $1) (param $x i32) (result i32)
   ;; CHECK-NEXT:  (block $x (result i32)
   ;; CHECK-NEXT:   (drop
   ;; CHECK-NEXT:    (br_if $x
@@ -305,7 +305,7 @@
     )
   )
 
-  ;; CHECK:      (func $restructure-br_if-value-effectful-corner-case-4 (type $0) (param $x i32) (result i32)
+  ;; CHECK:      (func $restructure-br_if-value-effectful-corner-case-4 (type $1) (param $x i32) (result i32)
   ;; CHECK-NEXT:  (block $x (result i32)
   ;; CHECK-NEXT:   (drop
   ;; CHECK-NEXT:    (br_if $x
@@ -340,7 +340,7 @@
     )
   )
 
-  ;; CHECK:      (func $restructure-select-no-multivalue (type $1)
+  ;; CHECK:      (func $restructure-select-no-multivalue (type $0)
   ;; CHECK-NEXT:  (tuple.drop 2
   ;; CHECK-NEXT:   (block $block (type $2) (result i32 i32)
   ;; CHECK-NEXT:    (tuple.drop 2
@@ -387,7 +387,7 @@
     )
   )
 
-  ;; CHECK:      (func $if-of-if (type $1)
+  ;; CHECK:      (func $if-of-if (type $0)
   ;; CHECK-NEXT:  (local $x i32)
   ;; CHECK-NEXT:  (if
   ;; CHECK-NEXT:   (select
@@ -421,7 +421,7 @@
     )
   )
 
-  ;; CHECK:      (func $if-of-if-but-side-effects (type $1)
+  ;; CHECK:      (func $if-of-if-but-side-effects (type $0)
   ;; CHECK-NEXT:  (local $x i32)
   ;; CHECK-NEXT:  (if
   ;; CHECK-NEXT:   (local.tee $x
@@ -460,7 +460,7 @@
     )
   )
 
-  ;; CHECK:      (func $if-of-if-but-too-costly (type $1)
+  ;; CHECK:      (func $if-of-if-but-too-costly (type $0)
   ;; CHECK-NEXT:  (local $x i32)
   ;; CHECK-NEXT:  (if
   ;; CHECK-NEXT:   (local.tee $x
@@ -515,7 +515,7 @@
     )
   )
 
-  ;; CHECK:      (func $if-of-if-but-inner-else (type $1)
+  ;; CHECK:      (func $if-of-if-but-inner-else (type $0)
   ;; CHECK-NEXT:  (local $x i32)
   ;; CHECK-NEXT:  (if
   ;; CHECK-NEXT:   (local.tee $x
@@ -555,7 +555,7 @@
     )
   )
 
-  ;; CHECK:      (func $if-of-if-but-outer-else (type $1)
+  ;; CHECK:      (func $if-of-if-but-outer-else (type $0)
   ;; CHECK-NEXT:  (local $x i32)
   ;; CHECK-NEXT:  (if
   ;; CHECK-NEXT:   (local.tee $x
@@ -595,7 +595,7 @@
     )
   )
 
-  ;; CHECK:      (func $unreachable-if (type $1)
+  ;; CHECK:      (func $unreachable-if (type $0)
   ;; CHECK-NEXT:  (block $block
   ;; CHECK-NEXT:   (if (result i32)
   ;; CHECK-NEXT:    (unreachable)
@@ -624,4 +624,38 @@
       )
     )
   )
+
+  ;; CHECK:      (func $loop-with-unreachable-if (type $0)
+  ;; CHECK-NEXT:  (loop $label
+  ;; CHECK-NEXT:   (if (result i32)
+  ;; CHECK-NEXT:    (unreachable)
+  ;; CHECK-NEXT:    (then
+  ;; CHECK-NEXT:     (unreachable)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:    (else
+  ;; CHECK-NEXT:     (i32.const 0)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:   (br $label)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $loop-with-unreachable-if
+    ;; We normally move brs right after an if into one of the if arms, when
+    ;; possible. That is almost possible here, but the if condition is
+    ;; unreachable, which allows one of the arms to have a concrete type. It is
+    ;; invalid to append to such an arm, so we should do nothing (leaving this
+    ;; for DCE).
+    (loop $label
+      (if (result i32)
+        (unreachable)
+        (then
+          (unreachable)
+        )
+        (else
+          (i32.const 0)
+        )
+      )
+      (br $label)
+    )
+  )
 )