[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