Skip to content

Commit

Permalink
[MachineOutliner][NFC] Refactor (#105398)
Browse files Browse the repository at this point in the history
This patch prepares the NFC groundwork for global outlining using
CGData, which will follow
#90074.

- The `MinRepeats` parameter is now explicitly passed to the
`getOutliningCandidateInfo` function, rather than relying on a default
value of 2. For local outlining, the minimum number of repetitions is
typically 2, but for the global outlining (mentioned above), we will
optimistically create a single `Candidate` for each `OutlinedFunction`
if stable hashes match a specific code sequence. This parameter is
adjusted accordingly in global outlining scenarios.
- I have also implemented `unique_ptr` for `OutlinedFunction` to ensure
safe and efficient memory management within `FunctionList`, avoiding
unnecessary implicit copies.

This depends on #101461.
This is a patch for
https://discourse.llvm.org/t/rfc-enhanced-machine-outliner-part-2-thinlto-nolto/78753.
  • Loading branch information
kyulee-com authored Aug 27, 2024
1 parent e19c3a7 commit 93b8d07
Show file tree
Hide file tree
Showing 10 changed files with 92 additions and 69 deletions.
9 changes: 6 additions & 3 deletions llvm/include/llvm/CodeGen/TargetInstrInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -2094,10 +2094,13 @@ class TargetInstrInfo : public MCInstrInfo {

/// Returns a \p outliner::OutlinedFunction struct containing target-specific
/// information for a set of outlining candidates. Returns std::nullopt if the
/// candidates are not suitable for outlining.
virtual std::optional<outliner::OutlinedFunction> getOutliningCandidateInfo(
/// candidates are not suitable for outlining. \p MinRepeats is the minimum
/// number of times the instruction sequence must be repeated.
virtual std::optional<std::unique_ptr<outliner::OutlinedFunction>>
getOutliningCandidateInfo(
const MachineModuleInfo &MMI,
std::vector<outliner::Candidate> &RepeatedSequenceLocs) const {
std::vector<outliner::Candidate> &RepeatedSequenceLocs,
unsigned MinRepeats) const {
llvm_unreachable(
"Target didn't implement TargetInstrInfo::getOutliningCandidateInfo!");
}
Expand Down
70 changes: 38 additions & 32 deletions llvm/lib/CodeGen/MachineOutliner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -456,16 +456,19 @@ struct MachineOutliner : public ModulePass {
/// \param Mapper Contains outlining mapping information.
/// \param[out] FunctionList Filled with a list of \p OutlinedFunctions
/// each type of candidate.
void findCandidates(InstructionMapper &Mapper,
std::vector<OutlinedFunction> &FunctionList);
void
findCandidates(InstructionMapper &Mapper,
std::vector<std::unique_ptr<OutlinedFunction>> &FunctionList);

/// Replace the sequences of instructions represented by \p OutlinedFunctions
/// with calls to functions.
///
/// \param M The module we are outlining from.
/// \param FunctionList A list of functions to be inserted into the module.
/// \param Mapper Contains the instruction mappings for the module.
bool outline(Module &M, std::vector<OutlinedFunction> &FunctionList,
/// \param[out] OutlinedFunctionNum The outlined function number.
bool outline(Module &M,
std::vector<std::unique_ptr<OutlinedFunction>> &FunctionList,
InstructionMapper &Mapper, unsigned &OutlinedFunctionNum);

/// Creates a function for \p OF and inserts it into the module.
Expand Down Expand Up @@ -583,7 +586,8 @@ void MachineOutliner::emitOutlinedFunctionRemark(OutlinedFunction &OF) {
}

void MachineOutliner::findCandidates(
InstructionMapper &Mapper, std::vector<OutlinedFunction> &FunctionList) {
InstructionMapper &Mapper,
std::vector<std::unique_ptr<OutlinedFunction>> &FunctionList) {
FunctionList.clear();
SuffixTree ST(Mapper.UnsignedVec, OutlinerLeafDescendants);

Expand Down Expand Up @@ -658,33 +662,36 @@ void MachineOutliner::findCandidates(
<< "\n");
LLVM_DEBUG(dbgs() << " Candidates kept: " << NumKept << "\n\n");
#endif
unsigned MinRepeats = 2;

// We've found something we might want to outline.
// Create an OutlinedFunction to store it and check if it'd be beneficial
// to outline.
if (CandidatesForRepeatedSeq.size() < 2)
if (CandidatesForRepeatedSeq.size() < MinRepeats)
continue;

// Arbitrarily choose a TII from the first candidate.
// FIXME: Should getOutliningCandidateInfo move to TargetMachine?
const TargetInstrInfo *TII =
CandidatesForRepeatedSeq[0].getMF()->getSubtarget().getInstrInfo();

std::optional<OutlinedFunction> OF =
TII->getOutliningCandidateInfo(*MMI, CandidatesForRepeatedSeq);
std::optional<std::unique_ptr<OutlinedFunction>> OF =
TII->getOutliningCandidateInfo(*MMI, CandidatesForRepeatedSeq,
MinRepeats);

// If we deleted too many candidates, then there's nothing worth outlining.
// FIXME: This should take target-specified instruction sizes into account.
if (!OF || OF->Candidates.size() < 2)
if (!OF.has_value() || OF.value()->Candidates.size() < MinRepeats)
continue;

// Is it better to outline this candidate than not?
if (OF->getBenefit() < OutlinerBenefitThreshold) {
emitNotOutliningCheaperRemark(StringLen, CandidatesForRepeatedSeq, *OF);
if (OF.value()->getBenefit() < OutlinerBenefitThreshold) {
emitNotOutliningCheaperRemark(StringLen, CandidatesForRepeatedSeq,
*OF.value());
continue;
}

FunctionList.push_back(*OF);
FunctionList.emplace_back(std::move(OF.value()));
}
}

Expand Down Expand Up @@ -827,71 +834,70 @@ MachineFunction *MachineOutliner::createOutlinedFunction(
return &MF;
}

bool MachineOutliner::outline(Module &M,
std::vector<OutlinedFunction> &FunctionList,
InstructionMapper &Mapper,
unsigned &OutlinedFunctionNum) {
bool MachineOutliner::outline(
Module &M, std::vector<std::unique_ptr<OutlinedFunction>> &FunctionList,
InstructionMapper &Mapper, unsigned &OutlinedFunctionNum) {
LLVM_DEBUG(dbgs() << "*** Outlining ***\n");
LLVM_DEBUG(dbgs() << "NUMBER OF POTENTIAL FUNCTIONS: " << FunctionList.size()
<< "\n");
bool OutlinedSomething = false;

// Sort by priority where priority := getNotOutlinedCost / getOutliningCost.
// The function with highest priority should be outlined first.
stable_sort(FunctionList,
[](const OutlinedFunction &LHS, const OutlinedFunction &RHS) {
return LHS.getNotOutlinedCost() * RHS.getOutliningCost() >
RHS.getNotOutlinedCost() * LHS.getOutliningCost();
});
stable_sort(FunctionList, [](const std::unique_ptr<OutlinedFunction> &LHS,
const std::unique_ptr<OutlinedFunction> &RHS) {
return LHS->getNotOutlinedCost() * RHS->getOutliningCost() >
RHS->getNotOutlinedCost() * LHS->getOutliningCost();
});

// Walk over each function, outlining them as we go along. Functions are
// outlined greedily, based off the sort above.
auto *UnsignedVecBegin = Mapper.UnsignedVec.begin();
LLVM_DEBUG(dbgs() << "WALKING FUNCTION LIST\n");
for (OutlinedFunction &OF : FunctionList) {
for (auto &OF : FunctionList) {
#ifndef NDEBUG
auto NumCandidatesBefore = OF.Candidates.size();
auto NumCandidatesBefore = OF->Candidates.size();
#endif
// If we outlined something that overlapped with a candidate in a previous
// step, then we can't outline from it.
erase_if(OF.Candidates, [&UnsignedVecBegin](Candidate &C) {
erase_if(OF->Candidates, [&UnsignedVecBegin](Candidate &C) {
return std::any_of(UnsignedVecBegin + C.getStartIdx(),
UnsignedVecBegin + C.getEndIdx() + 1, [](unsigned I) {
return I == static_cast<unsigned>(-1);
});
});

#ifndef NDEBUG
auto NumCandidatesAfter = OF.Candidates.size();
auto NumCandidatesAfter = OF->Candidates.size();
LLVM_DEBUG(dbgs() << "PRUNED: " << NumCandidatesBefore - NumCandidatesAfter
<< "/" << NumCandidatesBefore << " candidates\n");
#endif

// If we made it unbeneficial to outline this function, skip it.
if (OF.getBenefit() < OutlinerBenefitThreshold) {
LLVM_DEBUG(dbgs() << "SKIP: Expected benefit (" << OF.getBenefit()
if (OF->getBenefit() < OutlinerBenefitThreshold) {
LLVM_DEBUG(dbgs() << "SKIP: Expected benefit (" << OF->getBenefit()
<< " B) < threshold (" << OutlinerBenefitThreshold
<< " B)\n");
continue;
}

LLVM_DEBUG(dbgs() << "OUTLINE: Expected benefit (" << OF.getBenefit()
LLVM_DEBUG(dbgs() << "OUTLINE: Expected benefit (" << OF->getBenefit()
<< " B) > threshold (" << OutlinerBenefitThreshold
<< " B)\n");

// It's beneficial. Create the function and outline its sequence's
// occurrences.
OF.MF = createOutlinedFunction(M, OF, Mapper, OutlinedFunctionNum);
emitOutlinedFunctionRemark(OF);
OF->MF = createOutlinedFunction(M, *OF, Mapper, OutlinedFunctionNum);
emitOutlinedFunctionRemark(*OF);
FunctionsCreated++;
OutlinedFunctionNum++; // Created a function, move to the next name.
MachineFunction *MF = OF.MF;
MachineFunction *MF = OF->MF;
const TargetSubtargetInfo &STI = MF->getSubtarget();
const TargetInstrInfo &TII = *STI.getInstrInfo();

// Replace occurrences of the sequence with calls to the new function.
LLVM_DEBUG(dbgs() << "CREATE OUTLINED CALLS\n");
for (Candidate &C : OF.Candidates) {
for (Candidate &C : OF->Candidates) {
MachineBasicBlock &MBB = *C.getMBB();
MachineBasicBlock::iterator StartIt = C.begin();
MachineBasicBlock::iterator EndIt = std::prev(C.end());
Expand Down Expand Up @@ -1180,7 +1186,7 @@ bool MachineOutliner::doOutline(Module &M, unsigned &OutlinedFunctionNum) {

// Prepare instruction mappings for the suffix tree.
populateMapper(Mapper, M);
std::vector<OutlinedFunction> FunctionList;
std::vector<std::unique_ptr<OutlinedFunction>> FunctionList;

// Find all of the outlining candidates.
findCandidates(Mapper, FunctionList);
Expand Down
13 changes: 7 additions & 6 deletions llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8686,10 +8686,11 @@ static bool outliningCandidatesV8_3OpsConsensus(const outliner::Candidate &a,
return SubtargetA.hasV8_3aOps() == SubtargetB.hasV8_3aOps();
}

std::optional<outliner::OutlinedFunction>
std::optional<std::unique_ptr<outliner::OutlinedFunction>>
AArch64InstrInfo::getOutliningCandidateInfo(
const MachineModuleInfo &MMI,
std::vector<outliner::Candidate> &RepeatedSequenceLocs) const {
std::vector<outliner::Candidate> &RepeatedSequenceLocs,
unsigned MinRepeats) const {
unsigned SequenceSize = 0;
for (auto &MI : RepeatedSequenceLocs[0])
SequenceSize += getInstSizeInBytes(MI);
Expand Down Expand Up @@ -8803,7 +8804,7 @@ AArch64InstrInfo::getOutliningCandidateInfo(
llvm::erase_if(RepeatedSequenceLocs, hasIllegalSPModification);

// If the sequence doesn't have enough candidates left, then we're done.
if (RepeatedSequenceLocs.size() < 2)
if (RepeatedSequenceLocs.size() < MinRepeats)
return std::nullopt;
}

Expand Down Expand Up @@ -9050,7 +9051,7 @@ AArch64InstrInfo::getOutliningCandidateInfo(
}

// If we dropped all of the candidates, bail out here.
if (RepeatedSequenceLocs.size() < 2) {
if (RepeatedSequenceLocs.size() < MinRepeats) {
RepeatedSequenceLocs.clear();
return std::nullopt;
}
Expand Down Expand Up @@ -9093,8 +9094,8 @@ AArch64InstrInfo::getOutliningCandidateInfo(
if (FrameID != MachineOutlinerTailCall && CFICount > 0)
return std::nullopt;

return outliner::OutlinedFunction(RepeatedSequenceLocs, SequenceSize,
NumBytesToCreateFrame, FrameID);
return std::make_unique<outliner::OutlinedFunction>(
RepeatedSequenceLocs, SequenceSize, NumBytesToCreateFrame, FrameID);
}

void AArch64InstrInfo::mergeOutliningCandidateAttributes(
Expand Down
6 changes: 4 additions & 2 deletions llvm/lib/Target/AArch64/AArch64InstrInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -472,9 +472,11 @@ class AArch64InstrInfo final : public AArch64GenInstrInfo {

bool isFunctionSafeToOutlineFrom(MachineFunction &MF,
bool OutlineFromLinkOnceODRs) const override;
std::optional<outliner::OutlinedFunction> getOutliningCandidateInfo(
std::optional<std::unique_ptr<outliner::OutlinedFunction>>
getOutliningCandidateInfo(
const MachineModuleInfo &MMI,
std::vector<outliner::Candidate> &RepeatedSequenceLocs) const override;
std::vector<outliner::Candidate> &RepeatedSequenceLocs,
unsigned MinRepeats) const override;
void mergeOutliningCandidateAttributes(
Function &F, std::vector<outliner::Candidate> &Candidates) const override;
outliner::InstrType getOutliningTypeImpl(const MachineModuleInfo &MMI,
Expand Down
17 changes: 9 additions & 8 deletions llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5873,10 +5873,11 @@ static bool isLRAvailable(const TargetRegisterInfo &TRI,
return !Live;
}

std::optional<outliner::OutlinedFunction>
std::optional<std::unique_ptr<outliner::OutlinedFunction>>
ARMBaseInstrInfo::getOutliningCandidateInfo(
const MachineModuleInfo &MMI,
std::vector<outliner::Candidate> &RepeatedSequenceLocs) const {
std::vector<outliner::Candidate> &RepeatedSequenceLocs,
unsigned MinRepeats) const {
unsigned SequenceSize = 0;
for (auto &MI : RepeatedSequenceLocs[0])
SequenceSize += getInstSizeInBytes(MI);
Expand Down Expand Up @@ -5917,7 +5918,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo(
llvm::erase_if(RepeatedSequenceLocs, CantGuaranteeValueAcrossCall);

// If the sequence doesn't have enough candidates left, then we're done.
if (RepeatedSequenceLocs.size() < 2)
if (RepeatedSequenceLocs.size() < MinRepeats)
return std::nullopt;
}

Expand All @@ -5943,7 +5944,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo(
else
RepeatedSequenceLocs.erase(RepeatedSequenceLocs.begin(), NoBTI);

if (RepeatedSequenceLocs.size() < 2)
if (RepeatedSequenceLocs.size() < MinRepeats)
return std::nullopt;

// Likewise, partition the candidates according to PAC-RET enablement.
Expand All @@ -5960,7 +5961,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo(
else
RepeatedSequenceLocs.erase(RepeatedSequenceLocs.begin(), NoPAC);

if (RepeatedSequenceLocs.size() < 2)
if (RepeatedSequenceLocs.size() < MinRepeats)
return std::nullopt;

// At this point, we have only "safe" candidates to outline. Figure out
Expand Down Expand Up @@ -6064,7 +6065,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo(
RepeatedSequenceLocs.size() * Costs.CallDefault) {
RepeatedSequenceLocs = CandidatesWithoutStackFixups;
FrameID = MachineOutlinerNoLRSave;
if (RepeatedSequenceLocs.size() < 2)
if (RepeatedSequenceLocs.size() < MinRepeats)
return std::nullopt;
} else
SetCandidateCallInfo(MachineOutlinerDefault, Costs.CallDefault);
Expand All @@ -6090,8 +6091,8 @@ ARMBaseInstrInfo::getOutliningCandidateInfo(
NumBytesToCreateFrame += Costs.SaveRestoreLROnStack;
}

return outliner::OutlinedFunction(RepeatedSequenceLocs, SequenceSize,
NumBytesToCreateFrame, FrameID);
return std::make_unique<outliner::OutlinedFunction>(
RepeatedSequenceLocs, SequenceSize, NumBytesToCreateFrame, FrameID);
}

bool ARMBaseInstrInfo::checkAndUpdateStackOffset(MachineInstr *MI,
Expand Down
6 changes: 4 additions & 2 deletions llvm/lib/Target/ARM/ARMBaseInstrInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -356,9 +356,11 @@ class ARMBaseInstrInfo : public ARMGenInstrInfo {
/// ARM supports the MachineOutliner.
bool isFunctionSafeToOutlineFrom(MachineFunction &MF,
bool OutlineFromLinkOnceODRs) const override;
std::optional<outliner::OutlinedFunction> getOutliningCandidateInfo(
std::optional<std::unique_ptr<outliner::OutlinedFunction>>
getOutliningCandidateInfo(
const MachineModuleInfo &MMI,
std::vector<outliner::Candidate> &RepeatedSequenceLocs) const override;
std::vector<outliner::Candidate> &RepeatedSequenceLocs,
unsigned MinRepeats) const override;
void mergeOutliningCandidateAttributes(
Function &F, std::vector<outliner::Candidate> &Candidates) const override;
outliner::InstrType getOutliningTypeImpl(const MachineModuleInfo &MMI,
Expand Down
12 changes: 7 additions & 5 deletions llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2830,10 +2830,11 @@ bool RISCVInstrInfo::shouldOutlineFromFunctionByDefault(
return MF.getFunction().hasMinSize();
}

std::optional<outliner::OutlinedFunction>
std::optional<std::unique_ptr<outliner::OutlinedFunction>>
RISCVInstrInfo::getOutliningCandidateInfo(
const MachineModuleInfo &MMI,
std::vector<outliner::Candidate> &RepeatedSequenceLocs) const {
std::vector<outliner::Candidate> &RepeatedSequenceLocs,
unsigned MinRepeats) const {

// First we need to filter out candidates where the X5 register (IE t0) can't
// be used to setup the function call.
Expand All @@ -2845,7 +2846,7 @@ RISCVInstrInfo::getOutliningCandidateInfo(
llvm::erase_if(RepeatedSequenceLocs, CannotInsertCall);

// If the sequence doesn't have enough candidates left, then we're done.
if (RepeatedSequenceLocs.size() < 2)
if (RepeatedSequenceLocs.size() < MinRepeats)
return std::nullopt;

unsigned SequenceSize = 0;
Expand All @@ -2866,8 +2867,9 @@ RISCVInstrInfo::getOutliningCandidateInfo(
.hasStdExtCOrZca())
FrameOverhead = 2;

return outliner::OutlinedFunction(RepeatedSequenceLocs, SequenceSize,
FrameOverhead, MachineOutlinerDefault);
return std::make_unique<outliner::OutlinedFunction>(
RepeatedSequenceLocs, SequenceSize, FrameOverhead,
MachineOutlinerDefault);
}

outliner::InstrType
Expand Down
6 changes: 4 additions & 2 deletions llvm/lib/Target/RISCV/RISCVInstrInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,11 @@ class RISCVInstrInfo : public RISCVGenInstrInfo {
bool shouldOutlineFromFunctionByDefault(MachineFunction &MF) const override;

// Calculate target-specific information for a set of outlining candidates.
std::optional<outliner::OutlinedFunction> getOutliningCandidateInfo(
std::optional<std::unique_ptr<outliner::OutlinedFunction>>
getOutliningCandidateInfo(
const MachineModuleInfo &MMI,
std::vector<outliner::Candidate> &RepeatedSequenceLocs) const override;
std::vector<outliner::Candidate> &RepeatedSequenceLocs,
unsigned MinRepeats) const override;

// Return if/how a given MachineInstr should be outlined.
virtual outliner::InstrType
Expand Down
Loading

0 comments on commit 93b8d07

Please sign in to comment.