From 8bafca72573c7ba5e7b7711c5168e0609571661f Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Thu, 23 Jan 2025 14:18:55 +0000 Subject: [PATCH 1/2] [NFC][DebugInfo] Use iterators for instruction insertion in more places As part of the "RemoveDIs" work to eliminate debug intrinsics, we're replacing methods that use Instruction*'s as positions with iterators. This patch changes some more complex call-sites, those crossing file boundaries and where I've had to perform some minor rewrites. --- clang/lib/CodeGen/CodeGenFunction.h | 2 +- .../llvm/Transforms/Utils/BasicBlockUtils.h | 6 ++--- llvm/lib/CodeGen/CodeGenPrepare.cpp | 22 +++++++++---------- .../Target/Hexagon/HexagonVectorCombine.cpp | 14 ++++++------ llvm/lib/Transforms/Coroutines/CoroSplit.cpp | 16 ++++++++------ .../Instrumentation/AddressSanitizer.cpp | 2 +- .../Instrumentation/MemorySanitizer.cpp | 2 +- .../Transforms/Scalar/LoopStrengthReduce.cpp | 2 +- llvm/lib/Transforms/Utils/BasicBlockUtils.cpp | 12 +++++----- llvm/lib/Transforms/Utils/InlineFunction.cpp | 8 +++---- 10 files changed, 44 insertions(+), 42 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index fab27d4c22ed80..bdd6e3bd55a7fd 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -451,7 +451,7 @@ class CodeGenFunction : public CodeGenTypeCache { "EBB should be entry block of the current code gen function"); PostAllocaInsertPt = AllocaInsertPt->clone(); PostAllocaInsertPt->setName("postallocapt"); - PostAllocaInsertPt->insertAfter(AllocaInsertPt); + PostAllocaInsertPt->insertAfter(AllocaInsertPt->getIterator()); } return PostAllocaInsertPt; diff --git a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h index b447942ffbd676..4c717af0e501f3 100644 --- a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h +++ b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h @@ -540,7 +540,7 @@ inline void SplitBlockAndInsertIfThenElse(Value *Cond, Instruction *SplitBefore, /// SplitBefore. Returns the first insert point in the loop body, and the /// PHINode for the induction variable (i.e. "i" above). std::pair -SplitBlockAndInsertSimpleForLoop(Value *End, Instruction *SplitBefore); +SplitBlockAndInsertSimpleForLoop(Value *End, BasicBlock::iterator SplitBefore); /// Utility function for performing a given action on each lane of a vector /// with \p EC elements. To simplify porting legacy code, this defaults to @@ -551,7 +551,7 @@ SplitBlockAndInsertSimpleForLoop(Value *End, Instruction *SplitBefore); /// given index, and a value which is (at runtime) the index to access. /// This index *may* be a constant. void SplitBlockAndInsertForEachLane(ElementCount EC, Type *IndexTy, - Instruction *InsertBefore, + BasicBlock::iterator InsertBefore, std::function Func); /// Utility function for performing a given action on each lane of a vector @@ -563,7 +563,7 @@ void SplitBlockAndInsertForEachLane(ElementCount EC, Type *IndexTy, /// the given index, and a value which is (at runtime) the index to access. This /// index *may* be a constant. void SplitBlockAndInsertForEachLane( - Value *End, Instruction *InsertBefore, + Value *End, BasicBlock::iterator InsertBefore, std::function Func); /// Check whether BB is the merge point of a if-region. diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp index 7e9d705a7bef6c..94101ccca1096a 100644 --- a/llvm/lib/CodeGen/CodeGenPrepare.cpp +++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp @@ -2935,13 +2935,13 @@ bool CodeGenPrepare::dupRetToEnableTailCallOpts(BasicBlock *BB, // Make sure there are no instructions between the first instruction // and return. - const Instruction *BI = BB->getFirstNonPHI(); + BasicBlock::const_iterator BI = BB->getFirstNonPHIIt(); // Skip over debug and the bitcast. - while (isa(BI) || BI == BCI || BI == EVI || - isa(BI) || isLifetimeEndOrBitCastFor(BI) || - isFakeUse(BI)) - BI = BI->getNextNode(); - if (BI != RetI) + while (isa(BI) || &*BI == BCI || &*BI == EVI || + isa(BI) || isLifetimeEndOrBitCastFor(&*BI) || + isFakeUse(&*BI)) + BI = std::next(BI); + if (&*BI != RetI) return false; /// Only dup the ReturnInst if the CallInst is likely to be emitted as a tail @@ -3265,8 +3265,8 @@ class TypePromotionTransaction { /// Either an instruction: /// - Is the first in a basic block: BB is used. /// - Has a previous instruction: PrevInst is used. - union { - Instruction *PrevInst; + struct { + BasicBlock::iterator PrevInst; BasicBlock *BB; } Point; std::optional BeforeDbgRecord = std::nullopt; @@ -3286,7 +3286,7 @@ class TypePromotionTransaction { BeforeDbgRecord = Inst->getDbgReinsertionPosition(); if (HasPrevInstruction) { - Point.PrevInst = &*std::prev(Inst->getIterator()); + Point.PrevInst = std::prev(Inst->getIterator()); } else { Point.BB = BB; } @@ -3297,7 +3297,7 @@ class TypePromotionTransaction { if (HasPrevInstruction) { if (Inst->getParent()) Inst->removeFromParent(); - Inst->insertAfter(&*Point.PrevInst); + Inst->insertAfter(Point.PrevInst); } else { BasicBlock::iterator Position = Point.BB->getFirstInsertionPt(); if (Inst->getParent()) @@ -3317,7 +3317,7 @@ class TypePromotionTransaction { public: /// Move \p Inst before \p Before. - InstructionMoveBefore(Instruction *Inst, Instruction *Before) + InstructionMoveBefore(Instruction *Inst, BasicBlock::iterator Before) : TypePromotionAction(Inst), Position(Inst) { LLVM_DEBUG(dbgs() << "Do: move: " << *Inst << "\nbefore: " << *Before << "\n"); diff --git a/llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp b/llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp index ce933108b83b12..27f22758d28381 100644 --- a/llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp +++ b/llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp @@ -342,7 +342,7 @@ class AlignVectors { MoveList createLoadGroups(const AddrList &Group) const; MoveList createStoreGroups(const AddrList &Group) const; bool moveTogether(MoveGroup &Move) const; - template InstMap cloneBefore(Instruction *To, T &&Insts) const; + template InstMap cloneBefore(BasicBlock::iterator To, T &&Insts) const; void realignLoadGroup(IRBuilderBase &Builder, const ByteSpan &VSpan, int ScLen, Value *AlignVal, Value *AlignAddr) const; @@ -1046,7 +1046,7 @@ auto AlignVectors::moveTogether(MoveGroup &Move) const -> bool { if (Move.IsLoad) { // Move all the loads (and dependencies) to where the first load is. // Clone all deps to before Where, keeping order. - Move.Clones = cloneBefore(Where, Move.Deps); + Move.Clones = cloneBefore(Where->getIterator(), Move.Deps); // Move all main instructions to after Where, keeping order. ArrayRef Main(Move.Main); for (Instruction *M : Main) { @@ -1067,7 +1067,7 @@ auto AlignVectors::moveTogether(MoveGroup &Move) const -> bool { // Move all main instructions to before Where, inverting order. ArrayRef Main(Move.Main); for (Instruction *M : Main.drop_front(1)) { - M->moveBefore(Where); + M->moveBefore(Where->getIterator()); Where = M; } } @@ -1076,7 +1076,7 @@ auto AlignVectors::moveTogether(MoveGroup &Move) const -> bool { } template -auto AlignVectors::cloneBefore(Instruction *To, T &&Insts) const -> InstMap { +auto AlignVectors::cloneBefore(BasicBlock::iterator To, T &&Insts) const -> InstMap { InstMap Map; for (Instruction *I : Insts) { @@ -1200,10 +1200,10 @@ auto AlignVectors::realignLoadGroup(IRBuilderBase &Builder, VSpan.section(Start, Width).values()); }; - auto moveBefore = [this](Instruction *In, Instruction *To) { + auto moveBefore = [this](BasicBlock::iterator In, BasicBlock::iterator To) { // Move In and its upward dependencies to before To. assert(In->getParent() == To->getParent()); - DepList Deps = getUpwardDeps(In, To); + DepList Deps = getUpwardDeps(&*In, &*To); In->moveBefore(To); // DepList is sorted with respect to positions in the basic block. InstMap Map = cloneBefore(In, Deps); @@ -1236,7 +1236,7 @@ auto AlignVectors::realignLoadGroup(IRBuilderBase &Builder, // in order to check legality. if (auto *Load = dyn_cast(Loads[Index])) { if (!HVC.isSafeToMoveBeforeInBB(*Load, BasePos)) - moveBefore(Load, &*BasePos); + moveBefore(Load->getIterator(), BasePos); } LLVM_DEBUG(dbgs() << "Loads[" << Index << "]:" << *Loads[Index] << '\n'); } diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp index ff5df12c398c56..da387c5276408b 100644 --- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp +++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp @@ -1221,8 +1221,8 @@ static void handleNoSuspendCoroutine(coro::Shape &Shape) { // SimplifySuspendPoint needs to check that there is no calls between // coro_save and coro_suspend, since any of the calls may potentially resume // the coroutine and if that is the case we cannot eliminate the suspend point. -static bool hasCallsInBlockBetween(Instruction *From, Instruction *To) { - for (Instruction *I = From; I != To; I = I->getNextNode()) { +static bool hasCallsInBlockBetween(iterator_range R) { + for (Instruction &I : R) { // Assume that no intrinsic can resume the coroutine. if (isa(I)) continue; @@ -1256,7 +1256,7 @@ static bool hasCallsInBlocksBetween(BasicBlock *SaveBB, BasicBlock *ResDesBB) { Set.erase(ResDesBB); for (auto *BB : Set) - if (hasCallsInBlockBetween(BB->getFirstNonPHI(), nullptr)) + if (hasCallsInBlockBetween({BB->getFirstNonPHIIt(), BB->end()})) return true; return false; @@ -1265,17 +1265,19 @@ static bool hasCallsInBlocksBetween(BasicBlock *SaveBB, BasicBlock *ResDesBB) { static bool hasCallsBetween(Instruction *Save, Instruction *ResumeOrDestroy) { auto *SaveBB = Save->getParent(); auto *ResumeOrDestroyBB = ResumeOrDestroy->getParent(); + BasicBlock::iterator SaveIt = Save->getIterator(); + BasicBlock::iterator ResumeOrDestroyIt = ResumeOrDestroy->getIterator(); if (SaveBB == ResumeOrDestroyBB) - return hasCallsInBlockBetween(Save->getNextNode(), ResumeOrDestroy); + return hasCallsInBlockBetween({std::next(SaveIt), ResumeOrDestroyIt}); // Any calls from Save to the end of the block? - if (hasCallsInBlockBetween(Save->getNextNode(), nullptr)) + if (hasCallsInBlockBetween({std::next(SaveIt), SaveBB->end()})) return true; // Any calls from begging of the block up to ResumeOrDestroy? - if (hasCallsInBlockBetween(ResumeOrDestroyBB->getFirstNonPHI(), - ResumeOrDestroy)) + if (hasCallsInBlockBetween({ResumeOrDestroyBB->getFirstNonPHIIt(), + ResumeOrDestroyIt})) return true; // Any calls in all of the blocks between SaveBB and ResumeOrDestroyBB? diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp index e5f3b7f24bca7a..0aa0cbcfc48b3a 100644 --- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -1661,7 +1661,7 @@ void AddressSanitizer::instrumentMaskedLoadOrStore( if (Stride) Stride = IB.CreateZExtOrTrunc(Stride, IntptrTy); - SplitBlockAndInsertForEachLane(EVL, LoopInsertBefore, + SplitBlockAndInsertForEachLane(EVL, LoopInsertBefore->getIterator(), [&](IRBuilderBase &IRB, Value *Index) { Value *MaskElem = IRB.CreateExtractElement(Mask, Index); if (auto *MaskElemC = dyn_cast(MaskElem)) { diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp index 56d3eb10d73e95..10a796e0ce4d41 100644 --- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp @@ -1272,7 +1272,7 @@ struct MemorySanitizerVisitor : public InstVisitor { Value *End = IRB.CreateUDiv(RoundUp, ConstantInt::get(MS.IntptrTy, kOriginSize)); auto [InsertPt, Index] = - SplitBlockAndInsertSimpleForLoop(End, &*IRB.GetInsertPoint()); + SplitBlockAndInsertSimpleForLoop(End, IRB.GetInsertPoint()); IRB.SetInsertPoint(InsertPt); Value *GEP = IRB.CreateGEP(MS.OriginTy, OriginPtr, Index); diff --git a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp index 5a9a7ecdc13bfe..f57ec3f57b23b2 100644 --- a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp +++ b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp @@ -6138,7 +6138,7 @@ void LSRInstance::ImplementSolution( if (!llvm::all_of(BO->uses(), [&](Use &U) {return DT.dominates(IVIncInsertPos, U);})) continue; - BO->moveBefore(IVIncInsertPos); + BO->moveBefore(IVIncInsertPos->getIterator()); Changed = true; } diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp index 78116770009980..91aef9846863da 100644 --- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp +++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp @@ -1729,7 +1729,7 @@ void llvm::SplitBlockAndInsertIfThenElse( } std::pair -llvm::SplitBlockAndInsertSimpleForLoop(Value *End, Instruction *SplitBefore) { +llvm::SplitBlockAndInsertSimpleForLoop(Value *End, BasicBlock::iterator SplitBefore) { BasicBlock *LoopPred = SplitBefore->getParent(); BasicBlock *LoopBody = SplitBlock(SplitBefore->getParent(), SplitBefore); BasicBlock *LoopExit = SplitBlock(SplitBefore->getParent(), SplitBefore); @@ -1752,14 +1752,14 @@ llvm::SplitBlockAndInsertSimpleForLoop(Value *End, Instruction *SplitBefore) { IV->addIncoming(ConstantInt::get(Ty, 0), LoopPred); IV->addIncoming(IVNext, LoopBody); - return std::make_pair(LoopBody->getFirstNonPHI(), IV); + return std::make_pair(&*LoopBody->getFirstNonPHIIt(), IV); } void llvm::SplitBlockAndInsertForEachLane(ElementCount EC, - Type *IndexTy, Instruction *InsertBefore, + Type *IndexTy, BasicBlock::iterator InsertBefore, std::function Func) { - IRBuilder<> IRB(InsertBefore); + IRBuilder<> IRB(InsertBefore->getParent(), InsertBefore); if (EC.isScalable()) { Value *NumElements = IRB.CreateElementCount(IndexTy, EC); @@ -1780,10 +1780,10 @@ void llvm::SplitBlockAndInsertForEachLane(ElementCount EC, } void llvm::SplitBlockAndInsertForEachLane( - Value *EVL, Instruction *InsertBefore, + Value *EVL, BasicBlock::iterator InsertBefore, std::function Func) { - IRBuilder<> IRB(InsertBefore); + IRBuilder<> IRB(InsertBefore->getParent(), InsertBefore); Type *Ty = EVL->getType(); if (!isa(EVL)) { diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp index 6a8a468ebcae2b..adc40da07d9670 100644 --- a/llvm/lib/Transforms/Utils/InlineFunction.cpp +++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp @@ -184,14 +184,14 @@ namespace { } // end anonymous namespace static IntrinsicInst *getConvergenceEntry(BasicBlock &BB) { - auto *I = BB.getFirstNonPHI(); - while (I) { - if (auto *IntrinsicCall = dyn_cast(I)) { + BasicBlock::iterator It = BB.getFirstNonPHIIt(); + while (It != BB.end()) { + if (auto *IntrinsicCall = dyn_cast(It)) { if (IntrinsicCall->isEntry()) { return IntrinsicCall; } } - I = I->getNextNode(); + It = std::next(It); } return nullptr; } From a589eb12e56c1b5fefb382c19bcfded38c13e6d5 Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Mon, 27 Jan 2025 15:21:31 +0000 Subject: [PATCH 2/2] Fix one or two clang-format things --- llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h | 6 +++--- llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp | 6 ++++-- llvm/lib/Transforms/Coroutines/CoroSplit.cpp | 4 ++-- llvm/lib/Transforms/Utils/BasicBlockUtils.cpp | 11 ++++++----- 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h index 4c717af0e501f3..6faff3d1fd8e3d 100644 --- a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h +++ b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h @@ -550,9 +550,9 @@ SplitBlockAndInsertSimpleForLoop(Value *End, BasicBlock::iterator SplitBefore); /// IRBuilder whose insert point is correctly set for instantiating the /// given index, and a value which is (at runtime) the index to access. /// This index *may* be a constant. -void SplitBlockAndInsertForEachLane(ElementCount EC, Type *IndexTy, - BasicBlock::iterator InsertBefore, - std::function Func); +void SplitBlockAndInsertForEachLane( + ElementCount EC, Type *IndexTy, BasicBlock::iterator InsertBefore, + std::function Func); /// Utility function for performing a given action on each lane of a vector /// with \p EVL effective length. EVL is assumed > 0. To simplify porting legacy diff --git a/llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp b/llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp index 27f22758d28381..62b2839295d969 100644 --- a/llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp +++ b/llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp @@ -342,7 +342,8 @@ class AlignVectors { MoveList createLoadGroups(const AddrList &Group) const; MoveList createStoreGroups(const AddrList &Group) const; bool moveTogether(MoveGroup &Move) const; - template InstMap cloneBefore(BasicBlock::iterator To, T &&Insts) const; + template + InstMap cloneBefore(BasicBlock::iterator To, T &&Insts) const; void realignLoadGroup(IRBuilderBase &Builder, const ByteSpan &VSpan, int ScLen, Value *AlignVal, Value *AlignAddr) const; @@ -1076,7 +1077,8 @@ auto AlignVectors::moveTogether(MoveGroup &Move) const -> bool { } template -auto AlignVectors::cloneBefore(BasicBlock::iterator To, T &&Insts) const -> InstMap { +auto AlignVectors::cloneBefore(BasicBlock::iterator To, T &&Insts) const + -> InstMap { InstMap Map; for (Instruction *I : Insts) { diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp index da387c5276408b..2630ae19dabaa2 100644 --- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp +++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp @@ -1276,8 +1276,8 @@ static bool hasCallsBetween(Instruction *Save, Instruction *ResumeOrDestroy) { return true; // Any calls from begging of the block up to ResumeOrDestroy? - if (hasCallsInBlockBetween({ResumeOrDestroyBB->getFirstNonPHIIt(), - ResumeOrDestroyIt})) + if (hasCallsInBlockBetween( + {ResumeOrDestroyBB->getFirstNonPHIIt(), ResumeOrDestroyIt})) return true; // Any calls in all of the blocks between SaveBB and ResumeOrDestroyBB? diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp index 91aef9846863da..5d191c27b55056 100644 --- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp +++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp @@ -1728,8 +1728,9 @@ void llvm::SplitBlockAndInsertIfThenElse( } } -std::pair -llvm::SplitBlockAndInsertSimpleForLoop(Value *End, BasicBlock::iterator SplitBefore) { +std::pair +llvm::SplitBlockAndInsertSimpleForLoop(Value *End, + BasicBlock::iterator SplitBefore) { BasicBlock *LoopPred = SplitBefore->getParent(); BasicBlock *LoopBody = SplitBlock(SplitBefore->getParent(), SplitBefore); BasicBlock *LoopExit = SplitBlock(SplitBefore->getParent(), SplitBefore); @@ -1755,9 +1756,9 @@ llvm::SplitBlockAndInsertSimpleForLoop(Value *End, BasicBlock::iterator SplitBef return std::make_pair(&*LoopBody->getFirstNonPHIIt(), IV); } -void llvm::SplitBlockAndInsertForEachLane(ElementCount EC, - Type *IndexTy, BasicBlock::iterator InsertBefore, - std::function Func) { +void llvm::SplitBlockAndInsertForEachLane( + ElementCount EC, Type *IndexTy, BasicBlock::iterator InsertBefore, + std::function Func) { IRBuilder<> IRB(InsertBefore->getParent(), InsertBefore);