[NFC][DebugInfo] Make some block-start-position methods return iterators #124287
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
As part of the "RemoveDIs" work to eliminate debug intrinsics, we're replacing methods that use Instruction*'s as positions with iterators. A number of these (such as getFirstNonPHIOrDbg) are sufficiently infrequently used that we can just replace the pointer-returning version with an iterator-returning version, hopefully without much/any disruption. Thus this patch has getFirstNonPHIOrDbg and getFirstNonPHIOrDbgOrLifetime return an iterator, and updates all call-sites. There are no concerns about the iterators returned being converted to Instruction*'s and losing the debug-info bit: because the methods skip debug intrinsics, the iterator head bit is always false anyway.
llvmbot
added
lldb
backend:AArch64
backend:AMDGPU
PGO
|
@llvm/pr-subscribers-flang-openmp @llvm/pr-subscribers-llvm-ir Author: Jeremy Morse (jmorse) ChangesAs part of the "RemoveDIs" work to eliminate debug intrinsics, we're replacing methods that use Instruction*'s as positions with iterators. A number of these (such as getFirstNonPHIOrDbg) are sufficiently infrequently used that we can just replace the pointer-returning version with an iterator-returning version, hopefully without much/any disruption. Thus this patch has getFirstNonPHIOrDbg and getFirstNonPHIOrDbgOrLifetime return an iterator, and updates all call-sites. There are no concerns about the iterators returned being converted to Instruction*'s and losing the debug-info bit: because the methods skip debug intrinsics, the iterator head bit is always false anyway. Patch is 20.61 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/124287.diff 19 Files Affected:
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
index 6c728f34474898..a414ad652448e9 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -66,7 +66,7 @@ static llvm::Value *FindEntryInstruction(llvm::Function *function) {
if (function->empty())
return nullptr;
- return function->getEntryBlock().getFirstNonPHIOrDbg();
+ return &*function->getEntryBlock().getFirstNonPHIOrDbg();
}
IRForTarget::IRForTarget(lldb_private::ClangExpressionDeclMap *decl_map,
@@ -361,7 +361,7 @@ bool IRForTarget::CreateResultVariable(llvm::Function &llvm_function) {
// there's nothing to put into its equivalent persistent variable.
BasicBlock &entry_block(llvm_function.getEntryBlock());
- Instruction *first_entry_instruction(entry_block.getFirstNonPHIOrDbg());
+ Instruction *first_entry_instruction(&*entry_block.getFirstNonPHIOrDbg());
if (!first_entry_instruction)
return false;
@@ -1505,7 +1505,7 @@ bool IRForTarget::ReplaceVariables(Function &llvm_function) {
LLDB_LOG(log, "Arg: \"{0}\"", PrintValue(argument));
BasicBlock &entry_block(llvm_function.getEntryBlock());
- Instruction *FirstEntryInstruction(entry_block.getFirstNonPHIOrDbg());
+ Instruction *FirstEntryInstruction(&*entry_block.getFirstNonPHIOrDbg());
if (!FirstEntryInstruction) {
m_error_stream.Printf("Internal error [IRForTarget]: Couldn't find the "
diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index e22fe1e7e7dc8f..5df4dcecebc878 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -299,22 +299,20 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
/// Returns a pointer to the first instruction in this block that is not a
/// PHINode or a debug intrinsic, or any pseudo operation if \c SkipPseudoOp
/// is true.
- const Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) const;
- Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
- return const_cast<Instruction *>(
- static_cast<const BasicBlock *>(this)->getFirstNonPHIOrDbg(
- SkipPseudoOp));
+ InstListType::const_iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true) const;
+ InstListType::iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
+ return static_cast<const BasicBlock *>(this)->getFirstNonPHIOrDbg(
+ SkipPseudoOp).getNonConst();
}
/// Returns a pointer to the first instruction in this block that is not a
/// PHINode, a debug intrinsic, or a lifetime intrinsic, or any pseudo
/// operation if \c SkipPseudoOp is true.
- const Instruction *
+ InstListType::const_iterator
getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = true) const;
- Instruction *getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = true) {
- return const_cast<Instruction *>(
- static_cast<const BasicBlock *>(this)->getFirstNonPHIOrDbgOrLifetime(
- SkipPseudoOp));
+ InstListType::iterator getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = true) {
+ return static_cast<const BasicBlock *>(this)->getFirstNonPHIOrDbgOrLifetime(
+ SkipPseudoOp).getNonConst();
}
/// Returns an iterator to the first instruction in this block that is
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 7e9d705a7bef6c..fcd00d0558b713 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -990,7 +990,7 @@ bool CodeGenPrepare::isMergingEmptyBlockProfitable(BasicBlock *BB,
isa<IndirectBrInst>(Pred->getTerminator())))
return true;
- if (BB->getTerminator() != BB->getFirstNonPHIOrDbg())
+ if (BB->getTerminator() != &*BB->getFirstNonPHIOrDbg())
return true;
// We use a simple cost heuristic which determine skipping merging is
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index 0efc04cb2c8679..62cf9970ad3ff4 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -383,7 +383,8 @@ BasicBlock::const_iterator BasicBlock::getFirstNonPHIIt() const {
return It;
}
-const Instruction *BasicBlock::getFirstNonPHIOrDbg(bool SkipPseudoOp) const {
+BasicBlock::const_iterator
+BasicBlock::getFirstNonPHIOrDbg(bool SkipPseudoOp) const {
for (const Instruction &I : *this) {
if (isa<PHINode>(I) || isa<DbgInfoIntrinsic>(I))
continue;
@@ -391,12 +392,15 @@ const Instruction *BasicBlock::getFirstNonPHIOrDbg(bool SkipPseudoOp) const {
if (SkipPseudoOp && isa<PseudoProbeInst>(I))
continue;
- return &I;
+ BasicBlock::const_iterator It = I.getIterator();
+ // Signal that this comes after any debug records.
+ It.setHeadBit(false);
+ return It;
}
- return nullptr;
+ return end();
}
-const Instruction *
+BasicBlock::const_iterator
BasicBlock::getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp) const {
for (const Instruction &I : *this) {
if (isa<PHINode>(I) || isa<DbgInfoIntrinsic>(I))
@@ -408,9 +412,13 @@ BasicBlock::getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp) const {
if (SkipPseudoOp && isa<PseudoProbeInst>(I))
continue;
- return &I;
+ BasicBlock::const_iterator It = I.getIterator();
+ // Signal that this comes after any debug records.
+ It.setHeadBit(false);
+ return It;
+
}
- return nullptr;
+ return end();
}
BasicBlock::const_iterator BasicBlock::getFirstInsertionPt() const {
@@ -428,11 +436,10 @@ BasicBlock::const_iterator BasicBlock::getFirstInsertionPt() const {
}
BasicBlock::const_iterator BasicBlock::getFirstNonPHIOrDbgOrAlloca() const {
- const Instruction *FirstNonPHI = getFirstNonPHI();
- if (!FirstNonPHI)
+ const_iterator InsertPt = getFirstNonPHIIt();
+ if (InsertPt == end())
return end();
- const_iterator InsertPt = FirstNonPHI->getIterator();
if (InsertPt->isEHPad())
++InsertPt;
@@ -448,6 +455,9 @@ BasicBlock::const_iterator BasicBlock::getFirstNonPHIOrDbgOrAlloca() const {
++InsertPt;
}
}
+
+ // Signal that this comes after any debug records.
+ InsertPt.setHeadBit(false);
return InsertPt;
}
diff --git a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
index e2389145cf33f2..aae2fdaf5bec37 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
@@ -2188,7 +2188,7 @@ static std::optional<Instruction *> instCombineDMB(InstCombiner &IC,
NI = NI->getNextNonDebugInstruction();
if (!NI) {
if (auto *SuccBB = NIBB->getUniqueSuccessor())
- NI = SuccBB->getFirstNonPHIOrDbgOrLifetime();
+ NI = &*SuccBB->getFirstNonPHIOrDbgOrLifetime();
else
break;
}
diff --git a/llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp b/llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp
index 4ff6fc32b642dd..df0c2080e0795d 100644
--- a/llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp
+++ b/llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp
@@ -232,7 +232,7 @@ Value *SIAnnotateControlFlow::handleLoopCondition(
} else if (L->contains(Inst)) {
Insert = Term;
} else {
- Insert = L->getHeader()->getFirstNonPHIOrDbgOrLifetime();
+ Insert = &*L->getHeader()->getFirstNonPHIOrDbgOrLifetime();
}
return CreateBreak(Insert);
@@ -247,7 +247,7 @@ Value *SIAnnotateControlFlow::handleLoopCondition(
}
if (isa<Argument>(Cond)) {
- Instruction *Insert = L->getHeader()->getFirstNonPHIOrDbgOrLifetime();
+ Instruction *Insert = &*L->getHeader()->getFirstNonPHIOrDbgOrLifetime();
return CreateBreak(Insert);
}
diff --git a/llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp b/llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp
index 75fcf6829c5042..c734d3d4300737 100644
--- a/llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp
@@ -86,7 +86,7 @@ bool GenericToNVVM::runOnModule(Module &M) {
if (F.isDeclaration()) {
continue;
}
- IRBuilder<> Builder(F.getEntryBlock().getFirstNonPHIOrDbg());
+ IRBuilder<> Builder(&*F.getEntryBlock().getFirstNonPHIOrDbg());
for (BasicBlock &BB : F) {
for (Instruction &II : BB) {
for (unsigned i = 0, e = II.getNumOperands(); i < e; ++i) {
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 73d4fb9065831e..1195478fdff4cc 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -1656,7 +1656,8 @@ static Value *emitSetAndGetSwiftErrorValueAround(Instruction *Call,
Builder.SetInsertPoint(Call->getNextNode());
} else {
auto Invoke = cast<InvokeInst>(Call);
- Builder.SetInsertPoint(Invoke->getNormalDest()->getFirstNonPHIOrDbg());
+ BasicBlock::iterator It = Invoke->getNormalDest()->getFirstNonPHIOrDbg();
+ Builder.SetInsertPoint(It);
}
// Get the current swifterror value and store it to the alloca.
@@ -1697,7 +1698,7 @@ static void eliminateSwiftErrorAlloca(Function &F, AllocaInst *Alloca,
static void eliminateSwiftErrorArgument(Function &F, Argument &Arg,
coro::Shape &Shape,
SmallVectorImpl<AllocaInst*> &AllocasToPromote) {
- IRBuilder<> Builder(F.getEntryBlock().getFirstNonPHIOrDbg());
+ IRBuilder<> Builder(&F.getEntryBlock(), F.getEntryBlock().getFirstNonPHIOrDbg());
auto ArgTy = cast<PointerType>(Arg.getType());
auto ValueTy = PointerType::getUnqual(F.getContext());
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index ff5df12c398c56..3bf412e4e54a11 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -597,7 +597,7 @@ static void replaceSwiftErrorOps(Function &F, coro::Shape &Shape,
}
// Create a swifterror alloca.
- IRBuilder<> Builder(F.getEntryBlock().getFirstNonPHIOrDbg());
+ IRBuilder<> Builder(&F.getEntryBlock(), F.getEntryBlock().getFirstNonPHIOrDbg());
auto Alloca = Builder.CreateAlloca(ValueTy);
Alloca->setSwiftError(true);
diff --git a/llvm/lib/Transforms/IPO/IROutliner.cpp b/llvm/lib/Transforms/IPO/IROutliner.cpp
index 3f54106bd09fe9..41bc67f2b6891b 100644
--- a/llvm/lib/Transforms/IPO/IROutliner.cpp
+++ b/llvm/lib/Transforms/IPO/IROutliner.cpp
@@ -197,7 +197,7 @@ Value *OutlinableRegion::findCorrespondingValueIn(const OutlinableRegion &Other,
BasicBlock *
OutlinableRegion::findCorrespondingBlockIn(const OutlinableRegion &Other,
BasicBlock *BB) {
- Instruction *FirstNonPHI = BB->getFirstNonPHIOrDbg();
+ Instruction *FirstNonPHI = &*BB->getFirstNonPHIOrDbg();
assert(FirstNonPHI && "block is empty?");
Value *CorrespondingVal = findCorrespondingValueIn(Other, FirstNonPHI);
if (!CorrespondingVal)
diff --git a/llvm/lib/Transforms/IPO/SCCP.cpp b/llvm/lib/Transforms/IPO/SCCP.cpp
index e80c6f7c0f49d4..2afcdf09af0162 100644
--- a/llvm/lib/Transforms/IPO/SCCP.cpp
+++ b/llvm/lib/Transforms/IPO/SCCP.cpp
@@ -235,11 +235,11 @@ static bool runIPSCCP(
// nodes in executable blocks we found values for. The function's entry
// block is not part of BlocksToErase, so we have to handle it separately.
for (BasicBlock *BB : BlocksToErase) {
- NumInstRemoved += changeToUnreachable(BB->getFirstNonPHIOrDbg(),
+ NumInstRemoved += changeToUnreachable(&*BB->getFirstNonPHIOrDbg(),
/*PreserveLCSSA=*/false, &DTU);
}
if (!Solver.isBlockExecutable(&F.front()))
- NumInstRemoved += changeToUnreachable(F.front().getFirstNonPHIOrDbg(),
+ NumInstRemoved += changeToUnreachable(&*F.front().getFirstNonPHIOrDbg(),
/*PreserveLCSSA=*/false, &DTU);
BasicBlock *NewUnreachableBB = nullptr;
diff --git a/llvm/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp
index b978c54ef96fdf..e1d5b07405a095 100644
--- a/llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -1747,7 +1747,7 @@ void SampleProfileLoader::generateMDProfMetadata(Function &F) {
if (Weight != 0) {
if (Weight > MaxWeight) {
MaxWeight = Weight;
- MaxDestInst = Succ->getFirstNonPHIOrDbgOrLifetime();
+ MaxDestInst = &*Succ->getFirstNonPHIOrDbgOrLifetime();
}
}
}
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index f80bbffbab547e..4b42e86e25161b 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -462,8 +462,8 @@ Instruction *InstCombinerImpl::visitAllocaInst(AllocaInst &AI) {
// Get the first instruction in the entry block.
BasicBlock &EntryBlock = AI.getParent()->getParent()->getEntryBlock();
- Instruction *FirstInst = EntryBlock.getFirstNonPHIOrDbg();
- if (FirstInst != &AI) {
+ BasicBlock::iterator FirstInst = EntryBlock.getFirstNonPHIOrDbg();
+ if (&*FirstInst != &AI) {
// If the entry block doesn't start with a zero-size alloca then move
// this one to the start of the entry block. There is no problem with
// dominance as the array size was forced to a constant earlier already.
diff --git a/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp b/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
index bbc7a005b9ff4f..350b0c3754bb5e 100644
--- a/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
+++ b/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
@@ -415,7 +415,7 @@ static void splitCallSite(CallBase &CB,
// constant incoming values.
static bool isPredicatedOnPHI(CallBase &CB) {
BasicBlock *Parent = CB.getParent();
- if (&CB != Parent->getFirstNonPHIOrDbg())
+ if (&CB != &*Parent->getFirstNonPHIOrDbg())
return false;
for (auto &PN : Parent->phis()) {
diff --git a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
index c2f7c5dcaf1603..bcc56203a4beaf 100644
--- a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
+++ b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
@@ -770,7 +770,7 @@ static bool unswitchTrivialSwitch(Loop &L, SwitchInst &SI, DominatorTree &DT,
// instruction in the block.
auto *TI = BBToCheck.getTerminator();
bool isUnreachable = isa<UnreachableInst>(TI);
- return !isUnreachable || BBToCheck.getFirstNonPHIOrDbg() != TI;
+ return !isUnreachable || &*BBToCheck.getFirstNonPHIOrDbg() != TI;
};
SmallVector<int, 4> ExitCaseIndices;
diff --git a/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp b/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
index f34e9c5818dd67..b0105ae8fa1168 100644
--- a/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
+++ b/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
@@ -427,7 +427,7 @@ void llvm::moveInstructionsToTheBeginning(BasicBlock &FromBB, BasicBlock &ToBB,
DependenceInfo &DI) {
for (Instruction &I :
llvm::make_early_inc_range(llvm::drop_begin(llvm::reverse(FromBB)))) {
- Instruction *MovePos = ToBB.getFirstNonPHIOrDbg();
+ BasicBlock::iterator MovePos = ToBB.getFirstNonPHIOrDbg();
if (isSafeToMoveBefore(I, *MovePos, DT, &PDT, &DI))
I.moveBeforePreserving(MovePos);
diff --git a/llvm/lib/Transforms/Utils/IRNormalizer.cpp b/llvm/lib/Transforms/Utils/IRNormalizer.cpp
index 47ec7f3177db73..d36da331fedf72 100644
--- a/llvm/lib/Transforms/Utils/IRNormalizer.cpp
+++ b/llvm/lib/Transforms/Utils/IRNormalizer.cpp
@@ -475,7 +475,7 @@ void IRNormalizer::reorderInstructions(Function &F) const {
Call->getIntrinsicID() == Intrinsic::experimental_convergence_loop)
FirstNonPHIOrDbgOrAlloca++;
}
- Instruction->moveBefore(&*FirstNonPHIOrDbgOrAlloca);
+ Instruction->moveBefore(FirstNonPHIOrDbgOrAlloca);
TopologicalSort.pop();
}
}
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index cf3c2b360d0905..9cc754e71c0e46 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -6004,7 +6004,7 @@ static bool eliminateDeadSwitchCases(SwitchInst *SI, DomTreeUpdater *DTU,
/// the phi node, and set PhiIndex to BB's index in the phi node.
static PHINode *findPHIForConditionForwarding(ConstantInt *CaseValue,
BasicBlock *BB, int *PhiIndex) {
- if (BB->getFirstNonPHIOrDbg() != BB->getTerminator())
+ if (&*BB->getFirstNonPHIIt() != BB->getTerminator())
return nullptr; // BB must be empty to be a candidate for simplification.
if (!BB->getSinglePredecessor())
return nullptr; // BB must be dominated by the switch.
@@ -7885,7 +7885,7 @@ bool SimplifyCFGOpt::simplifyUncondBranch(BranchInst *BI,
Options.NeedCanonicalLoop &&
(!LoopHeaders.empty() && BB->hasNPredecessorsOrMore(2) &&
(is_contained(LoopHeaders, BB) || is_contained(LoopHeaders, Succ)));
- BasicBlock::iterator I = BB->getFirstNonPHIOrDbg(true)->getIterator();
+ BasicBlock::iterator I = BB->getFirstNonPHIOrDbg();
if (I->isTerminator() && BB != &BB->getParent()->getEntryBlock() &&
!NeedCanonicalLoop && TryToSimplifyUncondBranchFromEmptyBlock(BB, DTU))
return true;
diff --git a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
index 42616155f0cc32..c7fea4ab33372a 100644
--- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -4643,9 +4643,11 @@ TEST_F(OpenMPIRBuilderTest, CreateTeamsWithThreadLimit) {
dyn_cast<BranchInst>(PushNumTeamsCallInst->getNextNonDebugInstruction());
ASSERT_NE(BrInst, nullptr);
ASSERT_EQ(BrInst->getNumSuccessors(), 1U);
- Instruction *NextInstruction =
+ BasicBlock::iterator NextInstruction =
BrInst->getSuccessor(0)->getFirstNonPHIOrDbgOrLifetime();
- CallInst *ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
+ CallInst *ForkTeamsCI = nullptr;
+ if (NextInstruction != BrInst->getSuccessor(0)->end())
+ ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
ASSERT_NE(ForkTeamsCI, nullptr);
EXPECT_EQ(ForkTeamsCI->getCalledFunction(),
OMPBuilder.getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_fork_teams));
@@ -4698,9 +4700,11 @@ TEST_F(OpenMPIRBuilderTest, CreateTeamsWithNumTeamsUpper) {
dyn_cast<BranchInst>(PushNumTeamsCallInst->getNextNonDebugInstruction());
ASSERT_NE(BrInst, nullptr);
ASSERT_EQ(BrInst->getNumSuccessors(), 1U);
- Instruction *NextInstruction =
+ BasicBlock::iterator NextInstruction =
BrInst->getSuccessor(0)->getFirstNonPHIOrDbgOrLifetime();
- CallInst *ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
+ CallInst *ForkTeamsCI = nullptr;
+ if (NextInstruction != BrInst->getSuccessor(0)->end())
+ ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
ASSERT_NE(ForkTeamsCI, nullptr);
EXPECT_EQ(ForkTeamsCI->getCalledFunction(),
OMPBuilder.getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_fork_teams));
@@ -4756,9 +4760,11 @@ TEST_F(OpenMPIRBuilderTest, CreateTeamsWithNumTeamsBoth) {
dyn_cast<BranchInst>(PushNumTeamsCallInst->getNextNonDebugInstruction());
ASSERT_NE(BrInst, nullptr);
ASSERT_EQ(BrInst->getNumSuccessors(), 1U);
- Instruction *NextInstruction =
+ BasicBlock::iterator NextInstruction =
BrInst->getSuccessor(0)->getFirstNonPHIOrDbgOrLifetime();
- CallInst *ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
+ CallInst *ForkTeamsCI = nullptr;
+ if (NextInstruction != BrInst->getSuccessor(0)->end())
+ ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
ASSERT_NE(ForkTeamsCI, nullptr);
EXPECT_EQ(ForkTeamsCI->getCalledFunction(),
OMPBuilder.getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_fork_teams));
@@ -4820,9 +4826,11 @@ TEST_F(OpenMPIRBuilderTest, CreateTeamsWithNumTeamsAndThreadLimit) {
dyn_cast<BranchInst>(PushNumTeamsCallInst->getNextNonDebugInstruct...
[truncated]
|
|
@llvm/pr-subscribers-backend-nvptx Author: Jeremy Morse (jmorse) ChangesAs part of the "RemoveDIs" work to eliminate debug intrinsics, we're replacing methods that use Instruction*'s as positions with iterators. A number of these (such as getFirstNonPHIOrDbg) are sufficiently infrequently used that we can just replace the pointer-returning version with an iterator-returning version, hopefully without much/any disruption. Thus this patch has getFirstNonPHIOrDbg and getFirstNonPHIOrDbgOrLifetime return an iterator, and updates all call-sites. There are no concerns about the iterators returned being converted to Instruction*'s and losing the debug-info bit: because the methods skip debug intrinsics, the iterator head bit is always false anyway. Patch is 20.61 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/124287.diff 19 Files Affected:
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
index 6c728f34474898..a414ad652448e9 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -66,7 +66,7 @@ static llvm::Value *FindEntryInstruction(llvm::Function *function) {
if (function->empty())
return nullptr;
- return function->getEntryBlock().getFirstNonPHIOrDbg();
+ return &*function->getEntryBlock().getFirstNonPHIOrDbg();
}
IRForTarget::IRForTarget(lldb_private::ClangExpressionDeclMap *decl_map,
@@ -361,7 +361,7 @@ bool IRForTarget::CreateResultVariable(llvm::Function &llvm_function) {
// there's nothing to put into its equivalent persistent variable.
BasicBlock &entry_block(llvm_function.getEntryBlock());
- Instruction *first_entry_instruction(entry_block.getFirstNonPHIOrDbg());
+ Instruction *first_entry_instruction(&*entry_block.getFirstNonPHIOrDbg());
if (!first_entry_instruction)
return false;
@@ -1505,7 +1505,7 @@ bool IRForTarget::ReplaceVariables(Function &llvm_function) {
LLDB_LOG(log, "Arg: \"{0}\"", PrintValue(argument));
BasicBlock &entry_block(llvm_function.getEntryBlock());
- Instruction *FirstEntryInstruction(entry_block.getFirstNonPHIOrDbg());
+ Instruction *FirstEntryInstruction(&*entry_block.getFirstNonPHIOrDbg());
if (!FirstEntryInstruction) {
m_error_stream.Printf("Internal error [IRForTarget]: Couldn't find the "
diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index e22fe1e7e7dc8f..5df4dcecebc878 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -299,22 +299,20 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
/// Returns a pointer to the first instruction in this block that is not a
/// PHINode or a debug intrinsic, or any pseudo operation if \c SkipPseudoOp
/// is true.
- const Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) const;
- Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
- return const_cast<Instruction *>(
- static_cast<const BasicBlock *>(this)->getFirstNonPHIOrDbg(
- SkipPseudoOp));
+ InstListType::const_iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true) const;
+ InstListType::iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
+ return static_cast<const BasicBlock *>(this)->getFirstNonPHIOrDbg(
+ SkipPseudoOp).getNonConst();
}
/// Returns a pointer to the first instruction in this block that is not a
/// PHINode, a debug intrinsic, or a lifetime intrinsic, or any pseudo
/// operation if \c SkipPseudoOp is true.
- const Instruction *
+ InstListType::const_iterator
getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = true) const;
- Instruction *getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = true) {
- return const_cast<Instruction *>(
- static_cast<const BasicBlock *>(this)->getFirstNonPHIOrDbgOrLifetime(
- SkipPseudoOp));
+ InstListType::iterator getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = true) {
+ return static_cast<const BasicBlock *>(this)->getFirstNonPHIOrDbgOrLifetime(
+ SkipPseudoOp).getNonConst();
}
/// Returns an iterator to the first instruction in this block that is
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 7e9d705a7bef6c..fcd00d0558b713 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -990,7 +990,7 @@ bool CodeGenPrepare::isMergingEmptyBlockProfitable(BasicBlock *BB,
isa<IndirectBrInst>(Pred->getTerminator())))
return true;
- if (BB->getTerminator() != BB->getFirstNonPHIOrDbg())
+ if (BB->getTerminator() != &*BB->getFirstNonPHIOrDbg())
return true;
// We use a simple cost heuristic which determine skipping merging is
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index 0efc04cb2c8679..62cf9970ad3ff4 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -383,7 +383,8 @@ BasicBlock::const_iterator BasicBlock::getFirstNonPHIIt() const {
return It;
}
-const Instruction *BasicBlock::getFirstNonPHIOrDbg(bool SkipPseudoOp) const {
+BasicBlock::const_iterator
+BasicBlock::getFirstNonPHIOrDbg(bool SkipPseudoOp) const {
for (const Instruction &I : *this) {
if (isa<PHINode>(I) || isa<DbgInfoIntrinsic>(I))
continue;
@@ -391,12 +392,15 @@ const Instruction *BasicBlock::getFirstNonPHIOrDbg(bool SkipPseudoOp) const {
if (SkipPseudoOp && isa<PseudoProbeInst>(I))
continue;
- return &I;
+ BasicBlock::const_iterator It = I.getIterator();
+ // Signal that this comes after any debug records.
+ It.setHeadBit(false);
+ return It;
}
- return nullptr;
+ return end();
}
-const Instruction *
+BasicBlock::const_iterator
BasicBlock::getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp) const {
for (const Instruction &I : *this) {
if (isa<PHINode>(I) || isa<DbgInfoIntrinsic>(I))
@@ -408,9 +412,13 @@ BasicBlock::getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp) const {
if (SkipPseudoOp && isa<PseudoProbeInst>(I))
continue;
- return &I;
+ BasicBlock::const_iterator It = I.getIterator();
+ // Signal that this comes after any debug records.
+ It.setHeadBit(false);
+ return It;
+
}
- return nullptr;
+ return end();
}
BasicBlock::const_iterator BasicBlock::getFirstInsertionPt() const {
@@ -428,11 +436,10 @@ BasicBlock::const_iterator BasicBlock::getFirstInsertionPt() const {
}
BasicBlock::const_iterator BasicBlock::getFirstNonPHIOrDbgOrAlloca() const {
- const Instruction *FirstNonPHI = getFirstNonPHI();
- if (!FirstNonPHI)
+ const_iterator InsertPt = getFirstNonPHIIt();
+ if (InsertPt == end())
return end();
- const_iterator InsertPt = FirstNonPHI->getIterator();
if (InsertPt->isEHPad())
++InsertPt;
@@ -448,6 +455,9 @@ BasicBlock::const_iterator BasicBlock::getFirstNonPHIOrDbgOrAlloca() const {
++InsertPt;
}
}
+
+ // Signal that this comes after any debug records.
+ InsertPt.setHeadBit(false);
return InsertPt;
}
diff --git a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
index e2389145cf33f2..aae2fdaf5bec37 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
@@ -2188,7 +2188,7 @@ static std::optional<Instruction *> instCombineDMB(InstCombiner &IC,
NI = NI->getNextNonDebugInstruction();
if (!NI) {
if (auto *SuccBB = NIBB->getUniqueSuccessor())
- NI = SuccBB->getFirstNonPHIOrDbgOrLifetime();
+ NI = &*SuccBB->getFirstNonPHIOrDbgOrLifetime();
else
break;
}
diff --git a/llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp b/llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp
index 4ff6fc32b642dd..df0c2080e0795d 100644
--- a/llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp
+++ b/llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp
@@ -232,7 +232,7 @@ Value *SIAnnotateControlFlow::handleLoopCondition(
} else if (L->contains(Inst)) {
Insert = Term;
} else {
- Insert = L->getHeader()->getFirstNonPHIOrDbgOrLifetime();
+ Insert = &*L->getHeader()->getFirstNonPHIOrDbgOrLifetime();
}
return CreateBreak(Insert);
@@ -247,7 +247,7 @@ Value *SIAnnotateControlFlow::handleLoopCondition(
}
if (isa<Argument>(Cond)) {
- Instruction *Insert = L->getHeader()->getFirstNonPHIOrDbgOrLifetime();
+ Instruction *Insert = &*L->getHeader()->getFirstNonPHIOrDbgOrLifetime();
return CreateBreak(Insert);
}
diff --git a/llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp b/llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp
index 75fcf6829c5042..c734d3d4300737 100644
--- a/llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp
@@ -86,7 +86,7 @@ bool GenericToNVVM::runOnModule(Module &M) {
if (F.isDeclaration()) {
continue;
}
- IRBuilder<> Builder(F.getEntryBlock().getFirstNonPHIOrDbg());
+ IRBuilder<> Builder(&*F.getEntryBlock().getFirstNonPHIOrDbg());
for (BasicBlock &BB : F) {
for (Instruction &II : BB) {
for (unsigned i = 0, e = II.getNumOperands(); i < e; ++i) {
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 73d4fb9065831e..1195478fdff4cc 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -1656,7 +1656,8 @@ static Value *emitSetAndGetSwiftErrorValueAround(Instruction *Call,
Builder.SetInsertPoint(Call->getNextNode());
} else {
auto Invoke = cast<InvokeInst>(Call);
- Builder.SetInsertPoint(Invoke->getNormalDest()->getFirstNonPHIOrDbg());
+ BasicBlock::iterator It = Invoke->getNormalDest()->getFirstNonPHIOrDbg();
+ Builder.SetInsertPoint(It);
}
// Get the current swifterror value and store it to the alloca.
@@ -1697,7 +1698,7 @@ static void eliminateSwiftErrorAlloca(Function &F, AllocaInst *Alloca,
static void eliminateSwiftErrorArgument(Function &F, Argument &Arg,
coro::Shape &Shape,
SmallVectorImpl<AllocaInst*> &AllocasToPromote) {
- IRBuilder<> Builder(F.getEntryBlock().getFirstNonPHIOrDbg());
+ IRBuilder<> Builder(&F.getEntryBlock(), F.getEntryBlock().getFirstNonPHIOrDbg());
auto ArgTy = cast<PointerType>(Arg.getType());
auto ValueTy = PointerType::getUnqual(F.getContext());
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index ff5df12c398c56..3bf412e4e54a11 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -597,7 +597,7 @@ static void replaceSwiftErrorOps(Function &F, coro::Shape &Shape,
}
// Create a swifterror alloca.
- IRBuilder<> Builder(F.getEntryBlock().getFirstNonPHIOrDbg());
+ IRBuilder<> Builder(&F.getEntryBlock(), F.getEntryBlock().getFirstNonPHIOrDbg());
auto Alloca = Builder.CreateAlloca(ValueTy);
Alloca->setSwiftError(true);
diff --git a/llvm/lib/Transforms/IPO/IROutliner.cpp b/llvm/lib/Transforms/IPO/IROutliner.cpp
index 3f54106bd09fe9..41bc67f2b6891b 100644
--- a/llvm/lib/Transforms/IPO/IROutliner.cpp
+++ b/llvm/lib/Transforms/IPO/IROutliner.cpp
@@ -197,7 +197,7 @@ Value *OutlinableRegion::findCorrespondingValueIn(const OutlinableRegion &Other,
BasicBlock *
OutlinableRegion::findCorrespondingBlockIn(const OutlinableRegion &Other,
BasicBlock *BB) {
- Instruction *FirstNonPHI = BB->getFirstNonPHIOrDbg();
+ Instruction *FirstNonPHI = &*BB->getFirstNonPHIOrDbg();
assert(FirstNonPHI && "block is empty?");
Value *CorrespondingVal = findCorrespondingValueIn(Other, FirstNonPHI);
if (!CorrespondingVal)
diff --git a/llvm/lib/Transforms/IPO/SCCP.cpp b/llvm/lib/Transforms/IPO/SCCP.cpp
index e80c6f7c0f49d4..2afcdf09af0162 100644
--- a/llvm/lib/Transforms/IPO/SCCP.cpp
+++ b/llvm/lib/Transforms/IPO/SCCP.cpp
@@ -235,11 +235,11 @@ static bool runIPSCCP(
// nodes in executable blocks we found values for. The function's entry
// block is not part of BlocksToErase, so we have to handle it separately.
for (BasicBlock *BB : BlocksToErase) {
- NumInstRemoved += changeToUnreachable(BB->getFirstNonPHIOrDbg(),
+ NumInstRemoved += changeToUnreachable(&*BB->getFirstNonPHIOrDbg(),
/*PreserveLCSSA=*/false, &DTU);
}
if (!Solver.isBlockExecutable(&F.front()))
- NumInstRemoved += changeToUnreachable(F.front().getFirstNonPHIOrDbg(),
+ NumInstRemoved += changeToUnreachable(&*F.front().getFirstNonPHIOrDbg(),
/*PreserveLCSSA=*/false, &DTU);
BasicBlock *NewUnreachableBB = nullptr;
diff --git a/llvm/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp
index b978c54ef96fdf..e1d5b07405a095 100644
--- a/llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -1747,7 +1747,7 @@ void SampleProfileLoader::generateMDProfMetadata(Function &F) {
if (Weight != 0) {
if (Weight > MaxWeight) {
MaxWeight = Weight;
- MaxDestInst = Succ->getFirstNonPHIOrDbgOrLifetime();
+ MaxDestInst = &*Succ->getFirstNonPHIOrDbgOrLifetime();
}
}
}
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index f80bbffbab547e..4b42e86e25161b 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -462,8 +462,8 @@ Instruction *InstCombinerImpl::visitAllocaInst(AllocaInst &AI) {
// Get the first instruction in the entry block.
BasicBlock &EntryBlock = AI.getParent()->getParent()->getEntryBlock();
- Instruction *FirstInst = EntryBlock.getFirstNonPHIOrDbg();
- if (FirstInst != &AI) {
+ BasicBlock::iterator FirstInst = EntryBlock.getFirstNonPHIOrDbg();
+ if (&*FirstInst != &AI) {
// If the entry block doesn't start with a zero-size alloca then move
// this one to the start of the entry block. There is no problem with
// dominance as the array size was forced to a constant earlier already.
diff --git a/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp b/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
index bbc7a005b9ff4f..350b0c3754bb5e 100644
--- a/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
+++ b/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
@@ -415,7 +415,7 @@ static void splitCallSite(CallBase &CB,
// constant incoming values.
static bool isPredicatedOnPHI(CallBase &CB) {
BasicBlock *Parent = CB.getParent();
- if (&CB != Parent->getFirstNonPHIOrDbg())
+ if (&CB != &*Parent->getFirstNonPHIOrDbg())
return false;
for (auto &PN : Parent->phis()) {
diff --git a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
index c2f7c5dcaf1603..bcc56203a4beaf 100644
--- a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
+++ b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
@@ -770,7 +770,7 @@ static bool unswitchTrivialSwitch(Loop &L, SwitchInst &SI, DominatorTree &DT,
// instruction in the block.
auto *TI = BBToCheck.getTerminator();
bool isUnreachable = isa<UnreachableInst>(TI);
- return !isUnreachable || BBToCheck.getFirstNonPHIOrDbg() != TI;
+ return !isUnreachable || &*BBToCheck.getFirstNonPHIOrDbg() != TI;
};
SmallVector<int, 4> ExitCaseIndices;
diff --git a/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp b/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
index f34e9c5818dd67..b0105ae8fa1168 100644
--- a/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
+++ b/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
@@ -427,7 +427,7 @@ void llvm::moveInstructionsToTheBeginning(BasicBlock &FromBB, BasicBlock &ToBB,
DependenceInfo &DI) {
for (Instruction &I :
llvm::make_early_inc_range(llvm::drop_begin(llvm::reverse(FromBB)))) {
- Instruction *MovePos = ToBB.getFirstNonPHIOrDbg();
+ BasicBlock::iterator MovePos = ToBB.getFirstNonPHIOrDbg();
if (isSafeToMoveBefore(I, *MovePos, DT, &PDT, &DI))
I.moveBeforePreserving(MovePos);
diff --git a/llvm/lib/Transforms/Utils/IRNormalizer.cpp b/llvm/lib/Transforms/Utils/IRNormalizer.cpp
index 47ec7f3177db73..d36da331fedf72 100644
--- a/llvm/lib/Transforms/Utils/IRNormalizer.cpp
+++ b/llvm/lib/Transforms/Utils/IRNormalizer.cpp
@@ -475,7 +475,7 @@ void IRNormalizer::reorderInstructions(Function &F) const {
Call->getIntrinsicID() == Intrinsic::experimental_convergence_loop)
FirstNonPHIOrDbgOrAlloca++;
}
- Instruction->moveBefore(&*FirstNonPHIOrDbgOrAlloca);
+ Instruction->moveBefore(FirstNonPHIOrDbgOrAlloca);
TopologicalSort.pop();
}
}
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index cf3c2b360d0905..9cc754e71c0e46 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -6004,7 +6004,7 @@ static bool eliminateDeadSwitchCases(SwitchInst *SI, DomTreeUpdater *DTU,
/// the phi node, and set PhiIndex to BB's index in the phi node.
static PHINode *findPHIForConditionForwarding(ConstantInt *CaseValue,
BasicBlock *BB, int *PhiIndex) {
- if (BB->getFirstNonPHIOrDbg() != BB->getTerminator())
+ if (&*BB->getFirstNonPHIIt() != BB->getTerminator())
return nullptr; // BB must be empty to be a candidate for simplification.
if (!BB->getSinglePredecessor())
return nullptr; // BB must be dominated by the switch.
@@ -7885,7 +7885,7 @@ bool SimplifyCFGOpt::simplifyUncondBranch(BranchInst *BI,
Options.NeedCanonicalLoop &&
(!LoopHeaders.empty() && BB->hasNPredecessorsOrMore(2) &&
(is_contained(LoopHeaders, BB) || is_contained(LoopHeaders, Succ)));
- BasicBlock::iterator I = BB->getFirstNonPHIOrDbg(true)->getIterator();
+ BasicBlock::iterator I = BB->getFirstNonPHIOrDbg();
if (I->isTerminator() && BB != &BB->getParent()->getEntryBlock() &&
!NeedCanonicalLoop && TryToSimplifyUncondBranchFromEmptyBlock(BB, DTU))
return true;
diff --git a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
index 42616155f0cc32..c7fea4ab33372a 100644
--- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -4643,9 +4643,11 @@ TEST_F(OpenMPIRBuilderTest, CreateTeamsWithThreadLimit) {
dyn_cast<BranchInst>(PushNumTeamsCallInst->getNextNonDebugInstruction());
ASSERT_NE(BrInst, nullptr);
ASSERT_EQ(BrInst->getNumSuccessors(), 1U);
- Instruction *NextInstruction =
+ BasicBlock::iterator NextInstruction =
BrInst->getSuccessor(0)->getFirstNonPHIOrDbgOrLifetime();
- CallInst *ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
+ CallInst *ForkTeamsCI = nullptr;
+ if (NextInstruction != BrInst->getSuccessor(0)->end())
+ ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
ASSERT_NE(ForkTeamsCI, nullptr);
EXPECT_EQ(ForkTeamsCI->getCalledFunction(),
OMPBuilder.getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_fork_teams));
@@ -4698,9 +4700,11 @@ TEST_F(OpenMPIRBuilderTest, CreateTeamsWithNumTeamsUpper) {
dyn_cast<BranchInst>(PushNumTeamsCallInst->getNextNonDebugInstruction());
ASSERT_NE(BrInst, nullptr);
ASSERT_EQ(BrInst->getNumSuccessors(), 1U);
- Instruction *NextInstruction =
+ BasicBlock::iterator NextInstruction =
BrInst->getSuccessor(0)->getFirstNonPHIOrDbgOrLifetime();
- CallInst *ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
+ CallInst *ForkTeamsCI = nullptr;
+ if (NextInstruction != BrInst->getSuccessor(0)->end())
+ ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
ASSERT_NE(ForkTeamsCI, nullptr);
EXPECT_EQ(ForkTeamsCI->getCalledFunction(),
OMPBuilder.getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_fork_teams));
@@ -4756,9 +4760,11 @@ TEST_F(OpenMPIRBuilderTest, CreateTeamsWithNumTeamsBoth) {
dyn_cast<BranchInst>(PushNumTeamsCallInst->getNextNonDebugInstruction());
ASSERT_NE(BrInst, nullptr);
ASSERT_EQ(BrInst->getNumSuccessors(), 1U);
- Instruction *NextInstruction =
+ BasicBlock::iterator NextInstruction =
BrInst->getSuccessor(0)->getFirstNonPHIOrDbgOrLifetime();
- CallInst *ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
+ CallInst *ForkTeamsCI = nullptr;
+ if (NextInstruction != BrInst->getSuccessor(0)->end())
+ ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
ASSERT_NE(ForkTeamsCI, nullptr);
EXPECT_EQ(ForkTeamsCI->getCalledFunction(),
OMPBuilder.getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_fork_teams));
@@ -4820,9 +4826,11 @@ TEST_F(OpenMPIRBuilderTest, CreateTeamsWithNumTeamsAndThreadLimit) {
dyn_cast<BranchInst>(PushNumTeamsCallInst->getNextNonDebugInstruct...
[truncated]
|
|
@llvm/pr-subscribers-backend-aarch64 Author: Jeremy Morse (jmorse) ChangesAs part of the "RemoveDIs" work to eliminate debug intrinsics, we're replacing methods that use Instruction*'s as positions with iterators. A number of these (such as getFirstNonPHIOrDbg) are sufficiently infrequently used that we can just replace the pointer-returning version with an iterator-returning version, hopefully without much/any disruption. Thus this patch has getFirstNonPHIOrDbg and getFirstNonPHIOrDbgOrLifetime return an iterator, and updates all call-sites. There are no concerns about the iterators returned being converted to Instruction*'s and losing the debug-info bit: because the methods skip debug intrinsics, the iterator head bit is always false anyway. Patch is 20.61 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/124287.diff 19 Files Affected:
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
index 6c728f34474898..a414ad652448e9 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -66,7 +66,7 @@ static llvm::Value *FindEntryInstruction(llvm::Function *function) {
if (function->empty())
return nullptr;
- return function->getEntryBlock().getFirstNonPHIOrDbg();
+ return &*function->getEntryBlock().getFirstNonPHIOrDbg();
}
IRForTarget::IRForTarget(lldb_private::ClangExpressionDeclMap *decl_map,
@@ -361,7 +361,7 @@ bool IRForTarget::CreateResultVariable(llvm::Function &llvm_function) {
// there's nothing to put into its equivalent persistent variable.
BasicBlock &entry_block(llvm_function.getEntryBlock());
- Instruction *first_entry_instruction(entry_block.getFirstNonPHIOrDbg());
+ Instruction *first_entry_instruction(&*entry_block.getFirstNonPHIOrDbg());
if (!first_entry_instruction)
return false;
@@ -1505,7 +1505,7 @@ bool IRForTarget::ReplaceVariables(Function &llvm_function) {
LLDB_LOG(log, "Arg: \"{0}\"", PrintValue(argument));
BasicBlock &entry_block(llvm_function.getEntryBlock());
- Instruction *FirstEntryInstruction(entry_block.getFirstNonPHIOrDbg());
+ Instruction *FirstEntryInstruction(&*entry_block.getFirstNonPHIOrDbg());
if (!FirstEntryInstruction) {
m_error_stream.Printf("Internal error [IRForTarget]: Couldn't find the "
diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index e22fe1e7e7dc8f..5df4dcecebc878 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -299,22 +299,20 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
/// Returns a pointer to the first instruction in this block that is not a
/// PHINode or a debug intrinsic, or any pseudo operation if \c SkipPseudoOp
/// is true.
- const Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) const;
- Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
- return const_cast<Instruction *>(
- static_cast<const BasicBlock *>(this)->getFirstNonPHIOrDbg(
- SkipPseudoOp));
+ InstListType::const_iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true) const;
+ InstListType::iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
+ return static_cast<const BasicBlock *>(this)->getFirstNonPHIOrDbg(
+ SkipPseudoOp).getNonConst();
}
/// Returns a pointer to the first instruction in this block that is not a
/// PHINode, a debug intrinsic, or a lifetime intrinsic, or any pseudo
/// operation if \c SkipPseudoOp is true.
- const Instruction *
+ InstListType::const_iterator
getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = true) const;
- Instruction *getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = true) {
- return const_cast<Instruction *>(
- static_cast<const BasicBlock *>(this)->getFirstNonPHIOrDbgOrLifetime(
- SkipPseudoOp));
+ InstListType::iterator getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = true) {
+ return static_cast<const BasicBlock *>(this)->getFirstNonPHIOrDbgOrLifetime(
+ SkipPseudoOp).getNonConst();
}
/// Returns an iterator to the first instruction in this block that is
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 7e9d705a7bef6c..fcd00d0558b713 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -990,7 +990,7 @@ bool CodeGenPrepare::isMergingEmptyBlockProfitable(BasicBlock *BB,
isa<IndirectBrInst>(Pred->getTerminator())))
return true;
- if (BB->getTerminator() != BB->getFirstNonPHIOrDbg())
+ if (BB->getTerminator() != &*BB->getFirstNonPHIOrDbg())
return true;
// We use a simple cost heuristic which determine skipping merging is
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index 0efc04cb2c8679..62cf9970ad3ff4 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -383,7 +383,8 @@ BasicBlock::const_iterator BasicBlock::getFirstNonPHIIt() const {
return It;
}
-const Instruction *BasicBlock::getFirstNonPHIOrDbg(bool SkipPseudoOp) const {
+BasicBlock::const_iterator
+BasicBlock::getFirstNonPHIOrDbg(bool SkipPseudoOp) const {
for (const Instruction &I : *this) {
if (isa<PHINode>(I) || isa<DbgInfoIntrinsic>(I))
continue;
@@ -391,12 +392,15 @@ const Instruction *BasicBlock::getFirstNonPHIOrDbg(bool SkipPseudoOp) const {
if (SkipPseudoOp && isa<PseudoProbeInst>(I))
continue;
- return &I;
+ BasicBlock::const_iterator It = I.getIterator();
+ // Signal that this comes after any debug records.
+ It.setHeadBit(false);
+ return It;
}
- return nullptr;
+ return end();
}
-const Instruction *
+BasicBlock::const_iterator
BasicBlock::getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp) const {
for (const Instruction &I : *this) {
if (isa<PHINode>(I) || isa<DbgInfoIntrinsic>(I))
@@ -408,9 +412,13 @@ BasicBlock::getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp) const {
if (SkipPseudoOp && isa<PseudoProbeInst>(I))
continue;
- return &I;
+ BasicBlock::const_iterator It = I.getIterator();
+ // Signal that this comes after any debug records.
+ It.setHeadBit(false);
+ return It;
+
}
- return nullptr;
+ return end();
}
BasicBlock::const_iterator BasicBlock::getFirstInsertionPt() const {
@@ -428,11 +436,10 @@ BasicBlock::const_iterator BasicBlock::getFirstInsertionPt() const {
}
BasicBlock::const_iterator BasicBlock::getFirstNonPHIOrDbgOrAlloca() const {
- const Instruction *FirstNonPHI = getFirstNonPHI();
- if (!FirstNonPHI)
+ const_iterator InsertPt = getFirstNonPHIIt();
+ if (InsertPt == end())
return end();
- const_iterator InsertPt = FirstNonPHI->getIterator();
if (InsertPt->isEHPad())
++InsertPt;
@@ -448,6 +455,9 @@ BasicBlock::const_iterator BasicBlock::getFirstNonPHIOrDbgOrAlloca() const {
++InsertPt;
}
}
+
+ // Signal that this comes after any debug records.
+ InsertPt.setHeadBit(false);
return InsertPt;
}
diff --git a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
index e2389145cf33f2..aae2fdaf5bec37 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
@@ -2188,7 +2188,7 @@ static std::optional<Instruction *> instCombineDMB(InstCombiner &IC,
NI = NI->getNextNonDebugInstruction();
if (!NI) {
if (auto *SuccBB = NIBB->getUniqueSuccessor())
- NI = SuccBB->getFirstNonPHIOrDbgOrLifetime();
+ NI = &*SuccBB->getFirstNonPHIOrDbgOrLifetime();
else
break;
}
diff --git a/llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp b/llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp
index 4ff6fc32b642dd..df0c2080e0795d 100644
--- a/llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp
+++ b/llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp
@@ -232,7 +232,7 @@ Value *SIAnnotateControlFlow::handleLoopCondition(
} else if (L->contains(Inst)) {
Insert = Term;
} else {
- Insert = L->getHeader()->getFirstNonPHIOrDbgOrLifetime();
+ Insert = &*L->getHeader()->getFirstNonPHIOrDbgOrLifetime();
}
return CreateBreak(Insert);
@@ -247,7 +247,7 @@ Value *SIAnnotateControlFlow::handleLoopCondition(
}
if (isa<Argument>(Cond)) {
- Instruction *Insert = L->getHeader()->getFirstNonPHIOrDbgOrLifetime();
+ Instruction *Insert = &*L->getHeader()->getFirstNonPHIOrDbgOrLifetime();
return CreateBreak(Insert);
}
diff --git a/llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp b/llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp
index 75fcf6829c5042..c734d3d4300737 100644
--- a/llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp
@@ -86,7 +86,7 @@ bool GenericToNVVM::runOnModule(Module &M) {
if (F.isDeclaration()) {
continue;
}
- IRBuilder<> Builder(F.getEntryBlock().getFirstNonPHIOrDbg());
+ IRBuilder<> Builder(&*F.getEntryBlock().getFirstNonPHIOrDbg());
for (BasicBlock &BB : F) {
for (Instruction &II : BB) {
for (unsigned i = 0, e = II.getNumOperands(); i < e; ++i) {
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 73d4fb9065831e..1195478fdff4cc 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -1656,7 +1656,8 @@ static Value *emitSetAndGetSwiftErrorValueAround(Instruction *Call,
Builder.SetInsertPoint(Call->getNextNode());
} else {
auto Invoke = cast<InvokeInst>(Call);
- Builder.SetInsertPoint(Invoke->getNormalDest()->getFirstNonPHIOrDbg());
+ BasicBlock::iterator It = Invoke->getNormalDest()->getFirstNonPHIOrDbg();
+ Builder.SetInsertPoint(It);
}
// Get the current swifterror value and store it to the alloca.
@@ -1697,7 +1698,7 @@ static void eliminateSwiftErrorAlloca(Function &F, AllocaInst *Alloca,
static void eliminateSwiftErrorArgument(Function &F, Argument &Arg,
coro::Shape &Shape,
SmallVectorImpl<AllocaInst*> &AllocasToPromote) {
- IRBuilder<> Builder(F.getEntryBlock().getFirstNonPHIOrDbg());
+ IRBuilder<> Builder(&F.getEntryBlock(), F.getEntryBlock().getFirstNonPHIOrDbg());
auto ArgTy = cast<PointerType>(Arg.getType());
auto ValueTy = PointerType::getUnqual(F.getContext());
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index ff5df12c398c56..3bf412e4e54a11 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -597,7 +597,7 @@ static void replaceSwiftErrorOps(Function &F, coro::Shape &Shape,
}
// Create a swifterror alloca.
- IRBuilder<> Builder(F.getEntryBlock().getFirstNonPHIOrDbg());
+ IRBuilder<> Builder(&F.getEntryBlock(), F.getEntryBlock().getFirstNonPHIOrDbg());
auto Alloca = Builder.CreateAlloca(ValueTy);
Alloca->setSwiftError(true);
diff --git a/llvm/lib/Transforms/IPO/IROutliner.cpp b/llvm/lib/Transforms/IPO/IROutliner.cpp
index 3f54106bd09fe9..41bc67f2b6891b 100644
--- a/llvm/lib/Transforms/IPO/IROutliner.cpp
+++ b/llvm/lib/Transforms/IPO/IROutliner.cpp
@@ -197,7 +197,7 @@ Value *OutlinableRegion::findCorrespondingValueIn(const OutlinableRegion &Other,
BasicBlock *
OutlinableRegion::findCorrespondingBlockIn(const OutlinableRegion &Other,
BasicBlock *BB) {
- Instruction *FirstNonPHI = BB->getFirstNonPHIOrDbg();
+ Instruction *FirstNonPHI = &*BB->getFirstNonPHIOrDbg();
assert(FirstNonPHI && "block is empty?");
Value *CorrespondingVal = findCorrespondingValueIn(Other, FirstNonPHI);
if (!CorrespondingVal)
diff --git a/llvm/lib/Transforms/IPO/SCCP.cpp b/llvm/lib/Transforms/IPO/SCCP.cpp
index e80c6f7c0f49d4..2afcdf09af0162 100644
--- a/llvm/lib/Transforms/IPO/SCCP.cpp
+++ b/llvm/lib/Transforms/IPO/SCCP.cpp
@@ -235,11 +235,11 @@ static bool runIPSCCP(
// nodes in executable blocks we found values for. The function's entry
// block is not part of BlocksToErase, so we have to handle it separately.
for (BasicBlock *BB : BlocksToErase) {
- NumInstRemoved += changeToUnreachable(BB->getFirstNonPHIOrDbg(),
+ NumInstRemoved += changeToUnreachable(&*BB->getFirstNonPHIOrDbg(),
/*PreserveLCSSA=*/false, &DTU);
}
if (!Solver.isBlockExecutable(&F.front()))
- NumInstRemoved += changeToUnreachable(F.front().getFirstNonPHIOrDbg(),
+ NumInstRemoved += changeToUnreachable(&*F.front().getFirstNonPHIOrDbg(),
/*PreserveLCSSA=*/false, &DTU);
BasicBlock *NewUnreachableBB = nullptr;
diff --git a/llvm/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp
index b978c54ef96fdf..e1d5b07405a095 100644
--- a/llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -1747,7 +1747,7 @@ void SampleProfileLoader::generateMDProfMetadata(Function &F) {
if (Weight != 0) {
if (Weight > MaxWeight) {
MaxWeight = Weight;
- MaxDestInst = Succ->getFirstNonPHIOrDbgOrLifetime();
+ MaxDestInst = &*Succ->getFirstNonPHIOrDbgOrLifetime();
}
}
}
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index f80bbffbab547e..4b42e86e25161b 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -462,8 +462,8 @@ Instruction *InstCombinerImpl::visitAllocaInst(AllocaInst &AI) {
// Get the first instruction in the entry block.
BasicBlock &EntryBlock = AI.getParent()->getParent()->getEntryBlock();
- Instruction *FirstInst = EntryBlock.getFirstNonPHIOrDbg();
- if (FirstInst != &AI) {
+ BasicBlock::iterator FirstInst = EntryBlock.getFirstNonPHIOrDbg();
+ if (&*FirstInst != &AI) {
// If the entry block doesn't start with a zero-size alloca then move
// this one to the start of the entry block. There is no problem with
// dominance as the array size was forced to a constant earlier already.
diff --git a/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp b/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
index bbc7a005b9ff4f..350b0c3754bb5e 100644
--- a/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
+++ b/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
@@ -415,7 +415,7 @@ static void splitCallSite(CallBase &CB,
// constant incoming values.
static bool isPredicatedOnPHI(CallBase &CB) {
BasicBlock *Parent = CB.getParent();
- if (&CB != Parent->getFirstNonPHIOrDbg())
+ if (&CB != &*Parent->getFirstNonPHIOrDbg())
return false;
for (auto &PN : Parent->phis()) {
diff --git a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
index c2f7c5dcaf1603..bcc56203a4beaf 100644
--- a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
+++ b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
@@ -770,7 +770,7 @@ static bool unswitchTrivialSwitch(Loop &L, SwitchInst &SI, DominatorTree &DT,
// instruction in the block.
auto *TI = BBToCheck.getTerminator();
bool isUnreachable = isa<UnreachableInst>(TI);
- return !isUnreachable || BBToCheck.getFirstNonPHIOrDbg() != TI;
+ return !isUnreachable || &*BBToCheck.getFirstNonPHIOrDbg() != TI;
};
SmallVector<int, 4> ExitCaseIndices;
diff --git a/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp b/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
index f34e9c5818dd67..b0105ae8fa1168 100644
--- a/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
+++ b/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
@@ -427,7 +427,7 @@ void llvm::moveInstructionsToTheBeginning(BasicBlock &FromBB, BasicBlock &ToBB,
DependenceInfo &DI) {
for (Instruction &I :
llvm::make_early_inc_range(llvm::drop_begin(llvm::reverse(FromBB)))) {
- Instruction *MovePos = ToBB.getFirstNonPHIOrDbg();
+ BasicBlock::iterator MovePos = ToBB.getFirstNonPHIOrDbg();
if (isSafeToMoveBefore(I, *MovePos, DT, &PDT, &DI))
I.moveBeforePreserving(MovePos);
diff --git a/llvm/lib/Transforms/Utils/IRNormalizer.cpp b/llvm/lib/Transforms/Utils/IRNormalizer.cpp
index 47ec7f3177db73..d36da331fedf72 100644
--- a/llvm/lib/Transforms/Utils/IRNormalizer.cpp
+++ b/llvm/lib/Transforms/Utils/IRNormalizer.cpp
@@ -475,7 +475,7 @@ void IRNormalizer::reorderInstructions(Function &F) const {
Call->getIntrinsicID() == Intrinsic::experimental_convergence_loop)
FirstNonPHIOrDbgOrAlloca++;
}
- Instruction->moveBefore(&*FirstNonPHIOrDbgOrAlloca);
+ Instruction->moveBefore(FirstNonPHIOrDbgOrAlloca);
TopologicalSort.pop();
}
}
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index cf3c2b360d0905..9cc754e71c0e46 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -6004,7 +6004,7 @@ static bool eliminateDeadSwitchCases(SwitchInst *SI, DomTreeUpdater *DTU,
/// the phi node, and set PhiIndex to BB's index in the phi node.
static PHINode *findPHIForConditionForwarding(ConstantInt *CaseValue,
BasicBlock *BB, int *PhiIndex) {
- if (BB->getFirstNonPHIOrDbg() != BB->getTerminator())
+ if (&*BB->getFirstNonPHIIt() != BB->getTerminator())
return nullptr; // BB must be empty to be a candidate for simplification.
if (!BB->getSinglePredecessor())
return nullptr; // BB must be dominated by the switch.
@@ -7885,7 +7885,7 @@ bool SimplifyCFGOpt::simplifyUncondBranch(BranchInst *BI,
Options.NeedCanonicalLoop &&
(!LoopHeaders.empty() && BB->hasNPredecessorsOrMore(2) &&
(is_contained(LoopHeaders, BB) || is_contained(LoopHeaders, Succ)));
- BasicBlock::iterator I = BB->getFirstNonPHIOrDbg(true)->getIterator();
+ BasicBlock::iterator I = BB->getFirstNonPHIOrDbg();
if (I->isTerminator() && BB != &BB->getParent()->getEntryBlock() &&
!NeedCanonicalLoop && TryToSimplifyUncondBranchFromEmptyBlock(BB, DTU))
return true;
diff --git a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
index 42616155f0cc32..c7fea4ab33372a 100644
--- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -4643,9 +4643,11 @@ TEST_F(OpenMPIRBuilderTest, CreateTeamsWithThreadLimit) {
dyn_cast<BranchInst>(PushNumTeamsCallInst->getNextNonDebugInstruction());
ASSERT_NE(BrInst, nullptr);
ASSERT_EQ(BrInst->getNumSuccessors(), 1U);
- Instruction *NextInstruction =
+ BasicBlock::iterator NextInstruction =
BrInst->getSuccessor(0)->getFirstNonPHIOrDbgOrLifetime();
- CallInst *ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
+ CallInst *ForkTeamsCI = nullptr;
+ if (NextInstruction != BrInst->getSuccessor(0)->end())
+ ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
ASSERT_NE(ForkTeamsCI, nullptr);
EXPECT_EQ(ForkTeamsCI->getCalledFunction(),
OMPBuilder.getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_fork_teams));
@@ -4698,9 +4700,11 @@ TEST_F(OpenMPIRBuilderTest, CreateTeamsWithNumTeamsUpper) {
dyn_cast<BranchInst>(PushNumTeamsCallInst->getNextNonDebugInstruction());
ASSERT_NE(BrInst, nullptr);
ASSERT_EQ(BrInst->getNumSuccessors(), 1U);
- Instruction *NextInstruction =
+ BasicBlock::iterator NextInstruction =
BrInst->getSuccessor(0)->getFirstNonPHIOrDbgOrLifetime();
- CallInst *ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
+ CallInst *ForkTeamsCI = nullptr;
+ if (NextInstruction != BrInst->getSuccessor(0)->end())
+ ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
ASSERT_NE(ForkTeamsCI, nullptr);
EXPECT_EQ(ForkTeamsCI->getCalledFunction(),
OMPBuilder.getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_fork_teams));
@@ -4756,9 +4760,11 @@ TEST_F(OpenMPIRBuilderTest, CreateTeamsWithNumTeamsBoth) {
dyn_cast<BranchInst>(PushNumTeamsCallInst->getNextNonDebugInstruction());
ASSERT_NE(BrInst, nullptr);
ASSERT_EQ(BrInst->getNumSuccessors(), 1U);
- Instruction *NextInstruction =
+ BasicBlock::iterator NextInstruction =
BrInst->getSuccessor(0)->getFirstNonPHIOrDbgOrLifetime();
- CallInst *ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
+ CallInst *ForkTeamsCI = nullptr;
+ if (NextInstruction != BrInst->getSuccessor(0)->end())
+ ForkTeamsCI = dyn_cast_if_present<CallInst>(NextInstruction);
ASSERT_NE(ForkTeamsCI, nullptr);
EXPECT_EQ(ForkTeamsCI->getCalledFunction(),
OMPBuilder.getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_fork_teams));
@@ -4820,9 +4826,11 @@ TEST_F(OpenMPIRBuilderTest, CreateTeamsWithNumTeamsAndThreadLimit) {
dyn_cast<BranchInst>(PushNumTeamsCallInst->getNextNonDebugInstruct...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
OCHyams
approved these changes
Jan 27, 2025
llvm/lib/IR/BasicBlock.cpp
Outdated
Comment on lines
396
to
397
| // Signal that this comes after any debug records. | ||
| It.setHeadBit(false); |
There was a problem hiding this comment.
When does Instruction::getIterator return an iterator with head bit set? Can this be an assert (that it's not set) instead?
llvm/lib/IR/BasicBlock.cpp
Outdated
Comment on lines
416
to
417
| // Signal that this comes after any debug records. | ||
| It.setHeadBit(false); |
Comment on lines
+459
to
+460
| // Signal that this comes after any debug records. | ||
| InsertPt.setHeadBit(false); |
There was a problem hiding this comment.
The other two call-sites always generate iterators with a false head bit anyway, but there's a path from getFirstNonPHIIt to this line without InsertPt being assigned a different iterator. getFirstNonPHIIt will set the head bit, therefore we have to explicitly clear it here to ensure the position is "after" any debug records.
Comment on lines
1659
to
1660
| BasicBlock::iterator It = Invoke->getNormalDest()->getFirstNonPHIOrDbg(); | ||
| Builder.SetInsertPoint(It); |
There was a problem hiding this comment.
Why is this change necessary?
There was a problem hiding this comment.
Looks like it isn't, removing
jmorse
commented
Jan 27, 2025
Comment on lines
+459
to
+460
| // Signal that this comes after any debug records. | ||
| InsertPt.setHeadBit(false); |
There was a problem hiding this comment.
The other two call-sites always generate iterators with a false head bit anyway, but there's a path from getFirstNonPHIIt to this line without InsertPt being assigned a different iterator. getFirstNonPHIIt will set the head bit, therefore we have to explicitly clear it here to ensure the position is "after" any debug records.
Comment on lines
1659
to
1660
| BasicBlock::iterator It = Invoke->getNormalDest()->getFirstNonPHIOrDbg(); | ||
| Builder.SetInsertPoint(It); |
There was a problem hiding this comment.
Looks like it isn't, removing
|
Hi, in principle this change makes sense but it causes a bit of an issue with |
|
Ooooff, yeah we missed that. I think (90% confident) that overloading the relevant DIBuilder methods with iterator-taking versions should work fine, and we should be able to pick that into llvm20 before rc1, we'll look at it momentarily. (In theory downstream compatibility isn't a strong LLVM concern (?), but I think it'd be easy to support and ease transitions). |
|
Prototype here jmorse@8e68a72 , is that the sort of thing that would be useful? It's lacking unit test coverage right now. |
|
Yes, thank you, something like that looks great! I should be able to do some testing with that tomorrow to check to make sure I am not missing anything. |
|
Tested now, we do need to add some |
|
NB, I'm looking less likely to have time for this soon; if you're able to add some coverage to the unit tests (there'll be somewhere using those DIBuilder APIs that can duplicated to use iterator insertion instead), it'll speed things up. |
|
I'm taking a look, but I'm having some doubts about the change itself, |
|
Indeed, that's a pattern we've seen elsewhere in the instruction APIs -- you can select whether you want an instruction inserted into a block or not by whether you pass an instruction pointer or nullptr in. However, without a common "no-such-location" instruction iterator (or by making the argument std::optional) we can't replicate that with the new APIs. For things like the instruction constructor APIs we've used overloads, and forced users who don't want instruction insertion to call a function without an insertion location argument. Here, we'd have a |
I'll add a small note that it is possible to get around this - for an iterator |
|
Thanks, I knew it was possible (it had to be: it would be possible to create a dummy instruction, insert it at the specified point, and inspect its parent) but had not yet found the best way of doing it, I was trying to get it working with |
|
Thanks again, it ends up looking much better (IMO) with |
|
Given that https://llvm.org/docs/RemoveDIsDebugInfo.html specifically documents these functions as "no plans to deprecate", even though I personally feel like they are a bad idea, I just updated them so they compile with no deprecation warnings. Does this look like a good approach? Should I create a PR, or would you prefer a different approach and/or would you like to make some changes to it? |
|
The approach sounds reasonable enough to me that it's probably best to open a PR and continue discussion there - the C API is a bit of a special case, so it might warrant bringing in people who know more about it (and actually consume it) if a change is necessary, but I suspect the best option will be to just make them compile without deprecation warnings (as you've done) and try to ensure that consumers are warned of how to not use them incorrectly. |
hvdijk
added a commit
to hvdijk/llvm-project
that referenced
this pull request
Feb 6, 2025
After llvm#124287 updated several functions to return iterators rather than Instruction *, it was no longer straightforward to pass their result to DIBuilder. This commit updates DIBuilder methods to accept an InsertPosition instead, so that they can be called with an iterator (preferred), or with a deprecation warning an Instruction *, or a BasicBlock *. This commit also updates the existing calls to the DIBuilder methods to pass in iterators.
hvdijk
added a commit
to hvdijk/llvm-project
that referenced
this pull request
Feb 7, 2025
After llvm#124287 updated several functions to return iterators rather than Instruction *, it was no longer straightforward to pass their result to DIBuilder. This commit updates DIBuilder methods to accept an InsertPosition instead, so that they can be called with an iterator (preferred), or with a deprecation warning an Instruction *, or a BasicBlock *. This commit also updates the existing calls to the DIBuilder methods to pass in iterators.
hvdijk
added a commit
that referenced
this pull request
Feb 12, 2025
After #124287 updated several functions to return iterators rather than Instruction *, it was no longer straightforward to pass their result to DIBuilder. This commit updates DIBuilder methods to accept an InsertPosition instead, so that they can be called with an iterator (preferred), or with a deprecation warning an Instruction *, or a BasicBlock *. This commit also updates the existing calls to the DIBuilder methods to pass in iterators.
hvdijk
added a commit
that referenced
this pull request
Feb 13, 2025
…#126967) After #124287 updated several functions to return iterators rather than Instruction *, it was no longer straightforward to pass their result to DIBuilder. This commit updates DIBuilder methods to accept an InsertPosition instead, so that they can be called with an iterator (preferred), or with a deprecation warning an Instruction *, or a BasicBlock *. This commit also updates the existing calls to the DIBuilder methods to pass in iterators. As a special exception, DIBuilder::insertDeclare() keeps a separate overload accepting a BasicBlock *InsertAtEnd. This is because despite the name, this method does not insert at the end of the block, therefore this cannot be handled implicitly by using InsertPosition.
flovent
pushed a commit
to flovent/llvm-project
that referenced
this pull request
Feb 13, 2025
…26059) After llvm#124287 updated several functions to return iterators rather than Instruction *, it was no longer straightforward to pass their result to DIBuilder. This commit updates DIBuilder methods to accept an InsertPosition instead, so that they can be called with an iterator (preferred), or with a deprecation warning an Instruction *, or a BasicBlock *. This commit also updates the existing calls to the DIBuilder methods to pass in iterators.
flovent
pushed a commit
to flovent/llvm-project
that referenced
this pull request
Feb 13, 2025
…llvm#126967) After llvm#124287 updated several functions to return iterators rather than Instruction *, it was no longer straightforward to pass their result to DIBuilder. This commit updates DIBuilder methods to accept an InsertPosition instead, so that they can be called with an iterator (preferred), or with a deprecation warning an Instruction *, or a BasicBlock *. This commit also updates the existing calls to the DIBuilder methods to pass in iterators. As a special exception, DIBuilder::insertDeclare() keeps a separate overload accepting a BasicBlock *InsertAtEnd. This is because despite the name, this method does not insert at the end of the block, therefore this cannot be handled implicitly by using InsertPosition.
joaosaffran
pushed a commit
to joaosaffran/llvm-project
that referenced
this pull request
Feb 14, 2025
…26059) After llvm#124287 updated several functions to return iterators rather than Instruction *, it was no longer straightforward to pass their result to DIBuilder. This commit updates DIBuilder methods to accept an InsertPosition instead, so that they can be called with an iterator (preferred), or with a deprecation warning an Instruction *, or a BasicBlock *. This commit also updates the existing calls to the DIBuilder methods to pass in iterators.
joaosaffran
pushed a commit
to joaosaffran/llvm-project
that referenced
this pull request
Feb 14, 2025
…llvm#126967) After llvm#124287 updated several functions to return iterators rather than Instruction *, it was no longer straightforward to pass their result to DIBuilder. This commit updates DIBuilder methods to accept an InsertPosition instead, so that they can be called with an iterator (preferred), or with a deprecation warning an Instruction *, or a BasicBlock *. This commit also updates the existing calls to the DIBuilder methods to pass in iterators. As a special exception, DIBuilder::insertDeclare() keeps a separate overload accepting a BasicBlock *InsertAtEnd. This is because despite the name, this method does not insert at the end of the block, therefore this cannot be handled implicitly by using InsertPosition.
swift-ci
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Feb 14, 2025
…llvm#126967) After llvm#124287 updated several functions to return iterators rather than Instruction *, it was no longer straightforward to pass their result to DIBuilder. This commit updates DIBuilder methods to accept an InsertPosition instead, so that they can be called with an iterator (preferred), or with a deprecation warning an Instruction *, or a BasicBlock *. This commit also updates the existing calls to the DIBuilder methods to pass in iterators. As a special exception, DIBuilder::insertDeclare() keeps a separate overload accepting a BasicBlock *InsertAtEnd. This is because despite the name, this method does not insert at the end of the block, therefore this cannot be handled implicitly by using InsertPosition. (cherry picked from commit 1083ec6)
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Feb 19, 2025
…llvm#126967) After llvm#124287 updated several functions to return iterators rather than Instruction *, it was no longer straightforward to pass their result to DIBuilder. This commit updates DIBuilder methods to accept an InsertPosition instead, so that they can be called with an iterator (preferred), or with a deprecation warning an Instruction *, or a BasicBlock *. This commit also updates the existing calls to the DIBuilder methods to pass in iterators. As a special exception, DIBuilder::insertDeclare() keeps a separate overload accepting a BasicBlock *InsertAtEnd. This is because despite the name, this method does not insert at the end of the block, therefore this cannot be handled implicitly by using InsertPosition. (cherry picked from commit 1083ec6) Updated with fixes for heterogeneous DWARF patches. Fixes: SWDEV-515458
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Feb 24, 2025
…26059) After llvm#124287 updated several functions to return iterators rather than Instruction *, it was no longer straightforward to pass their result to DIBuilder. This commit updates DIBuilder methods to accept an InsertPosition instead, so that they can be called with an iterator (preferred), or with a deprecation warning an Instruction *, or a BasicBlock *. This commit also updates the existing calls to the DIBuilder methods to pass in iterators.
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Feb 24, 2025
…llvm#126967) After llvm#124287 updated several functions to return iterators rather than Instruction *, it was no longer straightforward to pass their result to DIBuilder. This commit updates DIBuilder methods to accept an InsertPosition instead, so that they can be called with an iterator (preferred), or with a deprecation warning an Instruction *, or a BasicBlock *. This commit also updates the existing calls to the DIBuilder methods to pass in iterators. As a special exception, DIBuilder::insertDeclare() keeps a separate overload accepting a BasicBlock *InsertAtEnd. This is because despite the name, this method does not insert at the end of the block, therefore this cannot be handled implicitly by using InsertPosition.
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.
As part of the "RemoveDIs" work to eliminate debug intrinsics, we're replacing methods that use Instruction*'s as positions with iterators. A number of these (such as getFirstNonPHIOrDbg) are sufficiently infrequently used that we can just replace the pointer-returning version with an iterator-returning version, hopefully without much/any disruption.
Thus this patch has getFirstNonPHIOrDbg and getFirstNonPHIOrDbgOrLifetime return an iterator, and updates all call-sites. There are no concerns about the iterators returned being converted to Instruction*'s and losing the debug-info bit: because the methods skip debug intrinsics, the iterator head bit is always false anyway.