Skip to content

Commit 10bd69a

Browse files
atrosinenkoatrosinenko
authored
[MachineOutliner] Refactor iterating over Candidate's instructions (#78972)
Make Candidate's front() and back() functions return references to MachineInstr and introduce begin() and end() returning iterators, the same way it is usually done in other container-like classes. This makes possible to iterate over the instructions contained in Candidate the same way one can iterate over MachineBasicBlock (note that begin() and end() return bundled iterators, just like MachineBasicBlock does, but no instr_begin() and instr_end() are defined yet).
1 parent 654131f commit 10bd69a

File tree

6 files changed

+69
-80
lines changed

6 files changed

+69
-80
lines changed

‎llvm/include/llvm/CodeGen/MachineOutliner.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ struct Candidate {
8787
// Compute liveness from the end of the block up to the beginning of the
8888
// outlining candidate.
8989
for (auto &MI : make_range(MBB->rbegin(),
90-
(MachineBasicBlock::reverse_iterator)front()))
90+
(MachineBasicBlock::reverse_iterator)begin()))
9191
FromEndOfBlockToStartOfSeq.stepBackward(MI);
9292
}
9393

@@ -100,7 +100,7 @@ struct Candidate {
100100
return;
101101
InSeqWasSet = true;
102102
InSeq.init(TRI);
103-
for (auto &MI : make_range(front(), std::next(back())))
103+
for (auto &MI : *this)
104104
InSeq.accumulate(MI);
105105
}
106106

