[clang] [llvm] [HLSL][RootSignature] Add parsing of optional parameters for RootDescriptor (PR #140151)
Finn Plummer via cfe-commits
cfe-commits at lists.llvm.org
Thu May 22 16:27:15 PDT 2025
https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/140151
>From 57d166785f06fcdb8e9c9a1b796ec89a9cc91070 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Thu, 15 May 2025 20:07:51 +0000
Subject: [PATCH 1/3] [HLSL][RootSignature] Add parsing of optional parameters
for RootParam
- define in-memory representation of optional non-flag parameters to
RootParam
- fill in data to parse these params in `parseRootParamParams`
- add unit tests
---
.../clang/Parse/ParseHLSLRootSignature.h | 2 +
clang/lib/Parse/ParseHLSLRootSignature.cpp | 40 +++++++++++++++++++
.../Parse/ParseHLSLRootSignatureTest.cpp | 10 ++++-
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 2 +
4 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 0d56d2a16a268..739a14d43584e 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -91,6 +91,8 @@ class RootSignatureParser {
struct ParsedRootDescriptorParams {
std::optional<llvm::hlsl::rootsig::Register> Reg;
+ std::optional<uint32_t> Space;
+ std::optional<llvm::hlsl::rootsig::ShaderVisibility> Visibility;
};
std::optional<ParsedRootDescriptorParams>
parseRootDescriptorParams(RootSignatureToken::Kind RegType);
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index da01df04ac78e..a77353c94e379 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -207,6 +207,13 @@ std::optional<RootDescriptor> RootSignatureParser::parseRootDescriptor() {
Descriptor.Reg = Params->Reg.value();
+ // Fill in optional values
+ if (Params->Space.has_value())
+ Param.Space = Params->Space.value();
+
+ if (Params->Visibility.has_value())
+ Param.Visibility = Params->Visibility.value();
+
if (consumeExpectedToken(TokenKind::pu_r_paren,
diag::err_hlsl_unexpected_end_of_params,
/*param of=*/TokenKind::kw_RootConstants))
@@ -435,6 +442,39 @@ RootSignatureParser::parseRootDescriptorParams(TokenKind RegType) {
Params.Reg = Reg;
}
+ // `space` `=` POS_INT
+ if (tryConsumeExpectedToken(TokenKind::kw_space)) {
+ if (Params.Space.has_value()) {
+ getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param)
+ << CurToken.TokKind;
+ return std::nullopt;
+ }
+
+ if (consumeExpectedToken(TokenKind::pu_equal))
+ return std::nullopt;
+
+ auto Space = parseUIntParam();
+ if (!Space.has_value())
+ return std::nullopt;
+ Params.Space = Space;
+ }
+
+ // `visibility` `=` SHADER_VISIBILITY
+ if (tryConsumeExpectedToken(TokenKind::kw_visibility)) {
+ if (Params.Visibility.has_value()) {
+ getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param)
+ << CurToken.TokKind;
+ return std::nullopt;
+ }
+
+ if (consumeExpectedToken(TokenKind::pu_equal))
+ return std::nullopt;
+
+ auto Visibility = parseShaderVisibility();
+ if (!Visibility.has_value())
+ return std::nullopt;
+ Params.Visibility = Visibility;
+ }
} while (tryConsumeExpectedToken(TokenKind::pu_comma));
return Params;
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 665ef5b518be7..57c55809c1386 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -347,8 +347,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootFlagsTest) {
TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
const llvm::StringLiteral Source = R"cc(
CBV(b0),
- SRV(t42),
- UAV(u34893247)
+ SRV(space = 4, t42, visibility = SHADER_VISIBILITY_GEOMETRY),
+ UAV(visibility = SHADER_VISIBILITY_HULL, u34893247)
)cc";
TrivialModuleLoader ModLoader;
@@ -371,18 +371,24 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::BReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 0u);
+ ASSERT_EQ(std::get<RootParam>(Elem).Space, 0u);
+ ASSERT_EQ(std::get<RootParam>(Elem).Visibility, ShaderVisibility::All);
Elem = Elements[1];
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::TReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 42u);
+ ASSERT_EQ(std::get<RootParam>(Elem).Space, 4u);
+ ASSERT_EQ(std::get<RootParam>(Elem).Visibility, ShaderVisibility::Geometry);
Elem = Elements[2];
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::UReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 34893247u);
+ ASSERT_EQ(std::get<RootParam>(Elem).Space, 0u);
+ ASSERT_EQ(std::get<RootParam>(Elem).Visibility, ShaderVisibility::Hull);
ASSERT_TRUE(Consumer->isSatisfied());
}
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 4b1bbd0e3bd12..7c496844ab7a6 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -90,6 +90,8 @@ using DescriptorType = llvm::dxil::ResourceClass;
struct RootDescriptor {
DescriptorType Type;
Register Reg;
+ uint32_t Space = 0;
+ ShaderVisibility Visibility = ShaderVisibility::All;
};
// Models the end of a descriptor table and stores its visibility
>From 94ec5c65c7e9e173dfa5e4467c8cce14a6faa593 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 21 May 2025 19:15:47 +0000
Subject: [PATCH 2/3] review: update-naming
---
.../unittests/Parse/ParseHLSLRootSignatureTest.cpp | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 57c55809c1386..07b4f625a2e05 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -371,24 +371,25 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::BReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 0u);
- ASSERT_EQ(std::get<RootParam>(Elem).Space, 0u);
- ASSERT_EQ(std::get<RootParam>(Elem).Visibility, ShaderVisibility::All);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Visibility, ShaderVisibility::All);
Elem = Elements[1];
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::TReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 42u);
- ASSERT_EQ(std::get<RootParam>(Elem).Space, 4u);
- ASSERT_EQ(std::get<RootParam>(Elem).Visibility, ShaderVisibility::Geometry);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 4u);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Visibility,
+ ShaderVisibility::Geometry);
Elem = Elements[2];
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::UReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 34893247u);
- ASSERT_EQ(std::get<RootParam>(Elem).Space, 0u);
- ASSERT_EQ(std::get<RootParam>(Elem).Visibility, ShaderVisibility::Hull);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Visibility, ShaderVisibility::Hull);
ASSERT_TRUE(Consumer->isSatisfied());
}
>From 532e22e3f4e5a6955101a0459479c9ac9f69b62b Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Thu, 22 May 2025 23:26:57 +0000
Subject: [PATCH 3/3] self-review: fix typos
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index a77353c94e379..92be49b8a96b2 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -209,10 +209,10 @@ std::optional<RootDescriptor> RootSignatureParser::parseRootDescriptor() {
// Fill in optional values
if (Params->Space.has_value())
- Param.Space = Params->Space.value();
+ Descriptor.Space = Params->Space.value();
if (Params->Visibility.has_value())
- Param.Visibility = Params->Visibility.value();
+ Descriptor.Visibility = Params->Visibility.value();
if (consumeExpectedToken(TokenKind::pu_r_paren,
diag::err_hlsl_unexpected_end_of_params,
More information about the cfe-commits
mailing list