[llvm] be7c859 - [CodeGenPrepare] Fix for using outdated/corrupt LoopInfo
Momchil Velikov via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 19 01:43:14 PDT 2023
Author: Momchil Velikov
Date: 2023-06-19T09:41:54+01:00
New Revision: be7c859b02306338d829a2f43ab72a6ee7e2faf9
URL: https://github.com/llvm/llvm-project/commit/be7c859b02306338d829a2f43ab72a6ee7e2faf9
DIFF: https://github.com/llvm/llvm-project/commit/be7c859b02306338d829a2f43ab72a6ee7e2faf9.diff
LOG: [CodeGenPrepare] Fix for using outdated/corrupt LoopInfo
Some transformation in CodeGenPrepare pass may create and/or delete
basic block, but they don't update the LoopInfo, so the LoopInfo may
end up containing dangling pointers and sometimes reused basic blocks,
which leads to "interesting" non-deterministic behaviour.
These transformations do not seem to alter the loop structure of the
function, and updating the loop info is quite straighforward.
Reviewed By: efriedma
Differential Revision: https://reviews.llvm.org/D150384
Change-Id: If8ab3905749ea6be94fbbacd54c5cfab5bc1fba1
Added:
Modified:
llvm/lib/CodeGen/CodeGenPrepare.cpp
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 410bfc53d63ab..fba31c65602b4 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -304,7 +304,7 @@ class CodeGenPrepare : public FunctionPass {
const TargetTransformInfo *TTI = nullptr;
const BasicBlockSectionsProfileReader *BBSectionsProfileReader = nullptr;
const TargetLibraryInfo *TLInfo = nullptr;
- const LoopInfo *LI = nullptr;
+ LoopInfo *LI = nullptr;
std::unique_ptr<BlockFrequencyInfo> BFI;
std::unique_ptr<BranchProbabilityInfo> BPI;
ProfileSummaryInfo *PSI = nullptr;
@@ -417,7 +417,7 @@ class CodeGenPrepare : public FunctionPass {
void removeAllAssertingVHReferences(Value *V);
bool eliminateAssumptions(Function &F);
- bool eliminateFallThrough(Function &F);
+ bool eliminateFallThrough(Function &F, DominatorTree *DT = nullptr);
bool eliminateMostlyEmptyBlocks(Function &F);
BasicBlock *findDestBlockOfMergeableEmptyBlock(BasicBlock *BB);
bool canMergeBlocks(const BasicBlock *BB, const BasicBlock *DestBB) const;
@@ -578,11 +578,15 @@ bool CodeGenPrepare::runOnFunction(Function &F) {
// Because the basic algorithm's complex is near O(N!).
IsHugeFunc = F.size() > HugeFuncThresholdInCGPP;
+ // Transformations above may invalidate dominator tree and/or loop info.
+ DT.reset();
+ LI->releaseMemory();
+ LI->analyze(getDT(F));
+
bool MadeChange = true;
bool FuncIterated = false;
while (MadeChange) {
MadeChange = false;
- DT.reset();
for (BasicBlock &BB : llvm::make_early_inc_range(F)) {
if (FuncIterated && !FreshBBs.contains(&BB))
@@ -591,6 +595,9 @@ bool CodeGenPrepare::runOnFunction(Function &F) {
ModifyDT ModifiedDTOnIteration = ModifyDT::NotModifyDT;
bool Changed = optimizeBlock(BB, ModifiedDTOnIteration);
+ if (ModifiedDTOnIteration == ModifyDT::ModifyBBDT)
+ DT.reset();
+
MadeChange |= Changed;
if (IsHugeFunc) {
// If the BB is updated, it may still has chance to be optimized.
@@ -606,9 +613,6 @@ bool CodeGenPrepare::runOnFunction(Function &F) {
FreshBBs.insert(&BB);
else if (FuncIterated)
FreshBBs.erase(&BB);
-
- if (ModifiedDTOnIteration == ModifyDT::ModifyBBDT)
- DT.reset();
} else {
// For small/normal functions, we restart BB iteration if the dominator
// tree of the Function was changed.
@@ -626,7 +630,12 @@ bool CodeGenPrepare::runOnFunction(Function &F) {
MadeChange |= optimizePhiTypes(F);
if (MadeChange)
- eliminateFallThrough(F);
+ eliminateFallThrough(F, DT.get());
+
+#ifndef NDEBUG
+ if (MadeChange && VerifyLoopInfo)
+ LI->verify(getDT(F));
+#endif
// Really free removed instructions during promotion.
for (Instruction *I : RemovedInsts)
@@ -759,7 +768,7 @@ void LLVM_ATTRIBUTE_UNUSED CodeGenPrepare::verifyBFIUpdates(Function &F) {
/// Merge basic blocks which are connected by a single edge, where one of the
/// basic blocks has a single successor pointing to the other basic block,
/// which has a single predecessor.
-bool CodeGenPrepare::eliminateFallThrough(Function &F) {
+bool CodeGenPrepare::eliminateFallThrough(Function &F, DominatorTree *DT) {
bool Changed = false;
// Scan all of the blocks in the function, except for the entry block.
// Use a temporary array to avoid iterator being invalidated when
@@ -781,13 +790,19 @@ bool CodeGenPrepare::eliminateFallThrough(Function &F) {
if (!SinglePred || SinglePred == BB || BB->hasAddressTaken())
continue;
+ // Make an effort to skip unreachable blocks.
+ if (DT && !DT->isReachableFromEntry(BB))
+ continue;
+
BranchInst *Term = dyn_cast<BranchInst>(SinglePred->getTerminator());
if (Term && !Term->isConditional()) {
Changed = true;
LLVM_DEBUG(dbgs() << "To merge:\n" << *BB << "\n\n\n");
// Merge BB into SinglePred and delete it.
- MergeBlockIntoPredecessor(BB);
+ MergeBlockIntoPredecessor(BB, /* DTU */ nullptr, LI, /* MSSAU */ nullptr,
+ /* MemDep */ nullptr,
+ /* PredecessorWithTwoSuccessors */ false, DT);
Preds.insert(SinglePred);
if (IsHugeFunc) {
@@ -2169,6 +2184,7 @@ static bool OptimizeExtractBits(BinaryOperator *ShiftI, ConstantInt *CI,
///
/// If the transform is performed, return true and set ModifiedDT to true.
static bool despeculateCountZeros(IntrinsicInst *CountZeros,
+ LoopInfo &LI,
const TargetLowering *TLI,
const DataLayout *DL, ModifyDT &ModifiedDT,
SmallSet<BasicBlock *, 32> &FreshBBs,
@@ -2208,6 +2224,13 @@ static bool despeculateCountZeros(IntrinsicInst *CountZeros,
if (IsHugeFunc)
FreshBBs.insert(EndBlock);
+ // Update the LoopInfo. The new blocks are in the same loop as the start
+ // block.
+ if (Loop *L = LI.getLoopFor(StartBlock)) {
+ L->addBasicBlockToLoop(CallBlock, LI);
+ L->addBasicBlockToLoop(EndBlock, LI);
+ }
+
// Set up a builder to create a compare, conditional branch, and PHI.
IRBuilder<> Builder(CountZeros->getContext());
Builder.SetInsertPoint(StartBlock->getTerminator());
@@ -2382,7 +2405,7 @@ bool CodeGenPrepare::optimizeCallInst(CallInst *CI, ModifyDT &ModifiedDT) {
case Intrinsic::cttz:
case Intrinsic::ctlz:
// If counting zeros is expensive, try to avoid it.
- return despeculateCountZeros(II, TLI, DL, ModifiedDT, FreshBBs,
+ return despeculateCountZeros(II, *LI, TLI, DL, ModifiedDT, FreshBBs,
IsHugeFunc);
case Intrinsic::fshl:
case Intrinsic::fshr:
@@ -2465,6 +2488,8 @@ bool CodeGenPrepare::dupRetToEnableTailCallOpts(BasicBlock *BB,
if (!RetI)
return false;
+ assert(LI->getLoopFor(BB) == nullptr && "A return block cannot be in a loop");
+
PHINode *PN = nullptr;
ExtractValueInst *EVI = nullptr;
BitCastInst *BCI = nullptr;
@@ -6116,7 +6141,7 @@ bool CodeGenPrepare::splitLargeGEPOffsets() {
NewBaseInsertPt = NewBaseInsertBB->getFirstInsertionPt();
else if (InvokeInst *Invoke = dyn_cast<InvokeInst>(BaseI)) {
NewBaseInsertBB =
- SplitEdge(NewBaseInsertBB, Invoke->getNormalDest());
+ SplitEdge(NewBaseInsertBB, Invoke->getNormalDest(), DT.get(), LI);
NewBaseInsertPt = NewBaseInsertBB->getFirstInsertionPt();
} else
NewBaseInsertPt = std::next(BaseI->getIterator());
@@ -6973,8 +6998,10 @@ bool CodeGenPrepare::optimizeSelectInst(SelectInst *SI) {
BasicBlock *EndBlock = StartBlock->splitBasicBlock(SplitPt, "select.end");
if (IsHugeFunc)
FreshBBs.insert(EndBlock);
+ Loop *L = LI->getLoopFor(StartBlock);
+ if (L)
+ L->addBasicBlockToLoop(EndBlock, *LI);
BFI->setBlockFreq(EndBlock, BFI->getBlockFreq(StartBlock).getFrequency());
-
// Delete the unconditional branch that was just created by the split.
StartBlock->getTerminator()->eraseFromParent();
@@ -6995,6 +7022,8 @@ bool CodeGenPrepare::optimizeSelectInst(SelectInst *SI) {
TrueBranch = BranchInst::Create(EndBlock, TrueBlock);
if (IsHugeFunc)
FreshBBs.insert(TrueBlock);
+ if (L)
+ L->addBasicBlockToLoop(TrueBlock, *LI);
TrueBranch->setDebugLoc(SI->getDebugLoc());
}
auto *TrueInst = cast<Instruction>(SI->getTrueValue());
@@ -7006,6 +7035,8 @@ bool CodeGenPrepare::optimizeSelectInst(SelectInst *SI) {
EndBlock->getParent(), EndBlock);
if (IsHugeFunc)
FreshBBs.insert(FalseBlock);
+ if (L)
+ L->addBasicBlockToLoop(FalseBlock, *LI);
FalseBranch = BranchInst::Create(EndBlock, FalseBlock);
FalseBranch->setDebugLoc(SI->getDebugLoc());
}
@@ -7024,6 +7055,8 @@ bool CodeGenPrepare::optimizeSelectInst(SelectInst *SI) {
EndBlock->getParent(), EndBlock);
if (IsHugeFunc)
FreshBBs.insert(FalseBlock);
+ if (L)
+ L->addBasicBlockToLoop(FalseBlock, *LI);
auto *FalseBranch = BranchInst::Create(EndBlock, FalseBlock);
FalseBranch->setDebugLoc(SI->getDebugLoc());
}
More information about the llvm-commits
mailing list