Skip to content

Commit 6942c64

Browse files
committed
[NFC][RemoveDIs] Prefer iterator-insertion over instructions
Continuing the patch series to get rid of debug intrinsics [0], instruction insertion needs to be done with iterators rather than instruction pointers, so that we can communicate information in the iterator class. This patch adds an iterator-taking insertBefore method and converts various call sites to take iterators. These are all sites where such debug-info needs to be preserved so that a stage2 clang can be built identically; it's likely that many more will need to be changed in the future. At this stage, this is just changing the spelling of a few operations, which will eventually become signifiant once the debug-info bearing iterator is used. [0] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939 Differential Revision: https://reviews.llvm.org/D152537
1 parent 3787fd9 commit 6942c64

36 files changed

+159
-109
lines changed

llvm/include/llvm/IR/BasicBlock.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,15 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
173173
static_cast<const BasicBlock *>(this)->getFirstNonPHI());
174174
}
175175

176+
/// Iterator returning form of getFirstNonPHI. Installed as a placeholder for
177+
/// the RemoveDIs project that will eventually remove debug intrinsics.
178+
InstListType::const_iterator getFirstNonPHIIt() const;
179+
InstListType::iterator getFirstNonPHIIt() {
180+
BasicBlock::iterator It =
181+
static_cast<const BasicBlock *>(this)->getFirstNonPHIIt().getNonConst();
182+
return It;
183+
}
184+
176185
/// Returns a pointer to the first instruction in this block that is not a
177186
/// PHINode or a debug intrinsic, or any pseudo operation if \c SkipPseudoOp
178187
/// is true.

llvm/include/llvm/IR/Instruction.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ class Instruction : public User,
123123
/// Insert an unlinked instruction into a basic block immediately before
124124
/// the specified instruction.
125125
void insertBefore(Instruction *InsertPos);
126+
void insertBefore(SymbolTableList<Instruction>::iterator InsertPos) {
127+
insertBefore(&*InsertPos);
128+
}
126129

127130
/// Insert an unlinked instruction into a basic block immediately after the
128131
/// specified instruction.
@@ -133,6 +136,11 @@ class Instruction : public User,
133136
SymbolTableList<Instruction>::iterator
134137
insertInto(BasicBlock *ParentBB, SymbolTableList<Instruction>::iterator It);
135138

139+
void insertBefore(BasicBlock &BB,
140+
SymbolTableList<Instruction>::iterator InsertPos) {
141+
insertInto(&BB, InsertPos);
142+
}
143+
136144
/// Unlink this instruction from its current basic block and insert it into
137145
/// the basic block that MovePos lives in, right before MovePos.
138146
void moveBefore(Instruction *MovePos);

