Skip to content

Commit

Permalink
[NFCI] SimplifyCFG: switch to non-permissive DomTree updates, where p…
Browse files Browse the repository at this point in the history 
…ossible

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.
  • Loading branch information
@LebedevRI
LebedevRI committed Jan 4, 2021
1 parent b4f519b commit 3fb5722
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 33 deletions.
1 change: 1 addition & 0 deletions llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
View file
Expand Up @@ -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;

Expand Down
67 changes: 34 additions & 33 deletions llvm/lib/Transforms/Utils/SimplifyCFG.cpp
View file
Expand Up @@ -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());
Expand Down Expand Up @@ -924,7 +925,7 @@ bool SimplifyCFGOpt::SimplifyEqualityComparisonWithOnlyPredecessor(
EraseTerminatorAndDCECond(TI);

if (DTU)
DTU->applyUpdatesPermissive(
DTU->applyUpdates(
{{DominatorTree::Delete, PredDef, ThisCases[0].Dest}});

return true;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -1554,7 +1556,7 @@ bool SimplifyCFGOpt::HoistThenElseCodeToIf(BranchInst *BI,

EraseTerminatorAndDCECond(BI);
if (DTU)
DTU->applyUpdatesPermissive(Updates);
DTU->applyUpdates(Updates);
return Changed;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -2660,14 +2662,15 @@ static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,

OldTI->eraseFromParent();
if (DTU)
DTU->applyUpdatesPermissive(Updates);
DTU->applyUpdates(Updates);

return true;
}

/// 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();
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -4711,15 +4713,15 @@ 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;
}
} 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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 different 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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 3fb5722

Please sign in to comment.