From 3ed59135c01fcc4d5b3ffa172575bbc74bbb0fb8 Mon Sep 17 00:00:00 2001 From: Kyungwoo Lee Date: Wed, 24 Apr 2024 09:40:34 -0700 Subject: [PATCH 1/4] [MachineOutliner][NFC] Refactor --- llvm/include/llvm/CodeGen/MachineOutliner.h | 5 +- llvm/include/llvm/CodeGen/TargetInstrInfo.h | 12 ++++- llvm/lib/CodeGen/MachineOutliner.cpp | 55 +++++++++++--------- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | 7 +-- llvm/lib/Target/AArch64/AArch64InstrInfo.h | 3 +- 5 files changed, 48 insertions(+), 34 deletions(-) diff --git a/llvm/include/llvm/CodeGen/MachineOutliner.h b/llvm/include/llvm/CodeGen/MachineOutliner.h index eaba6c9b18f2bb7..84937a8b563ac0e 100644 --- a/llvm/include/llvm/CodeGen/MachineOutliner.h +++ b/llvm/include/llvm/CodeGen/MachineOutliner.h @@ -234,11 +234,11 @@ struct OutlinedFunction { unsigned FrameConstructionID = 0; /// Return the number of candidates for this \p OutlinedFunction. - unsigned getOccurrenceCount() const { return Candidates.size(); } + virtual unsigned getOccurrenceCount() const { return Candidates.size(); } /// Return the number of bytes it would take to outline this /// function. - unsigned getOutliningCost() const { + virtual unsigned getOutliningCost() const { unsigned CallOverhead = 0; for (const Candidate &C : Candidates) CallOverhead += C.getCallOverhead(); @@ -272,6 +272,7 @@ struct OutlinedFunction { } OutlinedFunction() = delete; + virtual ~OutlinedFunction() = default; }; } // namespace outliner } // namespace llvm diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h index 882cadea2236956..a833a541e4e0255 100644 --- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h +++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h @@ -2088,14 +2088,22 @@ 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. + /// candidates are not suitable for outlining. \p MinRep is the minimum + /// number of times the instruction sequence must be repeated. virtual std::optional getOutliningCandidateInfo( const MachineModuleInfo &MMI, - std::vector &RepeatedSequenceLocs) const { + std::vector &RepeatedSequenceLocs, + unsigned MipRep) const { llvm_unreachable( "Target didn't implement TargetInstrInfo::getOutliningCandidateInfo!"); } + virtual std::optional getOutliningCandidateInfo( + const MachineModuleInfo &MMI, + std::vector &RepeatedSequenceLocs) const { + return getOutliningCandidateInfo(MMI, RepeatedSequenceLocs, /*MipRep=*/2); + } + /// Optional target hook to create the LLVM IR attributes for the outlined /// function. If overridden, the overriding function must call the default /// implementation. diff --git a/llvm/lib/CodeGen/MachineOutliner.cpp b/llvm/lib/CodeGen/MachineOutliner.cpp index 4b56a467b8d0766..eecf27613a2c318 100644 --- a/llvm/lib/CodeGen/MachineOutliner.cpp +++ b/llvm/lib/CodeGen/MachineOutliner.cpp @@ -456,8 +456,9 @@ 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 &FunctionList); + void + findCandidates(InstructionMapper &Mapper, + std::vector> &FunctionList); /// Replace the sequences of instructions represented by \p OutlinedFunctions /// with calls to functions. @@ -465,7 +466,9 @@ struct MachineOutliner : public ModulePass { /// \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 &FunctionList, + /// \param[out] OutlinedFunctionNum The outlined function number. + bool outline(Module &M, + std::vector> &FunctionList, InstructionMapper &Mapper, unsigned &OutlinedFunctionNum); /// Creates a function for \p OF and inserts it into the module. @@ -583,7 +586,8 @@ void MachineOutliner::emitOutlinedFunctionRemark(OutlinedFunction &OF) { } void MachineOutliner::findCandidates( - InstructionMapper &Mapper, std::vector &FunctionList) { + InstructionMapper &Mapper, + std::vector> &FunctionList) { FunctionList.clear(); SuffixTree ST(Mapper.UnsignedVec, OutlinerLeafDescendants); @@ -684,7 +688,7 @@ void MachineOutliner::findCandidates( continue; } - FunctionList.push_back(*OF); + FunctionList.push_back(std::make_unique(*OF)); } } @@ -827,10 +831,9 @@ MachineFunction *MachineOutliner::createOutlinedFunction( return &MF; } -bool MachineOutliner::outline(Module &M, - std::vector &FunctionList, - InstructionMapper &Mapper, - unsigned &OutlinedFunctionNum) { +bool MachineOutliner::outline( + Module &M, std::vector> &FunctionList, + InstructionMapper &Mapper, unsigned &OutlinedFunctionNum) { LLVM_DEBUG(dbgs() << "*** Outlining ***\n"); LLVM_DEBUG(dbgs() << "NUMBER OF POTENTIAL FUNCTIONS: " << FunctionList.size() << "\n"); @@ -838,23 +841,23 @@ bool MachineOutliner::outline(Module &M, // 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 &LHS, + const std::unique_ptr &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(-1); @@ -862,36 +865,36 @@ bool MachineOutliner::outline(Module &M, }); #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()); @@ -1180,7 +1183,7 @@ bool MachineOutliner::doOutline(Module &M, unsigned &OutlinedFunctionNum) { // Prepare instruction mappings for the suffix tree. populateMapper(Mapper, M); - std::vector FunctionList; + std::vector> FunctionList; // Find all of the outlining candidates. findCandidates(Mapper, FunctionList); diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp index 697ae510a95655c..156ab6568f833e7 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp @@ -8687,7 +8687,8 @@ static bool outliningCandidatesV8_3OpsConsensus(const outliner::Candidate &a, std::optional AArch64InstrInfo::getOutliningCandidateInfo( const MachineModuleInfo &MMI, - std::vector &RepeatedSequenceLocs) const { + std::vector &RepeatedSequenceLocs, + unsigned MinRep) const { unsigned SequenceSize = 0; for (auto &MI : RepeatedSequenceLocs[0]) SequenceSize += getInstSizeInBytes(MI); @@ -8801,7 +8802,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() < MinRep) return std::nullopt; } @@ -9048,7 +9049,7 @@ AArch64InstrInfo::getOutliningCandidateInfo( } // If we dropped all of the candidates, bail out here. - if (RepeatedSequenceLocs.size() < 2) { + if (RepeatedSequenceLocs.size() < MinRep) { RepeatedSequenceLocs.clear(); return std::nullopt; } diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.h b/llvm/lib/Target/AArch64/AArch64InstrInfo.h index a1f2fbff016312a..762fb9873065e66 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.h +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.h @@ -473,7 +473,8 @@ class AArch64InstrInfo final : public AArch64GenInstrInfo { bool OutlineFromLinkOnceODRs) const override; std::optional getOutliningCandidateInfo( const MachineModuleInfo &MMI, - std::vector &RepeatedSequenceLocs) const override; + std::vector &RepeatedSequenceLocs, + unsigned MinRep) const override; void mergeOutliningCandidateAttributes( Function &F, std::vector &Candidates) const override; outliner::InstrType getOutliningTypeImpl(const MachineModuleInfo &MMI, From e1fc322d9b4c9d23e9cb450f1e79491b90fd53d5 Mon Sep 17 00:00:00 2001 From: Kyungwoo Lee Date: Thu, 22 Aug 2024 00:08:01 -0700 Subject: [PATCH 2/4] Address comments from ellishg --- llvm/include/llvm/CodeGen/TargetInstrInfo.h | 13 ++++--------- llvm/lib/CodeGen/MachineOutliner.cpp | 15 +++++++++------ llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | 12 ++++++------ llvm/lib/Target/AArch64/AArch64InstrInfo.h | 5 +++-- llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp | 17 +++++++++-------- llvm/lib/Target/ARM/ARMBaseInstrInfo.h | 6 ++++-- llvm/lib/Target/RISCV/RISCVInstrInfo.cpp | 12 +++++++----- llvm/lib/Target/RISCV/RISCVInstrInfo.h | 6 ++++-- llvm/lib/Target/X86/X86InstrInfo.cpp | 16 +++++++++------- llvm/lib/Target/X86/X86InstrInfo.h | 6 ++++-- 10 files changed, 59 insertions(+), 49 deletions(-) diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h index a833a541e4e0255..3fc26dd85cb1dd7 100644 --- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h +++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h @@ -2088,22 +2088,17 @@ 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. \p MinRep is the minimum + /// candidates are not suitable for outlining. \p MinRepeates is the minimum /// number of times the instruction sequence must be repeated. - virtual std::optional getOutliningCandidateInfo( + virtual std::optional> + getOutliningCandidateInfo( const MachineModuleInfo &MMI, std::vector &RepeatedSequenceLocs, - unsigned MipRep) const { + unsigned MinRepeates) const { llvm_unreachable( "Target didn't implement TargetInstrInfo::getOutliningCandidateInfo!"); } - virtual std::optional getOutliningCandidateInfo( - const MachineModuleInfo &MMI, - std::vector &RepeatedSequenceLocs) const { - return getOutliningCandidateInfo(MMI, RepeatedSequenceLocs, /*MipRep=*/2); - } - /// Optional target hook to create the LLVM IR attributes for the outlined /// function. If overridden, the overriding function must call the default /// implementation. diff --git a/llvm/lib/CodeGen/MachineOutliner.cpp b/llvm/lib/CodeGen/MachineOutliner.cpp index eecf27613a2c318..ed194355f72addc 100644 --- a/llvm/lib/CodeGen/MachineOutliner.cpp +++ b/llvm/lib/CodeGen/MachineOutliner.cpp @@ -674,21 +674,24 @@ void MachineOutliner::findCandidates( const TargetInstrInfo *TII = CandidatesForRepeatedSeq[0].getMF()->getSubtarget().getInstrInfo(); - std::optional OF = - TII->getOutliningCandidateInfo(*MMI, CandidatesForRepeatedSeq); + unsigned MinRepeates = 2; + std::optional> OF = + TII->getOutliningCandidateInfo(*MMI, CandidatesForRepeatedSeq, + MinRepeates); // 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() < MinRepeates) 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(std::make_unique(*OF)); + FunctionList.emplace_back(std::move(OF.value())); } } diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp index 156ab6568f833e7..934cd2b72024c72 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp @@ -8684,11 +8684,11 @@ static bool outliningCandidatesV8_3OpsConsensus(const outliner::Candidate &a, return SubtargetA.hasV8_3aOps() == SubtargetB.hasV8_3aOps(); } -std::optional +std::optional> AArch64InstrInfo::getOutliningCandidateInfo( const MachineModuleInfo &MMI, std::vector &RepeatedSequenceLocs, - unsigned MinRep) const { + unsigned MinRepeates) const { unsigned SequenceSize = 0; for (auto &MI : RepeatedSequenceLocs[0]) SequenceSize += getInstSizeInBytes(MI); @@ -8802,7 +8802,7 @@ AArch64InstrInfo::getOutliningCandidateInfo( llvm::erase_if(RepeatedSequenceLocs, hasIllegalSPModification); // If the sequence doesn't have enough candidates left, then we're done. - if (RepeatedSequenceLocs.size() < MinRep) + if (RepeatedSequenceLocs.size() < MinRepeates) return std::nullopt; } @@ -9049,7 +9049,7 @@ AArch64InstrInfo::getOutliningCandidateInfo( } // If we dropped all of the candidates, bail out here. - if (RepeatedSequenceLocs.size() < MinRep) { + if (RepeatedSequenceLocs.size() < MinRepeates) { RepeatedSequenceLocs.clear(); return std::nullopt; } @@ -9092,8 +9092,8 @@ AArch64InstrInfo::getOutliningCandidateInfo( if (FrameID != MachineOutlinerTailCall && CFICount > 0) return std::nullopt; - return outliner::OutlinedFunction(RepeatedSequenceLocs, SequenceSize, - NumBytesToCreateFrame, FrameID); + return std::make_unique( + RepeatedSequenceLocs, SequenceSize, NumBytesToCreateFrame, FrameID); } void AArch64InstrInfo::mergeOutliningCandidateAttributes( diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.h b/llvm/lib/Target/AArch64/AArch64InstrInfo.h index 762fb9873065e66..4814d394e30a03c 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.h +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.h @@ -471,10 +471,11 @@ class AArch64InstrInfo final : public AArch64GenInstrInfo { bool isFunctionSafeToOutlineFrom(MachineFunction &MF, bool OutlineFromLinkOnceODRs) const override; - std::optional getOutliningCandidateInfo( + std::optional> + getOutliningCandidateInfo( const MachineModuleInfo &MMI, std::vector &RepeatedSequenceLocs, - unsigned MinRep) const override; + unsigned MinRepeates) const override; void mergeOutliningCandidateAttributes( Function &F, std::vector &Candidates) const override; outliner::InstrType getOutliningTypeImpl(const MachineModuleInfo &MMI, diff --git a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp index 1199052ca97e9c7..c3fc8b3f1c9bb91 100644 --- a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp +++ b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp @@ -5871,10 +5871,11 @@ static bool isLRAvailable(const TargetRegisterInfo &TRI, return !Live; } -std::optional +std::optional> ARMBaseInstrInfo::getOutliningCandidateInfo( const MachineModuleInfo &MMI, - std::vector &RepeatedSequenceLocs) const { + std::vector &RepeatedSequenceLocs, + unsigned MinRepeates) const { unsigned SequenceSize = 0; for (auto &MI : RepeatedSequenceLocs[0]) SequenceSize += getInstSizeInBytes(MI); @@ -5915,7 +5916,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() < MinRepeates) return std::nullopt; } @@ -5941,7 +5942,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo( else RepeatedSequenceLocs.erase(RepeatedSequenceLocs.begin(), NoBTI); - if (RepeatedSequenceLocs.size() < 2) + if (RepeatedSequenceLocs.size() < MinRepeates) return std::nullopt; // Likewise, partition the candidates according to PAC-RET enablement. @@ -5958,7 +5959,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo( else RepeatedSequenceLocs.erase(RepeatedSequenceLocs.begin(), NoPAC); - if (RepeatedSequenceLocs.size() < 2) + if (RepeatedSequenceLocs.size() < MinRepeates) return std::nullopt; // At this point, we have only "safe" candidates to outline. Figure out @@ -6062,7 +6063,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo( RepeatedSequenceLocs.size() * Costs.CallDefault) { RepeatedSequenceLocs = CandidatesWithoutStackFixups; FrameID = MachineOutlinerNoLRSave; - if (RepeatedSequenceLocs.size() < 2) + if (RepeatedSequenceLocs.size() < MinRepeates) return std::nullopt; } else SetCandidateCallInfo(MachineOutlinerDefault, Costs.CallDefault); @@ -6088,8 +6089,8 @@ ARMBaseInstrInfo::getOutliningCandidateInfo( NumBytesToCreateFrame += Costs.SaveRestoreLROnStack; } - return outliner::OutlinedFunction(RepeatedSequenceLocs, SequenceSize, - NumBytesToCreateFrame, FrameID); + return std::make_unique( + RepeatedSequenceLocs, SequenceSize, NumBytesToCreateFrame, FrameID); } bool ARMBaseInstrInfo::checkAndUpdateStackOffset(MachineInstr *MI, diff --git a/llvm/lib/Target/ARM/ARMBaseInstrInfo.h b/llvm/lib/Target/ARM/ARMBaseInstrInfo.h index 8521e3ef91399a5..b0ac6479ab6fdc7 100644 --- a/llvm/lib/Target/ARM/ARMBaseInstrInfo.h +++ b/llvm/lib/Target/ARM/ARMBaseInstrInfo.h @@ -355,9 +355,11 @@ class ARMBaseInstrInfo : public ARMGenInstrInfo { /// ARM supports the MachineOutliner. bool isFunctionSafeToOutlineFrom(MachineFunction &MF, bool OutlineFromLinkOnceODRs) const override; - std::optional getOutliningCandidateInfo( + std::optional> + getOutliningCandidateInfo( const MachineModuleInfo &MMI, - std::vector &RepeatedSequenceLocs) const override; + std::vector &RepeatedSequenceLocs, + unsigned MinRepeates) const override; void mergeOutliningCandidateAttributes( Function &F, std::vector &Candidates) const override; outliner::InstrType getOutliningTypeImpl(const MachineModuleInfo &MMI, diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp index 9dd79027d7a162b..766ee16393e6fbf 100644 --- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp +++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp @@ -2828,10 +2828,11 @@ bool RISCVInstrInfo::shouldOutlineFromFunctionByDefault( return MF.getFunction().hasMinSize(); } -std::optional +std::optional> RISCVInstrInfo::getOutliningCandidateInfo( const MachineModuleInfo &MMI, - std::vector &RepeatedSequenceLocs) const { + std::vector &RepeatedSequenceLocs, + unsigned MinRepeates) const { // First we need to filter out candidates where the X5 register (IE t0) can't // be used to setup the function call. @@ -2843,7 +2844,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() < MinRepeates) return std::nullopt; unsigned SequenceSize = 0; @@ -2864,8 +2865,9 @@ RISCVInstrInfo::getOutliningCandidateInfo( .hasStdExtCOrZca()) FrameOverhead = 2; - return outliner::OutlinedFunction(RepeatedSequenceLocs, SequenceSize, - FrameOverhead, MachineOutlinerDefault); + return std::make_unique( + RepeatedSequenceLocs, SequenceSize, FrameOverhead, + MachineOutlinerDefault); } outliner::InstrType diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.h b/llvm/lib/Target/RISCV/RISCVInstrInfo.h index ecb7982b3e5e36b..e431ae30839297f 100644 --- a/llvm/lib/Target/RISCV/RISCVInstrInfo.h +++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.h @@ -205,9 +205,11 @@ class RISCVInstrInfo : public RISCVGenInstrInfo { bool shouldOutlineFromFunctionByDefault(MachineFunction &MF) const override; // Calculate target-specific information for a set of outlining candidates. - std::optional getOutliningCandidateInfo( + std::optional> + getOutliningCandidateInfo( const MachineModuleInfo &MMI, - std::vector &RepeatedSequenceLocs) const override; + std::vector &RepeatedSequenceLocs, + unsigned MinRepeates) const override; // Return if/how a given MachineInstr should be outlined. virtual outliner::InstrType diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp index 39ba7ea777909c0..087e869c28fa3ac 100644 --- a/llvm/lib/Target/X86/X86InstrInfo.cpp +++ b/llvm/lib/Target/X86/X86InstrInfo.cpp @@ -10521,10 +10521,11 @@ FunctionPass *llvm::createCleanupLocalDynamicTLSPass() { /// enum MachineOutlinerClass { MachineOutlinerDefault, MachineOutlinerTailCall }; -std::optional +std::optional> X86InstrInfo::getOutliningCandidateInfo( const MachineModuleInfo &MMI, - std::vector &RepeatedSequenceLocs) const { + std::vector &RepeatedSequenceLocs, + unsigned MinRepeates) const { unsigned SequenceSize = 0; for (auto &MI : RepeatedSequenceLocs[0]) { // FIXME: x86 doesn't implement getInstSizeInBytes, so @@ -10561,9 +10562,10 @@ X86InstrInfo::getOutliningCandidateInfo( for (outliner::Candidate &C : RepeatedSequenceLocs) C.setCallInfo(MachineOutlinerTailCall, 1); - return outliner::OutlinedFunction(RepeatedSequenceLocs, SequenceSize, - 0, // Number of bytes to emit frame. - MachineOutlinerTailCall // Type of frame. + return std::make_unique( + RepeatedSequenceLocs, SequenceSize, + 0, // Number of bytes to emit frame. + MachineOutlinerTailCall // Type of frame. ); } @@ -10573,8 +10575,8 @@ X86InstrInfo::getOutliningCandidateInfo( for (outliner::Candidate &C : RepeatedSequenceLocs) C.setCallInfo(MachineOutlinerDefault, 1); - return outliner::OutlinedFunction(RepeatedSequenceLocs, SequenceSize, 1, - MachineOutlinerDefault); + return std::make_unique( + RepeatedSequenceLocs, SequenceSize, 1, MachineOutlinerDefault); } bool X86InstrInfo::isFunctionSafeToOutlineFrom( diff --git a/llvm/lib/Target/X86/X86InstrInfo.h b/llvm/lib/Target/X86/X86InstrInfo.h index 3100a9e5699f0a5..8593430d780d396 100644 --- a/llvm/lib/Target/X86/X86InstrInfo.h +++ b/llvm/lib/Target/X86/X86InstrInfo.h @@ -584,9 +584,11 @@ class X86InstrInfo final : public X86GenInstrInfo { ArrayRef> getSerializableDirectMachineOperandTargetFlags() const override; - std::optional getOutliningCandidateInfo( + std::optional> + getOutliningCandidateInfo( const MachineModuleInfo &MMI, - std::vector &RepeatedSequenceLocs) const override; + std::vector &RepeatedSequenceLocs, + unsigned MinRepeates) const override; bool isFunctionSafeToOutlineFrom(MachineFunction &MF, bool OutlineFromLinkOnceODRs) const override; From ab4a59f433c50f666ae3ffe8025f5c647630c797 Mon Sep 17 00:00:00 2001 From: Kyungwoo Lee Date: Mon, 26 Aug 2024 10:23:34 -0700 Subject: [PATCH 3/4] Address comments from michaelmaitland --- llvm/include/llvm/CodeGen/MachineOutliner.h | 5 ++--- llvm/include/llvm/CodeGen/TargetInstrInfo.h | 4 ++-- llvm/lib/CodeGen/MachineOutliner.cpp | 6 +++--- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | 6 +++--- llvm/lib/Target/AArch64/AArch64InstrInfo.h | 2 +- llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp | 10 +++++----- llvm/lib/Target/ARM/ARMBaseInstrInfo.h | 2 +- llvm/lib/Target/RISCV/RISCVInstrInfo.cpp | 4 ++-- llvm/lib/Target/RISCV/RISCVInstrInfo.h | 2 +- llvm/lib/Target/X86/X86InstrInfo.cpp | 2 +- llvm/lib/Target/X86/X86InstrInfo.h | 2 +- 11 files changed, 22 insertions(+), 23 deletions(-) diff --git a/llvm/include/llvm/CodeGen/MachineOutliner.h b/llvm/include/llvm/CodeGen/MachineOutliner.h index 84937a8b563ac0e..eaba6c9b18f2bb7 100644 --- a/llvm/include/llvm/CodeGen/MachineOutliner.h +++ b/llvm/include/llvm/CodeGen/MachineOutliner.h @@ -234,11 +234,11 @@ struct OutlinedFunction { unsigned FrameConstructionID = 0; /// Return the number of candidates for this \p OutlinedFunction. - virtual unsigned getOccurrenceCount() const { return Candidates.size(); } + unsigned getOccurrenceCount() const { return Candidates.size(); } /// Return the number of bytes it would take to outline this /// function. - virtual unsigned getOutliningCost() const { + unsigned getOutliningCost() const { unsigned CallOverhead = 0; for (const Candidate &C : Candidates) CallOverhead += C.getCallOverhead(); @@ -272,7 +272,6 @@ struct OutlinedFunction { } OutlinedFunction() = delete; - virtual ~OutlinedFunction() = default; }; } // namespace outliner } // namespace llvm diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h index 3fc26dd85cb1dd7..91382fd07aec937 100644 --- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h +++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h @@ -2088,13 +2088,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. \p MinRepeates is the minimum + /// candidates are not suitable for outlining. \p MinRepeats is the minimum /// number of times the instruction sequence must be repeated. virtual std::optional> getOutliningCandidateInfo( const MachineModuleInfo &MMI, std::vector &RepeatedSequenceLocs, - unsigned MinRepeates) const { + unsigned MinRepeats) const { llvm_unreachable( "Target didn't implement TargetInstrInfo::getOutliningCandidateInfo!"); } diff --git a/llvm/lib/CodeGen/MachineOutliner.cpp b/llvm/lib/CodeGen/MachineOutliner.cpp index ed194355f72addc..1644b0a8e118c69 100644 --- a/llvm/lib/CodeGen/MachineOutliner.cpp +++ b/llvm/lib/CodeGen/MachineOutliner.cpp @@ -674,14 +674,14 @@ void MachineOutliner::findCandidates( const TargetInstrInfo *TII = CandidatesForRepeatedSeq[0].getMF()->getSubtarget().getInstrInfo(); - unsigned MinRepeates = 2; + unsigned MinRepeats = 2; std::optional> OF = TII->getOutliningCandidateInfo(*MMI, CandidatesForRepeatedSeq, - MinRepeates); + 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.has_value() || OF.value()->Candidates.size() < MinRepeates) + if (!OF.has_value() || OF.value()->Candidates.size() < MinRepeats) continue; // Is it better to outline this candidate than not? diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp index 934cd2b72024c72..5fe4173d27d33d3 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp @@ -8688,7 +8688,7 @@ std::optional> AArch64InstrInfo::getOutliningCandidateInfo( const MachineModuleInfo &MMI, std::vector &RepeatedSequenceLocs, - unsigned MinRepeates) const { + unsigned MinRepeats) const { unsigned SequenceSize = 0; for (auto &MI : RepeatedSequenceLocs[0]) SequenceSize += getInstSizeInBytes(MI); @@ -8802,7 +8802,7 @@ AArch64InstrInfo::getOutliningCandidateInfo( llvm::erase_if(RepeatedSequenceLocs, hasIllegalSPModification); // If the sequence doesn't have enough candidates left, then we're done. - if (RepeatedSequenceLocs.size() < MinRepeates) + if (RepeatedSequenceLocs.size() < MinRepeats) return std::nullopt; } @@ -9049,7 +9049,7 @@ AArch64InstrInfo::getOutliningCandidateInfo( } // If we dropped all of the candidates, bail out here. - if (RepeatedSequenceLocs.size() < MinRepeates) { + if (RepeatedSequenceLocs.size() < MinRepeats) { RepeatedSequenceLocs.clear(); return std::nullopt; } diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.h b/llvm/lib/Target/AArch64/AArch64InstrInfo.h index 4814d394e30a03c..9cda7d86a3a1de2 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.h +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.h @@ -475,7 +475,7 @@ class AArch64InstrInfo final : public AArch64GenInstrInfo { getOutliningCandidateInfo( const MachineModuleInfo &MMI, std::vector &RepeatedSequenceLocs, - unsigned MinRepeates) const override; + unsigned MinRepeats) const override; void mergeOutliningCandidateAttributes( Function &F, std::vector &Candidates) const override; outliner::InstrType getOutliningTypeImpl(const MachineModuleInfo &MMI, diff --git a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp index c3fc8b3f1c9bb91..a87c0ace261a46d 100644 --- a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp +++ b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp @@ -5875,7 +5875,7 @@ std::optional> ARMBaseInstrInfo::getOutliningCandidateInfo( const MachineModuleInfo &MMI, std::vector &RepeatedSequenceLocs, - unsigned MinRepeates) const { + unsigned MinRepeats) const { unsigned SequenceSize = 0; for (auto &MI : RepeatedSequenceLocs[0]) SequenceSize += getInstSizeInBytes(MI); @@ -5916,7 +5916,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo( llvm::erase_if(RepeatedSequenceLocs, CantGuaranteeValueAcrossCall); // If the sequence doesn't have enough candidates left, then we're done. - if (RepeatedSequenceLocs.size() < MinRepeates) + if (RepeatedSequenceLocs.size() < MinRepeats) return std::nullopt; } @@ -5942,7 +5942,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo( else RepeatedSequenceLocs.erase(RepeatedSequenceLocs.begin(), NoBTI); - if (RepeatedSequenceLocs.size() < MinRepeates) + if (RepeatedSequenceLocs.size() < MinRepeats) return std::nullopt; // Likewise, partition the candidates according to PAC-RET enablement. @@ -5959,7 +5959,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo( else RepeatedSequenceLocs.erase(RepeatedSequenceLocs.begin(), NoPAC); - if (RepeatedSequenceLocs.size() < MinRepeates) + if (RepeatedSequenceLocs.size() < MinRepeats) return std::nullopt; // At this point, we have only "safe" candidates to outline. Figure out @@ -6063,7 +6063,7 @@ ARMBaseInstrInfo::getOutliningCandidateInfo( RepeatedSequenceLocs.size() * Costs.CallDefault) { RepeatedSequenceLocs = CandidatesWithoutStackFixups; FrameID = MachineOutlinerNoLRSave; - if (RepeatedSequenceLocs.size() < MinRepeates) + if (RepeatedSequenceLocs.size() < MinRepeats) return std::nullopt; } else SetCandidateCallInfo(MachineOutlinerDefault, Costs.CallDefault); diff --git a/llvm/lib/Target/ARM/ARMBaseInstrInfo.h b/llvm/lib/Target/ARM/ARMBaseInstrInfo.h index b0ac6479ab6fdc7..baffe78e8d0d2b6 100644 --- a/llvm/lib/Target/ARM/ARMBaseInstrInfo.h +++ b/llvm/lib/Target/ARM/ARMBaseInstrInfo.h @@ -359,7 +359,7 @@ class ARMBaseInstrInfo : public ARMGenInstrInfo { getOutliningCandidateInfo( const MachineModuleInfo &MMI, std::vector &RepeatedSequenceLocs, - unsigned MinRepeates) const override; + unsigned MinRepeats) const override; void mergeOutliningCandidateAttributes( Function &F, std::vector &Candidates) const override; outliner::InstrType getOutliningTypeImpl(const MachineModuleInfo &MMI, diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp index 766ee16393e6fbf..688a72ecfce8d46 100644 --- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp +++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp @@ -2832,7 +2832,7 @@ std::optional> RISCVInstrInfo::getOutliningCandidateInfo( const MachineModuleInfo &MMI, std::vector &RepeatedSequenceLocs, - unsigned MinRepeates) const { + 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. @@ -2844,7 +2844,7 @@ RISCVInstrInfo::getOutliningCandidateInfo( llvm::erase_if(RepeatedSequenceLocs, CannotInsertCall); // If the sequence doesn't have enough candidates left, then we're done. - if (RepeatedSequenceLocs.size() < MinRepeates) + if (RepeatedSequenceLocs.size() < MinRepeats) return std::nullopt; unsigned SequenceSize = 0; diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.h b/llvm/lib/Target/RISCV/RISCVInstrInfo.h index e431ae30839297f..e11cf2b2f767023 100644 --- a/llvm/lib/Target/RISCV/RISCVInstrInfo.h +++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.h @@ -209,7 +209,7 @@ class RISCVInstrInfo : public RISCVGenInstrInfo { getOutliningCandidateInfo( const MachineModuleInfo &MMI, std::vector &RepeatedSequenceLocs, - unsigned MinRepeates) const override; + unsigned MinRepeats) const override; // Return if/how a given MachineInstr should be outlined. virtual outliner::InstrType diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp index 087e869c28fa3ac..9ee00bd8d52707f 100644 --- a/llvm/lib/Target/X86/X86InstrInfo.cpp +++ b/llvm/lib/Target/X86/X86InstrInfo.cpp @@ -10525,7 +10525,7 @@ std::optional> X86InstrInfo::getOutliningCandidateInfo( const MachineModuleInfo &MMI, std::vector &RepeatedSequenceLocs, - unsigned MinRepeates) const { + unsigned MinRepeats) const { unsigned SequenceSize = 0; for (auto &MI : RepeatedSequenceLocs[0]) { // FIXME: x86 doesn't implement getInstSizeInBytes, so diff --git a/llvm/lib/Target/X86/X86InstrInfo.h b/llvm/lib/Target/X86/X86InstrInfo.h index 8593430d780d396..54987a186854dba 100644 --- a/llvm/lib/Target/X86/X86InstrInfo.h +++ b/llvm/lib/Target/X86/X86InstrInfo.h @@ -588,7 +588,7 @@ class X86InstrInfo final : public X86GenInstrInfo { getOutliningCandidateInfo( const MachineModuleInfo &MMI, std::vector &RepeatedSequenceLocs, - unsigned MinRepeates) const override; + unsigned MinRepeats) const override; bool isFunctionSafeToOutlineFrom(MachineFunction &MF, bool OutlineFromLinkOnceODRs) const override; From c72a0c17c9b62a40d87839150b03d8d63a56688e Mon Sep 17 00:00:00 2001 From: Kyungwoo Lee Date: Mon, 26 Aug 2024 17:15:03 -0700 Subject: [PATCH 4/4] Address comments from thevinster --- llvm/lib/CodeGen/MachineOutliner.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/CodeGen/MachineOutliner.cpp b/llvm/lib/CodeGen/MachineOutliner.cpp index 1644b0a8e118c69..42f410c277179bf 100644 --- a/llvm/lib/CodeGen/MachineOutliner.cpp +++ b/llvm/lib/CodeGen/MachineOutliner.cpp @@ -662,11 +662,12 @@ 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. @@ -674,7 +675,6 @@ void MachineOutliner::findCandidates( const TargetInstrInfo *TII = CandidatesForRepeatedSeq[0].getMF()->getSubtarget().getInstrInfo(); - unsigned MinRepeats = 2; std::optional> OF = TII->getOutliningCandidateInfo(*MMI, CandidatesForRepeatedSeq, MinRepeats);