@@ -135,8 +135,11 @@ struct Candidate {
135135
/// Returns the call overhead of this candidate if it is in the list.
136136
unsigned getCallOverhead() const { return CallOverhead; }
137137

138-
MachineBasicBlock::iterator &front() { return FirstInst; }
139-
MachineBasicBlock::iterator &back() { return LastInst; }
138+
MachineBasicBlock::iterator begin() { return FirstInst; }
139+
MachineBasicBlock::iterator end() { return std::next(LastInst); }
140+
141+
MachineInstr &front() { return *FirstInst; }
142+
MachineInstr &back() { return *LastInst; }
140143
MachineFunction *getMF() const { return MBB->getParent(); }
141144
MachineBasicBlock *getMBB() const { return MBB; }
142145

‎llvm/lib/CodeGen/MachineOutliner.cpp

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ void MachineOutliner::emitNotOutliningCheaperRemark(
525525
MachineOptimizationRemarkEmitter MORE(*(C.getMF()), nullptr);
526526
MORE.emit([&]() {
527527
MachineOptimizationRemarkMissed R(DEBUG_TYPE, "NotOutliningCheaper",
528-
C.front()->getDebugLoc(), C.getMBB());
528+
C.front().getDebugLoc(), C.getMBB());
529529
R << "Did not outline " << NV("Length", StringLen) << " instructions"
530530
<< " from " << NV("NumOccurrences", CandidatesForRepeatedSeq.size())
531531
<< " locations."
@@ -538,7 +538,7 @@ void MachineOutliner::emitNotOutliningCheaperRemark(
538538
// Tell the user the other places the candidate was found.
539539
for (unsigned i = 1, e = CandidatesForRepeatedSeq.size(); i < e; i++) {
540540
R << NV((Twine("OtherStartLoc") + Twine(i)).str(),
541-
CandidatesForRepeatedSeq[i].front()->getDebugLoc());
541+
CandidatesForRepeatedSeq[i].front().getDebugLoc());
542542
if (i != e - 1)
543543
R << ", ";
544544
}
@@ -563,7 +563,7 @@ void MachineOutliner::emitOutlinedFunctionRemark(OutlinedFunction &OF) {
563563
for (size_t i = 0, e = OF.Candidates.size(); i < e; i++) {
564564

565565
R << NV((Twine("StartLoc") + Twine(i)).str(),
566-
OF.Candidates[i].front()->getDebugLoc());
566+
OF.Candidates[i].front().getDebugLoc());
567567
if (i != e - 1)
568568
R << ", ";
569569
}
@@ -732,23 +732,22 @@ MachineFunction *MachineOutliner::createOutlinedFunction(
732732
// Insert the new function into the module.
733733
MF.insert(MF.begin(), &MBB);
734734

735-
MachineFunction *OriginalMF = FirstCand.front()->getMF();
735+
MachineFunction *OriginalMF = FirstCand.front().getMF();
736736
const std::vector<MCCFIInstruction> &Instrs =
737737
OriginalMF->getFrameInstructions();
738-
for (auto I = FirstCand.front(), E = std::next(FirstCand.back()); I != E;
739-
++I) {
740-
if (I->isDebugInstr())
738+
for (auto &MI : FirstCand) {
739+
if (MI.isDebugInstr())
741740
continue;
742741

743742
// Don't keep debug information for outlined instructions.
744743
auto DL = DebugLoc();
745-
if (I->isCFIInstruction()) {
746-
unsigned CFIIndex = I->getOperand(0).getCFIIndex();
744+
if (MI.isCFIInstruction()) {
745+
unsigned CFIIndex = MI.getOperand(0).getCFIIndex();
747746
MCCFIInstruction CFI = Instrs[CFIIndex];
748747
BuildMI(MBB, MBB.end(), DL, TII.get(TargetOpcode::CFI_INSTRUCTION))
749748
.addCFIIndex(MF.addFrameInst(CFI));
750749
} else {
751-
MachineInstr *NewMI = MF.CloneMachineInstr(&*I);
750+
MachineInstr *NewMI = MF.CloneMachineInstr(&MI);
752751
NewMI->dropMemRefs(MF);
753752
NewMI->setDebugLoc(DL);
754753
MBB.insert(MBB.end(), NewMI);
@@ -768,11 +767,11 @@ MachineFunction *MachineOutliner::createOutlinedFunction(
768767
LivePhysRegs LiveIns(TRI);
769768
for (auto &Cand : OF.Candidates) {
770769
// Figure out live-ins at the first instruction.
771-
MachineBasicBlock &OutlineBB = *Cand.front()->getParent();
770+
MachineBasicBlock &OutlineBB = *Cand.front().getParent();
772771
LivePhysRegs CandLiveIns(TRI);
773772
CandLiveIns.addLiveOuts(OutlineBB);
774773
for (const MachineInstr &MI :
775-
reverse(make_range(Cand.front(), OutlineBB.end())))
774+
reverse(make_range(Cand.begin(), OutlineBB.end())))
776775
CandLiveIns.stepBackward(MI);
777776

778777
// The live-in set for the outlined function is the union of the live-ins
@@ -884,8 +883,8 @@ bool MachineOutliner::outline(Module &M,
884883
LLVM_DEBUG(dbgs() << "CREATE OUTLINED CALLS\n");
885884
for (Candidate &C : OF.Candidates) {
886885
MachineBasicBlock &MBB = *C.getMBB();
887-
MachineBasicBlock::iterator StartIt = C.front();
888-
MachineBasicBlock::iterator EndIt = C.back();
886+
MachineBasicBlock::iterator StartIt = C.begin();
887+
MachineBasicBlock::iterator EndIt = std::prev(C.end());
889888

890889
// Insert the call.
891890
auto CallInst = TII.insertOutlinedCall(M, MBB, StartIt, *MF, C);

‎llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

Lines changed: 27 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -8230,11 +8230,11 @@ std::optional<outliner::OutlinedFunction>
82308230
AArch64InstrInfo::getOutliningCandidateInfo(
82318231
std::vector<outliner::Candidate> &RepeatedSequenceLocs) const {
82328232
outliner::Candidate &FirstCand = RepeatedSequenceLocs[0];
8233-
unsigned SequenceSize =
8234-
std::accumulate(FirstCand.front(), std::next(FirstCand.back()), 0,
8235-
[this](unsigned Sum, const MachineInstr &MI) {
8236-
return Sum + getInstSizeInBytes(MI);
8237-
});
8233+
8234+
unsigned SequenceSize = 0;
8235+
for (auto &MI : FirstCand)
8236+
SequenceSize += getInstSizeInBytes(MI);
8237+
82388238
unsigned NumBytesToCreateFrame = 0;
82398239

82408240
// We only allow outlining for functions having exactly matching return
@@ -8287,7 +8287,7 @@ AArch64InstrInfo::getOutliningCandidateInfo(
82878287
AArch64PAuth::getCheckerSizeInBytes(LRCheckMethod);
82888288
// Checking the authenticated LR value may significantly impact
82898289
// SequenceSize, so account for it for more precise results.
8290-
if (isTailCallReturnInst(*RepeatedSequenceLocs[0].back()))
8290+
if (isTailCallReturnInst(RepeatedSequenceLocs[0].back()))
82918291
SequenceSize += NumBytesToCheckLRInTCEpilogue;
82928292

82938293
// We have to check if sp modifying instructions would get outlined.
@@ -8296,47 +8296,43 @@ AArch64InstrInfo::getOutliningCandidateInfo(
82968296
// are not
82978297
auto hasIllegalSPModification = [&TRI](outliner::Candidate &C) {
82988298
int SPValue = 0;
8299-
MachineBasicBlock::iterator MBBI = C.front();
8300-
for (;;) {
8301-
if (MBBI->modifiesRegister(AArch64::SP, &TRI)) {
8302-
switch (MBBI->getOpcode()) {
8299+
for (auto &MI : C) {
8300+
if (MI.modifiesRegister(AArch64::SP, &TRI)) {
8301+
switch (MI.getOpcode()) {
83038302
case AArch64::ADDXri:
83048303
case AArch64::ADDWri:
8305-
assert(MBBI->getNumOperands() == 4 && "Wrong number of operands");
8306-
assert(MBBI->getOperand(2).isImm() &&
8304+
assert(MI.getNumOperands() == 4 && "Wrong number of operands");
8305+
assert(MI.getOperand(2).isImm() &&
83078306
"Expected operand to be immediate");
8308-
assert(MBBI->getOperand(1).isReg() &&
8307+
assert(MI.getOperand(1).isReg() &&
83098308
"Expected operand to be a register");
83108309
// Check if the add just increments sp. If so, we search for
83118310
// matching sub instructions that decrement sp. If not, the
83128311
// modification is illegal
8313-
if (MBBI->getOperand(1).getReg() == AArch64::SP)
8314-
SPValue += MBBI->getOperand(2).getImm();
8312+
if (MI.getOperand(1).getReg() == AArch64::SP)
8313+
SPValue += MI.getOperand(2).getImm();
83158314
else
83168315
return true;
83178316
break;
83188317
case AArch64::SUBXri:
83198318
case AArch64::SUBWri:
8320-
assert(MBBI->getNumOperands() == 4 && "Wrong number of operands");
8321-
assert(MBBI->getOperand(2).isImm() &&
8319+
assert(MI.getNumOperands() == 4 && "Wrong number of operands");
8320+
assert(MI.getOperand(2).isImm() &&
83228321
"Expected operand to be immediate");
8323-
assert(MBBI->getOperand(1).isReg() &&
8322+
assert(MI.getOperand(1).isReg() &&
83248323
"Expected operand to be a register");
83258324
// Check if the sub just decrements sp. If so, we search for
83268325
// matching add instructions that increment sp. If not, the
83278326
// modification is illegal
8328-
if (MBBI->getOperand(1).getReg() == AArch64::SP)
8329-
SPValue -= MBBI->getOperand(2).getImm();
8327+
if (MI.getOperand(1).getReg() == AArch64::SP)
8328+
SPValue -= MI.getOperand(2).getImm();
83308329
else
83318330
return true;
83328331
break;
83338332
default:
83348333
return true;
83358334
}
83368335
}
8337-
if (MBBI == C.back())
8338-
break;
8339-
++MBBI;
83408336
}
83418337
if (SPValue)
83428338
return true;
@@ -8357,7 +8353,7 @@ AArch64InstrInfo::getOutliningCandidateInfo(
83578353
for (outliner::Candidate &C : RepeatedSequenceLocs)
83588354
FlagsSetInAll &= C.Flags;
83598355

8360-
unsigned LastInstrOpcode = RepeatedSequenceLocs[0].back()->getOpcode();
8356+
unsigned LastInstrOpcode = RepeatedSequenceLocs[0].back().getOpcode();
83618357

83628358
// Helper lambda which sets call information for every candidate.
83638359
auto SetCandidateCallInfo =
@@ -8376,8 +8372,7 @@ AArch64InstrInfo::getOutliningCandidateInfo(
83768372
// We check to see if CFI Instructions are present, and if they are
83778373
// we find the number of CFI Instructions in the candidates.
83788374
unsigned CFICount = 0;
8379-
for (auto &I : make_range(RepeatedSequenceLocs[0].front(),
8380-
std::next(RepeatedSequenceLocs[0].back()))) {
8375+
for (auto &I : RepeatedSequenceLocs[0]) {
83818376
if (I.isCFIInstruction())
83828377
CFICount++;
83838378
}
@@ -8452,12 +8447,11 @@ AArch64InstrInfo::getOutliningCandidateInfo(
84528447

84538448
// True if it's possible to fix up each stack instruction in this sequence.
84548449
// Important for frames/call variants that modify the stack.
8455-
bool AllStackInstrsSafe = std::all_of(
8456-
FirstCand.front(), std::next(FirstCand.back()), IsSafeToFixup);
8450+
bool AllStackInstrsSafe = llvm::all_of(FirstCand, IsSafeToFixup);
84578451

84588452
// If the last instruction in any candidate is a terminator, then we should
84598453
// tail call all of the candidates.
8460-
if (RepeatedSequenceLocs[0].back()->isTerminator()) {
8454+
if (RepeatedSequenceLocs[0].back().isTerminator()) {
84618455
FrameID = MachineOutlinerTailCall;
84628456
NumBytesToCreateFrame = 0;
84638457
unsigned NumBytesForCall = 4 + NumBytesToCheckLRInTCEpilogue;
@@ -8583,9 +8577,8 @@ AArch64InstrInfo::getOutliningCandidateInfo(
85838577
//
85848578
if (FlagsSetInAll & MachineOutlinerMBBFlags::HasCalls) {
85858579
erase_if(RepeatedSequenceLocs, [this, &TRI](outliner::Candidate &C) {
8586-
return (std::any_of(
8587-
C.front(), std::next(C.back()),
8588-
[](const MachineInstr &MI) { return MI.isCall(); })) &&
8580+
auto IsCall = [](const MachineInstr &MI) { return MI.isCall(); };
8581+
return (llvm::any_of(C, IsCall)) &&
85898582
(!C.isAvailableAcrossAndOutOfSeq(AArch64::LR, TRI) ||
85908583
!findRegisterToSaveLRTo(C));
85918584
});
@@ -8605,7 +8598,7 @@ AArch64InstrInfo::getOutliningCandidateInfo(
86058598
// Check if the range contains a call. These require a save + restore of the
86068599
// link register.
86078600
bool ModStackToSaveLR = false;
8608-
if (std::any_of(FirstCand.front(), FirstCand.back(),
8601+
if (std::any_of(FirstCand.begin(), std::prev(FirstCand.end()),
86098602
[](const MachineInstr &MI) { return MI.isCall(); }))
86108603
ModStackToSaveLR = true;
86118604

@@ -8615,7 +8608,7 @@ AArch64InstrInfo::getOutliningCandidateInfo(
86158608
// it being valid to tail call this sequence. We should consider this as
86168609
// well.
86178610
else if (FrameID != MachineOutlinerThunk &&
8618-
FrameID != MachineOutlinerTailCall && FirstCand.back()->isCall())
8611+
FrameID != MachineOutlinerTailCall && FirstCand.back().isCall())
86198612
ModStackToSaveLR = true;
86208613

86218614
if (ModStackToSaveLR) {

‎llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5874,11 +5874,10 @@ std::optional<outliner::OutlinedFunction>
58745874
ARMBaseInstrInfo::getOutliningCandidateInfo(
58755875
std::vector<outliner::Candidate> &RepeatedSequenceLocs) const {
58765876
outliner::Candidate &FirstCand = RepeatedSequenceLocs[0];
5877-
unsigned SequenceSize =
5878-
std::accumulate(FirstCand.front(), std::next(FirstCand.back()), 0,
5879-
[this](unsigned Sum, const MachineInstr &MI) {
5880-
return Sum + getInstSizeInBytes(MI);
5881-
});
5877+
5878+
unsigned SequenceSize = 0;
5879+
for (auto &MI : FirstCand)
5880+
SequenceSize += getInstSizeInBytes(MI);
58825881

58835882
// Properties about candidate MBBs that hold for all of them.
58845883
unsigned FlagsSetInAll = 0xF;
@@ -5965,7 +5964,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo(
59655964
// At this point, we have only "safe" candidates to outline. Figure out
59665965
// frame + call instruction information.
59675966

5968-
unsigned LastInstrOpcode = RepeatedSequenceLocs[0].back()->getOpcode();
5967+
unsigned LastInstrOpcode = RepeatedSequenceLocs[0].back().getOpcode();
59695968

59705969
// Helper lambda which sets call information for every candidate.
59715970
auto SetCandidateCallInfo =
@@ -5998,7 +5997,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo(
59985997

59995998
// If the last instruction in any candidate is a terminator, then we should
60005999
// tail call all of the candidates.
6001-
if (RepeatedSequenceLocs[0].back()->isTerminator()) {
6000+
if (RepeatedSequenceLocs[0].back().isTerminator()) {
60026001
FrameID = MachineOutlinerTailCall;
60036002
NumBytesToCreateFrame = Costs.FrameTailCall;
60046003
SetCandidateCallInfo(MachineOutlinerTailCall, Costs.CallTailCall);
@@ -6024,7 +6023,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo(
60246023
const bool LRIsAvailable =
60256024
C.getMBB()->isReturnBlock() && !Last->isCall()
60266025
? isLRAvailable(TRI, Last,
6027-
(MachineBasicBlock::reverse_iterator)C.front())
6026+
(MachineBasicBlock::reverse_iterator)C.begin())
60286027
: C.isAvailableAcrossAndOutOfSeq(ARM::LR, TRI);
60296028
if (LRIsAvailable) {
60306029
FrameID = MachineOutlinerNoLRSave;
@@ -6072,7 +6071,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo(
60726071
if (FlagsSetInAll & MachineOutlinerMBBFlags::HasCalls) {
60736072
// check if the range contains a call. These require a save + restore of
60746073
// the link register.
6075-
if (std::any_of(FirstCand.front(), FirstCand.back(),
6074+
if (std::any_of(FirstCand.begin(), std::prev(FirstCand.end()),
60766075
[](const MachineInstr &MI) { return MI.isCall(); }))
60776076
NumBytesToCreateFrame += Costs.SaveRestoreLROnStack;
60786077

@@ -6082,7 +6081,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo(
60826081
// call without it being valid to tail call this sequence. We should
60836082
// consider this as well.
60846083
else if (FrameID != MachineOutlinerThunk &&
6085-
FrameID != MachineOutlinerTailCall && FirstCand.back()->isCall())
6084+
FrameID != MachineOutlinerTailCall && FirstCand.back().isCall())
60866085
NumBytesToCreateFrame += Costs.SaveRestoreLROnStack;
60876086
}
60886087

‎llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2434,10 +2434,8 @@ RISCVInstrInfo::getOutliningCandidateInfo(
24342434

24352435
unsigned SequenceSize = 0;
24362436

2437-
auto I = RepeatedSequenceLocs[0].front();
2438-
auto E = std::next(RepeatedSequenceLocs[0].back());
2439-
for (; I != E; ++I)
2440-
SequenceSize += getInstSizeInBytes(*I);
2437+
for (auto &MI : RepeatedSequenceLocs[0])
2438+
SequenceSize += getInstSizeInBytes(MI);
24412439

24422440
// call t0, function = 8 bytes.
24432441
unsigned CallOverhead = 8;

‎llvm/lib/Target/X86/X86InstrInfo.cpp

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10387,23 +10387,20 @@ enum MachineOutlinerClass { MachineOutlinerDefault, MachineOutlinerTailCall };
1038710387
std::optional<outliner::OutlinedFunction>
1038810388
X86InstrInfo::getOutliningCandidateInfo(
1038910389
std::vector<outliner::Candidate> &RepeatedSequenceLocs) const {
10390-
unsigned SequenceSize =
10391-
std::accumulate(RepeatedSequenceLocs[0].front(),
10392-
std::next(RepeatedSequenceLocs[0].back()), 0,
10393-
[](unsigned Sum, const MachineInstr &MI) {
10394-
// FIXME: x86 doesn't implement getInstSizeInBytes, so
10395-
// we can't tell the cost. Just assume each instruction
10396-
// is one byte.
10397-
if (MI.isDebugInstr() || MI.isKill())
10398-
return Sum;
10399-
return Sum + 1;
10400-
});
10390+
unsigned SequenceSize = 0;
10391+
for (auto &MI : RepeatedSequenceLocs[0]) {
10392+
// FIXME: x86 doesn't implement getInstSizeInBytes, so
10393+
// we can't tell the cost. Just assume each instruction
10394+
// is one byte.
10395+
if (MI.isDebugInstr() || MI.isKill())
10396+
continue;
10397+
SequenceSize += 1;
10398+
}
1040110399

1040210400
// We check to see if CFI Instructions are present, and if they are
1040310401
// we find the number of CFI Instructions in the candidates.
1040410402
unsigned CFICount = 0;
10405-
for (auto &I : make_range(RepeatedSequenceLocs[0].front(),
10406-
std::next(RepeatedSequenceLocs[0].back()))) {
10403+
for (auto &I : RepeatedSequenceLocs[0]) {
1040710404
if (I.isCFIInstruction())
1040810405
CFICount++;
1040910406
}
@@ -10422,7 +10419,7 @@ X86InstrInfo::getOutliningCandidateInfo(
1042210419
}
1042310420

1042410421
// FIXME: Use real size in bytes for call and ret instructions.
10425-
if (RepeatedSequenceLocs[0].back()->isTerminator()) {
10422+
if (RepeatedSequenceLocs[0].back().isTerminator()) {
1042610423
for (outliner::Candidate &C : RepeatedSequenceLocs)
1042710424
C.setCallInfo(MachineOutlinerTailCall, 1);
1042810425

0 commit comments

Comments
 (0)