[NFC][DebugInfo] Switch more call-sites to using iterator-insertion #124283
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
To finalise the "RemoveDIs" work removing debug intrinsics, we're updating call sites that insert instructions to use iterators instead. This set of changes are those where it's not immediately obvious that just calling getIterator to fetch an iterator is correct, and one or two places where more than one line needs to change. Overall the same rule holds though: iterators generated for the start of a block such as getFirstNonPHIIt need to be passed into insert/move methods without being unwrapped/rewrapped, everything else can use getIterator.
llvmbot
added
backend:Hexagon
vectorizers
coroutines
|
@llvm/pr-subscribers-backend-hexagon @llvm/pr-subscribers-coroutines Author: Jeremy Morse (jmorse) ChangesTo finalise the "RemoveDIs" work removing debug intrinsics, we're updating call sites that insert instructions to use iterators instead. This set of changes are those where it's not immediately obvious that just calling getIterator to fetch an iterator is correct, and one or two places where more than one line needs to change. Overall the same rule holds though: iterators generated for the start of a block such as getFirstNonPHIIt need to be passed into insert/move methods without being unwrapped/rewrapped, everything else can use getIterator. Patch is 22.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/124283.diff 16 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h b/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h
index 3aa30bec85c3a5..fbc76219ead867 100644
--- a/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h
+++ b/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h
@@ -170,7 +170,7 @@ class CoroIdInst : public AnyCoroIdInst {
Inst->eraseFromParent();
return;
}
- Inst->moveBefore(getCoroBegin()->getNextNode());
+ Inst->moveBefore(std::next(getCoroBegin()->getIterator()));
}
// Info argument of coro.id is
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 777391327f77c6..9548466f7fc0eb 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -678,7 +678,7 @@ void OpenMPIRBuilder::initialize() { initializeTypes(M); }
static void raiseUserConstantDataAllocasToEntryBlock(IRBuilderBase &Builder,
Function *Function) {
BasicBlock &EntryBlock = Function->getEntryBlock();
- Instruction *MoveLocInst = EntryBlock.getFirstNonPHI();
+ BasicBlock::iterator MoveLocInst = EntryBlock.getFirstNonPHIIt();
// Loop over blocks looking for constant allocas, skipping the entry block
// as any allocas there are already in the desired location.
@@ -6916,7 +6916,7 @@ static Expected<Function *> createOutlinedFunction(
Builder.CreateRetVoid();
// New Alloca IP at entry point of created device function.
- Builder.SetInsertPoint(EntryBB->getFirstNonPHI());
+ Builder.SetInsertPoint(EntryBB->getFirstNonPHIIt());
auto AllocaIP = Builder.saveIP();
Builder.SetInsertPoint(UserCodeEntryBB->getFirstNonPHIOrDbg());
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index 0efc04cb2c8679..aeeaafe3f3f08f 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -543,7 +543,7 @@ void BasicBlock::removePredecessor(BasicBlock *Pred,
}
bool BasicBlock::canSplitPredecessors() const {
- const Instruction *FirstNonPHI = getFirstNonPHI();
+ const_iterator FirstNonPHI = getFirstNonPHIIt();
if (isa<LandingPadInst>(FirstNonPHI))
return true;
// This is perhaps a little conservative because constructs like
@@ -675,11 +675,11 @@ void BasicBlock::replaceSuccessorsPhiUsesWith(BasicBlock *New) {
}
bool BasicBlock::isLandingPad() const {
- return isa<LandingPadInst>(getFirstNonPHI());
+ return isa<LandingPadInst>(getFirstNonPHIIt());
}
const LandingPadInst *BasicBlock::getLandingPadInst() const {
- return dyn_cast<LandingPadInst>(getFirstNonPHI());
+ return dyn_cast<LandingPadInst>(getFirstNonPHIIt());
}
std::optional<uint64_t> BasicBlock::getIrrLoopHeaderWeight() const {
diff --git a/llvm/lib/Target/Hexagon/HexagonOptimizeSZextends.cpp b/llvm/lib/Target/Hexagon/HexagonOptimizeSZextends.cpp
index bfd02802b78299..c29cf034ce0896 100644
--- a/llvm/lib/Target/Hexagon/HexagonOptimizeSZextends.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonOptimizeSZextends.cpp
@@ -81,7 +81,7 @@ bool HexagonOptimizeSZextends::runOnFunction(Function &F) {
assert (EVT::getEVT(SI->getType()) ==
(EVT::getEVT(Use->getType())));
Use->replaceAllUsesWith(SI);
- Instruction* First = &F.getEntryBlock().front();
+ BasicBlock::iterator First = F.getEntryBlock().begin();
SI->insertBefore(First);
Use->eraseFromParent();
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index e5f3b7f24bca7a..bad55849704f82 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3414,7 +3414,7 @@ void FunctionStackPoisoner::processStaticAllocas() {
assert(InsBeforeB == &F.getEntryBlock());
for (auto *AI : StaticAllocasToMoveUp)
if (AI->getParent() == InsBeforeB)
- AI->moveBefore(InsBefore);
+ AI->moveBefore(InsBefore->getIterator());
// Move stores of arguments into entry-block allocas as well. This prevents
// extra stack slots from being generated (to house the argument values until
@@ -3423,10 +3423,10 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector<Instruction *, 8> ArgInitInsts;
findStoresToUninstrumentedArgAllocas(ASan, *InsBefore, ArgInitInsts);
for (Instruction *ArgInitInst : ArgInitInsts)
- ArgInitInst->moveBefore(InsBefore);
+ ArgInitInst->moveBefore(InsBefore->getIterator());
// If we have a call to llvm.localescape, keep it in the entry block.
- if (LocalEscapeCall) LocalEscapeCall->moveBefore(InsBefore);
+ if (LocalEscapeCall) LocalEscapeCall->moveBefore(InsBefore->getIterator());
SmallVector<ASanStackVariableDescription, 16> SVD;
SVD.reserve(AllocaVec.size());
diff --git a/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp b/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
index bbc7a005b9ff4f..33c254900943bb 100644
--- a/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
+++ b/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
@@ -218,7 +218,7 @@ static bool canSplitCallSite(CallBase &CB, TargetTransformInfo &TTI) {
return true;
}
-static Instruction *cloneInstForMustTail(Instruction *I, Instruction *Before,
+static Instruction *cloneInstForMustTail(Instruction *I, BasicBlock::iterator Before,
Value *V) {
Instruction *Copy = I->clone();
Copy->setName(I->getName());
@@ -251,8 +251,8 @@ static void copyMustTailReturn(BasicBlock *SplitBB, Instruction *CI,
Instruction *TI = SplitBB->getTerminator();
Value *V = NewCI;
if (BCI)
- V = cloneInstForMustTail(BCI, TI, V);
- cloneInstForMustTail(RI, TI, IsVoid ? nullptr : V);
+ V = cloneInstForMustTail(BCI, TI->getIterator(), V);
+ cloneInstForMustTail(RI, TI->getIterator(), IsVoid ? nullptr : V);
// FIXME: remove TI here, `DuplicateInstructionsInSplitBetween` has a bug
// that prevents doing this now.
diff --git a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
index 5a9a7ecdc13bfe..b0ae19867d1f09 100644
--- a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -6176,11 +6176,11 @@ LSRInstance::LSRInstance(Loop *L, IVUsers &IU, ScalarEvolution &SE,
// CatchSwitchInst. Because the CatchSwitchInst cannot be split, there is
// no good place to stick any instructions.
if (auto *PN = dyn_cast<PHINode>(U.getUser())) {
- auto *FirstNonPHI = PN->getParent()->getFirstNonPHI();
+ auto FirstNonPHI = PN->getParent()->getFirstNonPHIIt();
if (isa<FuncletPadInst>(FirstNonPHI) ||
isa<CatchSwitchInst>(FirstNonPHI))
for (BasicBlock *PredBB : PN->blocks())
- if (isa<CatchSwitchInst>(PredBB->getFirstNonPHI()))
+ if (isa<CatchSwitchInst>(PredBB->getFirstNonPHIIt()))
return;
}
}
diff --git a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
index cc67a455672be4..60fbb689c33f3a 100644
--- a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
+++ b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
@@ -281,7 +281,7 @@ void MergedLoadStoreMotion::sinkStoresAndGEPs(BasicBlock *BB, StoreInst *S0,
auto *GEP0 = cast<GetElementPtrInst>(Ptr0);
auto *GEP1 = cast<GetElementPtrInst>(Ptr1);
Instruction *GEPNew = GEP0->clone();
- GEPNew->insertBefore(SNew);
+ GEPNew->insertBefore(SNew->getIterator());
GEPNew->applyMergedLocation(GEP0->getDebugLoc(), GEP1->getDebugLoc());
SNew->setOperand(1, GEPNew);
GEP0->replaceAllUsesWith(GEPNew);
diff --git a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
index 7b848ae547bd55..8a2491cd314bcb 100644
--- a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -1371,7 +1371,7 @@ static void recomputeLiveInValues(
// and inserts them before "InsertBefore". Returns rematerialized value
// which should be used after statepoint.
static Instruction *rematerializeChain(ArrayRef<Instruction *> ChainToBase,
- Instruction *InsertBefore,
+ BasicBlock::iterator InsertBefore,
Value *RootOfChain,
Value *AlternateLiveBase) {
Instruction *LastClonedValue = nullptr;
@@ -2185,16 +2185,16 @@ static void relocationViaAlloca(
// InvokeInst is a terminator so the store need to be inserted into its
// normal destination block.
BasicBlock *NormalDest = Invoke->getNormalDest();
- Store->insertBefore(NormalDest->getFirstNonPHI());
+ Store->insertBefore(NormalDest->getFirstNonPHIIt());
} else {
assert(!Inst->isTerminator() &&
"The only terminator that can produce a value is "
"InvokeInst which is handled above.");
- Store->insertAfter(Inst);
+ Store->insertAfter(Inst->getIterator());
}
} else {
assert(isa<Argument>(Def));
- Store->insertAfter(cast<Instruction>(Alloca));
+ Store->insertAfter(cast<Instruction>(Alloca)->getIterator());
}
}
@@ -2500,7 +2500,7 @@ static void rematerializeLiveValuesAtUses(
while (!Cand->user_empty()) {
Instruction *UserI = cast<Instruction>(*Cand->user_begin());
Instruction *RematChain = rematerializeChain(
- Record.ChainToBase, UserI, Record.RootOfChain, PointerToBase[Cand]);
+ Record.ChainToBase, UserI->getIterator(), Record.RootOfChain, PointerToBase[Cand]);
UserI->replaceUsesOfWith(Cand, RematChain);
PointerToBase[RematChain] = PointerToBase[Cand];
}
@@ -2573,16 +2573,16 @@ static void rematerializeLiveValues(CallBase *Call,
Instruction *InsertBefore = Call->getNextNode();
assert(InsertBefore);
Instruction *RematerializedValue =
- rematerializeChain(Record.ChainToBase, InsertBefore,
+ rematerializeChain(Record.ChainToBase, InsertBefore->getIterator(),
Record.RootOfChain, PointerToBase[LiveValue]);
Info.RematerializedValues[RematerializedValue] = LiveValue;
} else {
auto *Invoke = cast<InvokeInst>(Call);
- Instruction *NormalInsertBefore =
- &*Invoke->getNormalDest()->getFirstInsertionPt();
- Instruction *UnwindInsertBefore =
- &*Invoke->getUnwindDest()->getFirstInsertionPt();
+ BasicBlock::iterator NormalInsertBefore =
+ Invoke->getNormalDest()->getFirstInsertionPt();
+ BasicBlock::iterator UnwindInsertBefore =
+ Invoke->getUnwindDest()->getFirstInsertionPt();
Instruction *NormalRematerializedValue =
rematerializeChain(Record.ChainToBase, NormalInsertBefore,
@@ -3131,7 +3131,7 @@ bool RewriteStatepointsForGC::runOnFunction(Function &F, DominatorTree &DT,
// most instructions without side effects or memory access.
if (isa<ICmpInst>(Cond) && Cond->hasOneUse()) {
MadeChange = true;
- Cond->moveBefore(TI);
+ Cond->moveBefore(TI->getIterator());
}
}
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index f6179cadab4254..29240aaaa21be9 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -480,7 +480,7 @@ static void migrateDebugInfo(AllocaInst *OldAlloca, bool IsSplit,
// noted as slightly offset (in code) from the store. In practice this
// should have little effect on the debugging experience due to the fact
// that all the split stores should get the same line number.
- NewAssign->moveBefore(DbgAssign);
+ NewAssign->moveBefore(DbgAssign->getIterator());
NewAssign->setDebugLoc(DbgAssign->getDebugLoc());
LLVM_DEBUG(dbgs() << "Created new assign: " << *NewAssign << "\n");
@@ -1843,7 +1843,7 @@ static void rewriteMemOpOfSelect(SelectInst &SI, T &I,
CondMemOp.dropUBImplyingAttrsAndMetadata();
++NumLoadsSpeculated;
}
- CondMemOp.insertBefore(NewMemOpBB->getTerminator());
+ CondMemOp.insertBefore(NewMemOpBB->getTerminator()->getIterator());
Value *Ptr = SI.getOperand(1 + SuccIdx);
CondMemOp.setOperand(I.getPointerOperandIndex(), Ptr);
if (isa<LoadInst>(I)) {
diff --git a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
index c2f7c5dcaf1603..c5f08b084c4bf6 100644
--- a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
+++ b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
@@ -3303,8 +3303,8 @@ static bool isSafeForNoNTrivialUnswitching(Loop &L, LoopInfo &LI) {
// FIXME: We should teach SplitBlock to handle this and remove this
// restriction.
for (auto *ExitBB : ExitBlocks) {
- auto *I = ExitBB->getFirstNonPHI();
- if (isa<CleanupPadInst>(I) || isa<CatchSwitchInst>(I)) {
+ auto It = ExitBB->getFirstNonPHIIt();
+ if (isa<CleanupPadInst>(It) || isa<CatchSwitchInst>(It)) {
LLVM_DEBUG(dbgs() << "Cannot unswitch because of cleanuppad/catchswitch "
"in exit block\n");
return false;
diff --git a/llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp b/llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp
index b05ae00a1e0ea0..2d9a3d1f8a110a 100644
--- a/llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp
+++ b/llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp
@@ -474,9 +474,9 @@ struct AssumeSimplify {
AssumeBuilderState Builder(F.getParent());
/// For now it is initialized to the best value it could have
- Instruction *InsertPt = BB->getFirstNonPHI();
+ BasicBlock::iterator InsertPt = BB->getFirstNonPHIIt();
if (isa<LandingPadInst>(InsertPt))
- InsertPt = InsertPt->getNextNode();
+ InsertPt = std::next(InsertPt);
for (IntrinsicInst *I : make_range(Begin, End)) {
CleanupToDo.insert(I);
for (CallInst::BundleOpInfo &BOI : I->bundle_op_infos()) {
@@ -487,8 +487,8 @@ struct AssumeSimplify {
Builder.addKnowledge(RK);
if (auto *I = dyn_cast_or_null<Instruction>(RK.WasOn))
if (I->getParent() == InsertPt->getParent() &&
- (InsertPt->comesBefore(I) || InsertPt == I))
- InsertPt = I->getNextNode();
+ (InsertPt->comesBefore(I) || &*InsertPt == I))
+ InsertPt = I->getNextNode()->getIterator();
}
}
@@ -498,7 +498,7 @@ struct AssumeSimplify {
for (auto It = (*Begin)->getIterator(), E = InsertPt->getIterator();
It != E; --It)
if (!isGuaranteedToTransferExecutionToSuccessor(&*It)) {
- InsertPt = It->getNextNode();
+ InsertPt = std::next(It);
break;
}
auto *MergedAssume = Builder.build();
diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
index 78116770009980..7a55ac3e36fd83 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -836,7 +836,7 @@ BasicBlock *llvm::ehAwareSplitEdge(BasicBlock *BB, BasicBlock *Succ,
const CriticalEdgeSplittingOptions &Options,
const Twine &BBName) {
- auto *PadInst = Succ->getFirstNonPHI();
+ auto PadInst = Succ->getFirstNonPHIIt();
if (!LandingPadReplacement && !PadInst->isEHPad())
return SplitEdge(BB, Succ, Options.DT, Options.LI, Options.MSSAU, BBName);
@@ -981,7 +981,7 @@ BasicBlock *llvm::ehAwareSplitEdge(BasicBlock *BB, BasicBlock *Succ,
void llvm::createPHIsForSplitLoopExit(ArrayRef<BasicBlock *> Preds,
BasicBlock *SplitBB, BasicBlock *DestBB) {
// SplitBB shouldn't have anything non-trivial in it yet.
- assert((SplitBB->getFirstNonPHI() == SplitBB->getTerminator() ||
+ assert((&*SplitBB->getFirstNonPHIIt() == SplitBB->getTerminator() ||
SplitBB->isLandingPad()) &&
"SplitBB has non-PHI nodes!");
@@ -1450,7 +1450,7 @@ static void SplitLandingPadPredecessorsImpl(
// The new block unconditionally branches to the old block.
BranchInst *BI1 = BranchInst::Create(OrigBB, NewBB1);
- BI1->setDebugLoc(OrigBB->getFirstNonPHI()->getDebugLoc());
+ BI1->setDebugLoc(OrigBB->getFirstNonPHIIt()->getDebugLoc());
// Move the edges from Preds to point to NewBB1 instead of OrigBB.
for (BasicBlock *Pred : Preds) {
@@ -1491,7 +1491,7 @@ static void SplitLandingPadPredecessorsImpl(
// The new block unconditionally branches to the old block.
BranchInst *BI2 = BranchInst::Create(OrigBB, NewBB2);
- BI2->setDebugLoc(OrigBB->getFirstNonPHI()->getDebugLoc());
+ BI2->setDebugLoc(OrigBB->getFirstNonPHIIt()->getDebugLoc());
// Move the remaining edges from OrigBB to point to NewBB2.
for (BasicBlock *NewBB2Pred : NewBB2Preds)
diff --git a/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp b/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp
index 49209e33f2d1dd..e630ce8f7ac025 100644
--- a/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp
@@ -165,7 +165,7 @@ static bool processHeaderPhiOperands(BasicBlock *Header, BasicBlock *Latch,
// Move the phi operands of Header from Latch out of AftBlocks to InsertLoc.
static void moveHeaderPhiOperandsToForeBlocks(BasicBlock *Header,
BasicBlock *Latch,
- Instruction *InsertLoc,
+ BasicBlock::iterator InsertLoc,
BasicBlockSet &AftBlocks) {
// We need to ensure we move the instructions in the correct order,
// starting with the earliest required instruction and moving forward.
@@ -329,7 +329,7 @@ llvm::UnrollAndJamLoop(Loop *L, unsigned Count, unsigned TripCount,
// Move any instructions from fore phi operands from AftBlocks into Fore.
moveHeaderPhiOperandsToForeBlocks(
- Header, LatchBlock, ForeBlocksLast[0]->getTerminator(), AftBlocks);
+ Header, LatchBlock, ForeBlocksLast[0]->getTerminator()->getIterator(), AftBlocks);
// The current on-the-fly SSA update requires blocks to be processed in
// reverse postorder so that LastValueMap contains the correct value at each
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index cf3c2b360d0905..634dee1a8e13ec 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -5262,8 +5262,8 @@ bool SimplifyCFGOpt::simplifyBranchOnICmpChain(BranchInst *BI,
bool SimplifyCFGOpt::simplifyResume(ResumeInst *RI, IRBuilder<> &Builder) {
if (isa<PHINode>(RI->getValue()))
return simplifyCommonResume(RI);
- else if (isa<LandingPadInst>(RI->getParent()->getFirstNonPHI()) &&
- RI->getValue() == RI->getParent()->getFirstNonPHI())
+ else if (isa<LandingPadInst>(RI->getParent()->getFirstNonPHIIt()) &&
+ RI->getValue() == &*RI->getParent()->getFirstNonPHIIt())
// The resume must unwind the exception that caused control to branch here.
return simplifySingleResume(RI);
@@ -5298,7 +5298,7 @@ bool SimplifyCFGOpt::simplifyCommonResume(ResumeInst *RI) {
// Check that there are no other instructions except for debug and lifetime
// intrinsics between the phi's and resume instruction.
if (!isCleanupBlockEmpty(
- make_range(RI->getParent()->getFirstNonPHI(), BB->getTerminator())))
+ make_range(RI->getParent()->getFirstNonPHIIt(), BB->getTerminator()->getIterator())))
return false;
SmallSetVector<BasicBlock *, 4> TrivialUnwindBlocks;
@@ -5315,7 +5315,7 @@ bool SimplifyCFGOpt::simplifyCommonResume(ResumeInst *RI) {
if (IncomingBB->getUniqueSuccessor() != BB)
continue;
- auto *LandingPad = dyn_cast<LandingPadInst>(IncomingBB->getFirstNonPHI());
+ auto *LandingPad = dyn_cast<LandingPadInst>(IncomingBB->getFirstNonPHIIt());
// Not the landing pad that caused the control to branch here.
if (IncomingValue != LandingPad)
continue;
@@ -5364,7 +5364,7 @@ bool SimplifyCFGOpt::simplifyCommonResume(...
[truncated]
|
|
@llvm/pr-subscribers-llvm-transforms Author: Jeremy Morse (jmorse) ChangesTo finalise the "RemoveDIs" work removing debug intrinsics, we're updating call sites that insert instructions to use iterators instead. This set of changes are those where it's not immediately obvious that just calling getIterator to fetch an iterator is correct, and one or two places where more than one line needs to change. Overall the same rule holds though: iterators generated for the start of a block such as getFirstNonPHIIt need to be passed into insert/move methods without being unwrapped/rewrapped, everything else can use getIterator. Patch is 22.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/124283.diff 16 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h b/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h
index 3aa30bec85c3a5..fbc76219ead867 100644
--- a/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h
+++ b/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h
@@ -170,7 +170,7 @@ class CoroIdInst : public AnyCoroIdInst {
Inst->eraseFromParent();
return;
}
- Inst->moveBefore(getCoroBegin()->getNextNode());
+ Inst->moveBefore(std::next(getCoroBegin()->getIterator()));
}
// Info argument of coro.id is
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 777391327f77c6..9548466f7fc0eb 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -678,7 +678,7 @@ void OpenMPIRBuilder::initialize() { initializeTypes(M); }
static void raiseUserConstantDataAllocasToEntryBlock(IRBuilderBase &Builder,
Function *Function) {
BasicBlock &EntryBlock = Function->getEntryBlock();
- Instruction *MoveLocInst = EntryBlock.getFirstNonPHI();
+ BasicBlock::iterator MoveLocInst = EntryBlock.getFirstNonPHIIt();
// Loop over blocks looking for constant allocas, skipping the entry block
// as any allocas there are already in the desired location.
@@ -6916,7 +6916,7 @@ static Expected<Function *> createOutlinedFunction(
Builder.CreateRetVoid();
// New Alloca IP at entry point of created device function.
- Builder.SetInsertPoint(EntryBB->getFirstNonPHI());
+ Builder.SetInsertPoint(EntryBB->getFirstNonPHIIt());
auto AllocaIP = Builder.saveIP();
Builder.SetInsertPoint(UserCodeEntryBB->getFirstNonPHIOrDbg());
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index 0efc04cb2c8679..aeeaafe3f3f08f 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -543,7 +543,7 @@ void BasicBlock::removePredecessor(BasicBlock *Pred,
}
bool BasicBlock::canSplitPredecessors() const {
- const Instruction *FirstNonPHI = getFirstNonPHI();
+ const_iterator FirstNonPHI = getFirstNonPHIIt();
if (isa<LandingPadInst>(FirstNonPHI))
return true;
// This is perhaps a little conservative because constructs like
@@ -675,11 +675,11 @@ void BasicBlock::replaceSuccessorsPhiUsesWith(BasicBlock *New) {
}
bool BasicBlock::isLandingPad() const {
- return isa<LandingPadInst>(getFirstNonPHI());
+ return isa<LandingPadInst>(getFirstNonPHIIt());
}
const LandingPadInst *BasicBlock::getLandingPadInst() const {
- return dyn_cast<LandingPadInst>(getFirstNonPHI());
+ return dyn_cast<LandingPadInst>(getFirstNonPHIIt());
}
std::optional<uint64_t> BasicBlock::getIrrLoopHeaderWeight() const {
diff --git a/llvm/lib/Target/Hexagon/HexagonOptimizeSZextends.cpp b/llvm/lib/Target/Hexagon/HexagonOptimizeSZextends.cpp
index bfd02802b78299..c29cf034ce0896 100644
--- a/llvm/lib/Target/Hexagon/HexagonOptimizeSZextends.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonOptimizeSZextends.cpp
@@ -81,7 +81,7 @@ bool HexagonOptimizeSZextends::runOnFunction(Function &F) {
assert (EVT::getEVT(SI->getType()) ==
(EVT::getEVT(Use->getType())));
Use->replaceAllUsesWith(SI);
- Instruction* First = &F.getEntryBlock().front();
+ BasicBlock::iterator First = F.getEntryBlock().begin();
SI->insertBefore(First);
Use->eraseFromParent();
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index e5f3b7f24bca7a..bad55849704f82 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3414,7 +3414,7 @@ void FunctionStackPoisoner::processStaticAllocas() {
assert(InsBeforeB == &F.getEntryBlock());
for (auto *AI : StaticAllocasToMoveUp)
if (AI->getParent() == InsBeforeB)
- AI->moveBefore(InsBefore);
+ AI->moveBefore(InsBefore->getIterator());
// Move stores of arguments into entry-block allocas as well. This prevents
// extra stack slots from being generated (to house the argument values until
@@ -3423,10 +3423,10 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector<Instruction *, 8> ArgInitInsts;
findStoresToUninstrumentedArgAllocas(ASan, *InsBefore, ArgInitInsts);
for (Instruction *ArgInitInst : ArgInitInsts)
- ArgInitInst->moveBefore(InsBefore);
+ ArgInitInst->moveBefore(InsBefore->getIterator());
// If we have a call to llvm.localescape, keep it in the entry block.
- if (LocalEscapeCall) LocalEscapeCall->moveBefore(InsBefore);
+ if (LocalEscapeCall) LocalEscapeCall->moveBefore(InsBefore->getIterator());
SmallVector<ASanStackVariableDescription, 16> SVD;
SVD.reserve(AllocaVec.size());
diff --git a/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp b/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
index bbc7a005b9ff4f..33c254900943bb 100644
--- a/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
+++ b/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
@@ -218,7 +218,7 @@ static bool canSplitCallSite(CallBase &CB, TargetTransformInfo &TTI) {
return true;
}
-static Instruction *cloneInstForMustTail(Instruction *I, Instruction *Before,
+static Instruction *cloneInstForMustTail(Instruction *I, BasicBlock::iterator Before,
Value *V) {
Instruction *Copy = I->clone();
Copy->setName(I->getName());
@@ -251,8 +251,8 @@ static void copyMustTailReturn(BasicBlock *SplitBB, Instruction *CI,
Instruction *TI = SplitBB->getTerminator();
Value *V = NewCI;
if (BCI)
- V = cloneInstForMustTail(BCI, TI, V);
- cloneInstForMustTail(RI, TI, IsVoid ? nullptr : V);
+ V = cloneInstForMustTail(BCI, TI->getIterator(), V);
+ cloneInstForMustTail(RI, TI->getIterator(), IsVoid ? nullptr : V);
// FIXME: remove TI here, `DuplicateInstructionsInSplitBetween` has a bug
// that prevents doing this now.
diff --git a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
index 5a9a7ecdc13bfe..b0ae19867d1f09 100644
--- a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -6176,11 +6176,11 @@ LSRInstance::LSRInstance(Loop *L, IVUsers &IU, ScalarEvolution &SE,
// CatchSwitchInst. Because the CatchSwitchInst cannot be split, there is
// no good place to stick any instructions.
if (auto *PN = dyn_cast<PHINode>(U.getUser())) {
- auto *FirstNonPHI = PN->getParent()->getFirstNonPHI();
+ auto FirstNonPHI = PN->getParent()->getFirstNonPHIIt();
if (isa<FuncletPadInst>(FirstNonPHI) ||
isa<CatchSwitchInst>(FirstNonPHI))
for (BasicBlock *PredBB : PN->blocks())
- if (isa<CatchSwitchInst>(PredBB->getFirstNonPHI()))
+ if (isa<CatchSwitchInst>(PredBB->getFirstNonPHIIt()))
return;
}
}
diff --git a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
index cc67a455672be4..60fbb689c33f3a 100644
--- a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
+++ b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
@@ -281,7 +281,7 @@ void MergedLoadStoreMotion::sinkStoresAndGEPs(BasicBlock *BB, StoreInst *S0,
auto *GEP0 = cast<GetElementPtrInst>(Ptr0);
auto *GEP1 = cast<GetElementPtrInst>(Ptr1);
Instruction *GEPNew = GEP0->clone();
- GEPNew->insertBefore(SNew);
+ GEPNew->insertBefore(SNew->getIterator());
GEPNew->applyMergedLocation(GEP0->getDebugLoc(), GEP1->getDebugLoc());
SNew->setOperand(1, GEPNew);
GEP0->replaceAllUsesWith(GEPNew);
diff --git a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
index 7b848ae547bd55..8a2491cd314bcb 100644
--- a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -1371,7 +1371,7 @@ static void recomputeLiveInValues(
// and inserts them before "InsertBefore". Returns rematerialized value
// which should be used after statepoint.
static Instruction *rematerializeChain(ArrayRef<Instruction *> ChainToBase,
- Instruction *InsertBefore,
+ BasicBlock::iterator InsertBefore,
Value *RootOfChain,
Value *AlternateLiveBase) {
Instruction *LastClonedValue = nullptr;
@@ -2185,16 +2185,16 @@ static void relocationViaAlloca(
// InvokeInst is a terminator so the store need to be inserted into its
// normal destination block.
BasicBlock *NormalDest = Invoke->getNormalDest();
- Store->insertBefore(NormalDest->getFirstNonPHI());
+ Store->insertBefore(NormalDest->getFirstNonPHIIt());
} else {
assert(!Inst->isTerminator() &&
"The only terminator that can produce a value is "
"InvokeInst which is handled above.");
- Store->insertAfter(Inst);
+ Store->insertAfter(Inst->getIterator());
}
} else {
assert(isa<Argument>(Def));
- Store->insertAfter(cast<Instruction>(Alloca));
+ Store->insertAfter(cast<Instruction>(Alloca)->getIterator());
}
}
@@ -2500,7 +2500,7 @@ static void rematerializeLiveValuesAtUses(
while (!Cand->user_empty()) {
Instruction *UserI = cast<Instruction>(*Cand->user_begin());
Instruction *RematChain = rematerializeChain(
- Record.ChainToBase, UserI, Record.RootOfChain, PointerToBase[Cand]);
+ Record.ChainToBase, UserI->getIterator(), Record.RootOfChain, PointerToBase[Cand]);
UserI->replaceUsesOfWith(Cand, RematChain);
PointerToBase[RematChain] = PointerToBase[Cand];
}
@@ -2573,16 +2573,16 @@ static void rematerializeLiveValues(CallBase *Call,
Instruction *InsertBefore = Call->getNextNode();
assert(InsertBefore);
Instruction *RematerializedValue =
- rematerializeChain(Record.ChainToBase, InsertBefore,
+ rematerializeChain(Record.ChainToBase, InsertBefore->getIterator(),
Record.RootOfChain, PointerToBase[LiveValue]);
Info.RematerializedValues[RematerializedValue] = LiveValue;
} else {
auto *Invoke = cast<InvokeInst>(Call);
- Instruction *NormalInsertBefore =
- &*Invoke->getNormalDest()->getFirstInsertionPt();
- Instruction *UnwindInsertBefore =
- &*Invoke->getUnwindDest()->getFirstInsertionPt();
+ BasicBlock::iterator NormalInsertBefore =
+ Invoke->getNormalDest()->getFirstInsertionPt();
+ BasicBlock::iterator UnwindInsertBefore =
+ Invoke->getUnwindDest()->getFirstInsertionPt();
Instruction *NormalRematerializedValue =
rematerializeChain(Record.ChainToBase, NormalInsertBefore,
@@ -3131,7 +3131,7 @@ bool RewriteStatepointsForGC::runOnFunction(Function &F, DominatorTree &DT,
// most instructions without side effects or memory access.
if (isa<ICmpInst>(Cond) && Cond->hasOneUse()) {
MadeChange = true;
- Cond->moveBefore(TI);
+ Cond->moveBefore(TI->getIterator());
}
}
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index f6179cadab4254..29240aaaa21be9 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -480,7 +480,7 @@ static void migrateDebugInfo(AllocaInst *OldAlloca, bool IsSplit,
// noted as slightly offset (in code) from the store. In practice this
// should have little effect on the debugging experience due to the fact
// that all the split stores should get the same line number.
- NewAssign->moveBefore(DbgAssign);
+ NewAssign->moveBefore(DbgAssign->getIterator());
NewAssign->setDebugLoc(DbgAssign->getDebugLoc());
LLVM_DEBUG(dbgs() << "Created new assign: " << *NewAssign << "\n");
@@ -1843,7 +1843,7 @@ static void rewriteMemOpOfSelect(SelectInst &SI, T &I,
CondMemOp.dropUBImplyingAttrsAndMetadata();
++NumLoadsSpeculated;
}
- CondMemOp.insertBefore(NewMemOpBB->getTerminator());
+ CondMemOp.insertBefore(NewMemOpBB->getTerminator()->getIterator());
Value *Ptr = SI.getOperand(1 + SuccIdx);
CondMemOp.setOperand(I.getPointerOperandIndex(), Ptr);
if (isa<LoadInst>(I)) {
diff --git a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
index c2f7c5dcaf1603..c5f08b084c4bf6 100644
--- a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
+++ b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
@@ -3303,8 +3303,8 @@ static bool isSafeForNoNTrivialUnswitching(Loop &L, LoopInfo &LI) {
// FIXME: We should teach SplitBlock to handle this and remove this
// restriction.
for (auto *ExitBB : ExitBlocks) {
- auto *I = ExitBB->getFirstNonPHI();
- if (isa<CleanupPadInst>(I) || isa<CatchSwitchInst>(I)) {
+ auto It = ExitBB->getFirstNonPHIIt();
+ if (isa<CleanupPadInst>(It) || isa<CatchSwitchInst>(It)) {
LLVM_DEBUG(dbgs() << "Cannot unswitch because of cleanuppad/catchswitch "
"in exit block\n");
return false;
diff --git a/llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp b/llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp
index b05ae00a1e0ea0..2d9a3d1f8a110a 100644
--- a/llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp
+++ b/llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp
@@ -474,9 +474,9 @@ struct AssumeSimplify {
AssumeBuilderState Builder(F.getParent());
/// For now it is initialized to the best value it could have
- Instruction *InsertPt = BB->getFirstNonPHI();
+ BasicBlock::iterator InsertPt = BB->getFirstNonPHIIt();
if (isa<LandingPadInst>(InsertPt))
- InsertPt = InsertPt->getNextNode();
+ InsertPt = std::next(InsertPt);
for (IntrinsicInst *I : make_range(Begin, End)) {
CleanupToDo.insert(I);
for (CallInst::BundleOpInfo &BOI : I->bundle_op_infos()) {
@@ -487,8 +487,8 @@ struct AssumeSimplify {
Builder.addKnowledge(RK);
if (auto *I = dyn_cast_or_null<Instruction>(RK.WasOn))
if (I->getParent() == InsertPt->getParent() &&
- (InsertPt->comesBefore(I) || InsertPt == I))
- InsertPt = I->getNextNode();
+ (InsertPt->comesBefore(I) || &*InsertPt == I))
+ InsertPt = I->getNextNode()->getIterator();
}
}
@@ -498,7 +498,7 @@ struct AssumeSimplify {
for (auto It = (*Begin)->getIterator(), E = InsertPt->getIterator();
It != E; --It)
if (!isGuaranteedToTransferExecutionToSuccessor(&*It)) {
- InsertPt = It->getNextNode();
+ InsertPt = std::next(It);
break;
}
auto *MergedAssume = Builder.build();
diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
index 78116770009980..7a55ac3e36fd83 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -836,7 +836,7 @@ BasicBlock *llvm::ehAwareSplitEdge(BasicBlock *BB, BasicBlock *Succ,
const CriticalEdgeSplittingOptions &Options,
const Twine &BBName) {
- auto *PadInst = Succ->getFirstNonPHI();
+ auto PadInst = Succ->getFirstNonPHIIt();
if (!LandingPadReplacement && !PadInst->isEHPad())
return SplitEdge(BB, Succ, Options.DT, Options.LI, Options.MSSAU, BBName);
@@ -981,7 +981,7 @@ BasicBlock *llvm::ehAwareSplitEdge(BasicBlock *BB, BasicBlock *Succ,
void llvm::createPHIsForSplitLoopExit(ArrayRef<BasicBlock *> Preds,
BasicBlock *SplitBB, BasicBlock *DestBB) {
// SplitBB shouldn't have anything non-trivial in it yet.
- assert((SplitBB->getFirstNonPHI() == SplitBB->getTerminator() ||
+ assert((&*SplitBB->getFirstNonPHIIt() == SplitBB->getTerminator() ||
SplitBB->isLandingPad()) &&
"SplitBB has non-PHI nodes!");
@@ -1450,7 +1450,7 @@ static void SplitLandingPadPredecessorsImpl(
// The new block unconditionally branches to the old block.
BranchInst *BI1 = BranchInst::Create(OrigBB, NewBB1);
- BI1->setDebugLoc(OrigBB->getFirstNonPHI()->getDebugLoc());
+ BI1->setDebugLoc(OrigBB->getFirstNonPHIIt()->getDebugLoc());
// Move the edges from Preds to point to NewBB1 instead of OrigBB.
for (BasicBlock *Pred : Preds) {
@@ -1491,7 +1491,7 @@ static void SplitLandingPadPredecessorsImpl(
// The new block unconditionally branches to the old block.
BranchInst *BI2 = BranchInst::Create(OrigBB, NewBB2);
- BI2->setDebugLoc(OrigBB->getFirstNonPHI()->getDebugLoc());
+ BI2->setDebugLoc(OrigBB->getFirstNonPHIIt()->getDebugLoc());
// Move the remaining edges from OrigBB to point to NewBB2.
for (BasicBlock *NewBB2Pred : NewBB2Preds)
diff --git a/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp b/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp
index 49209e33f2d1dd..e630ce8f7ac025 100644
--- a/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp
@@ -165,7 +165,7 @@ static bool processHeaderPhiOperands(BasicBlock *Header, BasicBlock *Latch,
// Move the phi operands of Header from Latch out of AftBlocks to InsertLoc.
static void moveHeaderPhiOperandsToForeBlocks(BasicBlock *Header,
BasicBlock *Latch,
- Instruction *InsertLoc,
+ BasicBlock::iterator InsertLoc,
BasicBlockSet &AftBlocks) {
// We need to ensure we move the instructions in the correct order,
// starting with the earliest required instruction and moving forward.
@@ -329,7 +329,7 @@ llvm::UnrollAndJamLoop(Loop *L, unsigned Count, unsigned TripCount,
// Move any instructions from fore phi operands from AftBlocks into Fore.
moveHeaderPhiOperandsToForeBlocks(
- Header, LatchBlock, ForeBlocksLast[0]->getTerminator(), AftBlocks);
+ Header, LatchBlock, ForeBlocksLast[0]->getTerminator()->getIterator(), AftBlocks);
// The current on-the-fly SSA update requires blocks to be processed in
// reverse postorder so that LastValueMap contains the correct value at each
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index cf3c2b360d0905..634dee1a8e13ec 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -5262,8 +5262,8 @@ bool SimplifyCFGOpt::simplifyBranchOnICmpChain(BranchInst *BI,
bool SimplifyCFGOpt::simplifyResume(ResumeInst *RI, IRBuilder<> &Builder) {
if (isa<PHINode>(RI->getValue()))
return simplifyCommonResume(RI);
- else if (isa<LandingPadInst>(RI->getParent()->getFirstNonPHI()) &&
- RI->getValue() == RI->getParent()->getFirstNonPHI())
+ else if (isa<LandingPadInst>(RI->getParent()->getFirstNonPHIIt()) &&
+ RI->getValue() == &*RI->getParent()->getFirstNonPHIIt())
// The resume must unwind the exception that caused control to branch here.
return simplifySingleResume(RI);
@@ -5298,7 +5298,7 @@ bool SimplifyCFGOpt::simplifyCommonResume(ResumeInst *RI) {
// Check that there are no other instructions except for debug and lifetime
// intrinsics between the phi's and resume instruction.
if (!isCleanupBlockEmpty(
- make_range(RI->getParent()->getFirstNonPHI(), BB->getTerminator())))
+ make_range(RI->getParent()->getFirstNonPHIIt(), BB->getTerminator()->getIterator())))
return false;
SmallSetVector<BasicBlock *, 4> TrivialUnwindBlocks;
@@ -5315,7 +5315,7 @@ bool SimplifyCFGOpt::simplifyCommonResume(ResumeInst *RI) {
if (IncomingBB->getUniqueSuccessor() != BB)
continue;
- auto *LandingPad = dyn_cast<LandingPadInst>(IncomingBB->getFirstNonPHI());
+ auto *LandingPad = dyn_cast<LandingPadInst>(IncomingBB->getFirstNonPHIIt());
// Not the landing pad that caused the control to branch here.
if (IncomingValue != LandingPad)
continue;
@@ -5364,7 +5364,7 @@ bool SimplifyCFGOpt::simplifyCommonResume(...
[truncated]
|
You can test this locally with the following command:git-clang-format --diff c546b5317c518987a5f45dd4c4d25321a955c758 a1c2238a4ee79dba48f704cadaa21b9c8f43dca8 --extensions h,cpp -- llvm/include/llvm/Transforms/Coroutines/CoroInstr.h llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp llvm/lib/IR/BasicBlock.cpp llvm/lib/Target/Hexagon/HexagonOptimizeSZextends.cpp llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp llvm/lib/Transforms/Scalar/SROA.cpp llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp llvm/lib/Transforms/Utils/BasicBlockUtils.cpp llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp llvm/lib/Transforms/Utils/SimplifyCFG.cpp llvm/lib/Transforms/Vectorize/VPlan.cppView the diff from clang-format here.diff --git a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
index b0ae19867d..7dda2d2754 100644
--- a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -6176,12 +6176,11 @@ LSRInstance::LSRInstance(Loop *L, IVUsers &IU, ScalarEvolution &SE,
// CatchSwitchInst. Because the CatchSwitchInst cannot be split, there is
// no good place to stick any instructions.
if (auto *PN = dyn_cast<PHINode>(U.getUser())) {
- auto FirstNonPHI = PN->getParent()->getFirstNonPHIIt();
- if (isa<FuncletPadInst>(FirstNonPHI) ||
- isa<CatchSwitchInst>(FirstNonPHI))
- for (BasicBlock *PredBB : PN->blocks())
- if (isa<CatchSwitchInst>(PredBB->getFirstNonPHIIt()))
- return;
+ auto FirstNonPHI = PN->getParent()->getFirstNonPHIIt();
+ if (isa<FuncletPadInst>(FirstNonPHI) || isa<CatchSwitchInst>(FirstNonPHI))
+ for (BasicBlock *PredBB : PN->blocks())
+ if (isa<CatchSwitchInst>(PredBB->getFirstNonPHIIt()))
+ return;
}
}
|
OCHyams
approved these changes
Jan 27, 2025
| @@ -5449,7 +5448,7 @@ static bool removeEmptyCleanup(CleanupReturnInst *RI, DomTreeUpdater *DTU) { | |||
| } | |||
|
|
|||
| // Sink any remaining PHI nodes directly into UnwindDest. | |||
| Instruction *InsertPt = DestEHPad; | |||
| BasicBlock::iterator InsertPt = UnwindDest->getFirstNonPHIIt(); // XXX unsafe faff | |||
There was a problem hiding this comment.
Comment leftover from development? And why is it unsafe?
There was a problem hiding this comment.
Left over indeed, removing -- it was uh, "unsafe" because I thought some of these call-sites wouldn't recompile cleanly without touching headers, and I was on a laptop which didn't have enough energy left to totally recompile LLVM X_X.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To finalise the "RemoveDIs" work removing debug intrinsics, we're updating call sites that insert instructions to use iterators instead. This set of changes are those where it's not immediately obvious that just calling getIterator to fetch an iterator is correct, and one or two places where more than one line needs to change.
Overall the same rule holds though: iterators generated for the start of a block such as getFirstNonPHIIt need to be passed into insert/move methods without being unwrapped/rewrapped, everything else can use getIterator.