diff options
author | Alon Zakai <azakai@google.com> | 2024-10-10 08:40:24 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-10-10 08:40:24 -0700 |
commit | a8aa6602cdbedd04e69d362c01bbf378a44b395d (patch) | |
tree | 3cd354b15045a2bc69eca26892562947ea917f4e /src/passes/MergeBlocks.cpp | |
parent | debd24681cb4764e75936dd74bc33c41899b8a23 (diff) | |
download | binaryen-a8aa6602cdbedd04e69d362c01bbf378a44b395d.tar.gz binaryen-a8aa6602cdbedd04e69d362c01bbf378a44b395d.tar.bz2 binaryen-a8aa6602cdbedd04e69d362c01bbf378a44b395d.zip |
ReFinalize in MergeBlocks so we can optimize unreachable instructions too (#6994)
In #6984 we optimized dropped blocks even if they had unreachable code. In #6988
that part was reverted, and blocks with unreachable code were ignored once more.
However, I realized that the check was not actually for unreachable code, but for
having an unreachable child, so it would miss things like this:
(block
(block
..
(br $somewhere) ;; unreachable type, but no unreachable code
)
)
But it is useful to merge such blocks: we don't need the inner block here.
To fix this, just run ReFinalize if we change anything, which will propagate
unreachability as needed. I think MergeBlocks was written before we had
that utility, so it didn't use it...
This is not only useful for itself but will unblock an EH optimization in a
later PR, that has code in this form. It also simplifies the code by removing
the hasUnreachableChild checks.
Diffstat (limited to 'src/passes/MergeBlocks.cpp')
-rw-r--r-- | src/passes/MergeBlocks.cpp | 40 |
1 files changed, 15 insertions, 25 deletions
diff --git a/src/passes/MergeBlocks.cpp b/src/passes/MergeBlocks.cpp index 7518c73ca..147ba9f45 100644 --- a/src/passes/MergeBlocks.cpp +++ b/src/passes/MergeBlocks.cpp @@ -176,15 +176,6 @@ struct BreakValueDropper : public ControlFlowWalker<BreakValueDropper> { } }; -static bool hasUnreachableChild(Block* block) { - for (auto* test : block->list) { - if (test->type == Type::unreachable) { - return true; - } - } - return false; -} - // Checks for code after an unreachable element. static bool hasDeadCode(Block* block) { auto& list = block->list; @@ -206,11 +197,6 @@ static bool optimizeDroppedBlock(Drop* drop, PassOptions& options, BranchUtils::BranchSeekerCache& branchInfo) { assert(drop->value == block); - if (hasUnreachableChild(block)) { - // Don't move around unreachable code, as it can change types (leave it for - // DCE). - return false; - } if (block->name.is()) { // There may be breaks: see if we can remove their values. Expression* expression = block; @@ -241,8 +227,8 @@ static bool optimizeDroppedBlock(Drop* drop, return true; } -// Core block optimizer routine. -static void optimizeBlock(Block* curr, +// Core block optimizer routine. Returns true when we optimize. +static bool optimizeBlock(Block* curr, Module* module, PassOptions& passOptions, BranchUtils::BranchSeekerCache& branchInfo) { @@ -412,6 +398,7 @@ static void optimizeBlock(Block* curr, if (changed) { curr->finalize(curr->type); } + return changed; } void BreakValueDropper::visitBlock(Block* curr) { @@ -427,6 +414,8 @@ struct MergeBlocks return std::make_unique<MergeBlocks>(); } + bool refinalize = false; + BranchUtils::BranchSeekerCache branchInfo; void visitBlock(Block* curr) { @@ -438,6 +427,7 @@ struct MergeBlocks if (optimizeDroppedBlock( curr, block, *getModule(), getPassOptions(), branchInfo)) { replaceCurrent(block); + refinalize = true; } } } @@ -485,13 +475,6 @@ struct MergeBlocks } if (auto* block = child->dynCast<Block>()) { if (!block->name.is() && block->list.size() >= 2) { - // if we move around unreachable code, type changes could occur. avoid - // that, as anyhow it means we should have run dce before getting here - if (curr->type == Type::none && hasUnreachableChild(block)) { - // moving the block to the outside would replace a none with an - // unreachable - return outer; - } auto* back = block->list.back(); if (back->type == Type::unreachable) { // curr is not reachable, dce could remove it; don't try anything @@ -510,6 +493,7 @@ struct MergeBlocks return outer; } child = back; + refinalize = true; if (outer == nullptr) { // reuse the block, move it out block->list.back() = curr; @@ -600,8 +584,7 @@ struct MergeBlocks // too small for us to remove anything from (we cannot remove the last // element), or if it has unreachable code (leave that for dce), then give // up. - if (!block || block->name.is() || block->list.size() <= 1 || - hasUnreachableChild(block)) { + if (!block || block->name.is() || block->list.size() <= 1) { continueEarly(); continue; } @@ -682,6 +665,7 @@ struct MergeBlocks outerBlock->list.push_back(curr); outerBlock->finalize(curr->type); replaceCurrent(outerBlock); + refinalize = true; } } @@ -700,6 +684,12 @@ struct MergeBlocks outer = optimize(curr, curr->operands[i], outer); } } + + void visitFunction(Function* curr) { + if (refinalize) { + ReFinalize().walkFunctionInModule(curr, getModule()); + } + } }; Pass* createMergeBlocksPass() { return new MergeBlocks(); } |