remove-unused-brs: do not flow a value through a block if the block does not actually flow a value. fixes #1833 (#1835)

3 files changed, 81 insertions, 14 deletions

diff --git a/src/passes/RemoveUnusedBrs.cpp b/src/passes/RemoveUnusedBrs.cpp
index cb3ff69ee..58694b25c 100644
--- a/src/passes/RemoveUnusedBrs.cpp
+++ b/src/passes/RemoveUnusedBrs.cpp
@@ -106,10 +106,11 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
     } else if (auto* block = curr->dynCast<Block>()) {
       // any breaks flowing to here are unnecessary, as we get here anyhow
       auto name = block->name;
+      auto& list = block->list;
       if (name.is()) {
-        size_t size = flows.size();
-        size_t skip = 0;
-        for (size_t i = 0; i < size; i++) {
+        Index size = flows.size();
+        Index skip = 0;
+        for (Index i = 0; i < size; i++) {
           auto* flow = (*flows[i])->dynCast<Break>();
           if (flow && flow->name == name) {
             if (!flow->value) {
@@ -129,11 +130,22 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
           flows.resize(size - skip);
         }
         // drop a nop at the end of a block, which prevents a value flowing
-        while (block->list.size() > 0 && block->list.back()->is<Nop>()) {
-          block->list.resize(block->list.size() - 1);
+        while (list.size() > 0 && list.back()->is<Nop>()) {
+          list.resize(list.size() - 1);
           self->anotherCycle = true;
         }
       }
+      // A value flowing is only valid if it is a value that the block actually
+      // flows out. If it is never reached, it does not flow out, and may be
+      // invalid to represent as such.
+      auto size = list.size();
+      for (Index i = 0; i < size; i++) {
+        if (i != size - 1 && list[i]->type == unreachable) {
+          // No value flows out of this block.
+          self->stopValueFlow();
+          break;
+        }
+      }
     } else if (curr->is<Nop>()) {
       // ignore (could be result of a previous cycle)
       self->stopValueFlow();
@@ -515,7 +527,7 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
       super::doWalkFunction(func);
       assert(ifStack.empty());
       // flows may contain returns, which are flowing out and so can be optimized
-      for (size_t i = 0; i < flows.size(); i++) {
+      for (Index i = 0; i < flows.size(); i++) {
         auto* flow = (*flows[i])->dynCast<Return>();
         if (!flow) continue;
         if (!flow->value) {
diff --git a/test/passes/remove-unused-brs.txt b/test/passes/remove-unused-brs.txt
index 48cfad529..889e358c7 100644
--- a/test/passes/remove-unused-brs.txt
+++ b/test/passes/remove-unused-brs.txt
@@ -2212,21 +2212,20 @@
  (func $refinalize-need-br-value (; 101 ;) (type $2) (result i32)
   (loop $label$3 (result i32)
    (block $label$6 (result i32)
-    (block $label$10 (result i32)
+    (block $label$10
      (unreachable)
-     (block $label$503 (result i32)
+     (block $label$503
       (br_if $label$3
        (block $label$530 (result i32)
-        (block
-         (unreachable)
-         (drop
-          (i32.const 0)
-         )
+        (br_if $label$503
+         (i32.const 0)
         )
         (i32.const 0)
        )
       )
-      (i32.const 127)
+      (return
+       (i32.const 127)
+      )
      )
     )
    )
@@ -2404,4 +2403,32 @@
    (i32.const 1)
   )
  )
+ (func $do-not-flow-values-through-unreachable-code (; 113 ;) (type $2) (result i32)
+  (block $block
+   (unreachable)
+   (if
+    (i32.const 0)
+    (block $A
+     (return
+      (i32.const 0)
+     )
+    )
+    (nop)
+   )
+  )
+ )
+ (func $do-not-flow-values-through-unreachable-code-b (; 114 ;) (type $2) (result i32)
+  (loop $loop-in
+   (unreachable)
+   (if
+    (i32.const 0)
+    (block $A
+     (return
+      (i32.const 0)
+     )
+    )
+    (nop)
+   )
+  )
+ )
 )
diff --git a/test/passes/remove-unused-brs.wast b/test/passes/remove-unused-brs.wast
index 9d53f4fda..71f39e331 100644
--- a/test/passes/remove-unused-brs.wast
+++ b/test/passes/remove-unused-brs.wast
@@ -2011,5 +2011,33 @@
       )
     )
   )
+  (func $do-not-flow-values-through-unreachable-code (result i32)
+   (block
+    (unreachable)
+    (block $A
+     (if
+      (i32.const 0)
+      (return
+       (i32.const 0) ;; seems to flow out, but we are in unreachable code, and do not actually reach anywhere
+      )
+      (br $A) ;; can be a nop
+     )
+    )
+   )
+  )
+  (func $do-not-flow-values-through-unreachable-code-b (result i32)
+   (loop
+    (unreachable)
+    (block $A
+     (if
+      (i32.const 0)
+      (return
+       (i32.const 0)
+      )
+      (br $A)
+     )
+    )
+   )
+  )
 )