[clang] [llvm] [HLSL][RootSignature] Add parsing for empty RootParams (PR #140147)
Finn Plummer via cfe-commits
cfe-commits at lists.llvm.org
Wed May 21 11:54:49 PDT 2025
https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/140147
>From fc2fc60145e65a1b32542cbaf9b4bd6ce3b1509d Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Thu, 15 May 2025 17:48:07 +0000
Subject: [PATCH 1/2] [HLSL][RootSignature] Add parsing for empty RootParams
- define the RootParam in-memory struct containing its type
- add test harness for testing
---
.../clang/Parse/ParseHLSLRootSignature.h | 1 +
clang/lib/Parse/ParseHLSLRootSignature.cpp | 43 +++++++++++++++++++
.../Parse/ParseHLSLRootSignatureTest.cpp | 37 ++++++++++++++++
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 14 +++---
4 files changed, 90 insertions(+), 5 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 915266f8a36ae..b96416ab6c6c4 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -73,6 +73,7 @@ class RootSignatureParser {
/// Root Element parse methods:
std::optional<llvm::hlsl::rootsig::RootFlags> parseRootFlags();
std::optional<llvm::hlsl::rootsig::RootConstants> parseRootConstants();
+ std::optional<llvm::hlsl::rootsig::RootParam> parseRootParam();
std::optional<llvm::hlsl::rootsig::DescriptorTable> parseDescriptorTable();
std::optional<llvm::hlsl::rootsig::DescriptorTableClause>
parseDescriptorTableClause();
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 5603900429844..3ed60442eaa14 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -47,6 +47,14 @@ bool RootSignatureParser::parse() {
return true;
Elements.push_back(*Table);
}
+
+ if (tryConsumeExpectedToken(
+ {TokenKind::kw_CBV, TokenKind::kw_SRV, TokenKind::kw_UAV})) {
+ auto RootParam = parseRootParam();
+ if (!RootParam.has_value())
+ return true;
+ Elements.push_back(*RootParam);
+ }
} while (tryConsumeExpectedToken(TokenKind::pu_comma));
return consumeExpectedToken(TokenKind::end_of_stream,
@@ -155,6 +163,41 @@ std::optional<RootConstants> RootSignatureParser::parseRootConstants() {
return Constants;
}
+std::optional<RootParam> RootSignatureParser::parseRootParam() {
+ assert((CurToken.TokKind == TokenKind::kw_CBV ||
+ CurToken.TokKind == TokenKind::kw_SRV ||
+ CurToken.TokKind == TokenKind::kw_UAV) &&
+ "Expects to only be invoked starting at given keyword");
+
+ TokenKind ParamKind = CurToken.TokKind;
+
+ if (consumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
+ CurToken.TokKind))
+ return std::nullopt;
+
+ RootParam Param;
+ switch (ParamKind) {
+ default:
+ llvm_unreachable("Switch for consumed token was not provided");
+ case TokenKind::kw_CBV:
+ Param.Type = ParamType::CBuffer;
+ break;
+ case TokenKind::kw_SRV:
+ Param.Type = ParamType::SRV;
+ break;
+ case TokenKind::kw_UAV:
+ Param.Type = ParamType::UAV;
+ break;
+ }
+
+ if (consumeExpectedToken(TokenKind::pu_r_paren,
+ diag::err_hlsl_unexpected_end_of_params,
+ /*param of=*/TokenKind::kw_RootConstants))
+ return std::nullopt;
+
+ return Param;
+}
+
std::optional<DescriptorTable> RootSignatureParser::parseDescriptorTable() {
assert(CurToken.TokKind == TokenKind::kw_DescriptorTable &&
"Expects to only be invoked starting at given keyword");
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index c97f8d0b392d1..876dcd17f0389 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -344,6 +344,43 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootFlagsTest) {
ASSERT_TRUE(Consumer->isSatisfied());
}
+TEST_F(ParseHLSLRootSignatureTest, ValidParseRootParamsTest) {
+ const llvm::StringLiteral Source = R"cc(
+ CBV(),
+ SRV(),
+ UAV()
+ )cc";
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+ auto TokLoc = SourceLocation();
+
+ hlsl::RootSignatureLexer Lexer(Source, TokLoc);
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
+
+ // Test no diagnostics produced
+ Consumer->setNoDiag();
+
+ ASSERT_FALSE(Parser.parse());
+
+ ASSERT_EQ(Elements.size(), 3u);
+
+ RootElement Elem = Elements[0];
+ ASSERT_TRUE(std::holds_alternative<RootParam>(Elem));
+ ASSERT_EQ(std::get<RootParam>(Elem).Type, ParamType::CBuffer);
+
+ Elem = Elements[1];
+ ASSERT_TRUE(std::holds_alternative<RootParam>(Elem));
+ ASSERT_EQ(std::get<RootParam>(Elem).Type, ParamType::SRV);
+
+ Elem = Elements[2];
+ ASSERT_TRUE(std::holds_alternative<RootParam>(Elem));
+ ASSERT_EQ(std::get<RootParam>(Elem).Type, ParamType::UAV);
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
TEST_F(ParseHLSLRootSignatureTest, ValidTrailingCommaTest) {
// This test will checks we can handling trailing commas ','
const llvm::StringLiteral Source = R"cc(
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 9fdb40db9c23d..2bcae22c6cfb2 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -85,6 +85,11 @@ struct RootConstants {
ShaderVisibility Visibility = ShaderVisibility::All;
};
+using ParamType = llvm::dxil::ResourceClass;
+struct RootParam {
+ ParamType Type;
+};
+
// Models the end of a descriptor table and stores its visibility
struct DescriptorTable {
ShaderVisibility Visibility = ShaderVisibility::All;
@@ -125,8 +130,8 @@ struct DescriptorTableClause {
void dump(raw_ostream &OS) const;
};
-/// Models RootElement : RootFlags | RootConstants | DescriptorTable
-/// | DescriptorTableClause
+/// Models RootElement : RootFlags | RootConstants | RootParam
+/// | DescriptorTable | DescriptorTableClause
///
/// A Root Signature is modeled in-memory by an array of RootElements. These
/// aim to map closely to their DSL grammar reprsentation defined in the spec.
@@ -140,9 +145,8 @@ struct DescriptorTableClause {
/// The DescriptorTable is modelled by having its Clauses as the previous
/// RootElements in the array, and it holds a data member for the Visibility
/// parameter.
-using RootElement = std::variant<RootFlags, RootConstants, DescriptorTable,
- DescriptorTableClause>;
-
+using RootElement = std::variant<RootFlags, RootConstants, RootParam,
+ DescriptorTable, DescriptorTableClause>;
void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements);
class MetadataBuilder {
>From cddbc9a0bca4026bb756f42e1945a81d0cbd2865 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 21 May 2025 18:47:01 +0000
Subject: [PATCH 2/2] review: rename RootParam to RootDescriptor
---
.../clang/Parse/ParseHLSLRootSignature.h | 2 +-
clang/lib/Parse/ParseHLSLRootSignature.cpp | 22 +++++++++----------
.../Parse/ParseHLSLRootSignatureTest.cpp | 14 ++++++------
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 12 +++++-----
4 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index b96416ab6c6c4..939d45e0c01bc 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -73,7 +73,7 @@ class RootSignatureParser {
/// Root Element parse methods:
std::optional<llvm::hlsl::rootsig::RootFlags> parseRootFlags();
std::optional<llvm::hlsl::rootsig::RootConstants> parseRootConstants();
- std::optional<llvm::hlsl::rootsig::RootParam> parseRootParam();
+ std::optional<llvm::hlsl::rootsig::RootDescriptor> parseRootDescriptor();
std::optional<llvm::hlsl::rootsig::DescriptorTable> parseDescriptorTable();
std::optional<llvm::hlsl::rootsig::DescriptorTableClause>
parseDescriptorTableClause();
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 3ed60442eaa14..d71f2c22d5d4b 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -50,10 +50,10 @@ bool RootSignatureParser::parse() {
if (tryConsumeExpectedToken(
{TokenKind::kw_CBV, TokenKind::kw_SRV, TokenKind::kw_UAV})) {
- auto RootParam = parseRootParam();
- if (!RootParam.has_value())
+ auto Descriptor = parseRootDescriptor();
+ if (!Descriptor.has_value())
return true;
- Elements.push_back(*RootParam);
+ Elements.push_back(*Descriptor);
}
} while (tryConsumeExpectedToken(TokenKind::pu_comma));
@@ -163,30 +163,30 @@ std::optional<RootConstants> RootSignatureParser::parseRootConstants() {
return Constants;
}
-std::optional<RootParam> RootSignatureParser::parseRootParam() {
+std::optional<RootDescriptor> RootSignatureParser::parseRootDescriptor() {
assert((CurToken.TokKind == TokenKind::kw_CBV ||
CurToken.TokKind == TokenKind::kw_SRV ||
CurToken.TokKind == TokenKind::kw_UAV) &&
"Expects to only be invoked starting at given keyword");
- TokenKind ParamKind = CurToken.TokKind;
+ TokenKind DescriptorKind = CurToken.TokKind;
if (consumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
CurToken.TokKind))
return std::nullopt;
- RootParam Param;
- switch (ParamKind) {
+ RootDescriptor Descriptor;
+ switch (DescriptorKind) {
default:
llvm_unreachable("Switch for consumed token was not provided");
case TokenKind::kw_CBV:
- Param.Type = ParamType::CBuffer;
+ Descriptor.Type = DescriptorType::CBuffer;
break;
case TokenKind::kw_SRV:
- Param.Type = ParamType::SRV;
+ Descriptor.Type = DescriptorType::SRV;
break;
case TokenKind::kw_UAV:
- Param.Type = ParamType::UAV;
+ Descriptor.Type = DescriptorType::UAV;
break;
}
@@ -195,7 +195,7 @@ std::optional<RootParam> RootSignatureParser::parseRootParam() {
/*param of=*/TokenKind::kw_RootConstants))
return std::nullopt;
- return Param;
+ return Descriptor;
}
std::optional<DescriptorTable> RootSignatureParser::parseDescriptorTable() {
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 876dcd17f0389..7d03dda1f516b 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -344,7 +344,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootFlagsTest) {
ASSERT_TRUE(Consumer->isSatisfied());
}
-TEST_F(ParseHLSLRootSignatureTest, ValidParseRootParamsTest) {
+TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
const llvm::StringLiteral Source = R"cc(
CBV(),
SRV(),
@@ -367,16 +367,16 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootParamsTest) {
ASSERT_EQ(Elements.size(), 3u);
RootElement Elem = Elements[0];
- ASSERT_TRUE(std::holds_alternative<RootParam>(Elem));
- ASSERT_EQ(std::get<RootParam>(Elem).Type, ParamType::CBuffer);
+ ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
Elem = Elements[1];
- ASSERT_TRUE(std::holds_alternative<RootParam>(Elem));
- ASSERT_EQ(std::get<RootParam>(Elem).Type, ParamType::SRV);
+ ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV);
Elem = Elements[2];
- ASSERT_TRUE(std::holds_alternative<RootParam>(Elem));
- ASSERT_EQ(std::get<RootParam>(Elem).Type, ParamType::UAV);
+ ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV);
ASSERT_TRUE(Consumer->isSatisfied());
}
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 2bcae22c6cfb2..7829b842fa2be 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -85,9 +85,10 @@ struct RootConstants {
ShaderVisibility Visibility = ShaderVisibility::All;
};
-using ParamType = llvm::dxil::ResourceClass;
-struct RootParam {
- ParamType Type;
+using DescriptorType = llvm::dxil::ResourceClass;
+// Models RootDescriptor : CBV | SRV | UAV, by collecting like parameters
+struct RootDescriptor {
+ DescriptorType Type;
};
// Models the end of a descriptor table and stores its visibility
@@ -130,7 +131,7 @@ struct DescriptorTableClause {
void dump(raw_ostream &OS) const;
};
-/// Models RootElement : RootFlags | RootConstants | RootParam
+/// Models RootElement : RootFlags | RootConstants | RootDescriptor
/// | DescriptorTable | DescriptorTableClause
///
/// A Root Signature is modeled in-memory by an array of RootElements. These
@@ -145,8 +146,9 @@ struct DescriptorTableClause {
/// The DescriptorTable is modelled by having its Clauses as the previous
/// RootElements in the array, and it holds a data member for the Visibility
/// parameter.
-using RootElement = std::variant<RootFlags, RootConstants, RootParam,
+using RootElement = std::variant<RootFlags, RootConstants, RootDescriptor,
DescriptorTable, DescriptorTableClause>;
+
void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements);
class MetadataBuilder {
More information about the cfe-commits
mailing list