[llvm] 3fb5722 - [NFCI] SimplifyCFG: switch to non-permissive DomTree updates, where possible
Roman Lebedev via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 4 14:26:53 PST 2021
Author: Roman Lebedev
Date: 2021-01-05T01:26:36+03:00
New Revision: 3fb57222c4c0db02f13f32579fb83d0d488becad
URL: https://github.com/llvm/llvm-project/commit/3fb57222c4c0db02f13f32579fb83d0d488becad
DIFF: https://github.com/llvm/llvm-project/commit/3fb57222c4c0db02f13f32579fb83d0d488becad.diff
LOG: [NFCI] SimplifyCFG: switch to non-permissive DomTree updates, where possible
Notably, this doesn't switch *every* case, remaining cases
don't actually pass sanity checks in non-permissve mode,
and therefore require further analysis.
Note that SimplifyCFG still defaults to not preserving DomTree by default,
so this is effectively a NFC change.
Added:
Modified:
llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
index 3efdc0e9ea86..2c3454c46b30 100644
--- a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
+++ b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
@@ -77,6 +77,7 @@ STATISTIC(NumSimpl, "Number of blocks simplified");
/// If we have more than one empty (other than phi node) return blocks,
/// merge them together to promote recursive block merging.
+// FIXME: switch to non-permissive DomTreeUpdater::applyUpdates().
static bool mergeEmptyReturnBlocks(Function &F, DomTreeUpdater *DTU) {
bool Changed = false;
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index a4faf22b8294..567b2e02b71c 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -872,6 +872,7 @@ static void setBranchWeights(Instruction *I, uint32_t TrueWeight,
/// also a value comparison with the same value, and if that comparison
/// determines the outcome of this comparison. If so, simplify TI. This does a
/// very limited form of jump threading.
+// FIXME: switch to non-permissive DomTreeUpdater::applyUpdates().
bool SimplifyCFGOpt::SimplifyEqualityComparisonWithOnlyPredecessor(
Instruction *TI, BasicBlock *Pred, IRBuilder<> &Builder) {
Value *PredVal = isValueEqualityComparison(Pred->getTerminator());
@@ -924,7 +925,7 @@ bool SimplifyCFGOpt::SimplifyEqualityComparisonWithOnlyPredecessor(
EraseTerminatorAndDCECond(TI);
if (DTU)
- DTU->applyUpdatesPermissive(
+ DTU->applyUpdates(
{{DominatorTree::Delete, PredDef, ThisCases[0].Dest}});
return true;
@@ -956,7 +957,7 @@ bool SimplifyCFGOpt::SimplifyEqualityComparisonWithOnlyPredecessor(
if (I.second == 0)
Updates.push_back({DominatorTree::Delete, PredDef, I.first});
if (DTU)
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
LLVM_DEBUG(dbgs() << "Leaving: " << *TI << "\n");
return true;
@@ -1080,6 +1081,7 @@ static void FitWeights(MutableArrayRef<uint64_t> Weights) {
/// (either a switch or a branch on "X == c").
/// See if any of the predecessors of the terminator block are value comparisons
/// on the same value. If so, and if safe to do so, fold them together.
+// FIXME: switch to non-permissive DomTreeUpdater::applyUpdates().
bool SimplifyCFGOpt::FoldValueComparisonIntoPredecessors(Instruction *TI,
IRBuilder<> &Builder) {
BasicBlock *BB = TI->getParent();
@@ -1554,7 +1556,7 @@ bool SimplifyCFGOpt::HoistThenElseCodeToIf(BranchInst *BI,
EraseTerminatorAndDCECond(BI);
if (DTU)
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
return Changed;
}
@@ -2488,7 +2490,7 @@ static bool FoldCondBranchOnPHI(BranchInst *BI, DomTreeUpdater *DTU,
Updates.push_back({DominatorTree::Insert, PredBB, EdgeBB});
if (DTU)
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
// Recurse, simplifying any other constants.
return FoldCondBranchOnPHI(BI, DTU, DL, AC) || true;
@@ -2660,7 +2662,7 @@ static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,
OldTI->eraseFromParent();
if (DTU)
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
return true;
}
@@ -2668,6 +2670,7 @@ static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,
/// If we found a conditional branch that goes to two returning blocks,
/// try to merge them together into one return,
/// introducing a select if the return values disagree.
+// FIXME: switch to non-permissive DomTreeUpdater::applyUpdates().
bool SimplifyCFGOpt::SimplifyCondBranchToTwoReturns(BranchInst *BI,
IRBuilder<> &Builder) {
auto *BB = BI->getParent();
@@ -3169,7 +3172,7 @@ bool llvm::FoldBranchToCommonDest(BranchInst *BI, DomTreeUpdater *DTU,
}
}
// Update PHI Node.
- PHIs[i]->setIncomingValueForBlock(PBI->getParent(), MergedCond);
+ PHIs[i]->setIncomingValueForBlock(PBI->getParent(), MergedCond);
}
// PBI is changed to branch to UniqueSucc below. Remove itself from
@@ -3184,7 +3187,7 @@ bool llvm::FoldBranchToCommonDest(BranchInst *BI, DomTreeUpdater *DTU,
}
if (DTU)
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
// If BI was a loop latch, it may have had associated loop metadata.
// We need to copy it to the new latch, that is, PBI.
@@ -3561,9 +3564,8 @@ static bool tryWidenCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
OldSuccessor->removePredecessor(BI->getParent());
BI->setSuccessor(1, IfFalseBB);
if (DTU)
- DTU->applyUpdatesPermissive(
- {{DominatorTree::Delete, BI->getParent(), OldSuccessor},
- {DominatorTree::Insert, BI->getParent(), IfFalseBB}});
+ DTU->applyUpdates({{DominatorTree::Delete, BI->getParent(), OldSuccessor},
+ {DominatorTree::Insert, BI->getParent(), IfFalseBB}});
return true;
}
if (BI->getSuccessor(0) != IfFalseBB && // no inf looping
@@ -3573,9 +3575,8 @@ static bool tryWidenCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
OldSuccessor->removePredecessor(BI->getParent());
BI->setSuccessor(0, IfFalseBB);
if (DTU)
- DTU->applyUpdatesPermissive(
- {{DominatorTree::Delete, BI->getParent(), OldSuccessor},
- {DominatorTree::Insert, BI->getParent(), IfFalseBB}});
+ DTU->applyUpdates({{DominatorTree::Delete, BI->getParent(), OldSuccessor},
+ {DominatorTree::Insert, BI->getParent(), IfFalseBB}});
return true;
}
return false;
@@ -3767,7 +3768,7 @@ static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
Updates.push_back({DominatorTree::Insert, PBI->getParent(), Successor});
if (DTU)
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
// Update branch weight for PBI.
uint64_t PredTrueWeight, PredFalseWeight, SuccTrueWeight, SuccFalseWeight;
@@ -4096,7 +4097,7 @@ bool SimplifyCFGOpt::tryToSimplifyUncondBranchWithICmpInIt(
Updates.push_back({DominatorTree::Insert, NewBB, SuccBlock});
PHIUse->addIncoming(NewCst, NewBB);
if (DTU)
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
return true;
}
@@ -4222,7 +4223,7 @@ bool SimplifyCFGOpt::SimplifyBranchOnICmpChain(BranchInst *BI,
// Erase the old branch instruction.
EraseTerminatorAndDCECond(BI);
if (DTU)
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
LLVM_DEBUG(dbgs() << " ** 'icmp' chain result is:\n" << *BB << '\n');
return true;
@@ -4321,7 +4322,7 @@ bool SimplifyCFGOpt::simplifyCommonResume(ResumeInst *RI) {
TrivialBB->getTerminator()->eraseFromParent();
new UnreachableInst(RI->getContext(), TrivialBB);
if (DTU)
- DTU->applyUpdatesPermissive({{DominatorTree::Delete, TrivialBB, BB}});
+ DTU->applyUpdates({{DominatorTree::Delete, TrivialBB, BB}});
}
// Delete the resume block if all its predecessors have been removed.
@@ -4478,7 +4479,7 @@ static bool removeEmptyCleanup(CleanupReturnInst *RI, DomTreeUpdater *DTU) {
BasicBlock *PredBB = *PI++;
if (UnwindDest == nullptr) {
if (DTU)
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
Updates.clear();
removeUnwindEdge(PredBB, DTU);
++NumInvokes;
@@ -4491,7 +4492,7 @@ static bool removeEmptyCleanup(CleanupReturnInst *RI, DomTreeUpdater *DTU) {
}
if (DTU) {
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
DTU->deleteBB(BB);
} else
// The cleanup pad is now unreachable. Zap it.
@@ -4606,6 +4607,7 @@ bool SimplifyCFGOpt::simplifyReturn(ReturnInst *RI, IRBuilder<> &Builder) {
return false;
}
+// FIXME: switch to non-permissive DomTreeUpdater::applyUpdates().
bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) {
BasicBlock *BB = UI->getParent();
@@ -4711,7 +4713,7 @@ bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) {
} else if (auto *II = dyn_cast<InvokeInst>(TI)) {
if (II->getUnwindDest() == BB) {
if (DTU)
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
Updates.clear();
removeUnwindEdge(TI->getParent(), DTU);
Changed = true;
@@ -4719,7 +4721,7 @@ bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) {
} else if (auto *CSI = dyn_cast<CatchSwitchInst>(TI)) {
if (CSI->getUnwindDest() == BB) {
if (DTU)
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
Updates.clear();
removeUnwindEdge(TI->getParent(), DTU);
Changed = true;
@@ -4751,7 +4753,7 @@ bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) {
} else {
// Rewrite all preds to unwind to caller (or from invoke to call).
if (DTU)
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
Updates.clear();
SmallVector<BasicBlock *, 8> EHPreds(predecessors(Predecessor));
for (BasicBlock *EHPred : EHPreds)
@@ -4812,9 +4814,8 @@ static void createUnreachableSwitchDefault(SwitchInst *Switch,
auto *OrigDefaultBlock = Switch->getDefaultDest();
Switch->setDefaultDest(&*NewDefaultBlock);
if (DTU)
- DTU->applyUpdatesPermissive(
- {{DominatorTree::Delete, BB, OrigDefaultBlock},
- {DominatorTree::Insert, BB, &*NewDefaultBlock}});
+ DTU->applyUpdates({{DominatorTree::Delete, BB, OrigDefaultBlock},
+ {DominatorTree::Insert, BB, &*NewDefaultBlock}});
SplitBlock(&*NewDefaultBlock, &NewDefaultBlock->front(),
DTU ? &DTU->getDomTree() : nullptr);
SmallVector<DominatorTree::UpdateType, 2> Updates;
@@ -4824,7 +4825,7 @@ static void createUnreachableSwitchDefault(SwitchInst *Switch,
new UnreachableInst(Switch->getContext(), NewTerminator);
EraseTerminatorAndDCECond(NewTerminator);
if (DTU)
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
}
/// Turn a switch with two reachable destinations into an integer range
@@ -4949,8 +4950,7 @@ bool SimplifyCFGOpt::TurnSwitchRangeIntoICmp(SwitchInst *SI,
SI->eraseFromParent();
if (!HasDefault && DTU)
- DTU->applyUpdatesPermissive(
- {{DominatorTree::Delete, BB, UnreachableDefault}});
+ DTU->applyUpdates({{DominatorTree::Delete, BB, UnreachableDefault}});
return true;
}
@@ -5020,7 +5020,7 @@ static bool eliminateDeadSwitchCases(SwitchInst *SI, DomTreeUpdater *DTU,
if (I.second == 0)
Updates.push_back({DominatorTree::Delete, SI->getParent(), I.first});
if (DTU)
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
return true;
}
@@ -5399,7 +5399,7 @@ static void RemoveSwitchAfterSelectConversion(SwitchInst *SI, PHINode *PHI,
}
SI->eraseFromParent();
if (DTU)
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
}
/// If the switch is only used to initialize one or more
@@ -5810,6 +5810,7 @@ static void reuseTableCompare(
/// If the switch is only used to initialize one or more phi nodes in a common
/// successor block with
diff erent constant values, replace the switch with
/// lookup tables.
+// FIXME: switch to non-permissive DomTreeUpdater::applyUpdates().
static bool SwitchToLookupTable(SwitchInst *SI, IRBuilder<> &Builder,
DomTreeUpdater *DTU, const DataLayout &DL,
const TargetTransformInfo &TTI) {
@@ -6217,6 +6218,7 @@ bool SimplifyCFGOpt::simplifySwitch(SwitchInst *SI, IRBuilder<> &Builder) {
return false;
}
+// FIXME: switch to non-permissive DomTreeUpdater::applyUpdates().
bool SimplifyCFGOpt::simplifyIndirectBr(IndirectBrInst *IBI) {
BasicBlock *BB = IBI->getParent();
bool Changed = false;
@@ -6340,7 +6342,7 @@ static bool TryToMergeLandingPad(LandingPadInst *LPad, BranchInst *BI,
Builder.CreateUnreachable();
BI->eraseFromParent();
if (DTU)
- DTU->applyUpdatesPermissive(Updates);
+ DTU->applyUpdates(Updates);
return true;
}
return false;
@@ -6590,8 +6592,7 @@ static bool removeUndefIntroducingPredecessor(BasicBlock *BB,
: BI->getSuccessor(0));
BI->eraseFromParent();
if (DTU)
- DTU->applyUpdatesPermissive(
- {{DominatorTree::Delete, Predecessor, BB}});
+ DTU->applyUpdates({{DominatorTree::Delete, Predecessor, BB}});
return true;
}
// TODO: SwitchInst.
More information about the llvm-commits
mailing list