diff options
author | Alon Zakai <azakai@google.com> | 2022-08-22 14:31:04 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-08-22 14:31:04 -0700 |
commit | 0e0c2d9d45068c450ad5df5de47948532dd12c53 (patch) | |
tree | c8e3ee432f06e2f5b814a91466ff43aa305815ad | |
parent | 38c084ee386e257389d44c6200a403f74432e1af (diff) | |
download | binaryen-0e0c2d9d45068c450ad5df5de47948532dd12c53.tar.gz binaryen-0e0c2d9d45068c450ad5df5de47948532dd12c53.tar.bz2 binaryen-0e0c2d9d45068c450ad5df5de47948532dd12c53.zip |
RemoveUnusedBrs: Remove final block nops in all cases (#4954)
This fixes what looks like it might be a regression in #4943. It's not actually
an issue since it just affects wat files, but it did uncover an existing
inefficiency. The situation is this:
(block
..
(br $somewhere)
(nop)
)
Removing such a nop is always helpful, as the pass might see that that
br goes to where control flow is going anyhow, and the nop would
confuse it. We used to remove such nops only when the block had a name,
which is why wat testcases looks optimal, but we were actually doing the
less-efficient thing on real-world code. It was a minor inefficiency, though, as
the nop is quickly removed by later passes anyhow. Still, the fix is trivial (to
always remove such nops, regardless of a name on the block or not).
-rw-r--r-- | src/passes/RemoveUnusedBrs.cpp | 13 | ||||
-rw-r--r-- | test/passes/remove-unused-brs_enable-multivalue.txt | 13 |
2 files changed, 8 insertions, 18 deletions
diff --git a/src/passes/RemoveUnusedBrs.cpp b/src/passes/RemoveUnusedBrs.cpp index 3c4723c7f..b867dc4c2 100644 --- a/src/passes/RemoveUnusedBrs.cpp +++ b/src/passes/RemoveUnusedBrs.cpp @@ -203,11 +203,14 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> { if (skip > 0) { flows.resize(size - skip); } - // drop a nop at the end of a block, which prevents a value flowing - while (list.size() > 0 && list.back()->is<Nop>()) { - list.resize(list.size() - 1); - self->anotherCycle = true; - } + } + // Drop a nop at the end of a block, which prevents a value flowing. Note + // that this is worth doing regardless of whether we have a name on this + // block or not (which the if right above us checks) - such a nop is + // always unneeded and can limit later optimizations. + 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 diff --git a/test/passes/remove-unused-brs_enable-multivalue.txt b/test/passes/remove-unused-brs_enable-multivalue.txt index 8240bba22..d3180299c 100644 --- a/test/passes/remove-unused-brs_enable-multivalue.txt +++ b/test/passes/remove-unused-brs_enable-multivalue.txt @@ -23,7 +23,6 @@ (drop (i32.const 0) ) - (nop) ) ) ) @@ -46,7 +45,6 @@ (drop (i32.const 0) ) - (nop) ) ) ) @@ -58,7 +56,6 @@ (drop (i32.const 0) ) - (nop) ) ) ) @@ -140,7 +137,6 @@ (drop (i32.const 1) ) - (nop) ) ) ) @@ -152,7 +148,6 @@ (drop (i32.const 2) ) - (nop) ) ) ) @@ -231,7 +226,6 @@ (drop (i32.const 0) ) - (nop) ) ) ) @@ -385,7 +379,6 @@ (drop (i32.const 1) ) - (nop) ) ) (block $a20 @@ -393,7 +386,6 @@ (drop (i32.const 2) ) - (nop) ) ) ) @@ -526,7 +518,6 @@ (block $out49 (block (call $loops) - (nop) ) ) (br $in48) @@ -997,7 +988,6 @@ (br_if $shape$6$continue (local.get $0) ) - (nop) ) ) ) @@ -1790,7 +1780,6 @@ (drop (i32.const 0) ) - (nop) ) ) ) @@ -2326,8 +2315,6 @@ (i32.const 0) (block $label$3 (block - (nop) - (nop) ) ) (return |