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. _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits