-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[Clang][docs] Modify generator for HLSL semantics documentation #157841
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
HLSL semantics are split between system semantics with some kind of spelling, and user semantics with no actual spelling. Those have been documented as normal function attributes, but they'd benefit from a custom section with a slightly different handling. This will allow llvm#152537 to land.
@llvm/pr-subscribers-clang Author: Nathan Gauër (Keenuts) ChangesHLSL semantics are split between system semantics with some kind of spelling, and user semantics with no actual spelling. Those have been documented as normal function attributes, but they'd benefit from a custom section with a slightly different handling. This will allow #152537 to land. Full diff: https://github.com/llvm/llvm-project/pull/157841.diff 2 Files Affected:
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index ab3f9e4835802..ee212a9b50f36 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -8367,6 +8367,23 @@ flag.
}];
}
+def DocHLSLSemantics : DocumentationCategory<"HLSL Semantics"> {
+ let Content = [{
+A semantic is a string attached to a shader input or output that conveys
+information about the intended use of a parameter. Semantics are required on
+all variables passed between shader stages. The syntax for adding a semantic
+to a shader variable is shown here (Variable Syntax (DirectX HLSL)).
+
+In general, data passed between pipeline stages is completely generic and is
+not uniquely interpreted by the system; arbitrary semantics are allowed which
+have no special meaning. Parameters (in Direct3D 10 and later) which contain
+these special semantics are referred to as System-Value Semantics.
+
+More information is available here:
+https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-semantics
+ }];
+}
+
def WaveSizeDocs : Documentation {
let Category = DocCatFunction;
let Content = [{
@@ -8604,7 +8621,7 @@ randomized.
}
def HLSLSV_GroupThreadIDDocs : Documentation {
- let Category = DocCatFunction;
+ let Category = DocHLSLSemantics;
let Content = [{
The ``SV_GroupThreadID`` semantic, when applied to an input parameter, specifies which
individual thread within a thread group is executing in. This attribute is
@@ -8615,7 +8632,7 @@ The full documentation is available here: https://docs.microsoft.com/en-us/windo
}
def HLSLSV_GroupIDDocs : Documentation {
- let Category = DocCatFunction;
+ let Category = DocHLSLSemantics;
let Content = [{
The ``SV_GroupID`` semantic, when applied to an input parameter, specifies which
thread group a shader is executing in. This attribute is only supported in compute shaders.
@@ -8625,7 +8642,7 @@ The full documentation is available here: https://docs.microsoft.com/en-us/windo
}
def HLSLSV_GroupIndexDocs : Documentation {
- let Category = DocCatFunction;
+ let Category = DocHLSLSemantics;
let Content = [{
The ``SV_GroupIndex`` semantic, when applied to an input parameter, specifies a
data binding to map the group index to the specified parameter. This attribute
@@ -8682,7 +8699,7 @@ The full documentation is available here: https://learn.microsoft.com/en-us/wind
}
def HLSLSV_DispatchThreadIDDocs : Documentation {
- let Category = DocCatFunction;
+ let Category = DocHLSLSemantics;
let Content = [{
The ``SV_DispatchThreadID`` semantic, when applied to an input parameter,
specifies a data binding to map the global thread offset within the Dispatch
@@ -8697,7 +8714,7 @@ The full documentation is available here: https://docs.microsoft.com/en-us/windo
}
def HLSLSV_PositionDocs : Documentation {
- let Category = DocCatFunction;
+ let Category = DocHLSLSemantics;
let Content = [{
The ``SV_Position`` semantic, when applied to an input parameter in a pixel
shader, contains the location of the pixel center (x, y) in screen space.
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
index d63e79a5f5155..d5b08ab482b9e 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -5209,6 +5209,14 @@ class SpellingList {
Other.Spellings[Kind].end());
}
}
+
+ unsigned getSpellingCount() const {
+ unsigned Count = 0;
+ for (const auto &Item : Spellings)
+ if (Item.size() != 0)
+ ++Count;
+ return Count;
+ }
};
class DocumentationData {
@@ -5246,11 +5254,22 @@ GetAttributeHeadingAndSpellings(const Record &Documentation,
// documentation. This may not be a limiting factor since the spellings
// should generally be consistently applied across the category.
+ if (Cat == "HLSL Semantics") {
+ if (!Attribute.getName().starts_with("HLSL"))
+ PrintFatalError(Attribute.getLoc(),
+ "HLSL semantic attribute name must start with HLSL");
+
+ assert(Attribute.getName().size() > 4);
+ std::string Name = Attribute.getName().substr(4).str();
+ return std::make_pair(std::move(Name), SpellingList());
+ }
+
std::vector<FlattenedSpelling> Spellings = GetFlattenedSpellings(Attribute);
- if (Spellings.empty())
+ if (Spellings.empty()) {
PrintFatalError(Attribute.getLoc(),
"Attribute has no supported spellings; cannot be "
"documented");
+ }
// Determine the heading to be used for this attribute.
std::string Heading = Documentation.getValueAsString("Heading").str();
@@ -5296,37 +5315,39 @@ static void WriteDocumentation(const RecordKeeper &Records,
OS << ".. _" << Label << ":\n\n";
OS << Doc.Heading << "\n" << std::string(Doc.Heading.length(), '-') << "\n";
- // List what spelling syntaxes the attribute supports.
- // Note: "#pragma clang attribute" is handled outside the spelling kinds loop
- // so it must be last.
- OS << ".. csv-table:: Supported Syntaxes\n";
- OS << " :header: \"GNU\", \"C++11\", \"C23\", \"``__declspec``\",";
- OS << " \"Keyword\", \"``#pragma``\", \"HLSL Annotation\", \"``#pragma "
- "clang ";
- OS << "attribute``\"\n\n \"";
- for (size_t Kind = 0; Kind != NumSpellingKinds; ++Kind) {
- SpellingKind K = (SpellingKind)Kind;
- // TODO: List Microsoft (IDL-style attribute) spellings once we fully
- // support them.
- if (K == SpellingKind::Microsoft)
- continue;
+ if (Doc.SupportedSpellings.getSpellingCount() > 0) {
+ // List what spelling syntaxes the attribute supports.
+ // Note: "#pragma clang attribute" is handled outside the spelling kinds
+ // loop so it must be last.
+ OS << ".. csv-table:: Supported Syntaxes\n";
+ OS << " :header: \"GNU\", \"C++11\", \"C23\", \"``__declspec``\",";
+ OS << " \"Keyword\", \"``#pragma``\", \"HLSL Annotation\", \"``#pragma "
+ "clang ";
+ OS << "attribute``\"\n\n \"";
+ for (size_t Kind = 0; Kind != NumSpellingKinds; ++Kind) {
+ SpellingKind K = (SpellingKind)Kind;
+ // TODO: List Microsoft (IDL-style attribute) spellings once we fully
+ // support them.
+ if (K == SpellingKind::Microsoft)
+ continue;
- bool PrintedAny = false;
- for (StringRef Spelling : Doc.SupportedSpellings[K]) {
- if (PrintedAny)
- OS << " |br| ";
- OS << "``" << Spelling << "``";
- PrintedAny = true;
+ bool PrintedAny = false;
+ for (StringRef Spelling : Doc.SupportedSpellings[K]) {
+ if (PrintedAny)
+ OS << " |br| ";
+ OS << "``" << Spelling << "``";
+ PrintedAny = true;
+ }
+
+ OS << "\",\"";
}
- OS << "\",\"";
+ if (getPragmaAttributeSupport(Records).isAttributedSupported(
+ *Doc.Attribute))
+ OS << "Yes";
+ OS << "\"\n\n";
}
- if (getPragmaAttributeSupport(Records).isAttributedSupported(
- *Doc.Attribute))
- OS << "Yes";
- OS << "\"\n\n";
-
// If the attribute is deprecated, print a message about it, and possibly
// provide a replacement attribute.
if (!Doc.Documentation->isValueUnset("Deprecated")) {
|
Yeah, this seems to fix the issue I've hit. For the record, I've finally gotten around to a simpler reproducer:
|
Adding Erich Keans, might be a more accurate reviewer since this touches attributes |
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.
4 that are slightly above nit, but otherwise this looks good to me.
} | ||
|
||
unsigned getSpellingCount() const { | ||
unsigned Count = 0; |
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.
return llvm::accumulate(Spellings, 0, [](const auto &I) {return I.size() ? 1 : 0; });
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.
or perhaps:
bool hasSpelling() const {
return Spellings.end() != llvm::find_if(Spellings, [](const auto &I)->bool { return I.size(); });
}
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.
Still kept a loop as we have a C-array of vectors, so no proper iterators.
Might be able to use llvm's range with raw pointers, but a loop should be fine.
Changed to hasSpelling
as suggested still 😊
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.
Ooof, thats frustrating... we should probably swap that to a std::array one of these days.
Thanks for the reviews! Will merge tomorrow morning |
Follow up to llvm#157841, replacing the C-array with std::array so iterators can be used.
Follow up to #157841, replacing the C-array with std::array so iterators can be used. --------- Co-authored-by: Nikolas Klauser <[email protected]>
HLSL semantics are split between system semantics with some kind of spelling, and user semantics with no actual spelling. Those have been documented as normal function attributes, but they'd benefit from a custom section with a slightly different handling.
This will allow #152537 to land.
Verified the resulting RST file, and only diff are around HLSL semantics.
Rendered output:
