[CodeGen] Prune headers and move code out of line for build efficiency, NFC #135622
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…y, NFC I noticed these destructors taking time with -ftime-trace and moved some of them for minor build efficiency improvements. The main impact of moving destructors out of line is that it avoids requiring container fields containing other types from being complete, i.e. one can have uptr<T> or vector<T> as a field with an incomplete type T, and that means we can reduce transitive includes, as with LegalizerInfo.h. Move expensive getDebugOperandsForReg template out-of-line. The std::function instantiation shows up in time trace even if you don't use the function.
|
@llvm/pr-subscribers-backend-x86 Author: Reid Kleckner (rnk) ChangesI noticed these destructors taking time with -ftime-trace and moved some The main impact of moving destructors out of line is that it avoids Move expensive getDebugOperandsForReg template out-of-line. The Full diff: https://github.com/llvm/llvm-project/pull/135622.diff 9 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index 102b1eb07358e..a5d41733085de 100644
--- a/llvm/include/llvm/CodeGen/MachineInstr.h
+++ b/llvm/include/llvm/CodeGen/MachineInstr.h
@@ -612,26 +612,12 @@ class MachineInstr
/// Returns a range of all of the operands that correspond to a debug use of
/// \p Reg.
- template <typename Operand, typename Instruction>
- static iterator_range<
- filter_iterator<Operand *, std::function<bool(Operand &Op)>>>
- getDebugOperandsForReg(Instruction *MI, Register Reg) {
- std::function<bool(Operand & Op)> OpUsesReg(
- [Reg](Operand &Op) { return Op.isReg() && Op.getReg() == Reg; });
- return make_filter_range(MI->debug_operands(), OpUsesReg);
- }
iterator_range<filter_iterator<const MachineOperand *,
std::function<bool(const MachineOperand &Op)>>>
- getDebugOperandsForReg(Register Reg) const {
- return MachineInstr::getDebugOperandsForReg<const MachineOperand,
- const MachineInstr>(this, Reg);
- }
+ getDebugOperandsForReg(Register Reg) const;
iterator_range<filter_iterator<MachineOperand *,
std::function<bool(MachineOperand &Op)>>>
- getDebugOperandsForReg(Register Reg) {
- return MachineInstr::getDebugOperandsForReg<MachineOperand, MachineInstr>(
- this, Reg);
- }
+ getDebugOperandsForReg(Register Reg);
bool isDebugOperand(const MachineOperand *Op) const {
return Op >= adl_begin(debug_operands()) && Op <= adl_end(debug_operands());
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index 4f2f202f94841..0a36975f4f625 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -329,9 +329,6 @@ class TargetLoweringBase {
};
using ArgListTy = std::vector<ArgListEntry>;
- virtual void markLibCallAttributes(MachineFunction *MF, unsigned CC,
- ArgListTy &Args) const {};
-
static ISD::NodeType getExtendForContent(BooleanContent Content) {
switch (Content) {
case UndefinedBooleanContent:
@@ -350,7 +347,7 @@ class TargetLoweringBase {
explicit TargetLoweringBase(const TargetMachine &TM);
TargetLoweringBase(const TargetLoweringBase &) = delete;
TargetLoweringBase &operator=(const TargetLoweringBase &) = delete;
- virtual ~TargetLoweringBase() = default;
+ virtual ~TargetLoweringBase();
/// Return true if the target support strict float operation
bool isStrictFPEnabled() const {
@@ -3866,6 +3863,7 @@ class TargetLowering : public TargetLoweringBase {
TargetLowering &operator=(const TargetLowering &) = delete;
explicit TargetLowering(const TargetMachine &TM);
+ ~TargetLowering() override;
bool isPositionIndependent() const;
@@ -4592,6 +4590,9 @@ class TargetLowering : public TargetLoweringBase {
llvm_unreachable("Not Implemented");
}
+ virtual void markLibCallAttributes(MachineFunction *MF, unsigned CC,
+ ArgListTy &Args) const {}
+
/// This structure contains the information necessary for lowering
/// pointer-authenticating indirect calls. It is equivalent to the "ptrauth"
/// operand bundle found on the call instruction, if any.
diff --git a/llvm/include/llvm/Target/TargetMachine.h b/llvm/include/llvm/Target/TargetMachine.h
index 27eeb415ed644..2ae5c65ef2000 100644
--- a/llvm/include/llvm/Target/TargetMachine.h
+++ b/llvm/include/llvm/Target/TargetMachine.h
@@ -16,7 +16,6 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/PassManager.h"
-#include "llvm/MC/MCStreamer.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/CodeGen.h"
#include "llvm/Support/CommandLine.h"
@@ -45,6 +44,7 @@ class MCAsmInfo;
class MCContext;
class MCInstrInfo;
class MCRegisterInfo;
+class MCStreamer;
class MCSubtargetInfo;
class MCSymbol;
class raw_pwrite_stream;
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index 2409e601ceb46..8b888595cd846 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -794,6 +794,29 @@ bool MachineInstr::shouldUpdateAdditionalCallInfo() const {
return isCandidateForAdditionalCallInfo();
}
+template <typename Operand, typename Instruction>
+static iterator_range<
+ filter_iterator<Operand *, std::function<bool(Operand &Op)>>>
+getDebugOperandsForRegHelper(Instruction *MI, Register Reg) {
+ std::function<bool(Operand & Op)> OpUsesReg(
+ [Reg](Operand &Op) { return Op.isReg() && Op.getReg() == Reg; });
+ return make_filter_range(MI->debug_operands(), OpUsesReg);
+}
+
+iterator_range<filter_iterator<const MachineOperand *,
+ std::function<bool(const MachineOperand &Op)>>>
+MachineInstr::getDebugOperandsForReg(Register Reg) const {
+ return getDebugOperandsForRegHelper<const MachineOperand,
+ const MachineInstr>(this, Reg);
+}
+
+iterator_range<filter_iterator<MachineOperand *,
+ std::function<bool(MachineOperand &Op)>>>
+MachineInstr::getDebugOperandsForReg(Register Reg) {
+ return getDebugOperandsForRegHelper<MachineOperand, MachineInstr>(
+ this, Reg);
+}
+
unsigned MachineInstr::getNumExplicitOperands() const {
unsigned NumOperands = MCID->getNumOperands();
if (!MCID->isVariadic())
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index 89f806d8b1c30..5308593b5c988 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -41,6 +41,9 @@ using namespace llvm;
TargetLowering::TargetLowering(const TargetMachine &tm)
: TargetLoweringBase(tm) {}
+// Define the virtual destructor out-of-line for build efficiency.
+TargetLowering::~TargetLowering() = default;
+
const char *TargetLowering::getTargetNodeName(unsigned Opcode) const {
return nullptr;
}
diff --git a/llvm/lib/CodeGen/TargetLoweringBase.cpp b/llvm/lib/CodeGen/TargetLoweringBase.cpp
index 91ae9040581ee..483e52d16d537 100644
--- a/llvm/lib/CodeGen/TargetLoweringBase.cpp
+++ b/llvm/lib/CodeGen/TargetLoweringBase.cpp
@@ -668,6 +668,10 @@ TargetLoweringBase::TargetLoweringBase(const TargetMachine &tm)
RTLIB::initCmpLibcallCCs(CmpLibcallCCs);
}
+// Define the virtual destructor out-of-line to act as a key method to anchor
+// debug info (see coding standards).
+TargetLoweringBase::~TargetLoweringBase() = default;
+
void TargetLoweringBase::initActions() {
// All operations default to being supported.
memset(OpActions, 0, sizeof(OpActions));
diff --git a/llvm/lib/Target/X86/X86Subtarget.cpp b/llvm/lib/Target/X86/X86Subtarget.cpp
index b563f6ebce34e..81bfe4b28be8e 100644
--- a/llvm/lib/Target/X86/X86Subtarget.cpp
+++ b/llvm/lib/Target/X86/X86Subtarget.cpp
@@ -361,6 +361,9 @@ X86Subtarget::X86Subtarget(const Triple &TT, StringRef CPU, StringRef TuneCPU,
InstSelector.reset(createX86InstructionSelector(TM, *this, *RBI));
}
+// Define the virtual destructor out-of-line for build efficiency.
+X86Subtarget::~X86Subtarget() = default;
+
const CallLowering *X86Subtarget::getCallLowering() const {
return CallLoweringInfo.get();
}
diff --git a/llvm/lib/Target/X86/X86Subtarget.h b/llvm/lib/Target/X86/X86Subtarget.h
index 8f2d326a69398..b53350efc85f5 100644
--- a/llvm/lib/Target/X86/X86Subtarget.h
+++ b/llvm/lib/Target/X86/X86Subtarget.h
@@ -17,7 +17,6 @@
#include "X86ISelLowering.h"
#include "X86InstrInfo.h"
#include "X86SelectionDAGInfo.h"
-#include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
#include "llvm/CodeGen/TargetSubtargetInfo.h"
#include "llvm/IR/CallingConv.h"
#include "llvm/TargetParser/Triple.h"
@@ -114,6 +113,7 @@ class X86Subtarget final : public X86GenSubtargetInfo {
const X86TargetMachine &TM, MaybeAlign StackAlignOverride,
unsigned PreferVectorWidthOverride,
unsigned RequiredVectorWidth);
+ ~X86Subtarget() override;
const X86TargetLowering *getTargetLowering() const override {
return &TLInfo;
diff --git a/llvm/lib/Target/X86/X86TargetTransformInfo.cpp b/llvm/lib/Target/X86/X86TargetTransformInfo.cpp
index 7d168d33bb3e9..41e437a47ba29 100644
--- a/llvm/lib/Target/X86/X86TargetTransformInfo.cpp
+++ b/llvm/lib/Target/X86/X86TargetTransformInfo.cpp
@@ -49,6 +49,7 @@
//===----------------------------------------------------------------------===//
#include "X86TargetTransformInfo.h"
+#include "llvm/ADT/SmallBitVector.h"
#include "llvm/Analysis/TargetTransformInfo.h"
#include "llvm/CodeGen/BasicTTIImpl.h"
#include "llvm/CodeGen/CostTable.h"
|
|
@llvm/pr-subscribers-llvm-selectiondag Author: Reid Kleckner (rnk) ChangesI noticed these destructors taking time with -ftime-trace and moved some The main impact of moving destructors out of line is that it avoids Move expensive getDebugOperandsForReg template out-of-line. The Full diff: https://github.com/llvm/llvm-project/pull/135622.diff 9 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index 102b1eb07358e..a5d41733085de 100644
--- a/llvm/include/llvm/CodeGen/MachineInstr.h
+++ b/llvm/include/llvm/CodeGen/MachineInstr.h
@@ -612,26 +612,12 @@ class MachineInstr
/// Returns a range of all of the operands that correspond to a debug use of
/// \p Reg.
- template <typename Operand, typename Instruction>
- static iterator_range<
- filter_iterator<Operand *, std::function<bool(Operand &Op)>>>
- getDebugOperandsForReg(Instruction *MI, Register Reg) {
- std::function<bool(Operand & Op)> OpUsesReg(
- [Reg](Operand &Op) { return Op.isReg() && Op.getReg() == Reg; });
- return make_filter_range(MI->debug_operands(), OpUsesReg);
- }
iterator_range<filter_iterator<const MachineOperand *,
std::function<bool(const MachineOperand &Op)>>>
- getDebugOperandsForReg(Register Reg) const {
- return MachineInstr::getDebugOperandsForReg<const MachineOperand,
- const MachineInstr>(this, Reg);
- }
+ getDebugOperandsForReg(Register Reg) const;
iterator_range<filter_iterator<MachineOperand *,
std::function<bool(MachineOperand &Op)>>>
- getDebugOperandsForReg(Register Reg) {
- return MachineInstr::getDebugOperandsForReg<MachineOperand, MachineInstr>(
- this, Reg);
- }
+ getDebugOperandsForReg(Register Reg);
bool isDebugOperand(const MachineOperand *Op) const {
return Op >= adl_begin(debug_operands()) && Op <= adl_end(debug_operands());
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index 4f2f202f94841..0a36975f4f625 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -329,9 +329,6 @@ class TargetLoweringBase {
};
using ArgListTy = std::vector<ArgListEntry>;
- virtual void markLibCallAttributes(MachineFunction *MF, unsigned CC,
- ArgListTy &Args) const {};
-
static ISD::NodeType getExtendForContent(BooleanContent Content) {
switch (Content) {
case UndefinedBooleanContent:
@@ -350,7 +347,7 @@ class TargetLoweringBase {
explicit TargetLoweringBase(const TargetMachine &TM);
TargetLoweringBase(const TargetLoweringBase &) = delete;
TargetLoweringBase &operator=(const TargetLoweringBase &) = delete;
- virtual ~TargetLoweringBase() = default;
+ virtual ~TargetLoweringBase();
/// Return true if the target support strict float operation
bool isStrictFPEnabled() const {
@@ -3866,6 +3863,7 @@ class TargetLowering : public TargetLoweringBase {
TargetLowering &operator=(const TargetLowering &) = delete;
explicit TargetLowering(const TargetMachine &TM);
+ ~TargetLowering() override;
bool isPositionIndependent() const;
@@ -4592,6 +4590,9 @@ class TargetLowering : public TargetLoweringBase {
llvm_unreachable("Not Implemented");
}
+ virtual void markLibCallAttributes(MachineFunction *MF, unsigned CC,
+ ArgListTy &Args) const {}
+
/// This structure contains the information necessary for lowering
/// pointer-authenticating indirect calls. It is equivalent to the "ptrauth"
/// operand bundle found on the call instruction, if any.
diff --git a/llvm/include/llvm/Target/TargetMachine.h b/llvm/include/llvm/Target/TargetMachine.h
index 27eeb415ed644..2ae5c65ef2000 100644
--- a/llvm/include/llvm/Target/TargetMachine.h
+++ b/llvm/include/llvm/Target/TargetMachine.h
@@ -16,7 +16,6 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/PassManager.h"
-#include "llvm/MC/MCStreamer.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/CodeGen.h"
#include "llvm/Support/CommandLine.h"
@@ -45,6 +44,7 @@ class MCAsmInfo;
class MCContext;
class MCInstrInfo;
class MCRegisterInfo;
+class MCStreamer;
class MCSubtargetInfo;
class MCSymbol;
class raw_pwrite_stream;
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index 2409e601ceb46..8b888595cd846 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -794,6 +794,29 @@ bool MachineInstr::shouldUpdateAdditionalCallInfo() const {
return isCandidateForAdditionalCallInfo();
}
+template <typename Operand, typename Instruction>
+static iterator_range<
+ filter_iterator<Operand *, std::function<bool(Operand &Op)>>>
+getDebugOperandsForRegHelper(Instruction *MI, Register Reg) {
+ std::function<bool(Operand & Op)> OpUsesReg(
+ [Reg](Operand &Op) { return Op.isReg() && Op.getReg() == Reg; });
+ return make_filter_range(MI->debug_operands(), OpUsesReg);
+}
+
+iterator_range<filter_iterator<const MachineOperand *,
+ std::function<bool(const MachineOperand &Op)>>>
+MachineInstr::getDebugOperandsForReg(Register Reg) const {
+ return getDebugOperandsForRegHelper<const MachineOperand,
+ const MachineInstr>(this, Reg);
+}
+
+iterator_range<filter_iterator<MachineOperand *,
+ std::function<bool(MachineOperand &Op)>>>
+MachineInstr::getDebugOperandsForReg(Register Reg) {
+ return getDebugOperandsForRegHelper<MachineOperand, MachineInstr>(
+ this, Reg);
+}
+
unsigned MachineInstr::getNumExplicitOperands() const {
unsigned NumOperands = MCID->getNumOperands();
if (!MCID->isVariadic())
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index 89f806d8b1c30..5308593b5c988 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -41,6 +41,9 @@ using namespace llvm;
TargetLowering::TargetLowering(const TargetMachine &tm)
: TargetLoweringBase(tm) {}
+// Define the virtual destructor out-of-line for build efficiency.
+TargetLowering::~TargetLowering() = default;
+
const char *TargetLowering::getTargetNodeName(unsigned Opcode) const {
return nullptr;
}
diff --git a/llvm/lib/CodeGen/TargetLoweringBase.cpp b/llvm/lib/CodeGen/TargetLoweringBase.cpp
index 91ae9040581ee..483e52d16d537 100644
--- a/llvm/lib/CodeGen/TargetLoweringBase.cpp
+++ b/llvm/lib/CodeGen/TargetLoweringBase.cpp
@@ -668,6 +668,10 @@ TargetLoweringBase::TargetLoweringBase(const TargetMachine &tm)
RTLIB::initCmpLibcallCCs(CmpLibcallCCs);
}
+// Define the virtual destructor out-of-line to act as a key method to anchor
+// debug info (see coding standards).
+TargetLoweringBase::~TargetLoweringBase() = default;
+
void TargetLoweringBase::initActions() {
// All operations default to being supported.
memset(OpActions, 0, sizeof(OpActions));
diff --git a/llvm/lib/Target/X86/X86Subtarget.cpp b/llvm/lib/Target/X86/X86Subtarget.cpp
index b563f6ebce34e..81bfe4b28be8e 100644
--- a/llvm/lib/Target/X86/X86Subtarget.cpp
+++ b/llvm/lib/Target/X86/X86Subtarget.cpp
@@ -361,6 +361,9 @@ X86Subtarget::X86Subtarget(const Triple &TT, StringRef CPU, StringRef TuneCPU,
InstSelector.reset(createX86InstructionSelector(TM, *this, *RBI));
}
+// Define the virtual destructor out-of-line for build efficiency.
+X86Subtarget::~X86Subtarget() = default;
+
const CallLowering *X86Subtarget::getCallLowering() const {
return CallLoweringInfo.get();
}
diff --git a/llvm/lib/Target/X86/X86Subtarget.h b/llvm/lib/Target/X86/X86Subtarget.h
index 8f2d326a69398..b53350efc85f5 100644
--- a/llvm/lib/Target/X86/X86Subtarget.h
+++ b/llvm/lib/Target/X86/X86Subtarget.h
@@ -17,7 +17,6 @@
#include "X86ISelLowering.h"
#include "X86InstrInfo.h"
#include "X86SelectionDAGInfo.h"
-#include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
#include "llvm/CodeGen/TargetSubtargetInfo.h"
#include "llvm/IR/CallingConv.h"
#include "llvm/TargetParser/Triple.h"
@@ -114,6 +113,7 @@ class X86Subtarget final : public X86GenSubtargetInfo {
const X86TargetMachine &TM, MaybeAlign StackAlignOverride,
unsigned PreferVectorWidthOverride,
unsigned RequiredVectorWidth);
+ ~X86Subtarget() override;
const X86TargetLowering *getTargetLowering() const override {
return &TLInfo;
diff --git a/llvm/lib/Target/X86/X86TargetTransformInfo.cpp b/llvm/lib/Target/X86/X86TargetTransformInfo.cpp
index 7d168d33bb3e9..41e437a47ba29 100644
--- a/llvm/lib/Target/X86/X86TargetTransformInfo.cpp
+++ b/llvm/lib/Target/X86/X86TargetTransformInfo.cpp
@@ -49,6 +49,7 @@
//===----------------------------------------------------------------------===//
#include "X86TargetTransformInfo.h"
+#include "llvm/ADT/SmallBitVector.h"
#include "llvm/Analysis/TargetTransformInfo.h"
#include "llvm/CodeGen/BasicTTIImpl.h"
#include "llvm/CodeGen/CostTable.h"
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
I actually didn't intend to request review, I was just uploading this to run the premerge checks, especially to cover MSVC builds, which found something. |
phoebewang
reviewed
Apr 15, 2025
| @@ -49,6 +49,7 @@ | |||
| //===----------------------------------------------------------------------===// | |||
|
|
|||
| #include "X86TargetTransformInfo.h" | |||
| #include "llvm/ADT/SmallBitVector.h" | |||
There was a problem hiding this comment.
Is the change needed here?
There was a problem hiding this comment.
Yes, it was papered over due to transitive includes.
var-const
pushed a commit
to ldionne/llvm-project
that referenced
this pull request
Apr 17, 2025
…y, NFC (llvm#135622) I noticed these destructors taking time with -ftime-trace and moved some of them for minor build efficiency improvements. The main impact of moving destructors out of line is that it avoids requiring container fields containing other types from being complete, i.e. one can have uptr<T> or vector<T> as a field with an incomplete type T, and that means we can reduce transitive includes, as with LegalizerInfo.h. Move expensive getDebugOperandsForReg template out-of-line. The std::function instantiation shows up in time trace even if you don't use the function.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed these destructors taking time with -ftime-trace and moved some
of them for minor build efficiency improvements.
The main impact of moving destructors out of line is that it avoids
requiring container fields containing other types from being complete,
i.e. one can have uptr or vector as a field with an incomplete
type T, and that means we can reduce transitive includes, as with
LegalizerInfo.h.
Move expensive getDebugOperandsForReg template out-of-line. The
std::function instantiation shows up in time trace even if you don't use
the function.