[llvm] [SimplifyCFG] Avoid scanning functions multiple times in `removeUnreachableBlocks` (PR #213416)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 31 23:53:26 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: Yingwei Zheng (dtcxzyw)
<details>
<summary>Changes</summary>
`markAliveBlocks` scans instructions first to convert unreachable instructions into `unreachable`, then marks alive successors. When `iterativelySimplifyCFG` makes some changes, `removeUnreachableBlocks` will be called again and scan the whole function again, even if `iterativelySimplifyCFG` is unlikely to introduce new interesting patterns.
This patch adds a new option `SimplifyInsts` to `removeUnreachableBlocks`. When it is disabled, `markAliveBlocks` only performs a BFS traversal.
Although it is possible to cause regressions (unreachable-multi-basic-block-funclet.ll), it doesn't affect the optimization result in practice: https://github.com/dtcxzyw/llvm-opt-benchmark-nightly/pull/832
Compile-time improvement (approx -0.05%): https://llvm-compile-time-tracker.com/compare.php?from=6a898832ff382b1a288f9eb3bc5cd1f37d0fc29f&to=565856d52880ed13c697e921f498dc400bb76c17&stat=instructions:u
---
Patch is 20.47 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/213416.diff
4 Files Affected:
- (modified) llvm/include/llvm/Transforms/Utils/Local.h (+4-1)
- (modified) llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp (+5-2)
- (modified) llvm/lib/Transforms/Utils/Local.cpp (+161-154)
- (modified) llvm/test/Transforms/SimplifyCFG/unreachable-multi-basic-block-funclet.ll (+9-2)
``````````diff
diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h
index 62fa8ada3905f..1028d4e859e6d 100644
--- a/llvm/include/llvm/Transforms/Utils/Local.h
+++ b/llvm/include/llvm/Transforms/Utils/Local.h
@@ -411,11 +411,14 @@ LLVM_ABI Instruction *removeUnwindEdge(BasicBlock *BB,
DomTreeUpdater *DTU = nullptr);
/// Remove all blocks that can not be reached from the function's entry.
+/// When \p SimplifyInsts is true, it will also convert obviously unreachable
+/// instructions into unreachable (e.g, store to null).
///
/// Returns true if any basic block was removed.
LLVM_ABI bool removeUnreachableBlocks(Function &F,
DomTreeUpdater *DTU = nullptr,
- MemorySSAUpdater *MSSAU = nullptr);
+ MemorySSAUpdater *MSSAU = nullptr,
+ bool SimplifyInsts = true);
/// Combine the metadata of two instructions so that K can replace J. This
/// specifically handles the case of CSE-like transformations. Some
diff --git a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
index debf033c70c41..ece5b10688855 100644
--- a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
+++ b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
@@ -287,12 +287,15 @@ static bool simplifyFunctionCFGImpl(Function &F, const TargetTransformInfo &TTI,
// iterate between the two optimizations. We structure the code like this to
// avoid rerunning iterativelySimplifyCFG if the second pass of
// removeUnreachableBlocks doesn't do anything.
- if (!removeUnreachableBlocks(F, DT ? &DTU : nullptr))
+ // Avoid scanning instructions to reduce compile-time.
+ if (!removeUnreachableBlocks(F, DT ? &DTU : nullptr, /*MSSAU=*/nullptr,
+ /*SimplifyInsts=*/false))
return true;
do {
EverChanged = iterativelySimplifyCFG(F, TTI, DT ? &DTU : nullptr, Options);
- EverChanged |= removeUnreachableBlocks(F, DT ? &DTU : nullptr);
+ EverChanged |= removeUnreachableBlocks(
+ F, DT ? &DTU : nullptr, /*MSSAU=*/nullptr, /*SimplifyInsts=*/false);
} while (EverChanged);
return true;
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index b17740c0bc192..da3d648399f4d 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -2683,7 +2683,7 @@ BasicBlock *llvm::changeToInvokeAndSplitBasicBlock(CallInst *CI,
}
static bool markAliveBlocks(Function &F, SmallVectorImpl<bool> &Reachable,
- DomTreeUpdater *DTU = nullptr) {
+ DomTreeUpdater *DTU, bool SimplifyInsts) {
SmallVector<BasicBlock*, 128> Worklist;
BasicBlock *BB = &F.front();
Worklist.push_back(BB);
@@ -2695,174 +2695,180 @@ static bool markAliveBlocks(Function &F, SmallVectorImpl<bool> &Reachable,
// Do a quick scan of the basic block, turning any obviously unreachable
// instructions into LLVM unreachable insts. The instruction combining pass
// canonicalizes unreachable insts into stores to null or undef.
- for (Instruction &I : *BB) {
- if (auto *CI = dyn_cast<CallInst>(&I)) {
- Value *Callee = CI->getCalledOperand();
- // Handle intrinsic calls.
- if (Function *F = dyn_cast<Function>(Callee)) {
- auto IntrinsicID = F->getIntrinsicID();
- // Assumptions that are known to be false are equivalent to
- // unreachable. Also, if the condition is undefined, then we make the
- // choice most beneficial to the optimizer, and choose that to also be
- // unreachable.
- if (IntrinsicID == Intrinsic::assume) {
- if (match(CI->getArgOperand(0), m_CombineOr(m_Zero(), m_Undef()))) {
- // Don't insert a call to llvm.trap right before the unreachable.
- changeToUnreachable(CI, false, DTU);
- Changed = true;
- break;
- }
- } else if (IntrinsicID == Intrinsic::experimental_guard) {
- // A call to the guard intrinsic bails out of the current
- // compilation unit if the predicate passed to it is false. If the
- // predicate is a constant false, then we know the guard will bail
- // out of the current compile unconditionally, so all code following
- // it is dead.
- //
- // Note: unlike in llvm.assume, it is not "obviously profitable" for
- // guards to treat `undef` as `false` since a guard on `undef` can
- // still be useful for widening.
- if (match(CI->getArgOperand(0), m_Zero()))
- if (!isa<UnreachableInst>(CI->getNextNode())) {
- changeToUnreachable(CI->getNextNode(), false, DTU);
+ if (SimplifyInsts) {
+ for (Instruction &I : *BB) {
+ if (auto *CI = dyn_cast<CallInst>(&I)) {
+ Value *Callee = CI->getCalledOperand();
+ // Handle intrinsic calls.
+ if (Function *F = dyn_cast<Function>(Callee)) {
+ auto IntrinsicID = F->getIntrinsicID();
+ // Assumptions that are known to be false are equivalent to
+ // unreachable. Also, if the condition is undefined, then we make
+ // the choice most beneficial to the optimizer, and choose that to
+ // also be unreachable.
+ if (IntrinsicID == Intrinsic::assume) {
+ if (match(CI->getArgOperand(0),
+ m_CombineOr(m_Zero(), m_Undef()))) {
+ // Don't insert a call to llvm.trap right before the
+ // unreachable.
+ changeToUnreachable(CI, false, DTU);
Changed = true;
break;
}
+ } else if (IntrinsicID == Intrinsic::experimental_guard) {
+ // A call to the guard intrinsic bails out of the current
+ // compilation unit if the predicate passed to it is false. If the
+ // predicate is a constant false, then we know the guard will bail
+ // out of the current compile unconditionally, so all code
+ // following it is dead.
+ //
+ // Note: unlike in llvm.assume, it is not "obviously profitable"
+ // for guards to treat `undef` as `false` since a guard on `undef`
+ // can still be useful for widening.
+ if (match(CI->getArgOperand(0), m_Zero()))
+ if (!isa<UnreachableInst>(CI->getNextNode())) {
+ changeToUnreachable(CI->getNextNode(), false, DTU);
+ Changed = true;
+ break;
+ }
+ }
+ } else if ((isa<ConstantPointerNull>(Callee) &&
+ !NullPointerIsDefined(CI->getFunction(),
+ cast<PointerType>(Callee->getType())
+ ->getAddressSpace())) ||
+ isa<UndefValue>(Callee)) {
+ changeToUnreachable(CI, false, DTU);
+ Changed = true;
+ break;
}
- } else if ((isa<ConstantPointerNull>(Callee) &&
- !NullPointerIsDefined(CI->getFunction(),
- cast<PointerType>(Callee->getType())
- ->getAddressSpace())) ||
- isa<UndefValue>(Callee)) {
- changeToUnreachable(CI, false, DTU);
- Changed = true;
- break;
- }
- if (CI->doesNotReturn() && !CI->isMustTailCall()) {
- // If we found a call to a no-return function, insert an unreachable
- // instruction after it. Make sure there isn't *already* one there
- // though.
- if (!isa<UnreachableInst>(CI->getNextNode())) {
- // Don't insert a call to llvm.trap right before the unreachable.
- changeToUnreachable(CI->getNextNode(), false, DTU);
+ if (CI->doesNotReturn() && !CI->isMustTailCall()) {
+ // If we found a call to a no-return function, insert an unreachable
+ // instruction after it. Make sure there isn't *already* one there
+ // though.
+ if (!isa<UnreachableInst>(CI->getNextNode())) {
+ // Don't insert a call to llvm.trap right before the unreachable.
+ changeToUnreachable(CI->getNextNode(), false, DTU);
+ Changed = true;
+ }
+ break;
+ }
+ } else if (auto *SI = dyn_cast<StoreInst>(&I)) {
+ // Store to undef and store to null are undefined and used to signal
+ // that they should be changed to unreachable by passes that can't
+ // modify the CFG.
+
+ // Don't touch volatile stores.
+ if (SI->isVolatile())
+ continue;
+
+ Value *Ptr = SI->getOperand(1);
+
+ if (isa<UndefValue>(Ptr) ||
+ (isa<ConstantPointerNull>(Ptr) &&
+ !NullPointerIsDefined(SI->getFunction(),
+ SI->getPointerAddressSpace()))) {
+ changeToUnreachable(SI, false, DTU);
Changed = true;
+ break;
}
- break;
- }
- } else if (auto *SI = dyn_cast<StoreInst>(&I)) {
- // Store to undef and store to null are undefined and used to signal
- // that they should be changed to unreachable by passes that can't
- // modify the CFG.
-
- // Don't touch volatile stores.
- if (SI->isVolatile()) continue;
-
- Value *Ptr = SI->getOperand(1);
-
- if (isa<UndefValue>(Ptr) ||
- (isa<ConstantPointerNull>(Ptr) &&
- !NullPointerIsDefined(SI->getFunction(),
- SI->getPointerAddressSpace()))) {
- changeToUnreachable(SI, false, DTU);
- Changed = true;
- break;
}
}
- }
- Instruction *Terminator = BB->getTerminator();
- if (auto *II = dyn_cast<InvokeInst>(Terminator)) {
- // Turn invokes that call 'nounwind' functions into ordinary calls.
- Value *Callee = II->getCalledOperand();
- if ((isa<ConstantPointerNull>(Callee) &&
- !NullPointerIsDefined(BB->getParent())) ||
- isa<UndefValue>(Callee)) {
- changeToUnreachable(II, false, DTU);
- Changed = true;
- } else {
- if (II->doesNotReturn() &&
- !isa<UnreachableInst>(II->getNormalDest()->front())) {
- // If we found an invoke of a no-return function,
- // create a new empty basic block with an `unreachable` terminator,
- // and set it as the normal destination for the invoke,
- // unless that is already the case.
- // Note that the original normal destination could have other uses.
- BasicBlock *OrigNormalDest = II->getNormalDest();
- OrigNormalDest->removePredecessor(II->getParent());
- LLVMContext &Ctx = II->getContext();
- BasicBlock *UnreachableNormalDest = BasicBlock::Create(
- Ctx, OrigNormalDest->getName() + ".unreachable",
- II->getFunction(), OrigNormalDest);
- Reachable.resize(II->getFunction()->getMaxBlockNumber());
- auto *UI = new UnreachableInst(Ctx, UnreachableNormalDest);
- UI->setDebugLoc(DebugLoc::getTemporary());
- II->setNormalDest(UnreachableNormalDest);
- if (DTU)
- DTU->applyUpdates(
- {{DominatorTree::Delete, BB, OrigNormalDest},
- {DominatorTree::Insert, BB, UnreachableNormalDest}});
+ Instruction *Terminator = BB->getTerminator();
+ if (auto *II = dyn_cast<InvokeInst>(Terminator)) {
+ // Turn invokes that call 'nounwind' functions into ordinary calls.
+ Value *Callee = II->getCalledOperand();
+ if ((isa<ConstantPointerNull>(Callee) &&
+ !NullPointerIsDefined(BB->getParent())) ||
+ isa<UndefValue>(Callee)) {
+ changeToUnreachable(II, false, DTU);
Changed = true;
- }
- if (II->doesNotThrow() && canSimplifyInvokeNoUnwind(&F)) {
- if (II->use_empty() && !II->mayHaveSideEffects()) {
- // jump to the normal destination branch.
- BasicBlock *NormalDestBB = II->getNormalDest();
- BasicBlock *UnwindDestBB = II->getUnwindDest();
- UncondBrInst::Create(NormalDestBB, II->getIterator());
- UnwindDestBB->removePredecessor(II->getParent());
- II->eraseFromParent();
+ } else {
+ if (II->doesNotReturn() &&
+ !isa<UnreachableInst>(II->getNormalDest()->front())) {
+ // If we found an invoke of a no-return function,
+ // create a new empty basic block with an `unreachable` terminator,
+ // and set it as the normal destination for the invoke,
+ // unless that is already the case.
+ // Note that the original normal destination could have other uses.
+ BasicBlock *OrigNormalDest = II->getNormalDest();
+ OrigNormalDest->removePredecessor(II->getParent());
+ LLVMContext &Ctx = II->getContext();
+ BasicBlock *UnreachableNormalDest = BasicBlock::Create(
+ Ctx, OrigNormalDest->getName() + ".unreachable",
+ II->getFunction(), OrigNormalDest);
+ Reachable.resize(II->getFunction()->getMaxBlockNumber());
+ auto *UI = new UnreachableInst(Ctx, UnreachableNormalDest);
+ UI->setDebugLoc(DebugLoc::getTemporary());
+ II->setNormalDest(UnreachableNormalDest);
if (DTU)
- DTU->applyUpdates({{DominatorTree::Delete, BB, UnwindDestBB}});
- } else
- changeToCall(II, DTU);
- Changed = true;
- }
- }
- } else if (auto *CatchSwitch = dyn_cast<CatchSwitchInst>(Terminator)) {
- // Remove catchpads which cannot be reached.
- struct CatchPadDenseMapInfo {
- static unsigned getHashValue(CatchPadInst *CatchPad) {
- return static_cast<unsigned>(hash_combine_range(
- CatchPad->value_op_begin(), CatchPad->value_op_end()));
+ DTU->applyUpdates(
+ {{DominatorTree::Delete, BB, OrigNormalDest},
+ {DominatorTree::Insert, BB, UnreachableNormalDest}});
+ Changed = true;
+ }
+ if (II->doesNotThrow() && canSimplifyInvokeNoUnwind(&F)) {
+ if (II->use_empty() && !II->mayHaveSideEffects()) {
+ // jump to the normal destination branch.
+ BasicBlock *NormalDestBB = II->getNormalDest();
+ BasicBlock *UnwindDestBB = II->getUnwindDest();
+ UncondBrInst::Create(NormalDestBB, II->getIterator());
+ UnwindDestBB->removePredecessor(II->getParent());
+ II->eraseFromParent();
+ if (DTU)
+ DTU->applyUpdates({{DominatorTree::Delete, BB, UnwindDestBB}});
+ } else
+ changeToCall(II, DTU);
+ Changed = true;
+ }
}
+ } else if (auto *CatchSwitch = dyn_cast<CatchSwitchInst>(Terminator)) {
+ // Remove catchpads which cannot be reached.
+ struct CatchPadDenseMapInfo {
+ static unsigned getHashValue(CatchPadInst *CatchPad) {
+ return static_cast<unsigned>(hash_combine_range(
+ CatchPad->value_op_begin(), CatchPad->value_op_end()));
+ }
- static bool isEqual(CatchPadInst *LHS, CatchPadInst *RHS) {
- return LHS->isIdenticalTo(RHS);
- }
- };
-
- SmallDenseMap<BasicBlock *, int, 8> NumPerSuccessorCases;
- // Set of unique CatchPads.
- SmallDenseMap<CatchPadInst *, detail::DenseSetEmpty, 4,
- CatchPadDenseMapInfo, detail::DenseSetPair<CatchPadInst *>>
- HandlerSet;
- detail::DenseSetEmpty Empty;
- for (CatchSwitchInst::handler_iterator I = CatchSwitch->handler_begin(),
- E = CatchSwitch->handler_end();
- I != E; ++I) {
- BasicBlock *HandlerBB = *I;
- if (DTU)
- ++NumPerSuccessorCases[HandlerBB];
- auto *CatchPad = cast<CatchPadInst>(HandlerBB->getFirstNonPHIIt());
- if (!HandlerSet.insert({CatchPad, Empty}).second) {
+ static bool isEqual(CatchPadInst *LHS, CatchPadInst *RHS) {
+ return LHS->isIdenticalTo(RHS);
+ }
+ };
+
+ SmallDenseMap<BasicBlock *, int, 8> NumPerSuccessorCases;
+ // Set of unique CatchPads.
+ SmallDenseMap<CatchPadInst *, detail::DenseSetEmpty, 4,
+ CatchPadDenseMapInfo,
+ detail::DenseSetPair<CatchPadInst *>>
+ HandlerSet;
+ detail::DenseSetEmpty Empty;
+ for (CatchSwitchInst::handler_iterator I = CatchSwitch->handler_begin(),
+ E = CatchSwitch->handler_end();
+ I != E; ++I) {
+ BasicBlock *HandlerBB = *I;
if (DTU)
- --NumPerSuccessorCases[HandlerBB];
- CatchSwitch->removeHandler(I);
- --I;
- --E;
- Changed = true;
+ ++NumPerSuccessorCases[HandlerBB];
+ auto *CatchPad = cast<CatchPadInst>(HandlerBB->getFirstNonPHIIt());
+ if (!HandlerSet.insert({CatchPad, Empty}).second) {
+ if (DTU)
+ --NumPerSuccessorCases[HandlerBB];
+ CatchSwitch->removeHandler(I);
+ --I;
+ --E;
+ Changed = true;
+ }
+ }
+ if (DTU) {
+ std::vector<DominatorTree::UpdateType> Updates;
+ for (const std::pair<BasicBlock *, int> &I : NumPerSuccessorCases)
+ if (I.second == 0)
+ Updates.push_back({DominatorTree::Delete, BB, I.first});
+ DTU->applyUpdates(Updates);
}
}
- if (DTU) {
- std::vector<DominatorTree::UpdateType> Updates;
- for (const std::pair<BasicBlock *, int> &I : NumPerSuccessorCases)
- if (I.second == 0)
- Updates.push_back({DominatorTree::Delete, BB, I.first});
- DTU->applyUpdates(Updates);
- }
- }
- Changed |= ConstantFoldTerminator(BB, true, nullptr, DTU);
+ Changed |= ConstantFoldTerminator(BB, true, nullptr, DTU);
+ }
for (BasicBlock *Successor : successors(BB)) {
if (!Reachable[Successor->getNumber()]) {
Worklist.push_back(Successor);
@@ -2912,9 +2918,10 @@ Instruction *llvm::removeUnwindEdge(BasicBlock *BB, DomTreeUpdater *DTU) {
/// if they are in a dead cycle. Return true if a change was made, false
/// otherwise.
bool llvm::removeUnreachableBlocks(Function &F, DomTreeUpdater *DTU,
- MemorySSAUpdater *MSSAU) {
+ MemorySSAUpdater *MSSAU,
+ bool SimplifyInsts) {
SmallVector<bool, 16> Reachable(F.getMaxBlockNumber());
- bool Changed = markAliveBlocks(F, Reachable, DTU);
+ bool Changed = markAliveBlocks(F, Reachable, DTU, SimplifyInsts);
// Are there any blocks left to actually delete?
SmallSetVector<BasicBlock *, 8> BlocksToRemove;
diff --git a/llvm/test/Transforms/SimplifyCFG/unreachable-multi-basic-block-funclet.ll b/llvm/test/Transforms/SimplifyCFG/unreachable-multi-basic-block-funclet.ll
index 0f0fc78ec7add..ae33e9b24275d 100644
--- a/llvm/test/Transforms/SimplifyCFG/unreachable-multi-basic-block-funclet.ll
+++ b/llvm/test/Transforms/SimplifyCFG/unreachable-multi-basic-block-funclet.ll
@@ -198,9 +198,16 @@ define x86_thiscallcc ptr @baz(ptr %arg, ptr %arg1, ptr %arg2, i1 %arg3, ptr %ar
; CHECK-LABEL: define x86_thiscallcc ptr @baz(
; CHECK-SAME: ptr [[ARG:%.*]], ptr [[ARG1:%.*]], ptr [[ARG2:%.*]], i1 [[ARG3:%.*]], ptr [[ARG4:%.*]]) personality ptr null {
; CHECK-NEXT: [[BB:.*:]]
-; CHECK-NEXT: [[ALLOCA:%.*]] = alloca [2 x %struct.foo], align 4
-; CHECK-NEXT: [[INVOKE:%.*]] = call x86_thiscallcc ptr @quux(ptr null, ptr null, i32 0) #[[ATTR1:[0-9]+]]
+; CHECK-NEXT: [[ALLOCA:%.*]] = alloca [2 x [[STRUCT_FOO:%.*]]], align 4
+; CHECK-NEXT: [[INVOKE:%.*]] = invoke x86_t...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/213416
More information about the llvm-commits
mailing list