llvm/lib/CodeGen/CodeGenPrepare.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7097,7 +7097,8 @@ bool CodeGenPrepare::optimizeSelectInst(SelectInst *SI) {
70977097
// to get the PHI operand.
70987098
for (SelectInst *SI : llvm::reverse(ASI)) {
70997099
// The select itself is replaced with a PHI Node.
7100-
PHINode *PN = PHINode::Create(SI->getType(), 2, "", &EndBlock->front());
7100+
PHINode *PN = PHINode::Create(SI->getType(), 2, "");
7101+
PN->insertBefore(EndBlock->begin());
71017102
PN->takeName(SI);
71027103
PN->addIncoming(getTrueOrFalseValue(SI, true, INS), TrueBlock);
71037104
PN->addIncoming(getTrueOrFalseValue(SI, false, INS), FalseBlock);

llvm/lib/CodeGen/SelectOptimize.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,8 @@ void SelectOptimize::convertProfitableSIGroups(SelectGroups &ProfSIGroups) {
505505
for (auto It = ASI.rbegin(); It != ASI.rend(); ++It) {
506506
SelectInst *SI = *It;
507507
// The select itself is replaced with a PHI Node.
508-
PHINode *PN = PHINode::Create(SI->getType(), 2, "", &EndBlock->front());
508+
PHINode *PN = PHINode::Create(SI->getType(), 2, "");
509+
PN->insertBefore(EndBlock->begin());
509510
PN->takeName(SI);
510511
PN->addIncoming(getTrueOrFalseValue(SI, true, INS), TrueBlock);
511512
PN->addIncoming(getTrueOrFalseValue(SI, false, INS), FalseBlock);

llvm/lib/IR/BasicBlock.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,10 @@ const Instruction* BasicBlock::getFirstNonPHI() const {
220220
return nullptr;
221221
}
222222

223+
BasicBlock::const_iterator BasicBlock::getFirstNonPHIIt() const {
224+
return getFirstNonPHI()->getIterator();
225+
}
226+
223227
const Instruction *BasicBlock::getFirstNonPHIOrDbg(bool SkipPseudoOp) const {
224228
for (const Instruction &I : *this) {
225229
if (isa<PHINode>(I) || isa<DbgInfoIntrinsic>(I))

llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,8 @@ bool SIAnnotateControlFlow::handleLoop(BranchInst *Term) {
268268
return false;
269269

270270
BasicBlock *Target = Term->getSuccessor(1);
271-
PHINode *Broken = PHINode::Create(IntMask, 0, "phi.broken", &Target->front());
271+
PHINode *Broken = PHINode::Create(IntMask, 0, "phi.broken");
272+
Broken->insertBefore(Target->begin());
272273

273274
Value *Cond = Term->getCondition();
274275
Term->setCondition(BoolTrue);

llvm/lib/Target/PowerPC/PPCLoopInstrFormPrep.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -707,8 +707,8 @@ PPCLoopInstrFormPrep::rewriteForBase(Loop *L, const SCEVAddRecExpr *BasePtrSCEV,
707707
BasicBlock *LoopPredecessor = L->getLoopPredecessor();
708708

709709
PHINode *NewPHI = PHINode::Create(I8PtrTy, HeaderLoopPredCount,
710-
getInstrName(BaseMemI, PHINodeNameSuffix),
711-
Header->getFirstNonPHI());
710+
getInstrName(BaseMemI, PHINodeNameSuffix));
711+
NewPHI->insertBefore(Header->getFirstNonPHIIt());
712712

713713
Value *BasePtrStart = SCEVE.expandCodeFor(BasePtrStartSCEV, I8PtrTy,
714714
LoopPredecessor->getTerminator());

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2051,8 +2051,8 @@ static void movePHIValuesToInsertedBlock(BasicBlock *SuccBB,
20512051
int Index = PN->getBasicBlockIndex(InsertedBB);
20522052
Value *V = PN->getIncomingValue(Index);
20532053
PHINode *InputV = PHINode::Create(
2054-
V->getType(), 1, V->getName() + Twine(".") + SuccBB->getName(),
2055-
&InsertedBB->front());
2054+
V->getType(), 1, V->getName() + Twine(".") + SuccBB->getName());
2055+
InputV->insertBefore(InsertedBB->begin());
20562056
InputV->addIncoming(V, PredBB);
20572057
PN->setIncomingValue(Index, InputV);
20582058
PN = dyn_cast<PHINode>(PN->getNextNode());
@@ -2198,7 +2198,8 @@ static void rewritePHIs(BasicBlock &BB) {
21982198
// ehAwareSplitEdge will clone the LandingPad in all the edge blocks.
21992199
// We replace the original landing pad with a PHINode that will collect the
22002200
// results from all of them.
2201-
ReplPHI = PHINode::Create(LandingPad->getType(), 1, "", LandingPad);
2201+
ReplPHI = PHINode::Create(LandingPad->getType(), 1, "");
2202+
ReplPHI->insertBefore(LandingPad->getIterator());
22022203
ReplPHI->takeName(LandingPad);
22032204
LandingPad->replaceAllUsesWith(ReplPHI);
22042205
// We will erase the original landing pad at the end of this function after

llvm/lib/Transforms/Coroutines/CoroSplit.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,8 @@ static void createResumeEntryBlock(Function &F, coro::Shape &Shape) {
457457
Switch->addCase(IndexVal, ResumeBB);
458458

459459
cast<BranchInst>(SuspendBB->getTerminator())->setSuccessor(0, LandingBB);
460-
auto *PN = PHINode::Create(Builder.getInt8Ty(), 2, "", &LandingBB->front());
460+
auto *PN = PHINode::Create(Builder.getInt8Ty(), 2, "");
461+
PN->insertBefore(LandingBB->begin());
461462
S->replaceAllUsesWith(PN);
462463
PN->addIncoming(Builder.getInt8(-1), SuspendBB);
463464
PN->addIncoming(S, ResumeBB);

llvm/lib/Transforms/IPO/PartialInlining.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,17 +1042,18 @@ void PartialInlinerImpl::FunctionCloner::normalizeReturnBlock() const {
10421042
ClonedOI->ReturnBlock = ClonedOI->ReturnBlock->splitBasicBlock(
10431043
ClonedOI->ReturnBlock->getFirstNonPHI()->getIterator());
10441044
BasicBlock::iterator I = PreReturn->begin();
1045-
Instruction *Ins = &ClonedOI->ReturnBlock->front();
1045+
BasicBlock::iterator Ins = ClonedOI->ReturnBlock->begin();
10461046
SmallVector<Instruction *, 4> DeadPhis;
10471047
while (I != PreReturn->end()) {
10481048
PHINode *OldPhi = dyn_cast<PHINode>(I);
10491049
if (!OldPhi)
10501050
break;
10511051

10521052
PHINode *RetPhi =
1053-
PHINode::Create(OldPhi->getType(), NumPredsFromEntries + 1, "", Ins);
1053+
PHINode::Create(OldPhi->getType(), NumPredsFromEntries + 1, "");
1054+
RetPhi->insertBefore(Ins);
10541055
OldPhi->replaceAllUsesWith(RetPhi);
1055-
Ins = ClonedOI->ReturnBlock->getFirstNonPHI();
1056+
Ins = ClonedOI->ReturnBlock->getFirstNonPHIIt();
10561057

10571058
RetPhi->addIncoming(&*I, PreReturn);
10581059
for (BasicBlock *E : ClonedOI->ReturnBlockPreds) {

0 commit comments

Comments
 (0)