-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[TableGen, CodeGen, CHERI] Add support for the cPTR wildcard value type. #158426
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
cPTR is a wildcard CHERI capability value type, used analogously to iPTR. This allows TableGen patterns to abstract over CHERI capability widths. Co-authored-by: Jessica Clarke <[email protected]>
@llvm/pr-subscribers-llvm-selectiondag @llvm/pr-subscribers-llvm-globalisel Author: Owen Anderson (resistor) ChangescPTR is a wildcard CHERI capability value type, used analogously to iPTR. This allows TableGen patterns to abstract over CHERI capability widths. Co-authored-by: Jessica Clarke <[email protected]> Full diff: https://github.com/llvm/llvm-project/pull/158426.diff 7 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/ValueTypes.td b/llvm/include/llvm/CodeGen/ValueTypes.td
index 44edec98d20f3..c2208566ac8c9 100644
--- a/llvm/include/llvm/CodeGen/ValueTypes.td
+++ b/llvm/include/llvm/CodeGen/ValueTypes.td
@@ -367,6 +367,9 @@ def aarch64mfp8 : ValueType<8, 253>; // 8-bit value in FPR (AArch64)
def c64 : VTCheriCapability<64, 254>; // 64-bit CHERI capability value
def c128 : VTCheriCapability<128, 255>; // 128-bit CHERI capability value
+// Pseudo valuetype mapped to the current CHERI capability pointer size.
+def cPTR : VTAny<503>;
+
let isNormalValueType = false in {
def token : ValueType<0, 504>; // TokenTy
def MetadataVT : ValueType<0, 505> { // Metadata
diff --git a/llvm/include/llvm/CodeGenTypes/MachineValueType.h b/llvm/include/llvm/CodeGenTypes/MachineValueType.h
index 321fb6b601868..5c18c59ed592d 100644
--- a/llvm/include/llvm/CodeGenTypes/MachineValueType.h
+++ b/llvm/include/llvm/CodeGenTypes/MachineValueType.h
@@ -582,6 +582,12 @@ namespace llvm {
MVT::LAST_FP_SCALABLE_VECTOR_VALUETYPE,
force_iteration_on_noniterable_enum);
}
+
+ static auto cheri_capability_valuetypes() {
+ return enum_seq_inclusive(MVT::FIRST_CHERI_CAPABILITY_VALUETYPE,
+ MVT::LAST_CHERI_CAPABILITY_VALUETYPE,
+ force_iteration_on_noniterable_enum);
+ }
/// @}
};
diff --git a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
index f1f7cd72ef9f2..ac4a0fa1d13a2 100644
--- a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
@@ -335,6 +335,8 @@ bool TypeSetByHwMode::intersect(SetType &Out, const SetType &In) {
using WildPartT = std::pair<MVT, std::function<bool(MVT)>>;
static const WildPartT WildParts[] = {
{MVT::iPTR, [](MVT T) { return T.isScalarInteger() || T == MVT::iPTR; }},
+ {MVT::cPTR,
+ [](MVT T) { return T.isCheriCapability() || T == MVT::cPTR; }},
};
bool Changed = false;
@@ -816,6 +818,11 @@ void TypeInfer::expandOverloads(TypeSetByHwMode::SetType &Out,
if (Out.count(MVT::pAny)) {
Out.erase(MVT::pAny);
Out.insert(MVT::iPTR);
+ for (MVT T : MVT::cheri_capability_valuetypes()) {
+ if (Legal.count(T)) {
+ Out.insert(MVT::cPTR);
+ }
+ }
} else if (Out.count(MVT::iAny)) {
Out.erase(MVT::iAny);
for (MVT T : MVT::integer_valuetypes())
@@ -1647,9 +1654,11 @@ bool SDTypeConstraint::ApplyTypeConstraint(TreePatternNode &N,
case SDTCisVT:
// Operand must be a particular type.
return NodeToApply.UpdateNodeType(ResNo, VVT, TP);
- case SDTCisPtrTy:
- // Operand must be same as target pointer type.
- return NodeToApply.UpdateNodeType(ResNo, MVT::iPTR, TP);
+ case SDTCisPtrTy: {
+ // Operand must be a legal pointer (iPTR, or possibly cPTR) type.
+ const auto &PtrTys = TP.getDAGPatterns().getLegalPtrTypes();
+ return NodeToApply.UpdateNodeType(ResNo, PtrTys, TP);
+ }
case SDTCisInt:
// Require it to be one of the legal integer VTs.
return TI.EnforceInteger(NodeToApply.getExtType(ResNo));
@@ -3260,6 +3269,7 @@ CodeGenDAGPatterns::CodeGenDAGPatterns(const RecordKeeper &R,
PatternRewriterFn PatternRewriter)
: Records(R), Target(R), Intrinsics(R),
LegalVTS(Target.getLegalValueTypes()),
+ LegalPtrVTS(ComputeLegalPtrTypes()),
PatternRewriter(std::move(PatternRewriter)) {
ParseNodeInfo();
ParseNodeTransforms();
@@ -3295,6 +3305,36 @@ const Record *CodeGenDAGPatterns::getSDNodeNamed(StringRef Name) const {
return N;
}
+// Compute the subset of iPTR and cPTR legal for each mode, coalescing into the
+// default mode where possible to avoid predicate explosion.
+TypeSetByHwMode CodeGenDAGPatterns::ComputeLegalPtrTypes() const {
+ auto LegalPtrsForSet = [](const MachineValueTypeSet &In) {
+ MachineValueTypeSet Out;
+ Out.insert(MVT::iPTR);
+ for (MVT T : MVT::cheri_capability_valuetypes()) {
+ if (In.count(T)) {
+ Out.insert(MVT::cPTR);
+ break;
+ }
+ }
+ return Out;
+ };
+
+ const auto &LegalTypes = getLegalTypes();
+ MachineValueTypeSet LegalPtrsDefault =
+ LegalPtrsForSet(LegalTypes.get(DefaultMode));
+
+ TypeSetByHwMode LegalPtrTypes;
+ for (const auto &I : LegalTypes) {
+ MachineValueTypeSet S = LegalPtrsForSet(I.second);
+ if (I.first != DefaultMode && S == LegalPtrsDefault)
+ continue;
+ LegalPtrTypes.getOrCreate(I.first).insert(S);
+ }
+
+ return LegalPtrTypes;
+}
+
// Parse all of the SDNode definitions for the target, populating SDNodes.
void CodeGenDAGPatterns::ParseNodeInfo() {
const CodeGenHwModes &CGH = getTargetInfo().getHwModes();
diff --git a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.h b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.h
index 64fec275faa68..2ed8d1376b045 100644
--- a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.h
+++ b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.h
@@ -1135,6 +1135,7 @@ class CodeGenDAGPatterns {
std::vector<PatternToMatch> PatternsToMatch;
TypeSetByHwMode LegalVTS;
+ TypeSetByHwMode LegalPtrVTS;
using PatternRewriterFn = std::function<void(TreePattern *)>;
PatternRewriterFn PatternRewriter;
@@ -1148,6 +1149,7 @@ class CodeGenDAGPatterns {
CodeGenTarget &getTargetInfo() { return Target; }
const CodeGenTarget &getTargetInfo() const { return Target; }
const TypeSetByHwMode &getLegalTypes() const { return LegalVTS; }
+ const TypeSetByHwMode &getLegalPtrTypes() const { return LegalPtrVTS; }
const Record *getSDNodeNamed(StringRef Name) const;
@@ -1249,6 +1251,7 @@ class CodeGenDAGPatterns {
}
private:
+ TypeSetByHwMode ComputeLegalPtrTypes() const;
void ParseNodeInfo();
void ParseNodeTransforms();
void ParseComplexPatterns();
diff --git a/llvm/utils/TableGen/Common/DAGISelMatcher.cpp b/llvm/utils/TableGen/Common/DAGISelMatcher.cpp
index 255974624e8f0..4fdb386bf45e7 100644
--- a/llvm/utils/TableGen/Common/DAGISelMatcher.cpp
+++ b/llvm/utils/TableGen/Common/DAGISelMatcher.cpp
@@ -328,6 +328,14 @@ static bool TypesAreContradictory(MVT::SimpleValueType T1,
if (T1 == T2)
return false;
+ if (T1 == MVT::pAny)
+ return TypesAreContradictory(MVT::iPTR, T2) &&
+ TypesAreContradictory(MVT::cPTR, T2);
+
+ if (T2 == MVT::pAny)
+ return TypesAreContradictory(T1, MVT::iPTR) &&
+ TypesAreContradictory(T1, MVT::cPTR);
+
// If either type is about iPtr, then they don't conflict unless the other
// one is not a scalar integer type.
if (T1 == MVT::iPTR)
@@ -336,7 +344,13 @@ static bool TypesAreContradictory(MVT::SimpleValueType T1,
if (T2 == MVT::iPTR)
return !MVT(T1).isInteger() || MVT(T1).isVector();
- // Otherwise, they are two different non-iPTR types, they conflict.
+ if (T1 == MVT::cPTR)
+ return !MVT(T2).isCheriCapability() || MVT(T2).isVector();
+
+ if (T2 == MVT::cPTR)
+ return !MVT(T1).isCheriCapability() || MVT(T1).isVector();
+
+ // Otherwise, they are two different non-iPTR/cPTR types, they conflict.
return true;
}
diff --git a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp
index 8c8d5d77ebd73..746726cf2510e 100644
--- a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp
+++ b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp
@@ -1426,7 +1426,9 @@ Error OperandMatcher::addTypeCheckPredicate(const TypeSetByHwMode &VTy,
if (!VTy.isMachineValueType())
return failUnsupported("unsupported typeset");
- if (VTy.getMachineValueType() == MVT::iPTR && OperandIsAPointer) {
+ if ((VTy.getMachineValueType() == MVT::iPTR ||
+ VTy.getMachineValueType() == MVT::cPTR) &&
+ OperandIsAPointer) {
addPredicate<PointerToAnyOperandMatcher>(0);
return Error::success();
}
diff --git a/llvm/utils/TableGen/DAGISelMatcherOpt.cpp b/llvm/utils/TableGen/DAGISelMatcherOpt.cpp
index 8d8189983270e..268e6bbc4eee3 100644
--- a/llvm/utils/TableGen/DAGISelMatcherOpt.cpp
+++ b/llvm/utils/TableGen/DAGISelMatcherOpt.cpp
@@ -519,9 +519,9 @@ static void FactorScope(std::unique_ptr<Matcher> &MatcherPtr) {
CheckTypeMatcher *CTM = cast_or_null<CheckTypeMatcher>(
FindNodeWithKind(Optn, Matcher::CheckType));
if (!CTM ||
- // iPTR checks could alias any other case without us knowing, don't
- // bother with them.
- CTM->getType() == MVT::iPTR ||
+ // iPTR/cPTR checks could alias any other case without us knowing,
+ // don't bother with them.
+ CTM->getType() == MVT::iPTR || CTM->getType() == MVT::cPTR ||
// SwitchType only works for result #0.
CTM->getResNo() != 0 ||
// If the CheckType isn't at the start of the list, see if we can move
|
I don't understand enough about the tablegen internals to review this properly so I'll leave it to @jrtc27 and others. |
Nudge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test wouldn't hurt
Do you have a suggestion or a pointer to an existing test I could model on? I have plenty of integration-level tests of this functionality in the CHERI downstreams, but I don't have a unit test for it, and I'm not sure of a good way to write one. |
We've been getting better about adding tablegen tests, particularly for globalisel, so this looks most directly applicable I added a similar test recently here |
Looks like this introduced a compile-time regression: https://llvm-compile-time-tracker.com/compare.php?from=0c1087b377b49dafaa7d82942db11c2fd3fd4c11&to=b05101b86472826ec77d0c9b71a431917a8f2493&stat=instructions:u Any idea what's up here, given that nothing should actually be using cPTR yet? |
Not offhand, no. I wouldn't have expected anything to change. |
The only idea I have is that it perturbed tblgen output in some way, but re-reading the diff I don't see a clear way for that to have happened. |
I just verified that |
Oddly (if git bisect isn't lying to me), this commit makes
It's subtracting 0x76000 from the stack pointer in a loop, touching each page once and here it is after:
It's the same, but now it subtracts 0x17d000 from the stack pointer |
I'll continue to bisect backwards to see when the 0.5 mb allocation appeared. |
There was no one time. The stack usage of this function has just steadily crept upwards over time. It was already using 180k back in 2021 and that's as far back as I can easily build. |
Digging deeper, this commit increased the size of the TargetLoweringBase class to 1.5 MB, because the size of that class is quadratic in MVT::VALUETYPE_SIZE, and this commit increased that from 256 to 504. If this is intentional, perhaps the true issue here is that this shouldn't be stack-allocated in ARMAsmPrinter. Moving the ARMSubtarget from the stack to the heap does fix things. But I'm not sure if it was intentional for this commit to increase the size of that class by so much. |
If I had to guess, I'd guess the class being so much larger is probably also the cause of the compile-time regression. |
cPTR shouldn’t be any more of a real value type than iPTR. Presumably somewhere in the auto generation it’s being fed the wrong information about whether cPTR is real or tablegen-internal? |
def c128 : VTCheriCapability<128, 255>; // 128-bit CHERI capability value | ||
|
||
// Pseudo valuetype mapped to the current CHERI capability pointer size. | ||
// Should only be used in TableGen. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm maybe this regression is caused by this not being inside the let isNormalValueType = false in {
like iPTR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@abadams I wonder if moving this into the let block just below fixes your problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing that drops it back to 0.5 MB, yes. Though the class probably still shouldn't be on the stack.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@resistor are you planning to submit a PR to fix the regression or should I?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm traveling for the next few days. I can do it when I have time, but you might have time sooner than I do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I'll submit it shortly
Changes to llvm/include/llvm/IR/Intrinsics.td will trip up this test:
This is pretty annoying, as it's completely unrelated to such changes, but only the constants are changing. |
Changing it like this would help make it tolerant:
But I'm not sure that's what you'd like. |
Yeah those values do not matter for the purpose of this test, so using a |
Sent #161406 |
…ated changes (#161406) Changes to llvm/include/llvm/IR/Intrinsics.td may change the constants that are embedded in this test. Use wildcards, so that unrelated changes do not trip over this test failing. Fixes: llvm/llvm-project#158426
…pe. (llvm#158426) cPTR is a wildcard CHERI capability value type, used analogously to iPTR. This allows TableGen patterns to abstract over CHERI capability widths. Co-authored-by: Jessica Clarke <[email protected]>
llvm#161406) Changes to llvm/include/llvm/IR/Intrinsics.td may change the constants that are embedded in this test. Use wildcards, so that unrelated changes do not trip over this test failing. Fixes: llvm#158426
@resistor The test
Should those numbers be different on different platforms? Or is there something that can be tolerable? |
I would expect those to be stable across platforms, so the difference is surprising to me. |
cPTR is a wildcard CHERI capability value type, used analogously to iPTR. This allows TableGen patterns to abstract over CHERI capability widths.
Co-authored-by: Jessica Clarke [email protected]