[clang] [llvm] [NFC][DebugInfo] Use iterators for instruction insertion in more places (PR #124291)
Jeremy Morse via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 24 07:27:07 PST 2025
https://github.com/jmorse created https://github.com/llvm/llvm-project/pull/124291
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.
>From 8bafca72573c7ba5e7b7711c5168e0609571661f Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Thu, 23 Jan 2025 14:18:55 +0000
Subject: [PATCH] [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<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;
}
More information about the llvm-commits
mailing list