[Target][RISCV] Add HwMode support to subregister index size/offset. #86368
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
This is needed to proivde proper size and offset for the GPRPair subreg indices on RISC-V. Previously we marked them as having unknown size and offset, but this stops DwarfExpression::addMachineReg from being able to find the registers that make up the pair.
|
@llvm/pr-subscribers-backend-risc-v Author: Craig Topper (topperc) ChangesThis is needed to provide proper size and offset for the GPRPair subreg indices on RISC-V. The size of a GPR already uses HwMode. Previously we said the subreg indices have unknown size and offset, but this stops DwarfExpression::addMachineReg from being able to find the registers that make up the pair. I believe this fixes #85864 but need to verify. Full diff: https://github.com/llvm/llvm-project/pull/86368.diff 9 Files Affected:
diff --git a/llvm/include/llvm/Target/Target.td b/llvm/include/llvm/Target/Target.td
index cb1c0ed2513d45..648b967b26cd69 100644
--- a/llvm/include/llvm/Target/Target.td
+++ b/llvm/include/llvm/Target/Target.td
@@ -83,10 +83,24 @@ class RegInfoByHwMode<list<HwMode> Ms = [], list<RegInfo> Ts = []>
list<RegInfo> Objects = Ts;
}
+class SubRegRange<int size, int offset = 0> {
+ int Size = size; // Sub register size in bits.
+ int Offset = offset; // Offset of the first bit of the sub-reg index.
+}
+
+class SubRegRangeByHwMode<list<HwMode> Ms = [], list<SubRegRange> Ts = []>
+ : HwModeSelect<Ms> {
+ // The length of this list must be the same as the length of Ms.
+ list<SubRegRange> Objects = Ts;
+}
+
// SubRegIndex - Use instances of SubRegIndex to identify subregisters.
class SubRegIndex<int size, int offset = 0> {
string Namespace = "";
+ // The size/offset information, parameterized by a HW mode.
+ SubRegRangeByHwMode SubRegRanges;
+
// Size - Size (in bits) of the sub-registers represented by this index.
int Size = size;
diff --git a/llvm/lib/CodeGen/TargetRegisterInfo.cpp b/llvm/lib/CodeGen/TargetRegisterInfo.cpp
index 4120c74c23b193..4e06393f4cc1d3 100644
--- a/llvm/lib/CodeGen/TargetRegisterInfo.cpp
+++ b/llvm/lib/CodeGen/TargetRegisterInfo.cpp
@@ -595,13 +595,13 @@ bool TargetRegisterInfo::getCoveringSubRegIndexes(
unsigned TargetRegisterInfo::getSubRegIdxSize(unsigned Idx) const {
assert(Idx && Idx < getNumSubRegIndices() &&
"This is not a subregister index");
- return SubRegIdxRanges[Idx].Size;
+ return SubRegIdxRanges[HwMode * getNumSubRegIndices() + Idx].Size;
}
unsigned TargetRegisterInfo::getSubRegIdxOffset(unsigned Idx) const {
assert(Idx && Idx < getNumSubRegIndices() &&
"This is not a subregister index");
- return SubRegIdxRanges[Idx].Offset;
+ return SubRegIdxRanges[HwMode * getNumSubRegIndices() + Idx].Offset;
}
Register
diff --git a/llvm/lib/Target/RISCV/RISCVRegisterInfo.td b/llvm/lib/Target/RISCV/RISCVRegisterInfo.td
index 9da1f73681c68c..90c4a7193ee337 100644
--- a/llvm/lib/Target/RISCV/RISCVRegisterInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVRegisterInfo.td
@@ -64,9 +64,14 @@ def sub_vrm1_6 : ComposedSubRegIndex<sub_vrm2_3, sub_vrm1_0>;
def sub_vrm1_7 : ComposedSubRegIndex<sub_vrm2_3, sub_vrm1_1>;
// GPR sizes change with HwMode.
-// FIXME: Support HwMode in SubRegIndex?
-def sub_gpr_even : SubRegIndex<-1>;
-def sub_gpr_odd : SubRegIndex<-1, -1>;
+def sub_gpr_even : SubRegIndex<32> {
+ let SubRegRanges = SubRegRangeByHwMode<[RV32, RV64],
+ [SubRegRange<32>, SubRegRange<64>]>;
+}
+def sub_gpr_odd : SubRegIndex<32, 32> {
+ let SubRegRanges = SubRegRangeByHwMode<[RV32, RV64],
+ [SubRegRange<32, 32>, SubRegRange<64, 64>]>;
+}
} // Namespace = "RISCV"
// Integer registers
diff --git a/llvm/test/TableGen/ConcatenatedSubregs.td b/llvm/test/TableGen/ConcatenatedSubregs.td
index 5b354c94dca52c..b836bff6ff02cd 100644
--- a/llvm/test/TableGen/ConcatenatedSubregs.td
+++ b/llvm/test/TableGen/ConcatenatedSubregs.td
@@ -90,16 +90,16 @@ def TestTarget : Target;
// CHECK-LABEL: RegisterClass DRegs:
// CHECK-LABEL: SubRegIndex ssub1:
-// CHECK: Offset, Size: 16, 16
+// CHECK: Offset, Size: { Default:16, 16 }
// CHECK-LABEL: SubRegIndex sub0:
// CHECK-LABEL: SubRegIndex sub1:
// CHECK-LABEL: SubRegIndex sub2:
// Check inferred indexes:
// CHECK-LABEL: SubRegIndex ssub1_ssub2:
-// CHECK: Offset, Size: 16, 65535
+// CHECK: Offset, Size: { Default:16, 65535 }
// CHECK-LABEL: SubRegIndex ssub3_ssub4:
// CHECK-LABEL: SubRegIndex ssub0_ssub1_ssub2_ssub3:
-// CHECK: Offset, Size: 65535, 65535
+// CHECK: Offset, Size: { Default:65535, 65535 }
// CHECK-LABEL: SubRegIndex ssub1_ssub2_ssub3_ssub4:
// Check that all subregs are generated on some examples
diff --git a/llvm/utils/TableGen/CodeGenRegisters.cpp b/llvm/utils/TableGen/CodeGenRegisters.cpp
index e851d4c16bf7b6..624e8d5d54ba8f 100644
--- a/llvm/utils/TableGen/CodeGenRegisters.cpp
+++ b/llvm/utils/TableGen/CodeGenRegisters.cpp
@@ -47,19 +47,24 @@ using namespace llvm;
// CodeGenSubRegIndex
//===----------------------------------------------------------------------===//
-CodeGenSubRegIndex::CodeGenSubRegIndex(Record *R, unsigned Enum)
+CodeGenSubRegIndex::CodeGenSubRegIndex(Record *R, unsigned Enum,
+ const CodeGenHwModes &CGH)
: TheDef(R), EnumValue(Enum), AllSuperRegsCovered(true), Artificial(true) {
Name = std::string(R->getName());
if (R->getValue("Namespace"))
Namespace = std::string(R->getValueAsString("Namespace"));
- Size = R->getValueAsInt("Size");
- Offset = R->getValueAsInt("Offset");
+
+ if (const RecordVal *RV = R->getValue("SubRegRanges"))
+ if (auto *DI = dyn_cast_or_null<DefInit>(RV->getValue()))
+ Range = SubRegRangeByHwMode(DI->getDef(), CGH);
+ if (!Range.hasDefault())
+ Range.insertSubRegRangeForMode(DefaultMode, SubRegRange(R));
}
CodeGenSubRegIndex::CodeGenSubRegIndex(StringRef N, StringRef Nspace,
unsigned Enum)
: TheDef(nullptr), Name(std::string(N)), Namespace(std::string(Nspace)),
- Size(-1), Offset(-1), EnumValue(Enum), AllSuperRegsCovered(true),
+ Range(SubRegRange(-1, -1)), EnumValue(Enum), AllSuperRegsCovered(true),
Artificial(true) {}
std::string CodeGenSubRegIndex::getQualifiedName() const {
@@ -81,7 +86,7 @@ void CodeGenSubRegIndex::updateComponents(CodeGenRegBank &RegBank) {
"ComposedOf must have exactly two entries");
CodeGenSubRegIndex *A = RegBank.getSubRegIdx(Comps[0]);
CodeGenSubRegIndex *B = RegBank.getSubRegIdx(Comps[1]);
- CodeGenSubRegIndex *X = A->addComposite(B, this);
+ CodeGenSubRegIndex *X = A->addComposite(B, this, RegBank.getHwModes());
if (X)
PrintFatalError(TheDef->getLoc(), "Ambiguous ComposedOf entries");
}
@@ -518,7 +523,8 @@ void CodeGenRegister::computeSecondarySubRegs(CodeGenRegBank &RegBank) {
// Each part of Cand is a sub-register of this. Make the full Cand also
// a sub-register with a concatenated sub-register index.
- CodeGenSubRegIndex *Concat = RegBank.getConcatSubRegIndex(Parts);
+ CodeGenSubRegIndex *Concat =
+ RegBank.getConcatSubRegIndex(Parts, RegBank.getHwModes());
std::pair<CodeGenSubRegIndex *, CodeGenRegister *> NewSubReg =
std::pair(Concat, Cand);
@@ -542,7 +548,7 @@ void CodeGenRegister::computeSecondarySubRegs(CodeGenRegBank &RegBank) {
PrintFatalError(TheDef->getLoc(), "No SubRegIndex for " +
SubReg.second->getName() +
" in " + getName());
- NewIdx->addComposite(SubReg.first, SubIdx);
+ NewIdx->addComposite(SubReg.first, SubIdx, RegBank.getHwModes());
}
}
}
@@ -1315,7 +1321,7 @@ CodeGenSubRegIndex *CodeGenRegBank::getSubRegIdx(Record *Def) {
CodeGenSubRegIndex *&Idx = Def2SubRegIdx[Def];
if (Idx)
return Idx;
- SubRegIndices.emplace_back(Def, SubRegIndices.size() + 1);
+ SubRegIndices.emplace_back(Def, SubRegIndices.size() + 1, getHwModes());
Idx = &SubRegIndices.back();
return Idx;
}
@@ -1379,12 +1385,13 @@ CodeGenRegBank::getCompositeSubRegIndex(CodeGenSubRegIndex *A,
// None exists, synthesize one.
std::string Name = A->getName() + "_then_" + B->getName();
Comp = createSubRegIndex(Name, A->getNamespace());
- A->addComposite(B, Comp);
+ A->addComposite(B, Comp, getHwModes());
return Comp;
}
CodeGenSubRegIndex *CodeGenRegBank::getConcatSubRegIndex(
- const SmallVector<CodeGenSubRegIndex *, 8> &Parts) {
+ const SmallVector<CodeGenSubRegIndex *, 8> &Parts,
+ const CodeGenHwModes &CGH) {
assert(Parts.size() > 1 && "Need two parts to concatenate");
#ifndef NDEBUG
for (CodeGenSubRegIndex *Idx : Parts) {
@@ -1399,28 +1406,47 @@ CodeGenSubRegIndex *CodeGenRegBank::getConcatSubRegIndex(
// None exists, synthesize one.
std::string Name = Parts.front()->getName();
- // Determine whether all parts are contiguous.
- bool IsContinuous = true;
- unsigned Size = Parts.front()->Size;
- unsigned LastOffset = Parts.front()->Offset;
- unsigned LastSize = Parts.front()->Size;
const unsigned UnknownSize = (uint16_t)-1;
+
for (unsigned i = 1, e = Parts.size(); i != e; ++i) {
Name += '_';
Name += Parts[i]->getName();
- if (Size == UnknownSize || Parts[i]->Size == UnknownSize)
- Size = UnknownSize;
- else
- Size += Parts[i]->Size;
- if (LastSize == UnknownSize || Parts[i]->Offset != (LastOffset + LastSize))
- IsContinuous = false;
- LastOffset = Parts[i]->Offset;
- LastSize = Parts[i]->Size;
}
+
Idx = createSubRegIndex(Name, Parts.front()->getNamespace());
- Idx->Size = Size;
- Idx->Offset = IsContinuous ? Parts.front()->Offset : -1;
Idx->ConcatenationOf.assign(Parts.begin(), Parts.end());
+
+ unsigned NumModes = CGH.getNumModeIds();
+ for (unsigned M = 0; M < NumModes; ++M) {
+ const CodeGenSubRegIndex *Part = Parts.front();
+
+ // Determine whether all parts are contiguous.
+ bool IsContinuous = true;
+ const SubRegRange &FirstPartRange = Part->Range.get(M);
+ unsigned Size = FirstPartRange.Size;
+ unsigned LastOffset = FirstPartRange.Offset;
+ unsigned LastSize = FirstPartRange.Size;
+
+ for (unsigned i = 1, e = Parts.size(); i != e; ++i) {
+ Part = Parts[i];
+ Name += '_';
+ Name += Part->getName();
+
+ const SubRegRange &PartRange = Part->Range.get(M);
+ if (Size == UnknownSize || PartRange.Size == UnknownSize)
+ Size = UnknownSize;
+ else
+ Size += PartRange.Size;
+ if (LastSize == UnknownSize ||
+ PartRange.Offset != (LastOffset + LastSize))
+ IsContinuous = false;
+ LastOffset = PartRange.Offset;
+ LastSize = PartRange.Size;
+ }
+ unsigned Offset = IsContinuous ? FirstPartRange.Offset : -1;
+ Idx->Range.get(M) = SubRegRange(Size, Offset);
+ }
+
return Idx;
}
@@ -1504,7 +1530,8 @@ void CodeGenRegBank::computeComposites() {
assert(Idx3 && "Sub-register doesn't have an index");
// Conflicting composition? Emit a warning but allow it.
- if (CodeGenSubRegIndex *Prev = Idx1->addComposite(Idx2, Idx3)) {
+ if (CodeGenSubRegIndex *Prev =
+ Idx1->addComposite(Idx2, Idx3, getHwModes())) {
// If the composition was not user-defined, always emit a warning.
if (!UserDefined.count({Idx1, Idx2}) ||
agree(compose(Idx1, Idx2), SubRegAction.at(Idx3)))
diff --git a/llvm/utils/TableGen/CodeGenRegisters.h b/llvm/utils/TableGen/CodeGenRegisters.h
index c34f376ea99db3..9058baea2b23e9 100644
--- a/llvm/utils/TableGen/CodeGenRegisters.h
+++ b/llvm/utils/TableGen/CodeGenRegisters.h
@@ -68,8 +68,7 @@ class CodeGenSubRegIndex {
std::string Namespace;
public:
- uint16_t Size;
- uint16_t Offset;
+ SubRegRangeByHwMode Range;
const unsigned EnumValue;
mutable LaneBitmask LaneMask;
mutable SmallVector<MaskRolPair, 1> CompositionLaneMaskTransform;
@@ -86,7 +85,7 @@ class CodeGenSubRegIndex {
// indexes are not used to create new register classes.
bool Artificial;
- CodeGenSubRegIndex(Record *R, unsigned Enum);
+ CodeGenSubRegIndex(Record *R, unsigned Enum, const CodeGenHwModes &CGH);
CodeGenSubRegIndex(StringRef N, StringRef Nspace, unsigned Enum);
CodeGenSubRegIndex(CodeGenSubRegIndex &) = delete;
@@ -108,19 +107,42 @@ class CodeGenSubRegIndex {
// Add a composite subreg index: this+A = B.
// Return a conflicting composite, or NULL
- CodeGenSubRegIndex *addComposite(CodeGenSubRegIndex *A,
- CodeGenSubRegIndex *B) {
+ CodeGenSubRegIndex *addComposite(CodeGenSubRegIndex *A, CodeGenSubRegIndex *B,
+ const CodeGenHwModes &CGH) {
assert(A && B);
std::pair<CompMap::iterator, bool> Ins = Composed.insert(std::pair(A, B));
+
// Synthetic subreg indices that aren't contiguous (for instance ARM
// register tuples) don't have a bit range, so it's OK to let
// B->Offset == -1. For the other cases, accumulate the offset and set
// the size here. Only do so if there is no offset yet though.
- if ((Offset != (uint16_t)-1 && A->Offset != (uint16_t)-1) &&
- (B->Offset == (uint16_t)-1)) {
- B->Offset = Offset + A->Offset;
- B->Size = A->Size;
+ unsigned NumModes = CGH.getNumModeIds();
+ // Skip default mode.
+ for (unsigned M = 0; M < NumModes; ++M) {
+ // Handle DefaultMode last.
+ if (M == DefaultMode)
+ continue;
+ SubRegRange &Range = this->Range.get(M);
+ SubRegRange &ARange = A->Range.get(M);
+ SubRegRange &BRange = B->Range.get(M);
+
+ if (Range.Offset != (uint16_t)-1 && ARange.Offset != (uint16_t)-1 &&
+ BRange.Offset == (uint16_t)-1) {
+ BRange.Offset = Range.Offset + ARange.Offset;
+ BRange.Size = ARange.Size;
+ }
+ }
+
+ // Now handle default.
+ SubRegRange &Range = this->Range.get(DefaultMode);
+ SubRegRange &ARange = A->Range.get(DefaultMode);
+ SubRegRange &BRange = B->Range.get(DefaultMode);
+ if (Range.Offset != (uint16_t)-1 && ARange.Offset != (uint16_t)-1 &&
+ BRange.Offset == (uint16_t)-1) {
+ BRange.Offset = Range.Offset + ARange.Offset;
+ BRange.Size = ARange.Size;
}
+
return (Ins.second || Ins.first->second == B) ? nullptr : Ins.first->second;
}
@@ -681,7 +703,8 @@ class CodeGenRegBank {
// Find or create a sub-register index representing the concatenation of
// non-overlapping sibling indices.
CodeGenSubRegIndex *
- getConcatSubRegIndex(const SmallVector<CodeGenSubRegIndex *, 8> &);
+ getConcatSubRegIndex(const SmallVector<CodeGenSubRegIndex *, 8> &,
+ const CodeGenHwModes &CGH);
const std::deque<CodeGenRegister> &getRegisters() const { return Registers; }
diff --git a/llvm/utils/TableGen/InfoByHwMode.cpp b/llvm/utils/TableGen/InfoByHwMode.cpp
index 5496408bb0d306..cacf4ece667137 100644
--- a/llvm/utils/TableGen/InfoByHwMode.cpp
+++ b/llvm/utils/TableGen/InfoByHwMode.cpp
@@ -183,6 +183,20 @@ void RegSizeInfoByHwMode::writeToStream(raw_ostream &OS) const {
OS << '}';
}
+SubRegRange::SubRegRange(Record *R) {
+ Size = R->getValueAsInt("Size");
+ Offset = R->getValueAsInt("Offset");
+}
+
+SubRegRangeByHwMode::SubRegRangeByHwMode(Record *R, const CodeGenHwModes &CGH) {
+ const HwModeSelect &MS = CGH.getHwModeSelect(R);
+ for (const HwModeSelect::PairType &P : MS.Items) {
+ auto I = Map.insert({P.first, SubRegRange(P.second)});
+ assert(I.second && "Duplicate entry?");
+ (void)I;
+ }
+}
+
EncodingInfoByHwMode::EncodingInfoByHwMode(Record *R,
const CodeGenHwModes &CGH) {
const HwModeSelect &MS = CGH.getHwModeSelect(R);
diff --git a/llvm/utils/TableGen/InfoByHwMode.h b/llvm/utils/TableGen/InfoByHwMode.h
index 1909913c50c6e0..dd0b9830d757e8 100644
--- a/llvm/utils/TableGen/InfoByHwMode.h
+++ b/llvm/utils/TableGen/InfoByHwMode.h
@@ -176,6 +176,8 @@ struct ValueTypeByHwMode : public InfoByHwMode<MVT> {
ValueTypeByHwMode getValueTypeByHwMode(Record *Rec, const CodeGenHwModes &CGH);
+raw_ostream &operator<<(raw_ostream &OS, const ValueTypeByHwMode &T);
+
struct RegSizeInfo {
unsigned RegSize;
unsigned SpillSize;
@@ -213,10 +215,27 @@ struct RegSizeInfoByHwMode : public InfoByHwMode<RegSizeInfo> {
}
};
-raw_ostream &operator<<(raw_ostream &OS, const ValueTypeByHwMode &T);
raw_ostream &operator<<(raw_ostream &OS, const RegSizeInfo &T);
raw_ostream &operator<<(raw_ostream &OS, const RegSizeInfoByHwMode &T);
+struct SubRegRange {
+ uint16_t Size;
+ uint16_t Offset;
+
+ SubRegRange(Record *R);
+ SubRegRange(uint16_t Size, uint16_t Offset) : Size(Size), Offset(Offset) {}
+};
+
+struct SubRegRangeByHwMode : public InfoByHwMode<SubRegRange> {
+ SubRegRangeByHwMode(Record *R, const CodeGenHwModes &CGH);
+ SubRegRangeByHwMode(SubRegRange Range) { Map.insert({DefaultMode, Range}); }
+ SubRegRangeByHwMode() = default;
+
+ void insertSubRegRangeForMode(unsigned Mode, SubRegRange Info) {
+ Map.insert(std::pair(Mode, Info));
+ }
+};
+
struct EncodingInfoByHwMode : public InfoByHwMode<Record *> {
EncodingInfoByHwMode(Record *R, const CodeGenHwModes &CGH);
EncodingInfoByHwMode() = default;
diff --git a/llvm/utils/TableGen/RegisterInfoEmitter.cpp b/llvm/utils/TableGen/RegisterInfoEmitter.cpp
index c4fc1930488cf7..350faa87fc0e2a 100644
--- a/llvm/utils/TableGen/RegisterInfoEmitter.cpp
+++ b/llvm/utils/TableGen/RegisterInfoEmitter.cpp
@@ -1245,10 +1245,13 @@ void RegisterInfoEmitter::runTargetDesc(raw_ostream &OS, CodeGenTarget &Target,
// Emit the table of sub-register index sizes.
OS << "static const TargetRegisterInfo::SubRegCoveredBits "
"SubRegIdxRangeTable[] = {\n";
- OS << " { " << (uint16_t)-1 << ", " << (uint16_t)-1 << " },\n";
- for (const auto &Idx : SubRegIndices) {
- OS << " { " << Idx.Offset << ", " << Idx.Size << " },\t// "
- << Idx.getName() << "\n";
+ for (unsigned M = 0; M < NumModes; ++M) {
+ OS << " { " << (uint16_t)-1 << ", " << (uint16_t)-1 << " },\n";
+ for (const auto &Idx : SubRegIndices) {
+ const SubRegRange &Range = Idx.Range.get(M);
+ OS << " { " << Range.Offset << ", " << Range.Size << " },\t// "
+ << Idx.getName() << "\n";
+ }
}
OS << "};\n\n";
@@ -1864,7 +1867,12 @@ void RegisterInfoEmitter::debugDump(raw_ostream &OS) {
OS << "SubRegIndex " << SRI.getName() << ":\n";
OS << "\tLaneMask: " << PrintLaneMask(SRI.LaneMask) << '\n';
OS << "\tAllSuperRegsCovered: " << SRI.AllSuperRegsCovered << '\n';
- OS << "\tOffset, Size: " << SRI.Offset << ", " << SRI.Size << '\n';
+ OS << "\tOffset, Size: {";
+ for (unsigned M = 0; M != NumModes; ++M) {
+ const SubRegRange &Range = SRI.Range.get(M);
+ OS << ' ' << getModeName(M) << ':' << Range.Offset << ", " << Range.Size;
+ }
+ OS << " }\n";
}
for (const CodeGenRegister &R : RegBank.getRegisters()) {
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
✅ With the latest revision this PR passed the Python code formatter. |
jrtc27
reviewed
Mar 23, 2024
| // FIXME: Support HwMode in SubRegIndex? | ||
| def sub_gpr_even : SubRegIndex<-1>; | ||
| def sub_gpr_odd : SubRegIndex<-1, -1>; | ||
| def sub_gpr_even : SubRegIndex<32> { |
There was a problem hiding this comment.
Leave these as -1 so we never accidentally use 32 for non-RV32?
There was a problem hiding this comment.
Isn't 32 consistent with the size we pass to the non-HwMode part of this?
class GPRRegisterClass<dag regList>
: RegisterClass<"RISCV", [XLenVT, XLenFVT, i32], 32, regList> {
let RegInfos = XLenRI;
}
There was a problem hiding this comment.
I guess if we do that then it's fine
| @@ -90,16 +90,16 @@ def TestTarget : Target; | |||
| // CHECK-LABEL: RegisterClass DRegs: | |||
|
|
|||
| // CHECK-LABEL: SubRegIndex ssub1: | |||
| // CHECK: Offset, Size: 16, 16 | |||
| // CHECK: Offset, Size: { Default:16, 16 } | |||
There was a problem hiding this comment.
Perhaps this should use extra {} to group the pair? One could be forgiven for thinking the Default only applies to the first argument, and it'll look very strange once you add in more HwMode's.
There was a problem hiding this comment.
I split the Offset and Size on different lines.
superZWT123
reviewed
Mar 24, 2024
| Name += '_'; | ||
| Name += Part->getName(); | ||
|
|
||
| const SubRegRange &PartRange = Part->Range.get(M); |
There was a problem hiding this comment.
Hi, topperc :
In your previous implementation with ‘R->getValue(“SubRegRanges”)’, it appears that ‘Range’ indeed only includes the HwModes that control the ‘SubRegRanges’, right? Now, if I were to define a new HwMode but not use it to control anything, would invoking ‘Range.get(M)’ lead to an error? This is because ‘getNumModeIds()’ returns the IDs of all HwModes, while ‘Range’ only contains those that actually control the ‘SubRegRanges’.
There was a problem hiding this comment.
Range.get(M) will return the value from the DefaultMode if the mode isn't present.
bjope
reviewed
Mar 25, 2024
|
I guess the RISCV definitions can be seen as serving as an example on how to use this. |
bjope
reviewed
Mar 27, 2024
llvm/include/llvm/Target/Target.td
Outdated
| @@ -99,6 +99,9 @@ class SubRegIndex<int size, int offset = 0> { | |||
| string Namespace = ""; | |||
|
|
|||
| // The size/offset information, parameterized by a HW mode. | |||
| // If the HwModes provided for SubRegRanges does not the DefaultMode, the | |||
There was a problem hiding this comment.
typo ( /does not /does not include / )
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.
This is needed to provide proper size and offset for the GPRPair subreg indices on RISC-V. The size of a GPR already uses HwMode. Previously we said the subreg indices have unknown size and offset, but this stops DwarfExpression::addMachineReg from being able to find the registers that make up the pair.
I believe this fixes #85864 but need to verify.