Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[HLSL][RootSignature] Add parsing of flags to RootDescriptor #140152

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

Merged
merged 3 commits into from
May 23, 2025

Conversation

inbelic
Copy link
Contributor

@inbelic inbelic commented May 15, 2025

  • 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

Final part of and resolves #126577

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels May 15, 2025
@llvmbot
Copy link
Member

llvmbot commented May 15, 2025

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Finn Plummer (inbelic)

Changes
  • 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

Final part of and resolves #126577


Full diff: https://github.com/llvm/llvm-project/pull/140152.diff

4 Files Affected:

  • (modified) clang/include/clang/Parse/ParseHLSLRootSignature.h (+3)
  • (modified) clang/lib/Parse/ParseHLSLRootSignature.cpp (+60)
  • (modified) clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp (+17-3)
  • (modified) llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h (+25)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 436d217cec5b1..7b9168290d62a 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<ParsedRootParamParams>
   parseRootParamParams(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 edb61f29f10d7..faf261cc9b7fe 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -193,6 +193,7 @@ std::optional<RootParam> RootSignatureParser::parseRootParam() {
     ExpectedReg = TokenKind::uReg;
     break;
   }
+  Param.setDefaultFlags();
 
   auto Params = parseRootParamParams(ExpectedReg);
   if (!Params.has_value())
@@ -214,6 +215,9 @@ std::optional<RootParam> RootSignatureParser::parseRootParam() {
   if (Params->Visibility.has_value())
     Param.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::parseRootParamParams(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 02bf38dcb110f..7ed286589f8fa 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -347,8 +347,11 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootFlagsTest) {
 TEST_F(ParseHLSLRootSignatureTest, ValidParseRootParamsTest) {
   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, ValidParseRootParamsTest) {
 
   ASSERT_FALSE(Parser.parse());
 
-  ASSERT_EQ(Elements.size(), 3u);
+  ASSERT_EQ(Elements.size(), 4u);
 
   RootElement Elem = Elements[0];
   ASSERT_TRUE(std::holds_alternative<RootParam>(Elem));
@@ -372,6 +375,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootParamsTest) {
   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::DataStaticWhileSetAtExecute);
 
   Elem = Elements[1];
   ASSERT_TRUE(std::holds_alternative<RootParam>(Elem));
@@ -380,6 +385,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootParamsTest) {
   ASSERT_EQ(std::get<RootParam>(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<RootParam>(Elem).Flags, RootDescriptorFlags::ValidFlags);
 
   Elem = Elements[2];
   ASSERT_TRUE(std::holds_alternative<RootParam>(Elem));
@@ -388,6 +394,14 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootParamsTest) {
   ASSERT_EQ(std::get<RootParam>(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<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 7aa55215abae3..98fa5f09429e3 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,
@@ -91,6 +99,23 @@ struct RootParam {
   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

@inbelic inbelic linked an issue May 16, 2025 that may be closed by this pull request
4 tasks
llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h Outdated Show resolved Hide resolved
Copy link
Contributor

@joaosaffran joaosaffran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Just some minor comments and questions for my own clarification

@inbelic inbelic changed the title [HLSL][RootSignature] Add parsing of flags to RootParam [HLSL][RootSignature] Add parsing of flags to RootDescriptor May 21, 2025
@alsepkow
Copy link
Contributor

Reviewed but don't have approval permissions yet. LGTM!

inbelic added 2 commits May 23, 2025 02:27
- 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
@inbelic inbelic changed the base branch from users/inbelic/pr-140151 to main May 23, 2025 02:29
@inbelic inbelic force-pushed the inbelic/rs-flags-root-param branch from 7f48b6f to c3b4c86 Compare May 23, 2025 02:30
@inbelic inbelic merged commit 7549f42 into llvm:main May 23, 2025
11 of 12 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented May 23, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-gcc-ubuntu running on sie-linux-worker3 while building clang,llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/18255

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
...
PASS: ScudoStandalone-Unit :: ./ScudoCxxUnitTest-x86_64-Test/0/3 (90305 of 90314)
PASS: lit :: discovery.py (90306 of 90314)
PASS: lit :: shtest-external-shell-kill.py (90307 of 90314)
PASS: lit :: googletest-timeout.py (90308 of 90314)
PASS: lit :: selecting.py (90309 of 90314)
PASS: lit :: shtest-timeout.py (90310 of 90314)
PASS: lit :: max-time.py (90311 of 90314)
PASS: lit :: shtest-shell.py (90312 of 90314)
PASS: lit :: shtest-define.py (90313 of 90314)
TIMEOUT: AddressSanitizer-x86_64-linux :: TestCases/asan_lsan_deadlock.cpp (90314 of 90314)
******************** TEST 'AddressSanitizer-x86_64-linux :: TestCases/asan_lsan_deadlock.cpp' FAILED ********************
Exit Code: -9
Timeout: Reached timeout of 900 seconds

Command Output (stderr):
--
/home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/./bin/clang  --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only  -m64  -O0 /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/compiler-rt/test/asan/TestCases/asan_lsan_deadlock.cpp -o /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/runtimes/runtimes-bins/compiler-rt/test/asan/X86_64LinuxConfig/TestCases/Output/asan_lsan_deadlock.cpp.tmp # RUN: at line 4
+ /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/./bin/clang --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only -m64 -O0 /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/compiler-rt/test/asan/TestCases/asan_lsan_deadlock.cpp -o /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/runtimes/runtimes-bins/compiler-rt/test/asan/X86_64LinuxConfig/TestCases/Output/asan_lsan_deadlock.cpp.tmp
env ASAN_OPTIONS=detect_leaks=1 not  /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/runtimes/runtimes-bins/compiler-rt/test/asan/X86_64LinuxConfig/TestCases/Output/asan_lsan_deadlock.cpp.tmp 2>&1 | FileCheck /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/compiler-rt/test/asan/TestCases/asan_lsan_deadlock.cpp # RUN: at line 5
+ env ASAN_OPTIONS=detect_leaks=1 not /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/runtimes/runtimes-bins/compiler-rt/test/asan/X86_64LinuxConfig/TestCases/Output/asan_lsan_deadlock.cpp.tmp
+ FileCheck /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/compiler-rt/test/asan/TestCases/asan_lsan_deadlock.cpp

--

********************
********************
Timed Out Tests (1):
  AddressSanitizer-x86_64-linux :: TestCases/asan_lsan_deadlock.cpp


Testing Time: 1158.63s

Total Discovered Tests: 124909
  Skipped          :     38 (0.03%)
  Unsupported      :   2701 (2.16%)
  Passed           : 121879 (97.57%)
  Expectedly Failed:    290 (0.23%)
  Timed Out        :      1 (0.00%)
FAILED: CMakeFiles/check-all /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/CMakeFiles/check-all 
cd /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build && /usr/bin/python3.8 /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/./bin/llvm-lit --verbose --timeout=900 --param USE_Z3_SOLVER=0 /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/utils/mlgo-utils /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/projects/cross-project-tests /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/tools/lld/test /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/tools/clang/tools/extra/include-cleaner/test /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/tools/clang/tools/extra/test /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/tools/clang/test @/home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/runtimes/runtimes-bins/lit.tests /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/utils/lit /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/test
ninja: build stopped: subcommand failed.

@llvm-ci
Copy link
Collaborator

llvm-ci commented May 23, 2025

LLVM Buildbot has detected a new failure on builder openmp-s390x-linux running on systemz-1 while building clang,llvm at step 6 "test-openmp".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/11980

Here is the relevant piece of the build log for the reference
Step 6 (test-openmp) failure: 1200 seconds without output running [b'ninja', b'-j 4', b'check-openmp'], attempting to kill
...
PASS: ompd-test :: openmp_examples/example_2.c (443 of 453)
PASS: ompd-test :: openmp_examples/example_4.c (444 of 453)
PASS: ompd-test :: openmp_examples/example_5.c (445 of 453)
PASS: ompd-test :: openmp_examples/example_task.c (446 of 453)
UNSUPPORTED: ompd-test :: openmp_examples/ompd_bt.c (447 of 453)
PASS: ompd-test :: openmp_examples/fibonacci.c (448 of 453)
UNSUPPORTED: ompd-test :: openmp_examples/ompd_parallel.c (449 of 453)
PASS: ompd-test :: openmp_examples/parallel.c (450 of 453)
PASS: ompd-test :: openmp_examples/nested.c (451 of 453)
PASS: ompd-test :: openmp_examples/ompd_icvs.c (452 of 453)
command timed out: 1200 seconds without output running [b'ninja', b'-j 4', b'check-openmp'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1331.122268

@inbelic inbelic deleted the inbelic/rs-flags-root-param branch June 2, 2025 20:33
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
…0152)

- defines RootDescriptorFlags in-memory representation
- defines parseRootDescriptorFlags to be DXC compatible. This is why we
support multiple `|` flags even though validation will assert that only one
flag is set
- add unit tests to demonstrate functionality

Final part of and resolves
llvm#126577
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[HLSL] Implement Root Signature Parsing of Root Parameters
5 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.