llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-compiler-rt-sanitizer @llvm/pr-subscribers-backend-hexagon Author: Jeremy Morse (jmorse) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/124291.diff 10 Files Affected: - (modified) clang/lib/CodeGen/CodeGenFunction.h (+1-1) - (modified) llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h (+3-3) - (modified) llvm/lib/CodeGen/CodeGenPrepare.cpp (+11-11) - (modified) llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp (+7-7) - (modified) llvm/lib/Transforms/Coroutines/CoroSplit.cpp (+9-7) - (modified) llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp (+1-1) - (modified) llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp (+1-1) - (modified) llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp (+1-1) - (modified) llvm/lib/Transforms/Utils/BasicBlockUtils.cpp (+6-6) - (modified) llvm/lib/Transforms/Utils/InlineFunction.cpp (+4-4) ``````````diff 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<Instruction*, Value*> -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<void(IRBuilderBase&, Value*)> 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<void(IRBuilderBase &, Value *)> 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<DbgInfoIntrinsic>(BI) || BI == BCI || BI == EVI || - isa<PseudoProbeInst>(BI) || isLifetimeEndOrBitCastFor(BI) || - isFakeUse(BI)) - BI = BI->getNextNode(); - if (BI != RetI) + while (isa<DbgInfoIntrinsic>(BI) || &*BI == BCI || &*BI == EVI || + isa<PseudoProbeInst>(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<DbgRecord::self_iterator> 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 <typename T> InstMap cloneBefore(Instruction *To, T &&Insts) const; + template <typename T> 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<Instruction *> 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<Instruction *> 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 <typename T> -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<Instruction>(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<BasicBlock::iterator> R) { + for (Instruction &I : R) { // Assume that no intrinsic can resume the coroutine. if (isa<IntrinsicInst>(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<ConstantInt>(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<MemorySanitizerVisitor> { 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<Instruction*, Value*> -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<void(IRBuilderBase&, Value*)> 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<void(IRBuilderBase &, Value *)> Func) { - IRBuilder<> IRB(InsertBefore); + IRBuilder<> IRB(InsertBefore->getParent(), InsertBefore); Type *Ty = EVL->getType(); if (!isa<ConstantInt>(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<ConvergenceControlInst>(I)) { + BasicBlock::iterator It = BB.getFirstNonPHIIt(); + while (It != BB.end()) { + if (auto *IntrinsicCall = dyn_cast<ConvergenceControlInst>(It)) { if (IntrinsicCall->isEntry()) { return IntrinsicCall; } } - I = I->getNextNode(); + It = std::next(It); } return nullptr; } `````````` </details> https://github.com/llvm/llvm-project/pull/124291 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits