shylie/llvm-project
1
0
Fork 0
Code Issues Pull Requests Actions 6 Packages Projects Releases Wiki Activity

[LoopFusion] Fix out-of-date LoopInfo being used during fusion (#189452)

Browse Source 
This is fix for
[187902](https://github.com/llvm/llvm-project/issues/187902), where
`LoopInfo` is not in a valid state at the beginning of `ScalarEvolution::createSCEVIter`.

The reason for the bug is that, `mergeLatch()` is called at a place
where control flow and dominator trees have been updated but `LoopInfo`
has not completed the update yet. `mergeLatch()` calls into
`ScalarEvolution` that uses `LoopInfo`, where out-of-date `LoopInfo` would
result in crash or unpredictable results.

This patch moves `mergeLatch()` to the place where `LoopInfo` has
completed its update and hence is in a valid state.
This commit is contained in:
Congzhe 2026-04-06 16:35:28 -04:00 committed by GitHub
parent 1a0ca1019d
commit fbe6d79465
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 39 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
llvm/include/llvm/Transforms/Utils/CodeMoverUtils.h
View File

@ -26,6 +26,7 @@ class DependenceInfo;
class DominatorTree;
class Instruction;
class PostDominatorTree;
class ScalarEvolution;
/// Return true if \p I can be safely moved before \p InsertPoint.
LLVM_ABI bool isSafeToMoveBefore(Instruction &I, Instruction &InsertPoint,
@ -43,18 +44,16 @@ LLVM_ABI bool isSafeToMoveBefore(BasicBlock &BB, Instruction &InsertPoint,
/// Move instructions, in an order-preserving manner, from \p FromBB to the
/// beginning of \p ToBB when proven safe.
LLVM_ABI void moveInstructionsToTheBeginning(BasicBlock &FromBB,
BasicBlock &ToBB,
DominatorTree &DT,
const PostDominatorTree &PDT,
DependenceInfo &DI);
LLVM_ABI void
moveInstructionsToTheBeginning(BasicBlock &FromBB, BasicBlock &ToBB,
DominatorTree &DT, const PostDominatorTree &PDT,
DependenceInfo &DI, ScalarEvolution &SE);
/// Move instructions, in an order-preserving manner, from \p FromBB to the end
/// of \p ToBB when proven safe.
LLVM_ABI void moveInstructionsToTheEnd(BasicBlock &FromBB, BasicBlock &ToBB,
DominatorTree &DT,
const PostDominatorTree &PDT,
DependenceInfo &DI);
DependenceInfo &DI, ScalarEvolution &SE);
/// In case that two BBs \p ThisBlock and \p OtherBlock are control flow
/// equivalent but they do not strictly dominate and post-dominate each

48
llvm/lib/Transforms/Scalar/LoopFuse.cpp
View File

@ -1479,7 +1479,7 @@ private:
/// Move instructions from FC0.Latch to FC1.Latch. If FC0.Latch has an unique
/// successor, then merge FC0.Latch with its unique successor.
void mergeLatch(const FusionCandidate &FC0, const FusionCandidate &FC1) {
moveInstructionsToTheBeginning(*FC0.Latch, *FC1.Latch, DT, PDT, DI);
moveInstructionsToTheBeginning(*FC0.Latch, *FC1.Latch, DT, PDT, DI, SE);
if (BasicBlock *Succ = FC0.Latch->getUniqueSuccessor()) {
MergeBlockIntoPredecessor(Succ, &DTU, &LI);
DTU.flush();
@ -1524,7 +1524,7 @@ private:
// Move instructions from the preheader of FC1 to the end of the preheader
// of FC0.
moveInstructionsToTheEnd(*FC1.Preheader, *FC0.Preheader, DT, PDT, DI);
moveInstructionsToTheEnd(*FC1.Preheader, *FC0.Preheader, DT, PDT, DI, SE);
// Fusing guarded loops is handled slightly differently than non-guarded
// loops and has been broken out into a separate method instead of trying to
@ -1676,19 +1676,6 @@ private:
SE.forgetLoop(FC1.L);
SE.forgetLoop(FC0.L);
// Move instructions from FC0.Latch to FC1.Latch.
// Note: mergeLatch requires an updated DT.
mergeLatch(FC0, FC1);
// Forget block dispositions as well, so that there are no dangling
// pointers to erased/free'ed blocks. It should be done after mergeLatch()
// since merging the latches may affect the dispositions.
SE.forgetBlockAndLoopDispositions();
// Forget the cached SCEV values including the induction variable that may
// have changed after the fusion.
SE.forgetLoop(FC0.L);
// Merge the loops.
SmallVector<BasicBlock *, 8> Blocks(FC1.L->blocks());
for (BasicBlock *BB : Blocks) {
@ -1708,6 +1695,15 @@ private:
// Delete the now empty loop L1.
LI.erase(FC1.L);
// Forget block dispositions as well, so that there are no dangling
// pointers to erased/free'ed blocks. It should be done after mergeLatch()
// since merging the latches may affect the dispositions.
SE.forgetBlockAndLoopDispositions();
// Move instructions from FC0.Latch to FC1.Latch.
// Note: mergeLatch requires an updated DT.
mergeLatch(FC0, FC1);
#ifndef NDEBUG
assert(!verifyFunction(*FC0.Header->getParent(), &errs()));
assert(DT.verify(DominatorTree::VerificationLevel::Fast));
@ -1781,11 +1777,11 @@ private:
// of the FC0 Exit block to the beginning of the exit block of FC1.
moveInstructionsToTheBeginning(
(FC0.Peeled ? *FC0ExitBlockSuccessor : *FC0.ExitBlock), *FC1.ExitBlock,
DT, PDT, DI);
DT, PDT, DI, SE);
// Move instructions from the guard block of FC1 to the end of the guard
// block of FC0.
moveInstructionsToTheEnd(*FC1GuardBlock, *FC0GuardBlock, DT, PDT, DI);
moveInstructionsToTheEnd(*FC1GuardBlock, *FC0GuardBlock, DT, PDT, DI, SE);
assert(FC0NonLoopBlock == FC1GuardBlock && "Loops are not adjacent");
@ -1978,15 +1974,6 @@ private:
SE.forgetLoop(FC1.L);
SE.forgetLoop(FC0.L);
// Move instructions from FC0.Latch to FC1.Latch.
// Note: mergeLatch requires an updated DT.
mergeLatch(FC0, FC1);
// Forget block dispositions as well, so that there are no dangling
// pointers to erased/free'ed blocks. It should be done after mergeLatch()
// since merging the latches may affect the dispositions.
SE.forgetBlockAndLoopDispositions();
// Merge the loops.
SmallVector<BasicBlock *, 8> Blocks(FC1.L->blocks());
for (BasicBlock *BB : Blocks) {
@ -2006,6 +1993,15 @@ private:
// Delete the now empty loop L1.
LI.erase(FC1.L);
// Forget block dispositions as well, so that there are no dangling
// pointers to erased/free'ed blocks. It should be done after mergeLatch()
// since merging the latches may affect the dispositions.
SE.forgetBlockAndLoopDispositions();
// Move instructions from FC0.Latch to FC1.Latch.
// Note: mergeLatch requires an updated DT.
mergeLatch(FC0, FC1);
#ifndef NDEBUG
assert(!verifyFunction(*FC0.Header->getParent(), &errs()));
assert(DT.verify(DominatorTree::VerificationLevel::Fast));

15
llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
View File

@ -194,25 +194,32 @@ bool llvm::isSafeToMoveBefore(BasicBlock &BB, Instruction &InsertPoint,
void llvm::moveInstructionsToTheBeginning(BasicBlock &FromBB, BasicBlock &ToBB,
DominatorTree &DT,
const PostDominatorTree &PDT,
DependenceInfo &DI) {
DependenceInfo &DI,
ScalarEvolution &SE) {
for (Instruction &I :
llvm::make_early_inc_range(llvm::drop_begin(llvm::reverse(FromBB)))) {
BasicBlock::iterator MovePos = ToBB.getFirstNonPHIOrDbg();
if (isSafeToMoveBefore(I, *MovePos, DT, &PDT, &DI))
if (isSafeToMoveBefore(I, *MovePos, DT, &PDT, &DI)) {
// Update SCEV to ensure it remains valid throughout the function.
SE.forgetValue(&I);
I.moveBeforePreserving(MovePos);
}
}
}
void llvm::moveInstructionsToTheEnd(BasicBlock &FromBB, BasicBlock &ToBB,
DominatorTree &DT,
const PostDominatorTree &PDT,
DependenceInfo &DI) {
DependenceInfo &DI, ScalarEvolution &SE) {
Instruction *MovePos = ToBB.getTerminator();
while (FromBB.size() > 1) {
Instruction &I = FromBB.front();
if (isSafeToMoveBefore(I, *MovePos, DT, &PDT, &DI))
if (isSafeToMoveBefore(I, *MovePos, DT, &PDT, &DI)) {
// Update SCEV to ensure it remains valid throughout the function.
SE.forgetValue(&I);
I.moveBeforePreserving(MovePos->getIterator());
}
}
}