[clang] [llvm] [HLSL][RootSignature] Add parsing of flags to RootDescriptor (PR #140152)
Finn Plummer via cfe-commits
cfe-commits at lists.llvm.org
Thu May 22 19:47:44 PDT 2025
https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/140152
>From 3daccf83b60e2a5435097b950eaf3cf07a39b440 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Thu, 15 May 2025 20:31:30 +0000
Subject: [PATCH 1/3] [HLSL][RootSignature] Add parsing of flags to RootParam
- defines RootDescriptorFlags in-memory representation
- defines parseRootDescriptorFlags to be DXC compatible. This is why we
support multiple `|` flags even validation will assert that only one
flag is set...
- add unit tests to demonstrate functionality
---
.../clang/Parse/ParseHLSLRootSignature.h | 3 +
clang/lib/Parse/ParseHLSLRootSignature.cpp | 60 +++++++++++++++++++
.../Parse/ParseHLSLRootSignatureTest.cpp | 20 ++++++-
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 25 ++++++++
4 files changed, 105 insertions(+), 3 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 739a14d43584e..3754b4a7b595f 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -93,6 +93,7 @@ class RootSignatureParser {
std::optional<llvm::hlsl::rootsig::Register> Reg;
std::optional<uint32_t> Space;
std::optional<llvm::hlsl::rootsig::ShaderVisibility> Visibility;
+ std::optional<llvm::hlsl::rootsig::RootDescriptorFlags> Flags;
};
std::optional<ParsedRootDescriptorParams>
parseRootDescriptorParams(RootSignatureToken::Kind RegType);
@@ -113,6 +114,8 @@ class RootSignatureParser {
/// Parsing methods of various enums
std::optional<llvm::hlsl::rootsig::ShaderVisibility> parseShaderVisibility();
+ std::optional<llvm::hlsl::rootsig::RootDescriptorFlags>
+ parseRootDescriptorFlags();
std::optional<llvm::hlsl::rootsig::DescriptorRangeFlags>
parseDescriptorRangeFlags();
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 92be49b8a96b2..5a31c06d4848f 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -193,6 +193,7 @@ std::optional<RootDescriptor> RootSignatureParser::parseRootDescriptor() {
ExpectedReg = TokenKind::uReg;
break;
}
+ Param.setDefaultFlags();
auto Params = parseRootDescriptorParams(ExpectedReg);
if (!Params.has_value())
@@ -214,6 +215,9 @@ std::optional<RootDescriptor> RootSignatureParser::parseRootDescriptor() {
if (Params->Visibility.has_value())
Descriptor.Visibility = Params->Visibility.value();
+ if (Params->Flags.has_value())
+ Param.Flags = Params->Flags.value();
+
if (consumeExpectedToken(TokenKind::pu_r_paren,
diag::err_hlsl_unexpected_end_of_params,
/*param of=*/TokenKind::kw_RootConstants))
@@ -475,6 +479,23 @@ RootSignatureParser::parseRootDescriptorParams(TokenKind RegType) {
return std::nullopt;
Params.Visibility = Visibility;
}
+
+ // `flags` `=` ROOT_DESCRIPTOR_FLAGS
+ if (tryConsumeExpectedToken(TokenKind::kw_flags)) {
+ if (Params.Flags.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 Flags = parseRootDescriptorFlags();
+ if (!Flags.has_value())
+ return std::nullopt;
+ Params.Flags = Flags;
+ }
} while (tryConsumeExpectedToken(TokenKind::pu_comma));
return Params;
@@ -654,6 +675,45 @@ RootSignatureParser::parseShaderVisibility() {
return std::nullopt;
}
+std::optional<llvm::hlsl::rootsig::RootDescriptorFlags>
+RootSignatureParser::parseRootDescriptorFlags() {
+ assert(CurToken.TokKind == TokenKind::pu_equal &&
+ "Expects to only be invoked starting at given keyword");
+
+ // Handle the edge-case of '0' to specify no flags set
+ if (tryConsumeExpectedToken(TokenKind::int_literal)) {
+ if (!verifyZeroFlag()) {
+ getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_non_zero_flag);
+ return std::nullopt;
+ }
+ return RootDescriptorFlags::None;
+ }
+
+ TokenKind Expected[] = {
+#define ROOT_DESCRIPTOR_FLAG_ENUM(NAME, LIT) TokenKind::en_##NAME,
+#include "clang/Lex/HLSLRootSignatureTokenKinds.def"
+ };
+
+ std::optional<RootDescriptorFlags> Flags;
+
+ do {
+ if (tryConsumeExpectedToken(Expected)) {
+ switch (CurToken.TokKind) {
+#define ROOT_DESCRIPTOR_FLAG_ENUM(NAME, LIT) \
+ case TokenKind::en_##NAME: \
+ Flags = \
+ maybeOrFlag<RootDescriptorFlags>(Flags, RootDescriptorFlags::NAME); \
+ break;
+#include "clang/Lex/HLSLRootSignatureTokenKinds.def"
+ default:
+ llvm_unreachable("Switch for consumed enum token was not provided");
+ }
+ }
+ } while (tryConsumeExpectedToken(TokenKind::pu_or));
+
+ return Flags;
+}
+
std::optional<llvm::hlsl::rootsig::DescriptorRangeFlags>
RootSignatureParser::parseDescriptorRangeFlags() {
assert(CurToken.TokKind == TokenKind::pu_equal &&
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 07b4f625a2e05..063208888a0a8 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -347,8 +347,11 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootFlagsTest) {
TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
const llvm::StringLiteral Source = R"cc(
CBV(b0),
- SRV(space = 4, t42, visibility = SHADER_VISIBILITY_GEOMETRY),
- UAV(visibility = SHADER_VISIBILITY_HULL, u34893247)
+ SRV(space = 4, t42, visibility = SHADER_VISIBILITY_GEOMETRY,
+ flags = DATA_VOLATILE | DATA_STATIC | DATA_STATIC_WHILE_SET_AT_EXECUTE
+ ),
+ UAV(visibility = SHADER_VISIBILITY_HULL, u34893247),
+ CBV(b0, flags = 0),
)cc";
TrivialModuleLoader ModLoader;
@@ -364,7 +367,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
ASSERT_FALSE(Parser.parse());
- ASSERT_EQ(Elements.size(), 3u);
+ ASSERT_EQ(Elements.size(), 4u);
RootElement Elem = Elements[0];
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
@@ -373,6 +376,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 0u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Visibility, ShaderVisibility::All);
+ ASSERT_EQ(std::get<RootParam>(Elem).Flags,
+ RootDescriptorFlags::DataStaticWhileSetAtExecute);
Elem = Elements[1];
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
@@ -382,6 +387,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 4u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Visibility,
ShaderVisibility::Geometry);
+ ASSERT_EQ(std::get<RootParam>(Elem).Flags, RootDescriptorFlags::ValidFlags);
Elem = Elements[2];
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
@@ -390,6 +396,14 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 34893247u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Visibility, ShaderVisibility::Hull);
+ ASSERT_EQ(std::get<RootParam>(Elem).Flags, RootDescriptorFlags::DataVolatile);
+
+ Elem = Elements[3];
+ ASSERT_EQ(std::get<RootParam>(Elem).Reg.ViewType, RegisterType::BReg);
+ ASSERT_EQ(std::get<RootParam>(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<RootParam>(Elem).Flags, RootDescriptorFlags::None);
ASSERT_TRUE(Consumer->isSatisfied());
}
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 7c496844ab7a6..31b08028d0aca 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -46,6 +46,14 @@ enum class RootFlags : uint32_t {
ValidFlags = 0x00000fff
};
+enum class RootDescriptorFlags : unsigned {
+ None = 0,
+ DataVolatile = 0x2,
+ DataStaticWhileSetAtExecute = 0x4,
+ DataStatic = 0x8,
+ ValidFlags = 0xe,
+};
+
enum class DescriptorRangeFlags : unsigned {
None = 0,
DescriptorsVolatile = 0x1,
@@ -92,6 +100,23 @@ struct RootDescriptor {
Register Reg;
uint32_t Space = 0;
ShaderVisibility Visibility = ShaderVisibility::All;
+ RootDescriptorFlags Flags;
+
+ void setDefaultFlags() {
+ assert(Type != ParamType::Sampler &&
+ "Sampler is not a valid type of ParamType");
+ switch (Type) {
+ case ParamType::CBuffer:
+ case ParamType::SRV:
+ Flags = RootDescriptorFlags::DataStaticWhileSetAtExecute;
+ break;
+ case ParamType::UAV:
+ Flags = RootDescriptorFlags::DataVolatile;
+ break;
+ case ParamType::Sampler:
+ break;
+ }
+ }
};
// Models the end of a descriptor table and stores its visibility
>From c3b4c8644b2f18c0028ba89f58574e161dc39dd6 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 21 May 2025 19:20:34 +0000
Subject: [PATCH 2/3] review: update naming to RootDescriptor
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 4 ++--
.../Parse/ParseHLSLRootSignatureTest.cpp | 21 ++++++++++++-------
2 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 5a31c06d4848f..5181aae3f2d3b 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -193,7 +193,7 @@ std::optional<RootDescriptor> RootSignatureParser::parseRootDescriptor() {
ExpectedReg = TokenKind::uReg;
break;
}
- Param.setDefaultFlags();
+ Descriptor.setDefaultFlags();
auto Params = parseRootDescriptorParams(ExpectedReg);
if (!Params.has_value())
@@ -216,7 +216,7 @@ std::optional<RootDescriptor> RootSignatureParser::parseRootDescriptor() {
Descriptor.Visibility = Params->Visibility.value();
if (Params->Flags.has_value())
- Param.Flags = Params->Flags.value();
+ Descriptor.Flags = Params->Flags.value();
if (consumeExpectedToken(TokenKind::pu_r_paren,
diag::err_hlsl_unexpected_end_of_params,
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 063208888a0a8..64e780a61baf8 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -376,7 +376,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 0u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Visibility, ShaderVisibility::All);
- ASSERT_EQ(std::get<RootParam>(Elem).Flags,
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags,
RootDescriptorFlags::DataStaticWhileSetAtExecute);
Elem = Elements[1];
@@ -387,7 +387,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 4u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Visibility,
ShaderVisibility::Geometry);
- ASSERT_EQ(std::get<RootParam>(Elem).Flags, RootDescriptorFlags::ValidFlags);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags,
+ RootDescriptorFlags::ValidFlags);
Elem = Elements[2];
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
@@ -396,14 +397,18 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 34893247u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Visibility, ShaderVisibility::Hull);
- ASSERT_EQ(std::get<RootParam>(Elem).Flags, RootDescriptorFlags::DataVolatile);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags,
+ RootDescriptorFlags::DataVolatile);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags,
+ RootDescriptorFlags::DataVolatile);
Elem = Elements[3];
- ASSERT_EQ(std::get<RootParam>(Elem).Reg.ViewType, RegisterType::BReg);
- ASSERT_EQ(std::get<RootParam>(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<RootParam>(Elem).Flags, RootDescriptorFlags::None);
+ 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<RootDescriptor>(Elem).Space, 0u);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Visibility, ShaderVisibility::All);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, RootDescriptorFlags::None);
ASSERT_TRUE(Consumer->isSatisfied());
}
>From 710cc3e81dcb204b97bb4821a978a152a9273d51 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Fri, 23 May 2025 02:46:02 +0000
Subject: [PATCH 3/3] self-review: touch-up
---
llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 31b08028d0aca..40394ca0a5b3f 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -93,7 +93,7 @@ struct RootConstants {
ShaderVisibility Visibility = ShaderVisibility::All;
};
-using DescriptorType = llvm::dxil::ResourceClass;
+enum class DescriptorType : uint8_t { SRV = 0, UAV, CBuffer };
// Models RootDescriptor : CBV | SRV | UAV, by collecting like parameters
struct RootDescriptor {
DescriptorType Type;
@@ -103,18 +103,14 @@ struct RootDescriptor {
RootDescriptorFlags Flags;
void setDefaultFlags() {
- assert(Type != ParamType::Sampler &&
- "Sampler is not a valid type of ParamType");
switch (Type) {
- case ParamType::CBuffer:
- case ParamType::SRV:
+ case DescriptorType::CBuffer:
+ case DescriptorType::SRV:
Flags = RootDescriptorFlags::DataStaticWhileSetAtExecute;
break;
- case ParamType::UAV:
+ case DescriptorType::UAV:
Flags = RootDescriptorFlags::DataVolatile;
break;
- case ParamType::Sampler:
- break;
}
}
};
More information about the cfe-commits
mailing list