summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThomas Lively <tlively123@gmail.com>2024-12-02 15:18:57 -0800
committerGitHub <noreply@github.com>2024-12-02 23:18:57 +0000
commit74782d217ed15dd73b58b1636c563aa51334a576 (patch)
tree4a4de972ae57b23ff6e0b02fbccb853a45cde881
parent31c988b30556ef000bd2212754d7fc5beebf08d2 (diff)
downloadbinaryen-74782d217ed15dd73b58b1636c563aa51334a576.tar.gz
binaryen-74782d217ed15dd73b58b1636c563aa51334a576.tar.bz2
binaryen-74782d217ed15dd73b58b1636c563aa51334a576.zip
Do not sink blocks into ifs with unreachable conditions (#7129)
RemoveUnusedBrs sinks blocks into If arms when those arms contain branches to the blocks and the other arm and condition do not. Now that we type Ifs with unreachable conditions as unreachable, it is possible for the If arms to have a different type than the block that would be sunk, so sinking the block would produce invalid IR. Fix the problem by never sinking blocks into Ifs with unreachable conditions. Fixes #7128.
-rw-r--r--src/passes/RemoveUnusedBrs.cpp6
-rw-r--r--test/lit/passes/remove-unused-brs.wast30
-rw-r--r--test/passes/remove-unused-brs_enable-multivalue.txt26
3 files changed, 49 insertions, 13 deletions
diff --git a/src/passes/RemoveUnusedBrs.cpp b/src/passes/RemoveUnusedBrs.cpp
index 44549f68d..32f7bd48a 100644
--- a/src/passes/RemoveUnusedBrs.cpp
+++ b/src/passes/RemoveUnusedBrs.cpp
@@ -757,6 +757,12 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
replaceCurrent(loop);
worked = true;
} else if (auto* iff = curr->list[0]->dynCast<If>()) {
+ if (iff->condition->type == Type::unreachable) {
+ // The block result type may not be compatible with the arm result
+ // types since the unreachable If can satisfy any type of block.
+ // Just leave this for DCE.
+ return;
+ }
// The label can't be used in the condition.
if (BranchUtils::BranchSeeker::count(iff->condition, curr->name) ==
0) {
diff --git a/test/lit/passes/remove-unused-brs.wast b/test/lit/passes/remove-unused-brs.wast
index 3380db6b7..2019d37a4 100644
--- a/test/lit/passes/remove-unused-brs.wast
+++ b/test/lit/passes/remove-unused-brs.wast
@@ -594,4 +594,34 @@
)
)
)
+
+ ;; CHECK: (func $unreachable-if (type $1)
+ ;; CHECK-NEXT: (block $block
+ ;; CHECK-NEXT: (if (result i32)
+ ;; CHECK-NEXT: (unreachable)
+ ;; CHECK-NEXT: (then
+ ;; CHECK-NEXT: (i32.const 0)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (else
+ ;; CHECK-NEXT: (br $block)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ (func $unreachable-if
+ ;; Regression test for a problem where blocks were sunk into ifs with
+ ;; unreachable conditions, causing validation errors when the block type was
+ ;; incompatible with the if type.
+ (block $block
+ (if (result i32)
+ (unreachable)
+ (then
+ (i32.const 0)
+ )
+ (else
+ (br $block)
+ )
+ )
+ )
+ )
)
diff --git a/test/passes/remove-unused-brs_enable-multivalue.txt b/test/passes/remove-unused-brs_enable-multivalue.txt
index 54780f0d9..a20f2905b 100644
--- a/test/passes/remove-unused-brs_enable-multivalue.txt
+++ b/test/passes/remove-unused-brs_enable-multivalue.txt
@@ -2389,12 +2389,12 @@
(loop $label$1
(br_if $label$1
(block $label$2
- (if
- (block $label$4
- (unreachable)
- )
- (then
- (block $label$3
+ (block $label$3
+ (if
+ (block $label$4
+ (unreachable)
+ )
+ (then
(br $label$3)
)
)
@@ -2405,15 +2405,15 @@
)
)
(func $if-arm-unreachable
- (if
- (unreachable)
- (then
- (block $label$1
+ (block $label$1
+ (if
+ (unreachable)
+ (then
(nop)
)
- )
- (else
- (unreachable)
+ (else
+ (unreachable)
+ )
)
)
)