From 41e9c4b88c28b0a6f3820b45000cedeced89206c Mon Sep 17 00:00:00 2001 From: Amara Emerson Date: Sat, 18 Mar 2023 23:17:28 -0700 Subject: [PATCH] [NFC][Outliner] Delete default ctors for Candidate & OutlinedFunction. I think it's good practice to avoid having default ctors unless they're really valid/useful. For OutlinedFunction the default ctor was used to represent a bail-out value for getOutliningCandidateInfo(), so I changed the API to return an optional instead which seems a tad cleaner. Differential Revision: https://reviews.llvm.org/D146375 --- llvm/include/llvm/CodeGen/MachineOutliner.h | 4 ++-- llvm/include/llvm/CodeGen/TargetInstrInfo.h | 5 +++-- llvm/lib/CodeGen/MachineOutliner.cpp | 10 +++++----- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | 15 ++++++++------- llvm/lib/Target/AArch64/AArch64InstrInfo.h | 3 ++- llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp | 9 +++++---- llvm/lib/Target/ARM/ARMBaseInstrInfo.h | 2 +- llvm/lib/Target/RISCV/RISCVInstrInfo.cpp | 5 +++-- llvm/lib/Target/RISCV/RISCVInstrInfo.h | 2 +- llvm/lib/Target/X86/X86InstrInfo.cpp | 8 +++++--- llvm/lib/Target/X86/X86InstrInfo.h | 2 +- 11 files changed, 36 insertions(+), 29 deletions(-) diff --git a/llvm/include/llvm/CodeGen/MachineOutliner.h b/llvm/include/llvm/CodeGen/MachineOutliner.h index f968089..d0ff02f 100644 --- a/llvm/include/llvm/CodeGen/MachineOutliner.h +++ b/llvm/include/llvm/CodeGen/MachineOutliner.h @@ -199,7 +199,7 @@ public: unsigned FunctionIdx, unsigned Flags) : StartIdx(StartIdx), Len(Len), FirstInst(FirstInst), LastInst(LastInst), MBB(MBB), FunctionIdx(FunctionIdx), Flags(Flags) {} - Candidate() = default; + Candidate() = delete; /// Used to ensure that \p Candidates are outlined in an order that /// preserves the start and end indices of other \p Candidates. @@ -268,7 +268,7 @@ public: C.Benefit = B; } - OutlinedFunction() = default; + OutlinedFunction() = delete; }; } // namespace outliner } // namespace llvm diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h index 92c283a..b4a7bbb 100644 --- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h +++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h @@ -1966,8 +1966,9 @@ public: } /// Returns a \p outliner::OutlinedFunction struct containing target-specific - /// information for a set of outlining candidates. - virtual outliner::OutlinedFunction getOutliningCandidateInfo( + /// information for a set of outlining candidates. Returns None if the + /// candidates are not suitable for outlining. + virtual std::optional getOutliningCandidateInfo( std::vector &RepeatedSequenceLocs) const { llvm_unreachable( "Target didn't implement TargetInstrInfo::getOutliningCandidateInfo!"); diff --git a/llvm/lib/CodeGen/MachineOutliner.cpp b/llvm/lib/CodeGen/MachineOutliner.cpp index 856b9bf..8d72208 100644 --- a/llvm/lib/CodeGen/MachineOutliner.cpp +++ b/llvm/lib/CodeGen/MachineOutliner.cpp @@ -655,21 +655,21 @@ void MachineOutliner::findCandidates( const TargetInstrInfo *TII = CandidatesForRepeatedSeq[0].getMF()->getSubtarget().getInstrInfo(); - OutlinedFunction OF = + std::optional OF = TII->getOutliningCandidateInfo(CandidatesForRepeatedSeq); // If we deleted too many candidates, then there's nothing worth outlining. // FIXME: This should take target-specified instruction sizes into account. - if (OF.Candidates.size() < 2) + if (!OF || OF->Candidates.size() < 2) continue; // Is it better to outline this candidate than not? - if (OF.getBenefit() < 1) { - emitNotOutliningCheaperRemark(StringLen, CandidatesForRepeatedSeq, OF); + if (OF->getBenefit() < 1) { + emitNotOutliningCheaperRemark(StringLen, CandidatesForRepeatedSeq, *OF); continue; } - FunctionList.push_back(OF); + FunctionList.push_back(*OF); } } diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp index 54ad05b..b1cfd68 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp @@ -7151,7 +7151,8 @@ static bool outliningCandidatesV8_3OpsConsensus(const outliner::Candidate &a, return SubtargetA.hasV8_3aOps() == SubtargetB.hasV8_3aOps(); } -outliner::OutlinedFunction AArch64InstrInfo::getOutliningCandidateInfo( +std::optional +AArch64InstrInfo::getOutliningCandidateInfo( std::vector &RepeatedSequenceLocs) const { outliner::Candidate &FirstCand = RepeatedSequenceLocs[0]; unsigned SequenceSize = @@ -7181,7 +7182,7 @@ outliner::OutlinedFunction AArch64InstrInfo::getOutliningCandidateInfo( } return true; }) != RepeatedSequenceLocs.end()) { - return outliner::OutlinedFunction(); + return std::nullopt; } // Since at this point all candidates agree on their return address signing @@ -7259,7 +7260,7 @@ outliner::OutlinedFunction AArch64InstrInfo::getOutliningCandidateInfo( // If the sequence doesn't have enough candidates left, then we're done. if (RepeatedSequenceLocs.size() < 2) - return outliner::OutlinedFunction(); + return std::nullopt; } // Properties about candidate MBBs that hold for all of them. @@ -7304,7 +7305,7 @@ outliner::OutlinedFunction AArch64InstrInfo::getOutliningCandidateInfo( C.getMF()->getFrameInstructions(); if (CFICount > 0 && CFICount != CFIInstructions.size()) - return outliner::OutlinedFunction(); + return std::nullopt; } // Returns true if an instructions is safe to fix up, false otherwise. @@ -7506,7 +7507,7 @@ outliner::OutlinedFunction AArch64InstrInfo::getOutliningCandidateInfo( // If we dropped all of the candidates, bail out here. if (RepeatedSequenceLocs.size() < 2) { RepeatedSequenceLocs.clear(); - return outliner::OutlinedFunction(); + return std::nullopt; } } @@ -7533,7 +7534,7 @@ outliner::OutlinedFunction AArch64InstrInfo::getOutliningCandidateInfo( // We can't fix up the stack. Bail out. if (!AllStackInstrsSafe) { RepeatedSequenceLocs.clear(); - return outliner::OutlinedFunction(); + return std::nullopt; } // Save + restore LR. @@ -7544,7 +7545,7 @@ outliner::OutlinedFunction AArch64InstrInfo::getOutliningCandidateInfo( // If we have CFI instructions, we can only outline if the outlined section // can be a tail call if (FrameID != MachineOutlinerTailCall && CFICount > 0) - return outliner::OutlinedFunction(); + return std::nullopt; return outliner::OutlinedFunction(RepeatedSequenceLocs, SequenceSize, NumBytesToCreateFrame, FrameID); diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.h b/llvm/lib/Target/AArch64/AArch64InstrInfo.h index 96ec055..59fd05a 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.h +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.h @@ -17,6 +17,7 @@ #include "AArch64RegisterInfo.h" #include "llvm/CodeGen/TargetInstrInfo.h" #include "llvm/Support/TypeSize.h" +#include #define GET_INSTRINFO_HEADER #include "AArch64GenInstrInfo.inc" @@ -289,7 +290,7 @@ public: bool isFunctionSafeToOutlineFrom(MachineFunction &MF, bool OutlineFromLinkOnceODRs) const override; - outliner::OutlinedFunction getOutliningCandidateInfo( + std::optional getOutliningCandidateInfo( std::vector &RepeatedSequenceLocs) const override; outliner::InstrType getOutliningTypeImpl(MachineBasicBlock::iterator &MIT, unsigned Flags) const override; diff --git a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp index 77557c3..4a158b3 100644 --- a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp +++ b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp @@ -5869,7 +5869,8 @@ static bool isLRAvailable(const TargetRegisterInfo &TRI, return !Live; } -outliner::OutlinedFunction ARMBaseInstrInfo::getOutliningCandidateInfo( +std::optional +ARMBaseInstrInfo::getOutliningCandidateInfo( std::vector &RepeatedSequenceLocs) const { outliner::Candidate &FirstCand = RepeatedSequenceLocs[0]; unsigned SequenceSize = @@ -5915,7 +5916,7 @@ outliner::OutlinedFunction ARMBaseInstrInfo::getOutliningCandidateInfo( // If the sequence doesn't have enough candidates left, then we're done. if (RepeatedSequenceLocs.size() < 2) - return outliner::OutlinedFunction(); + return std::nullopt; } // We expect the majority of the outlining candidates to be in consensus with @@ -5941,7 +5942,7 @@ outliner::OutlinedFunction ARMBaseInstrInfo::getOutliningCandidateInfo( RepeatedSequenceLocs.erase(RepeatedSequenceLocs.begin(), NoBTI); if (RepeatedSequenceLocs.size() < 2) - return outliner::OutlinedFunction(); + return std::nullopt; // Likewise, partition the candidates according to PAC-RET enablement. auto NoPAC = @@ -5958,7 +5959,7 @@ outliner::OutlinedFunction ARMBaseInstrInfo::getOutliningCandidateInfo( RepeatedSequenceLocs.erase(RepeatedSequenceLocs.begin(), NoPAC); if (RepeatedSequenceLocs.size() < 2) - return outliner::OutlinedFunction(); + return std::nullopt; // At this point, we have only "safe" candidates to outline. Figure out // frame + call instruction information. diff --git a/llvm/lib/Target/ARM/ARMBaseInstrInfo.h b/llvm/lib/Target/ARM/ARMBaseInstrInfo.h index 70c3be2..5efcc1a 100644 --- a/llvm/lib/Target/ARM/ARMBaseInstrInfo.h +++ b/llvm/lib/Target/ARM/ARMBaseInstrInfo.h @@ -348,7 +348,7 @@ public: /// ARM supports the MachineOutliner. bool isFunctionSafeToOutlineFrom(MachineFunction &MF, bool OutlineFromLinkOnceODRs) const override; - outliner::OutlinedFunction getOutliningCandidateInfo( + std::optional getOutliningCandidateInfo( std::vector &RepeatedSequenceLocs) const override; void mergeOutliningCandidateAttributes( Function &F, std::vector &Candidates) const override; diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp index d525365..d1a8a41 100644 --- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp +++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp @@ -1925,7 +1925,8 @@ bool RISCVInstrInfo::shouldOutlineFromFunctionByDefault( return MF.getFunction().hasMinSize(); } -outliner::OutlinedFunction RISCVInstrInfo::getOutliningCandidateInfo( +std::optional +RISCVInstrInfo::getOutliningCandidateInfo( std::vector &RepeatedSequenceLocs) const { // First we need to filter out candidates where the X5 register (IE t0) can't @@ -1939,7 +1940,7 @@ outliner::OutlinedFunction RISCVInstrInfo::getOutliningCandidateInfo( // If the sequence doesn't have enough candidates left, then we're done. if (RepeatedSequenceLocs.size() < 2) - return outliner::OutlinedFunction(); + return std::nullopt; unsigned SequenceSize = 0; diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.h b/llvm/lib/Target/RISCV/RISCVInstrInfo.h index cc84e9c..e3b394e 100644 --- a/llvm/lib/Target/RISCV/RISCVInstrInfo.h +++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.h @@ -159,7 +159,7 @@ public: bool shouldOutlineFromFunctionByDefault(MachineFunction &MF) const override; // Calculate target-specific information for a set of outlining candidates. - outliner::OutlinedFunction getOutliningCandidateInfo( + std::optional getOutliningCandidateInfo( std::vector &RepeatedSequenceLocs) const override; // Return if/how a given MachineInstr should be outlined. diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp index 557e18c..dbf5e8d 100644 --- a/llvm/lib/Target/X86/X86InstrInfo.cpp +++ b/llvm/lib/Target/X86/X86InstrInfo.cpp @@ -43,6 +43,7 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/raw_ostream.h" #include "llvm/Target/TargetOptions.h" +#include using namespace llvm; @@ -9598,7 +9599,8 @@ enum MachineOutlinerClass { MachineOutlinerTailCall }; -outliner::OutlinedFunction X86InstrInfo::getOutliningCandidateInfo( +std::optional +X86InstrInfo::getOutliningCandidateInfo( std::vector &RepeatedSequenceLocs) const { unsigned SequenceSize = std::accumulate(RepeatedSequenceLocs[0].front(), @@ -9631,7 +9633,7 @@ outliner::OutlinedFunction X86InstrInfo::getOutliningCandidateInfo( C.getMF()->getFrameInstructions(); if (CFICount > 0 && CFICount != CFIInstructions.size()) - return outliner::OutlinedFunction(); + return std::nullopt; } // FIXME: Use real size in bytes for call and ret instructions. @@ -9646,7 +9648,7 @@ outliner::OutlinedFunction X86InstrInfo::getOutliningCandidateInfo( } if (CFICount > 0) - return outliner::OutlinedFunction(); + return std::nullopt; for (outliner::Candidate &C : RepeatedSequenceLocs) C.setCallInfo(MachineOutlinerDefault, 1); diff --git a/llvm/lib/Target/X86/X86InstrInfo.h b/llvm/lib/Target/X86/X86InstrInfo.h index ff588b1..f8016b3 100644 --- a/llvm/lib/Target/X86/X86InstrInfo.h +++ b/llvm/lib/Target/X86/X86InstrInfo.h @@ -551,7 +551,7 @@ public: ArrayRef> getSerializableDirectMachineOperandTargetFlags() const override; - outliner::OutlinedFunction getOutliningCandidateInfo( + std::optional getOutliningCandidateInfo( std::vector &RepeatedSequenceLocs) const override; bool isFunctionSafeToOutlineFrom(MachineFunction &MF, -- 2.7.4