From fbe6d794650a3c9d586c020fcf2b4fba3faa2dad Mon Sep 17 00:00:00 2001 From: Congzhe Date: Mon, 6 Apr 2026 16:35:28 -0400 Subject: [PATCH] [LoopFusion] Fix out-of-date LoopInfo being used during fusion (#189452) 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. --- .../llvm/Transforms/Utils/CodeMoverUtils.h | 13 +++-- llvm/lib/Transforms/Scalar/LoopFuse.cpp | 48 +++++++++---------- llvm/lib/Transforms/Utils/CodeMoverUtils.cpp | 15 ++++-- 3 files changed, 39 insertions(+), 37 deletions(-) diff --git a/llvm/include/llvm/Transforms/Utils/CodeMoverUtils.h b/llvm/include/llvm/Transforms/Utils/CodeMoverUtils.h index d473f7092f62..0c5b32500099 100644 --- a/llvm/include/llvm/Transforms/Utils/CodeMoverUtils.h +++ b/llvm/include/llvm/Transforms/Utils/CodeMoverUtils.h @@ -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 diff --git a/llvm/lib/Transforms/Scalar/LoopFuse.cpp b/llvm/lib/Transforms/Scalar/LoopFuse.cpp index 23bba139212e..b577bf658461 100644 --- a/llvm/lib/Transforms/Scalar/LoopFuse.cpp +++ b/llvm/lib/Transforms/Scalar/LoopFuse.cpp @@ -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 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 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)); diff --git a/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp b/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp index 989ccafadb1a..eaf2a708e31d 100644 --- a/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp +++ b/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp @@ -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()); + } } }