[llvm] [MachineOutliner] Refactor iterating over Candidate's instructions (PR #78972)
Anatoly Trosinenko via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 22 05:53:21 PST 2024
https://github.com/atrosinenko created https://github.com/llvm/llvm-project/pull/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).
>From 1eef917b256fda124575ea122d7a65f6c92fd5ec Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosinenko at accesssoftek.com>
Date: Sun, 21 Jan 2024 12:07:09 +0300
Subject: [PATCH] [MachineOutliner] Refactor iterating over Candidate's
instructions
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).
---
llvm/include/llvm/CodeGen/MachineOutliner.h | 11 ++--
llvm/lib/CodeGen/MachineOutliner.cpp | 27 +++++----
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | 61 +++++++++-----------
llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp | 19 +++---
llvm/lib/Target/RISCV/RISCVInstrInfo.cpp | 6 +-
llvm/lib/Target/X86/X86InstrInfo.cpp | 25 ++++----
6 files changed, 69 insertions(+), 80 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/MachineOutliner.h b/llvm/include/llvm/CodeGen/MachineOutliner.h
index d0ff02fea4ff9b7..eaba6c9b18f2bb7 100644
--- a/llvm/include/llvm/CodeGen/MachineOutliner.h
+++ b/llvm/include/llvm/CodeGen/MachineOutliner.h
@@ -87,7 +87,7 @@ struct Candidate {
// Compute liveness from the end of the block up to the beginning of the
// outlining candidate.
for (auto &MI : make_range(MBB->rbegin(),
- (MachineBasicBlock::reverse_iterator)front()))
+ (MachineBasicBlock::reverse_iterator)begin()))
FromEndOfBlockToStartOfSeq.stepBackward(MI);
}
@@ -100,7 +100,7 @@ struct Candidate {
return;
InSeqWasSet = true;
InSeq.init(TRI);
- for (auto &MI : make_range(front(), std::next(back())))
+ for (auto &MI : *this)
InSeq.accumulate(MI);
}
@@ -135,8 +135,11 @@ struct Candidate {
/// Returns the call overhead of this candidate if it is in the list.
unsigned getCallOverhead() const { return CallOverhead; }
- MachineBasicBlock::iterator &front() { return FirstInst; }
- MachineBasicBlock::iterator &back() { return LastInst; }
+ MachineBasicBlock::iterator begin() { return FirstInst; }
+ MachineBasicBlock::iterator end() { return std::next(LastInst); }
+
+ MachineInstr &front() { return *FirstInst; }
+ MachineInstr &back() { return *LastInst; }
MachineFunction *getMF() const { return MBB->getParent(); }
MachineBasicBlock *getMBB() const { return MBB; }
diff --git a/llvm/lib/CodeGen/MachineOutliner.cpp b/llvm/lib/CodeGen/MachineOutliner.cpp
index a0769105c929246..b8d3b2e30e6e6a7 100644
--- a/llvm/lib/CodeGen/MachineOutliner.cpp
+++ b/llvm/lib/CodeGen/MachineOutliner.cpp
@@ -525,7 +525,7 @@ void MachineOutliner::emitNotOutliningCheaperRemark(
MachineOptimizationRemarkEmitter MORE(*(C.getMF()), nullptr);
MORE.emit([&]() {
MachineOptimizationRemarkMissed R(DEBUG_TYPE, "NotOutliningCheaper",
- C.front()->getDebugLoc(), C.getMBB());
+ C.front().getDebugLoc(), C.getMBB());
R << "Did not outline " << NV("Length", StringLen) << " instructions"
<< " from " << NV("NumOccurrences", CandidatesForRepeatedSeq.size())
<< " locations."
@@ -538,7 +538,7 @@ void MachineOutliner::emitNotOutliningCheaperRemark(
// Tell the user the other places the candidate was found.
for (unsigned i = 1, e = CandidatesForRepeatedSeq.size(); i < e; i++) {
R << NV((Twine("OtherStartLoc") + Twine(i)).str(),
- CandidatesForRepeatedSeq[i].front()->getDebugLoc());
+ CandidatesForRepeatedSeq[i].front().getDebugLoc());
if (i != e - 1)
R << ", ";
}
@@ -563,7 +563,7 @@ void MachineOutliner::emitOutlinedFunctionRemark(OutlinedFunction &OF) {
for (size_t i = 0, e = OF.Candidates.size(); i < e; i++) {
R << NV((Twine("StartLoc") + Twine(i)).str(),
- OF.Candidates[i].front()->getDebugLoc());
+ OF.Candidates[i].front().getDebugLoc());
if (i != e - 1)
R << ", ";
}
@@ -732,23 +732,22 @@ MachineFunction *MachineOutliner::createOutlinedFunction(
// Insert the new function into the module.
MF.insert(MF.begin(), &MBB);
- MachineFunction *OriginalMF = FirstCand.front()->getMF();
+ MachineFunction *OriginalMF = FirstCand.front().getMF();
const std::vector<MCCFIInstruction> &Instrs =
OriginalMF->getFrameInstructions();
- for (auto I = FirstCand.front(), E = std::next(FirstCand.back()); I != E;
- ++I) {
- if (I->isDebugInstr())
+ for (auto &MI : FirstCand) {
+ if (MI.isDebugInstr())
continue;
// Don't keep debug information for outlined instructions.
auto DL = DebugLoc();
- if (I->isCFIInstruction()) {
- unsigned CFIIndex = I->getOperand(0).getCFIIndex();
+ if (MI.isCFIInstruction()) {
+ unsigned CFIIndex = MI.getOperand(0).getCFIIndex();
MCCFIInstruction CFI = Instrs[CFIIndex];
BuildMI(MBB, MBB.end(), DL, TII.get(TargetOpcode::CFI_INSTRUCTION))
.addCFIIndex(MF.addFrameInst(CFI));
} else {
- MachineInstr *NewMI = MF.CloneMachineInstr(&*I);
+ MachineInstr *NewMI = MF.CloneMachineInstr(&MI);
NewMI->dropMemRefs(MF);
NewMI->setDebugLoc(DL);
MBB.insert(MBB.end(), NewMI);
@@ -768,11 +767,11 @@ MachineFunction *MachineOutliner::createOutlinedFunction(
LivePhysRegs LiveIns(TRI);
for (auto &Cand : OF.Candidates) {
// Figure out live-ins at the first instruction.
- MachineBasicBlock &OutlineBB = *Cand.front()->getParent();
+ MachineBasicBlock &OutlineBB = *Cand.front().getParent();
LivePhysRegs CandLiveIns(TRI);
CandLiveIns.addLiveOuts(OutlineBB);
for (const MachineInstr &MI :
- reverse(make_range(Cand.front(), OutlineBB.end())))
+ reverse(make_range(Cand.begin(), OutlineBB.end())))
CandLiveIns.stepBackward(MI);
// The live-in set for the outlined function is the union of the live-ins
@@ -884,8 +883,8 @@ bool MachineOutliner::outline(Module &M,
LLVM_DEBUG(dbgs() << "CREATE OUTLINED CALLS\n");
for (Candidate &C : OF.Candidates) {
MachineBasicBlock &MBB = *C.getMBB();
- MachineBasicBlock::iterator StartIt = C.front();
- MachineBasicBlock::iterator EndIt = C.back();
+ MachineBasicBlock::iterator StartIt = C.begin();
+ MachineBasicBlock::iterator EndIt = std::prev(C.end());
// Insert the call.
auto CallInst = TII.insertOutlinedCall(M, MBB, StartIt, *MF, C);
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index 42b7a6418032adc..68b4750973417a0 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -8227,11 +8227,11 @@ std::optional<outliner::OutlinedFunction>
AArch64InstrInfo::getOutliningCandidateInfo(
std::vector<outliner::Candidate> &RepeatedSequenceLocs) const {
outliner::Candidate &FirstCand = RepeatedSequenceLocs[0];
- unsigned SequenceSize =
- std::accumulate(FirstCand.front(), std::next(FirstCand.back()), 0,
- [this](unsigned Sum, const MachineInstr &MI) {
- return Sum + getInstSizeInBytes(MI);
- });
+
+ unsigned SequenceSize = 0;
+ for (auto &MI : FirstCand)
+ SequenceSize += getInstSizeInBytes(MI);
+
unsigned NumBytesToCreateFrame = 0;
// We only allow outlining for functions having exactly matching return
@@ -8284,7 +8284,7 @@ AArch64InstrInfo::getOutliningCandidateInfo(
AArch64PAuth::getCheckerSizeInBytes(LRCheckMethod);
// Checking the authenticated LR value may significantly impact
// SequenceSize, so account for it for more precise results.
- if (isTailCallReturnInst(*RepeatedSequenceLocs[0].back()))
+ if (isTailCallReturnInst(RepeatedSequenceLocs[0].back()))
SequenceSize += NumBytesToCheckLRInTCEpilogue;
// We have to check if sp modifying instructions would get outlined.
@@ -8293,37 +8293,36 @@ AArch64InstrInfo::getOutliningCandidateInfo(
// are not
auto hasIllegalSPModification = [&TRI](outliner::Candidate &C) {
int SPValue = 0;
- MachineBasicBlock::iterator MBBI = C.front();
- for (;;) {
- if (MBBI->modifiesRegister(AArch64::SP, &TRI)) {
- switch (MBBI->getOpcode()) {
+ for (auto &MI : C) {
+ if (MI.modifiesRegister(AArch64::SP, &TRI)) {
+ switch (MI.getOpcode()) {
case AArch64::ADDXri:
case AArch64::ADDWri:
- assert(MBBI->getNumOperands() == 4 && "Wrong number of operands");
- assert(MBBI->getOperand(2).isImm() &&
+ assert(MI.getNumOperands() == 4 && "Wrong number of operands");
+ assert(MI.getOperand(2).isImm() &&
"Expected operand to be immediate");
- assert(MBBI->getOperand(1).isReg() &&
+ assert(MI.getOperand(1).isReg() &&
"Expected operand to be a register");
// Check if the add just increments sp. If so, we search for
// matching sub instructions that decrement sp. If not, the
// modification is illegal
- if (MBBI->getOperand(1).getReg() == AArch64::SP)
- SPValue += MBBI->getOperand(2).getImm();
+ if (MI.getOperand(1).getReg() == AArch64::SP)
+ SPValue += MI.getOperand(2).getImm();
else
return true;
break;
case AArch64::SUBXri:
case AArch64::SUBWri:
- assert(MBBI->getNumOperands() == 4 && "Wrong number of operands");
- assert(MBBI->getOperand(2).isImm() &&
+ assert(MI.getNumOperands() == 4 && "Wrong number of operands");
+ assert(MI.getOperand(2).isImm() &&
"Expected operand to be immediate");
- assert(MBBI->getOperand(1).isReg() &&
+ assert(MI.getOperand(1).isReg() &&
"Expected operand to be a register");
// Check if the sub just decrements sp. If so, we search for
// matching add instructions that increment sp. If not, the
// modification is illegal
- if (MBBI->getOperand(1).getReg() == AArch64::SP)
- SPValue -= MBBI->getOperand(2).getImm();
+ if (MI.getOperand(1).getReg() == AArch64::SP)
+ SPValue -= MI.getOperand(2).getImm();
else
return true;
break;
@@ -8331,9 +8330,6 @@ AArch64InstrInfo::getOutliningCandidateInfo(
return true;
}
}
- if (MBBI == C.back())
- break;
- ++MBBI;
}
if (SPValue)
return true;
@@ -8354,7 +8350,7 @@ AArch64InstrInfo::getOutliningCandidateInfo(
for (outliner::Candidate &C : RepeatedSequenceLocs)
FlagsSetInAll &= C.Flags;
- unsigned LastInstrOpcode = RepeatedSequenceLocs[0].back()->getOpcode();
+ unsigned LastInstrOpcode = RepeatedSequenceLocs[0].back().getOpcode();
// Helper lambda which sets call information for every candidate.
auto SetCandidateCallInfo =
@@ -8373,8 +8369,7 @@ AArch64InstrInfo::getOutliningCandidateInfo(
// We check to see if CFI Instructions are present, and if they are
// we find the number of CFI Instructions in the candidates.
unsigned CFICount = 0;
- for (auto &I : make_range(RepeatedSequenceLocs[0].front(),
- std::next(RepeatedSequenceLocs[0].back()))) {
+ for (auto &I : RepeatedSequenceLocs[0]) {
if (I.isCFIInstruction())
CFICount++;
}
@@ -8449,12 +8444,11 @@ AArch64InstrInfo::getOutliningCandidateInfo(
// True if it's possible to fix up each stack instruction in this sequence.
// Important for frames/call variants that modify the stack.
- bool AllStackInstrsSafe = std::all_of(
- FirstCand.front(), std::next(FirstCand.back()), IsSafeToFixup);
+ bool AllStackInstrsSafe = llvm::all_of(FirstCand, IsSafeToFixup);
// If the last instruction in any candidate is a terminator, then we should
// tail call all of the candidates.
- if (RepeatedSequenceLocs[0].back()->isTerminator()) {
+ if (RepeatedSequenceLocs[0].back().isTerminator()) {
FrameID = MachineOutlinerTailCall;
NumBytesToCreateFrame = 0;
unsigned NumBytesForCall = 4 + NumBytesToCheckLRInTCEpilogue;
@@ -8580,9 +8574,8 @@ AArch64InstrInfo::getOutliningCandidateInfo(
//
if (FlagsSetInAll & MachineOutlinerMBBFlags::HasCalls) {
erase_if(RepeatedSequenceLocs, [this, &TRI](outliner::Candidate &C) {
- return (std::any_of(
- C.front(), std::next(C.back()),
- [](const MachineInstr &MI) { return MI.isCall(); })) &&
+ auto IsCall = [](const MachineInstr &MI) { return MI.isCall(); };
+ return (llvm::any_of(C, IsCall)) &&
(!C.isAvailableAcrossAndOutOfSeq(AArch64::LR, TRI) ||
!findRegisterToSaveLRTo(C));
});
@@ -8602,7 +8595,7 @@ AArch64InstrInfo::getOutliningCandidateInfo(
// Check if the range contains a call. These require a save + restore of the
// link register.
bool ModStackToSaveLR = false;
- if (std::any_of(FirstCand.front(), FirstCand.back(),
+ if (std::any_of(FirstCand.begin(), std::prev(FirstCand.end()),
[](const MachineInstr &MI) { return MI.isCall(); }))
ModStackToSaveLR = true;
@@ -8612,7 +8605,7 @@ AArch64InstrInfo::getOutliningCandidateInfo(
// it being valid to tail call this sequence. We should consider this as
// well.
else if (FrameID != MachineOutlinerThunk &&
- FrameID != MachineOutlinerTailCall && FirstCand.back()->isCall())
+ FrameID != MachineOutlinerTailCall && FirstCand.back().isCall())
ModStackToSaveLR = true;
if (ModStackToSaveLR) {
diff --git a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
index ef02dc99701149d..4bf65be6f10262e 100644
--- a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
@@ -5874,11 +5874,10 @@ std::optional<outliner::OutlinedFunction>
ARMBaseInstrInfo::getOutliningCandidateInfo(
std::vector<outliner::Candidate> &RepeatedSequenceLocs) const {
outliner::Candidate &FirstCand = RepeatedSequenceLocs[0];
- unsigned SequenceSize =
- std::accumulate(FirstCand.front(), std::next(FirstCand.back()), 0,
- [this](unsigned Sum, const MachineInstr &MI) {
- return Sum + getInstSizeInBytes(MI);
- });
+
+ unsigned SequenceSize = 0;
+ for (auto &MI : FirstCand)
+ SequenceSize += getInstSizeInBytes(MI);
// Properties about candidate MBBs that hold for all of them.
unsigned FlagsSetInAll = 0xF;
@@ -5965,7 +5964,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo(
// At this point, we have only "safe" candidates to outline. Figure out
// frame + call instruction information.
- unsigned LastInstrOpcode = RepeatedSequenceLocs[0].back()->getOpcode();
+ unsigned LastInstrOpcode = RepeatedSequenceLocs[0].back().getOpcode();
// Helper lambda which sets call information for every candidate.
auto SetCandidateCallInfo =
@@ -5998,7 +5997,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo(
// If the last instruction in any candidate is a terminator, then we should
// tail call all of the candidates.
- if (RepeatedSequenceLocs[0].back()->isTerminator()) {
+ if (RepeatedSequenceLocs[0].back().isTerminator()) {
FrameID = MachineOutlinerTailCall;
NumBytesToCreateFrame = Costs.FrameTailCall;
SetCandidateCallInfo(MachineOutlinerTailCall, Costs.CallTailCall);
@@ -6024,7 +6023,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo(
const bool LRIsAvailable =
C.getMBB()->isReturnBlock() && !Last->isCall()
? isLRAvailable(TRI, Last,
- (MachineBasicBlock::reverse_iterator)C.front())
+ (MachineBasicBlock::reverse_iterator)C.begin())
: C.isAvailableAcrossAndOutOfSeq(ARM::LR, TRI);
if (LRIsAvailable) {
FrameID = MachineOutlinerNoLRSave;
@@ -6072,7 +6071,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo(
if (FlagsSetInAll & MachineOutlinerMBBFlags::HasCalls) {
// check if the range contains a call. These require a save + restore of
// the link register.
- if (std::any_of(FirstCand.front(), FirstCand.back(),
+ if (std::any_of(FirstCand.begin(), std::prev(FirstCand.end()),
[](const MachineInstr &MI) { return MI.isCall(); }))
NumBytesToCreateFrame += Costs.SaveRestoreLROnStack;
@@ -6082,7 +6081,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo(
// call without it being valid to tail call this sequence. We should
// consider this as well.
else if (FrameID != MachineOutlinerThunk &&
- FrameID != MachineOutlinerTailCall && FirstCand.back()->isCall())
+ FrameID != MachineOutlinerTailCall && FirstCand.back().isCall())
NumBytesToCreateFrame += Costs.SaveRestoreLROnStack;
}
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 9813c7a70dfc315..bed8a3dba000eaa 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -2434,10 +2434,8 @@ RISCVInstrInfo::getOutliningCandidateInfo(
unsigned SequenceSize = 0;
- auto I = RepeatedSequenceLocs[0].front();
- auto E = std::next(RepeatedSequenceLocs[0].back());
- for (; I != E; ++I)
- SequenceSize += getInstSizeInBytes(*I);
+ for (auto &MI : RepeatedSequenceLocs[0])
+ SequenceSize += getInstSizeInBytes(MI);
// call t0, function = 8 bytes.
unsigned CallOverhead = 8;
diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp
index 36022ef35118fe8..b07665351e63bb0 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.cpp
+++ b/llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -10389,23 +10389,20 @@ enum MachineOutlinerClass { MachineOutlinerDefault, MachineOutlinerTailCall };
std::optional<outliner::OutlinedFunction>
X86InstrInfo::getOutliningCandidateInfo(
std::vector<outliner::Candidate> &RepeatedSequenceLocs) const {
- unsigned SequenceSize =
- std::accumulate(RepeatedSequenceLocs[0].front(),
- std::next(RepeatedSequenceLocs[0].back()), 0,
- [](unsigned Sum, const MachineInstr &MI) {
- // FIXME: x86 doesn't implement getInstSizeInBytes, so
- // we can't tell the cost. Just assume each instruction
- // is one byte.
- if (MI.isDebugInstr() || MI.isKill())
- return Sum;
- return Sum + 1;
- });
+ unsigned SequenceSize = 0;
+ for (auto &MI : RepeatedSequenceLocs[0]) {
+ // FIXME: x86 doesn't implement getInstSizeInBytes, so
+ // we can't tell the cost. Just assume each instruction
+ // is one byte.
+ if (MI.isDebugInstr() || MI.isKill())
+ continue;
+ SequenceSize += 1;
+ }
// We check to see if CFI Instructions are present, and if they are
// we find the number of CFI Instructions in the candidates.
unsigned CFICount = 0;
- for (auto &I : make_range(RepeatedSequenceLocs[0].front(),
- std::next(RepeatedSequenceLocs[0].back()))) {
+ for (auto &I : RepeatedSequenceLocs[0]) {
if (I.isCFIInstruction())
CFICount++;
}
@@ -10424,7 +10421,7 @@ X86InstrInfo::getOutliningCandidateInfo(
}
// FIXME: Use real size in bytes for call and ret instructions.
- if (RepeatedSequenceLocs[0].back()->isTerminator()) {
+ if (RepeatedSequenceLocs[0].back().isTerminator()) {
for (outliner::Candidate &C : RepeatedSequenceLocs)
C.setCallInfo(MachineOutlinerTailCall, 1);
More information about the llvm-commits
mailing list