[llvm] 2070fe7 - [NFCI][SimplifyCFG] Don't form DTU updates if we aren't going to apply them
Roman Lebedev via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 24 14:03:05 PDT 2021
Author: Roman Lebedev
Date: 2021-03-25T00:02:37+03:00
New Revision: 2070fe7144fcf76f0800c423c11882c9b872da6c
URL: https://github.com/llvm/llvm-project/commit/2070fe7144fcf76f0800c423c11882c9b872da6c
DIFF: https://github.com/llvm/llvm-project/commit/2070fe7144fcf76f0800c423c11882c9b872da6c.diff
LOG: [NFCI][SimplifyCFG] Don't form DTU updates if we aren't going to apply them
I think we may want to have a thin wrapper over a vector to deduplicate
those `if(DTU)` predicates, and instead do them in the `insert()` itself.
Added:
Modified:
llvm/lib/Transforms/Utils/Local.cpp
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index e1ea7c8e27a9..5b3472c7348e 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -2403,22 +2403,25 @@ static bool markAliveBlocks(Function &F,
E = CatchSwitch->handler_end();
I != E; ++I) {
BasicBlock *HandlerBB = *I;
- ++NumPerSuccessorCases[HandlerBB];
+ if (DTU)
+ ++NumPerSuccessorCases[HandlerBB];
auto *CatchPad = cast<CatchPadInst>(HandlerBB->getFirstNonPHI());
if (!HandlerSet.insert({CatchPad, Empty}).second) {
- --NumPerSuccessorCases[HandlerBB];
+ if (DTU)
+ --NumPerSuccessorCases[HandlerBB];
CatchSwitch->removeHandler(I);
--I;
--E;
Changed = true;
}
}
- 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});
- if (DTU)
+ 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);
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 255bc2621d85..c34ef3dec26d 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -914,20 +914,23 @@ bool SimplifyCFGOpt::SimplifyEqualityComparisonWithOnlyPredecessor(
for (SwitchInst::CaseIt i = SI->case_end(), e = SI->case_begin(); i != e;) {
--i;
auto *Successor = i->getCaseSuccessor();
- ++NumPerSuccessorCases[Successor];
+ if (DTU)
+ ++NumPerSuccessorCases[Successor];
if (DeadCases.count(i->getCaseValue())) {
Successor->removePredecessor(PredDef);
SI.removeCase(i);
- --NumPerSuccessorCases[Successor];
+ if (DTU)
+ --NumPerSuccessorCases[Successor];
}
}
- std::vector<DominatorTree::UpdateType> Updates;
- for (const std::pair<BasicBlock *, int> &I : NumPerSuccessorCases)
- if (I.second == 0)
- Updates.push_back({DominatorTree::Delete, PredDef, I.first});
- if (DTU)
+ if (DTU) {
+ std::vector<DominatorTree::UpdateType> Updates;
+ for (const std::pair<BasicBlock *, int> &I : NumPerSuccessorCases)
+ if (I.second == 0)
+ Updates.push_back({DominatorTree::Delete, PredDef, I.first});
DTU->applyUpdates(Updates);
+ }
LLVM_DEBUG(dbgs() << "Leaving: " << *TI << "\n");
return true;
@@ -1177,7 +1180,7 @@ bool SimplifyCFGOpt::PerformValueComparisonIntoPredecessorFolding(
// Reconstruct the new switch statement we will be building.
if (PredDefault != BBDefault) {
PredDefault->removePredecessor(Pred);
- if (PredDefault != BB)
+ if (DTU && PredDefault != BB)
Updates.push_back({DominatorTree::Delete, Pred, PredDefault});
PredDefault = BBDefault;
++NewSuccessors[BBDefault];
@@ -1259,7 +1262,7 @@ bool SimplifyCFGOpt::PerformValueComparisonIntoPredecessorFolding(
(void)I;
AddPredecessorToBlock(NewSuccessor.first, Pred, BB);
}
- if (!is_contained(successors(Pred), NewSuccessor.first))
+ if (DTU && !is_contained(successors(Pred), NewSuccessor.first))
Updates.push_back({DominatorTree::Insert, Pred, NewSuccessor.first});
}
@@ -1299,18 +1302,21 @@ bool SimplifyCFGOpt::PerformValueComparisonIntoPredecessorFolding(
InfLoopBlock =
BasicBlock::Create(BB->getContext(), "infloop", BB->getParent());
BranchInst::Create(InfLoopBlock, InfLoopBlock);
- Updates.push_back({DominatorTree::Insert, InfLoopBlock, InfLoopBlock});
+ if (DTU)
+ Updates.push_back(
+ {DominatorTree::Insert, InfLoopBlock, InfLoopBlock});
}
NewSI->setSuccessor(i, InfLoopBlock);
}
- if (InfLoopBlock)
- Updates.push_back({DominatorTree::Insert, Pred, InfLoopBlock});
+ if (DTU) {
+ if (InfLoopBlock)
+ Updates.push_back({DominatorTree::Insert, Pred, InfLoopBlock});
- Updates.push_back({DominatorTree::Delete, Pred, BB});
+ Updates.push_back({DominatorTree::Delete, Pred, BB});
- if (DTU)
DTU->applyUpdates(Updates);
+ }
++NumFoldValueComparisonIntoPredecessors;
return true;
@@ -1587,10 +1593,13 @@ bool SimplifyCFGOpt::HoistThenElseCodeToIf(BranchInst *BI,
// Update any PHI nodes in our new successors.
for (BasicBlock *Succ : successors(BB1)) {
AddPredecessorToBlock(Succ, BIParent, BB1);
- Updates.push_back({DominatorTree::Insert, BIParent, Succ});
+ if (DTU)
+ Updates.push_back({DominatorTree::Insert, BIParent, Succ});
}
- for (BasicBlock *Succ : successors(BI))
- Updates.push_back({DominatorTree::Delete, BIParent, Succ});
+
+ if (DTU)
+ for (BasicBlock *Succ : successors(BI))
+ Updates.push_back({DominatorTree::Delete, BIParent, Succ});
EraseTerminatorAndDCECond(BI);
if (DTU)
@@ -2474,7 +2483,8 @@ static bool FoldCondBranchOnPHI(BranchInst *BI, DomTreeUpdater *DTU,
BasicBlock::Create(BB->getContext(), RealDest->getName() + ".critedge",
RealDest->getParent(), RealDest);
BranchInst *CritEdgeBranch = BranchInst::Create(RealDest, EdgeBB);
- Updates.push_back({DominatorTree::Insert, EdgeBB, RealDest});
+ if (DTU)
+ Updates.push_back({DominatorTree::Insert, EdgeBB, RealDest});
CritEdgeBranch->setDebugLoc(BI->getDebugLoc());
// Update PHI nodes.
@@ -2533,11 +2543,12 @@ static bool FoldCondBranchOnPHI(BranchInst *BI, DomTreeUpdater *DTU,
PredBBTI->setSuccessor(i, EdgeBB);
}
- Updates.push_back({DominatorTree::Insert, PredBB, EdgeBB});
- Updates.push_back({DominatorTree::Delete, PredBB, BB});
+ if (DTU) {
+ Updates.push_back({DominatorTree::Insert, PredBB, EdgeBB});
+ Updates.push_back({DominatorTree::Delete, PredBB, BB});
- if (DTU)
DTU->applyUpdates(Updates);
+ }
// Recurse, simplifying any other constants.
return FoldCondBranchOnPHI(BI, DTU, DL, AC) || true;
@@ -3660,7 +3671,8 @@ static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
BasicBlock *InfLoopBlock =
BasicBlock::Create(BB->getContext(), "infloop", BB->getParent());
BranchInst::Create(InfLoopBlock, InfLoopBlock);
- Updates.push_back({DominatorTree::Insert, InfLoopBlock, InfLoopBlock});
+ if (DTU)
+ Updates.push_back({DominatorTree::Insert, InfLoopBlock, InfLoopBlock});
OtherDest = InfLoopBlock;
}
@@ -3687,11 +3699,12 @@ static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
PBI->setSuccessor(0, CommonDest);
PBI->setSuccessor(1, OtherDest);
- Updates.push_back({DominatorTree::Insert, PBI->getParent(), OtherDest});
- Updates.push_back({DominatorTree::Delete, PBI->getParent(), RemovedDest});
+ if (DTU) {
+ Updates.push_back({DominatorTree::Insert, PBI->getParent(), OtherDest});
+ Updates.push_back({DominatorTree::Delete, PBI->getParent(), RemovedDest});
- if (DTU)
DTU->applyUpdates(Updates);
+ }
// Update branch weight for PBI.
uint64_t PredTrueWeight, PredFalseWeight, SuccTrueWeight, SuccFalseWeight;
@@ -4010,17 +4023,19 @@ bool SimplifyCFGOpt::tryToSimplifyUncondBranchWithICmpInIt(
SIW.setSuccessorWeight(0, *NewW);
}
SIW.addCase(Cst, NewBB, NewW);
- Updates.push_back({DominatorTree::Insert, Pred, NewBB});
+ if (DTU)
+ Updates.push_back({DominatorTree::Insert, Pred, NewBB});
}
// NewBB branches to the phi block, add the uncond branch and the phi entry.
Builder.SetInsertPoint(NewBB);
Builder.SetCurrentDebugLocation(SI->getDebugLoc());
Builder.CreateBr(SuccBlock);
- Updates.push_back({DominatorTree::Insert, NewBB, SuccBlock});
PHIUse->addIncoming(NewCst, NewBB);
- if (DTU)
+ if (DTU) {
+ Updates.push_back({DominatorTree::Insert, NewBB, SuccBlock});
DTU->applyUpdates(Updates);
+ }
return true;
}
@@ -4106,7 +4121,8 @@ bool SimplifyCFGOpt::SimplifyBranchOnICmpChain(BranchInst *BI,
OldTI->eraseFromParent();
- Updates.push_back({DominatorTree::Insert, BB, EdgeBB});
+ if (DTU)
+ Updates.push_back({DominatorTree::Insert, BB, EdgeBB});
// If there are PHI nodes in EdgeBB, then we need to add a new entry to them
// for the edge we just added.
@@ -4394,16 +4410,19 @@ static bool removeEmptyCleanup(CleanupReturnInst *RI, DomTreeUpdater *DTU) {
// We use make_early_inc_range here because we may remove some predecessors.
for (BasicBlock *PredBB : llvm::make_early_inc_range(predecessors(BB))) {
if (UnwindDest == nullptr) {
- if (DTU)
+ if (DTU) {
DTU->applyUpdates(Updates);
- Updates.clear();
+ Updates.clear();
+ }
removeUnwindEdge(PredBB, DTU);
++NumInvokes;
} else {
Instruction *TI = PredBB->getTerminator();
TI->replaceUsesOfWith(BB, UnwindDest);
- Updates.push_back({DominatorTree::Insert, PredBB, UnwindDest});
- Updates.push_back({DominatorTree::Delete, PredBB, BB});
+ if (DTU) {
+ Updates.push_back({DominatorTree::Insert, PredBB, UnwindDest});
+ Updates.push_back({DominatorTree::Delete, PredBB, BB});
+ }
}
}
@@ -4610,7 +4629,8 @@ bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) {
EraseTerminatorAndDCECond(BI);
Changed = true;
}
- Updates.push_back({DominatorTree::Delete, Predecessor, BB});
+ if (DTU)
+ Updates.push_back({DominatorTree::Delete, Predecessor, BB});
} else if (auto *SI = dyn_cast<SwitchInst>(TI)) {
SwitchInstProfUpdateWrapper SU(*SI);
for (auto i = SU->case_begin(), e = SU->case_end(); i != e;) {
@@ -4624,21 +4644,23 @@ bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) {
Changed = true;
}
// Note that the default destination can't be removed!
- if (SI->getDefaultDest() != BB)
+ if (DTU && SI->getDefaultDest() != BB)
Updates.push_back({DominatorTree::Delete, Predecessor, BB});
} else if (auto *II = dyn_cast<InvokeInst>(TI)) {
if (II->getUnwindDest() == BB) {
- if (DTU)
+ if (DTU) {
DTU->applyUpdates(Updates);
- Updates.clear();
+ Updates.clear();
+ }
removeUnwindEdge(TI->getParent(), DTU);
Changed = true;
}
} else if (auto *CSI = dyn_cast<CatchSwitchInst>(TI)) {
if (CSI->getUnwindDest() == BB) {
- if (DTU)
+ if (DTU) {
DTU->applyUpdates(Updates);
- Updates.clear();
+ Updates.clear();
+ }
removeUnwindEdge(TI->getParent(), DTU);
Changed = true;
continue;
@@ -4654,23 +4676,28 @@ bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) {
Changed = true;
}
}
- Updates.push_back({DominatorTree::Delete, Predecessor, BB});
+ if (DTU)
+ Updates.push_back({DominatorTree::Delete, Predecessor, BB});
if (CSI->getNumHandlers() == 0) {
if (CSI->hasUnwindDest()) {
// Redirect all predecessors of the block containing CatchSwitchInst
// to instead branch to the CatchSwitchInst's unwind destination.
- for (auto *PredecessorOfPredecessor : predecessors(Predecessor)) {
- Updates.push_back({DominatorTree::Insert, PredecessorOfPredecessor,
- CSI->getUnwindDest()});
- Updates.push_back(
- {DominatorTree::Delete, PredecessorOfPredecessor, Predecessor});
+ if (DTU) {
+ for (auto *PredecessorOfPredecessor : predecessors(Predecessor)) {
+ Updates.push_back({DominatorTree::Insert,
+ PredecessorOfPredecessor,
+ CSI->getUnwindDest()});
+ Updates.push_back({DominatorTree::Delete,
+ PredecessorOfPredecessor, Predecessor});
+ }
}
Predecessor->replaceAllUsesWith(CSI->getUnwindDest());
} else {
// Rewrite all preds to unwind to caller (or from invoke to call).
- if (DTU)
+ if (DTU) {
DTU->applyUpdates(Updates);
- Updates.clear();
+ Updates.clear();
+ }
SmallVector<BasicBlock *, 8> EHPreds(predecessors(Predecessor));
for (BasicBlock *EHPred : EHPreds)
removeUnwindEdge(EHPred, DTU);
@@ -4684,7 +4711,8 @@ bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) {
(void)CRI;
assert(CRI->hasUnwindDest() && CRI->getUnwindDest() == BB &&
"Expected to always have an unwind to BB.");
- Updates.push_back({DominatorTree::Delete, Predecessor, BB});
+ if (DTU)
+ Updates.push_back({DominatorTree::Delete, Predecessor, BB});
new UnreachableInst(TI->getContext(), TI);
TI->eraseFromParent();
Changed = true;
@@ -4731,8 +4759,9 @@ static void createUnreachableSwitchDefault(SwitchInst *Switch,
{DominatorTree::Delete, BB, OrigDefaultBlock}});
SplitBlock(&*NewDefaultBlock, &NewDefaultBlock->front(), DTU);
SmallVector<DominatorTree::UpdateType, 2> Updates;
- for (auto *Successor : successors(NewDefaultBlock))
- Updates.push_back({DominatorTree::Delete, NewDefaultBlock, Successor});
+ if (DTU)
+ for (auto *Successor : successors(NewDefaultBlock))
+ Updates.push_back({DominatorTree::Delete, NewDefaultBlock, Successor});
auto *NewTerminator = NewDefaultBlock->getTerminator();
new UnreachableInst(Switch->getContext(), NewTerminator);
EraseTerminatorAndDCECond(NewTerminator);
@@ -4887,12 +4916,14 @@ static bool eliminateDeadSwitchCases(SwitchInst *SI, DomTreeUpdater *DTU,
SmallMapVector<BasicBlock *, int, 8> NumPerSuccessorCases;
for (auto &Case : SI->cases()) {
auto *Successor = Case.getCaseSuccessor();
- ++NumPerSuccessorCases[Successor];
+ if (DTU)
+ ++NumPerSuccessorCases[Successor];
const APInt &CaseVal = Case.getCaseValue()->getValue();
if (Known.Zero.intersects(CaseVal) || !Known.One.isSubsetOf(CaseVal) ||
(CaseVal.getMinSignedBits() > MaxSignificantBitsInCond)) {
DeadCases.push_back(Case.getCaseValue());
- --NumPerSuccessorCases[Successor];
+ if (DTU)
+ --NumPerSuccessorCases[Successor];
LLVM_DEBUG(dbgs() << "SimplifyCFG: switch case " << CaseVal
<< " is dead.\n");
}
@@ -4927,12 +4958,13 @@ static bool eliminateDeadSwitchCases(SwitchInst *SI, DomTreeUpdater *DTU,
SIW.removeCase(CaseI);
}
- std::vector<DominatorTree::UpdateType> Updates;
- for (const std::pair<BasicBlock *, int> &I : NumPerSuccessorCases)
- if (I.second == 0)
- Updates.push_back({DominatorTree::Delete, SI->getParent(), I.first});
- if (DTU)
+ if (DTU) {
+ std::vector<DominatorTree::UpdateType> Updates;
+ for (const std::pair<BasicBlock *, int> &I : NumPerSuccessorCases)
+ if (I.second == 0)
+ Updates.push_back({DominatorTree::Delete, SI->getParent(), I.first});
DTU->applyUpdates(Updates);
+ }
return true;
}
@@ -5296,7 +5328,7 @@ static void RemoveSwitchAfterSelectConversion(SwitchInst *SI, PHINode *PHI,
BasicBlock *SelectBB = SI->getParent();
BasicBlock *DestBB = PHI->getParent();
- if (!is_contained(predecessors(DestBB), SelectBB))
+ if (DTU && !is_contained(predecessors(DestBB), SelectBB))
Updates.push_back({DominatorTree::Insert, SelectBB, DestBB});
Builder.CreateBr(DestBB);
@@ -5312,7 +5344,8 @@ static void RemoveSwitchAfterSelectConversion(SwitchInst *SI, PHINode *PHI,
if (Succ == DestBB)
continue;
Succ->removePredecessor(SelectBB);
- Updates.push_back({DominatorTree::Delete, SelectBB, Succ});
+ if (DTU)
+ Updates.push_back({DominatorTree::Delete, SelectBB, Succ});
}
SI->eraseFromParent();
if (DTU)
@@ -5860,7 +5893,8 @@ static bool SwitchToLookupTable(SwitchInst *SI, IRBuilder<> &Builder,
if (!DefaultIsReachable || GeneratingCoveredLookupTable) {
Builder.CreateBr(LookupBB);
- Updates.push_back({DominatorTree::Insert, BB, LookupBB});
+ if (DTU)
+ Updates.push_back({DominatorTree::Insert, BB, LookupBB});
// Note: We call removeProdecessor later since we need to be able to get the
// PHI value for the default case in case we're using a bit mask.
} else {
@@ -5868,7 +5902,8 @@ static bool SwitchToLookupTable(SwitchInst *SI, IRBuilder<> &Builder,
TableIndex, ConstantInt::get(MinCaseVal->getType(), TableSize));
RangeCheckBranch =
Builder.CreateCondBr(Cmp, LookupBB, SI->getDefaultDest());
- Updates.push_back({DominatorTree::Insert, BB, LookupBB});
+ if (DTU)
+ Updates.push_back({DominatorTree::Insert, BB, LookupBB});
}
// Populate the BB that does the lookups.
@@ -5906,8 +5941,10 @@ static bool SwitchToLookupTable(SwitchInst *SI, IRBuilder<> &Builder,
Value *LoBit = Builder.CreateTrunc(
Shifted, Type::getInt1Ty(Mod.getContext()), "switch.lobit");
Builder.CreateCondBr(LoBit, LookupBB, SI->getDefaultDest());
- Updates.push_back({DominatorTree::Insert, MaskBB, LookupBB});
- Updates.push_back({DominatorTree::Insert, MaskBB, SI->getDefaultDest()});
+ if (DTU) {
+ Updates.push_back({DominatorTree::Insert, MaskBB, LookupBB});
+ Updates.push_back({DominatorTree::Insert, MaskBB, SI->getDefaultDest()});
+ }
Builder.SetInsertPoint(LookupBB);
AddPredecessorToBlock(SI->getDefaultDest(), MaskBB, BB);
}
@@ -5917,7 +5954,8 @@ static bool SwitchToLookupTable(SwitchInst *SI, IRBuilder<> &Builder,
// do not delete PHINodes here.
SI->getDefaultDest()->removePredecessor(BB,
/*KeepOneInputPHIs=*/true);
- Updates.push_back({DominatorTree::Delete, BB, SI->getDefaultDest()});
+ if (DTU)
+ Updates.push_back({DominatorTree::Delete, BB, SI->getDefaultDest()});
}
bool ReturnedEarly = false;
@@ -5956,7 +5994,8 @@ static bool SwitchToLookupTable(SwitchInst *SI, IRBuilder<> &Builder,
if (!ReturnedEarly) {
Builder.CreateBr(CommonDest);
- Updates.push_back({DominatorTree::Insert, LookupBB, CommonDest});
+ if (DTU)
+ Updates.push_back({DominatorTree::Insert, LookupBB, CommonDest});
}
// Remove the switch.
@@ -6239,8 +6278,10 @@ static bool TryToMergeLandingPad(LandingPadInst *LPad, BranchInst *BI,
assert(II->getNormalDest() != BB && II->getUnwindDest() == BB &&
"unexpected successor");
II->setUnwindDest(OtherPred);
- Updates.push_back({DominatorTree::Insert, Pred, OtherPred});
- Updates.push_back({DominatorTree::Delete, Pred, BB});
+ if (DTU) {
+ Updates.push_back({DominatorTree::Insert, Pred, OtherPred});
+ Updates.push_back({DominatorTree::Delete, Pred, BB});
+ }
}
// The debug info in OtherPred doesn't cover the merged control flow that
@@ -6256,7 +6297,8 @@ static bool TryToMergeLandingPad(LandingPadInst *LPad, BranchInst *BI,
Succs.insert(succ_begin(BB), succ_end(BB));
for (BasicBlock *Succ : Succs) {
Succ->removePredecessor(BB);
- Updates.push_back({DominatorTree::Delete, BB, Succ});
+ if (DTU)
+ Updates.push_back({DominatorTree::Delete, BB, Succ});
}
IRBuilder<> Builder(BI);
More information about the llvm-commits
mailing list