CodeGen: Eliminate dynamic relocations in the register superclass tables. #119122
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
s-barannikov
reviewed
Dec 8, 2024
s-barannikov
reviewed
Dec 8, 2024
|
@llvm/pr-subscribers-tablegen @llvm/pr-subscribers-backend-arm Author: Owen Anderson (resistor) ChangesFull diff: https://github.com/llvm/llvm-project/pull/119122.diff 6 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
index 292fa3c94969be..c69c5205bf9e3c 100644
--- a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
@@ -42,11 +42,38 @@ class VirtRegMap;
class LiveIntervals;
class LiveInterval;
+/// TargetSuperClassIterator enumerates all super-registers of RegClass.
+class TargetSuperClassIterator
+ : public iterator_adaptor_base<TargetSuperClassIterator, const unsigned *> {
+public:
+ /// Constructs an end iterator.
+ TargetSuperClassIterator() = default;
+
+ TargetSuperClassIterator(const unsigned *V, const TargetRegisterClass *T) { I = V; }
+
+ const TargetRegisterClass &operator*() const { return *I; }
+
+ using iterator_adaptor_base::operator++;
+
+ bool operator==(const TargetSuperClassIterator &Other) const {
+ // End can be represented either with a nullptr or with a ptr to
+ // a sentinel value of ~0U. They must compare equal.
+ bool SelfIsEnd = !I || *I == ~0U;
+ bool OtherIsEnd = !Other.I || *Other.I == ~0U;
+ if (SelfIsEnd && OtherIsEnd)
+ return true;
+
+ return I == Other.I;
+ }
+
+ /// Returns true if this iterator is not yet at the end.
+ bool isValid() const { return I && *I != ~0U; }
+};
+
class TargetRegisterClass {
public:
using iterator = const MCPhysReg *;
using const_iterator = const MCPhysReg *;
- using sc_iterator = const TargetRegisterClass* const *;
// Instance variables filled by tablegen, do not use!
const MCRegisterClass *MC;
@@ -67,7 +94,7 @@ class TargetRegisterClass {
/// Whether a combination of subregisters can cover every register in the
/// class. See also the CoveredBySubRegs description in Target.td.
const bool CoveredBySubRegs;
- const sc_iterator SuperClasses;
+ const unsigned *SuperClasses;
ArrayRef<MCPhysReg> (*OrderFunc)(const MachineFunction&);
/// Return the register class ID number.
@@ -178,15 +205,13 @@ class TargetRegisterClass {
/// Returns a NULL-terminated list of super-classes. The
/// classes are ordered by ID which is also a topological ordering from large
/// to small classes. The list does NOT include the current class.
- sc_iterator getSuperClasses() const {
- return SuperClasses;
+ iterator_range<TargetSuperClassIterator> superclasses() const {
+ return make_range({SuperClasses}, TargetSuperClassIterator());
}
/// Return true if this TargetRegisterClass is a subset
/// class of at least one other TargetRegisterClass.
- bool isASubClass() const {
- return SuperClasses[0] != nullptr;
- }
+ bool isASubClass() const { return SuperClasses[0] != ~0U; }
/// Returns the preferred order for allocating registers from this register
/// class in MF. The raw order comes directly from the .td file and may
diff --git a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
index a1f068f0e049bd..ade447871545e9 100644
--- a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
@@ -262,30 +262,30 @@ bool ARMBaseRegisterInfo::isInlineAsmReadOnlyReg(const MachineFunction &MF,
const TargetRegisterClass *
ARMBaseRegisterInfo::getLargestLegalSuperClass(const TargetRegisterClass *RC,
const MachineFunction &MF) const {
- const TargetRegisterClass *Super = RC;
- TargetRegisterClass::sc_iterator I = RC->getSuperClasses();
+ unsigned SuperID = RC->getID();
+ auto I = RC->superclasses().begin();
do {
- switch (Super->getID()) {
+ switch (SuperID) {
case ARM::GPRRegClassID:
case ARM::SPRRegClassID:
case ARM::DPRRegClassID:
case ARM::GPRPairRegClassID:
- return Super;
+ return getRegClass(SuperID);
case ARM::QPRRegClassID:
case ARM::QQPRRegClassID:
case ARM::QQQQPRRegClassID:
if (MF.getSubtarget<ARMSubtarget>().hasNEON())
- return Super;
+ return getRegClass(SuperID);
break;
case ARM::MQPRRegClassID:
case ARM::MQQPRRegClassID:
case ARM::MQQQQPRRegClassID:
if (MF.getSubtarget<ARMSubtarget>().hasMVEIntegerOps())
- return Super;
+ return getRegClass(SuperID);
break;
}
- Super = *I++;
- } while (Super);
+ SuperID = *I++;
+ } while (SuperID != ~0U);
return RC;
}
diff --git a/llvm/lib/Target/Hexagon/HexagonRegisterInfo.cpp b/llvm/lib/Target/Hexagon/HexagonRegisterInfo.cpp
index d4d121e4380089..d159d00201ad7a 100644
--- a/llvm/lib/Target/Hexagon/HexagonRegisterInfo.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonRegisterInfo.cpp
@@ -431,8 +431,9 @@ unsigned HexagonRegisterInfo::getHexagonSubRegIndex(
return WSub[GenIdx];
}
- if (const TargetRegisterClass *SuperRC = *RC.getSuperClasses())
- return getHexagonSubRegIndex(*SuperRC, GenIdx);
+ unsigned SuperID = *RC.superclasses().begin();
+ if (SuperID != ~0U)
+ return getHexagonSubRegIndex(*getRegClass(SuperID), GenIdx);
llvm_unreachable("Invalid register class");
}
diff --git a/llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp b/llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
index 43dfc4108f8384..019d4cfa33fbaf 100644
--- a/llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
+++ b/llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
@@ -692,21 +692,23 @@ PPCRegisterInfo::getLargestLegalSuperClass(const TargetRegisterClass *RC,
InflateGPRC++;
}
- for (const auto *I = RC->getSuperClasses(); *I; ++I) {
- if (getRegSizeInBits(**I) != getRegSizeInBits(*RC))
+ for (unsigned SuperID : RC->superclasses()) {
+ if (getRegSizeInBits(*getRegClass(SuperID)) != getRegSizeInBits(*RC))
continue;
- switch ((*I)->getID()) {
+ switch (SuperID) {
case PPC::VSSRCRegClassID:
- return Subtarget.hasP8Vector() ? *I : DefaultSuperclass;
+ return Subtarget.hasP8Vector() ? getRegClass(SuperID)
+ : DefaultSuperclass;
case PPC::VSFRCRegClassID:
case PPC::VSRCRegClassID:
- return *I;
+ return getRegClass(SuperID);
case PPC::VSRpRCRegClassID:
- return Subtarget.pairedVectorMemops() ? *I : DefaultSuperclass;
+ return Subtarget.pairedVectorMemops() ? getRegClass(SuperID)
+ : DefaultSuperclass;
case PPC::ACCRCRegClassID:
case PPC::UACCRCRegClassID:
- return Subtarget.hasMMA() ? *I : DefaultSuperclass;
+ return Subtarget.hasMMA() ? getRegClass(SuperID) : DefaultSuperclass;
}
}
}
diff --git a/llvm/lib/Target/X86/X86RegisterInfo.cpp b/llvm/lib/Target/X86/X86RegisterInfo.cpp
index 50db211c99d882..84b9310279fcb3 100644
--- a/llvm/lib/Target/X86/X86RegisterInfo.cpp
+++ b/llvm/lib/Target/X86/X86RegisterInfo.cpp
@@ -123,7 +123,7 @@ X86RegisterInfo::getLargestLegalSuperClass(const TargetRegisterClass *RC,
const X86Subtarget &Subtarget = MF.getSubtarget<X86Subtarget>();
const TargetRegisterClass *Super = RC;
- TargetRegisterClass::sc_iterator I = RC->getSuperClasses();
+ auto I = RC->superclasses().begin();
do {
switch (Super->getID()) {
case X86::FR32RegClassID:
@@ -172,7 +172,8 @@ X86RegisterInfo::getLargestLegalSuperClass(const TargetRegisterClass *RC,
if (getRegSizeInBits(*Super) == getRegSizeInBits(*RC))
return Super;
}
- Super = *I++;
+ Super = I.isValid() ? getRegClass(*I) : nullptr;
+ ++I;
} while (Super);
return RC;
}
diff --git a/llvm/utils/TableGen/RegisterInfoEmitter.cpp b/llvm/utils/TableGen/RegisterInfoEmitter.cpp
index a6f87119aca5ba..a0b4668f152563 100644
--- a/llvm/utils/TableGen/RegisterInfoEmitter.cpp
+++ b/llvm/utils/TableGen/RegisterInfoEmitter.cpp
@@ -1286,8 +1286,8 @@ void RegisterInfoEmitter::runTargetDesc(raw_ostream &OS) {
}
OS << "};\n";
- OS << "\nstatic const TargetRegisterClass *const "
- << "NullRegClasses[] = { nullptr };\n\n";
+ OS << "\nstatic const unsigned "
+ << "NullRegClasses[] = { ~0U };\n\n";
// Emit register class bit mask tables. The first bit mask emitted for a
// register class, RC, is the set of sub-classes, including RC itself.
@@ -1348,11 +1348,10 @@ void RegisterInfoEmitter::runTargetDesc(raw_ostream &OS) {
if (Supers.empty())
continue;
- OS << "static const TargetRegisterClass *const " << RC.getName()
- << "Superclasses[] = {\n";
+ OS << "static unsigned const " << RC.getName() << "Superclasses[] = {\n";
for (const auto *Super : Supers)
- OS << " &" << Super->getQualifiedName() << "RegClass,\n";
- OS << " nullptr\n};\n\n";
+ OS << " " << Super->getQualifiedIdName() << ",\n";
+ OS << " ~0U\n};\n\n";
}
// Emit methods.
|
|
@llvm/pr-subscribers-backend-powerpc Author: Owen Anderson (resistor) ChangesFull diff: https://github.com/llvm/llvm-project/pull/119122.diff 6 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
index 292fa3c94969be..c69c5205bf9e3c 100644
--- a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
@@ -42,11 +42,38 @@ class VirtRegMap;
class LiveIntervals;
class LiveInterval;
+/// TargetSuperClassIterator enumerates all super-registers of RegClass.
+class TargetSuperClassIterator
+ : public iterator_adaptor_base<TargetSuperClassIterator, const unsigned *> {
+public:
+ /// Constructs an end iterator.
+ TargetSuperClassIterator() = default;
+
+ TargetSuperClassIterator(const unsigned *V, const TargetRegisterClass *T) { I = V; }
+
+ const TargetRegisterClass &operator*() const { return *I; }
+
+ using iterator_adaptor_base::operator++;
+
+ bool operator==(const TargetSuperClassIterator &Other) const {
+ // End can be represented either with a nullptr or with a ptr to
+ // a sentinel value of ~0U. They must compare equal.
+ bool SelfIsEnd = !I || *I == ~0U;
+ bool OtherIsEnd = !Other.I || *Other.I == ~0U;
+ if (SelfIsEnd && OtherIsEnd)
+ return true;
+
+ return I == Other.I;
+ }
+
+ /// Returns true if this iterator is not yet at the end.
+ bool isValid() const { return I && *I != ~0U; }
+};
+
class TargetRegisterClass {
public:
using iterator = const MCPhysReg *;
using const_iterator = const MCPhysReg *;
- using sc_iterator = const TargetRegisterClass* const *;
// Instance variables filled by tablegen, do not use!
const MCRegisterClass *MC;
@@ -67,7 +94,7 @@ class TargetRegisterClass {
/// Whether a combination of subregisters can cover every register in the
/// class. See also the CoveredBySubRegs description in Target.td.
const bool CoveredBySubRegs;
- const sc_iterator SuperClasses;
+ const unsigned *SuperClasses;
ArrayRef<MCPhysReg> (*OrderFunc)(const MachineFunction&);
/// Return the register class ID number.
@@ -178,15 +205,13 @@ class TargetRegisterClass {
/// Returns a NULL-terminated list of super-classes. The
/// classes are ordered by ID which is also a topological ordering from large
/// to small classes. The list does NOT include the current class.
- sc_iterator getSuperClasses() const {
- return SuperClasses;
+ iterator_range<TargetSuperClassIterator> superclasses() const {
+ return make_range({SuperClasses}, TargetSuperClassIterator());
}
/// Return true if this TargetRegisterClass is a subset
/// class of at least one other TargetRegisterClass.
- bool isASubClass() const {
- return SuperClasses[0] != nullptr;
- }
+ bool isASubClass() const { return SuperClasses[0] != ~0U; }
/// Returns the preferred order for allocating registers from this register
/// class in MF. The raw order comes directly from the .td file and may
diff --git a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
index a1f068f0e049bd..ade447871545e9 100644
--- a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
@@ -262,30 +262,30 @@ bool ARMBaseRegisterInfo::isInlineAsmReadOnlyReg(const MachineFunction &MF,
const TargetRegisterClass *
ARMBaseRegisterInfo::getLargestLegalSuperClass(const TargetRegisterClass *RC,
const MachineFunction &MF) const {
- const TargetRegisterClass *Super = RC;
- TargetRegisterClass::sc_iterator I = RC->getSuperClasses();
+ unsigned SuperID = RC->getID();
+ auto I = RC->superclasses().begin();
do {
- switch (Super->getID()) {
+ switch (SuperID) {
case ARM::GPRRegClassID:
case ARM::SPRRegClassID:
case ARM::DPRRegClassID:
case ARM::GPRPairRegClassID:
- return Super;
+ return getRegClass(SuperID);
case ARM::QPRRegClassID:
case ARM::QQPRRegClassID:
case ARM::QQQQPRRegClassID:
if (MF.getSubtarget<ARMSubtarget>().hasNEON())
- return Super;
+ return getRegClass(SuperID);
break;
case ARM::MQPRRegClassID:
case ARM::MQQPRRegClassID:
case ARM::MQQQQPRRegClassID:
if (MF.getSubtarget<ARMSubtarget>().hasMVEIntegerOps())
- return Super;
+ return getRegClass(SuperID);
break;
}
- Super = *I++;
- } while (Super);
+ SuperID = *I++;
+ } while (SuperID != ~0U);
return RC;
}
diff --git a/llvm/lib/Target/Hexagon/HexagonRegisterInfo.cpp b/llvm/lib/Target/Hexagon/HexagonRegisterInfo.cpp
index d4d121e4380089..d159d00201ad7a 100644
--- a/llvm/lib/Target/Hexagon/HexagonRegisterInfo.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonRegisterInfo.cpp
@@ -431,8 +431,9 @@ unsigned HexagonRegisterInfo::getHexagonSubRegIndex(
return WSub[GenIdx];
}
- if (const TargetRegisterClass *SuperRC = *RC.getSuperClasses())
- return getHexagonSubRegIndex(*SuperRC, GenIdx);
+ unsigned SuperID = *RC.superclasses().begin();
+ if (SuperID != ~0U)
+ return getHexagonSubRegIndex(*getRegClass(SuperID), GenIdx);
llvm_unreachable("Invalid register class");
}
diff --git a/llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp b/llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
index 43dfc4108f8384..019d4cfa33fbaf 100644
--- a/llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
+++ b/llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
@@ -692,21 +692,23 @@ PPCRegisterInfo::getLargestLegalSuperClass(const TargetRegisterClass *RC,
InflateGPRC++;
}
- for (const auto *I = RC->getSuperClasses(); *I; ++I) {
- if (getRegSizeInBits(**I) != getRegSizeInBits(*RC))
+ for (unsigned SuperID : RC->superclasses()) {
+ if (getRegSizeInBits(*getRegClass(SuperID)) != getRegSizeInBits(*RC))
continue;
- switch ((*I)->getID()) {
+ switch (SuperID) {
case PPC::VSSRCRegClassID:
- return Subtarget.hasP8Vector() ? *I : DefaultSuperclass;
+ return Subtarget.hasP8Vector() ? getRegClass(SuperID)
+ : DefaultSuperclass;
case PPC::VSFRCRegClassID:
case PPC::VSRCRegClassID:
- return *I;
+ return getRegClass(SuperID);
case PPC::VSRpRCRegClassID:
- return Subtarget.pairedVectorMemops() ? *I : DefaultSuperclass;
+ return Subtarget.pairedVectorMemops() ? getRegClass(SuperID)
+ : DefaultSuperclass;
case PPC::ACCRCRegClassID:
case PPC::UACCRCRegClassID:
- return Subtarget.hasMMA() ? *I : DefaultSuperclass;
+ return Subtarget.hasMMA() ? getRegClass(SuperID) : DefaultSuperclass;
}
}
}
diff --git a/llvm/lib/Target/X86/X86RegisterInfo.cpp b/llvm/lib/Target/X86/X86RegisterInfo.cpp
index 50db211c99d882..84b9310279fcb3 100644
--- a/llvm/lib/Target/X86/X86RegisterInfo.cpp
+++ b/llvm/lib/Target/X86/X86RegisterInfo.cpp
@@ -123,7 +123,7 @@ X86RegisterInfo::getLargestLegalSuperClass(const TargetRegisterClass *RC,
const X86Subtarget &Subtarget = MF.getSubtarget<X86Subtarget>();
const TargetRegisterClass *Super = RC;
- TargetRegisterClass::sc_iterator I = RC->getSuperClasses();
+ auto I = RC->superclasses().begin();
do {
switch (Super->getID()) {
case X86::FR32RegClassID:
@@ -172,7 +172,8 @@ X86RegisterInfo::getLargestLegalSuperClass(const TargetRegisterClass *RC,
if (getRegSizeInBits(*Super) == getRegSizeInBits(*RC))
return Super;
}
- Super = *I++;
+ Super = I.isValid() ? getRegClass(*I) : nullptr;
+ ++I;
} while (Super);
return RC;
}
diff --git a/llvm/utils/TableGen/RegisterInfoEmitter.cpp b/llvm/utils/TableGen/RegisterInfoEmitter.cpp
index a6f87119aca5ba..a0b4668f152563 100644
--- a/llvm/utils/TableGen/RegisterInfoEmitter.cpp
+++ b/llvm/utils/TableGen/RegisterInfoEmitter.cpp
@@ -1286,8 +1286,8 @@ void RegisterInfoEmitter::runTargetDesc(raw_ostream &OS) {
}
OS << "};\n";
- OS << "\nstatic const TargetRegisterClass *const "
- << "NullRegClasses[] = { nullptr };\n\n";
+ OS << "\nstatic const unsigned "
+ << "NullRegClasses[] = { ~0U };\n\n";
// Emit register class bit mask tables. The first bit mask emitted for a
// register class, RC, is the set of sub-classes, including RC itself.
@@ -1348,11 +1348,10 @@ void RegisterInfoEmitter::runTargetDesc(raw_ostream &OS) {
if (Supers.empty())
continue;
- OS << "static const TargetRegisterClass *const " << RC.getName()
- << "Superclasses[] = {\n";
+ OS << "static unsigned const " << RC.getName() << "Superclasses[] = {\n";
for (const auto *Super : Supers)
- OS << " &" << Super->getQualifiedName() << "RegClass,\n";
- OS << " nullptr\n};\n\n";
+ OS << " " << Super->getQualifiedIdName() << ",\n";
+ OS << " ~0U\n};\n\n";
}
// Emit methods.
|
s-barannikov
approved these changes
Dec 8, 2024
|
Just to give some motivation, this PR (after #118734 merges) will remove another 13% of non-vtable dynamic relocations from the These are also hard to see as no one table is terribly large, but there are a large number of them. There are also more opportunities remaining here, notably the dynamic relocations required in some of the Also, could the same change done here for superclass tables be done for subreg tables? That's the other one that seems to show up as a lot of separate tables, although not as much as superclasses. |
I'll take a look at the others once this one is in. I expect some of them to be straight forward, but the ID -> TargetRegisterClass table will be tricky. |
|
cc @llvm/pr-subscribers-backend-hexagon |
jayfoad
reviewed
Dec 9, 2024
I figured out a way to get rid of this, by storing the size of the list. PTAL |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
jayfoad
reviewed
Dec 10, 2024
There was a problem hiding this comment.
No specific objection to this patch, but it seems like it could still be cleaned up. If we're storing the size of the list then why do we also need a sentinel value? I'd also be inclined to make superclasses() return an ArrayRef instead of iterator_range (but maybe that's just my own personal prejudice against unnecessary iterators).
Done. |
|
Probably from this patch https://lab.llvm.org/buildbot/#/builders/25/builds/4805 ? |
|
This other patch definitely caused an ASAN failure and I just reverted it: #119138 |
|
Looking at the error logs, it definitely appears to be #119138, not this one |
|
I see those as well |
|
Ok, that's a different error that does come from this commit. Reverting. |
|
Reverted in e940353 |
resistor
added a commit
to resistor/llvm-project
that referenced
this pull request
Dec 11, 2024
…les. This reapplies llvm#119122 with a fix for UBSAN errors in the X86 backend related to incrementing a nullptr.
resistor
added a commit
to resistor/llvm-project
that referenced
this pull request
Dec 11, 2024
…les. This reapplies llvm#119122 with a fix for UBSAN errors in the X86 backend related to incrementing a nullptr.
jayfoad
reviewed
Dec 11, 2024
| @@ -123,7 +123,8 @@ X86RegisterInfo::getLargestLegalSuperClass(const TargetRegisterClass *RC, | |||
| const X86Subtarget &Subtarget = MF.getSubtarget<X86Subtarget>(); | |||
|
|
|||
| const TargetRegisterClass *Super = RC; | |||
| TargetRegisterClass::sc_iterator I = RC->getSuperClasses(); | |||
| auto I = RC->superclasses().begin(); | |||
There was a problem hiding this comment.
I think this code is really trying to iterate over RC and all its (proper) superclasses, so maybe it could be rewritten like:
for (auto ID : concat<unsigned>({RC->getID()}, RC->superclasses()))
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.
No description provided.