Skip to content

[HLSL][RootSignature] Add space, visibility enums to StaticSampler #140306

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

Open
wants to merge 1 commit into
base: users/inbelic/pr-140305
Choose a base branch
from

Conversation

inbelic
Copy link
Contributor

@inbelic inbelic commented May 16, 2025

  • adds the space and visibility parameters to StaticSampler
  • adds basic unit tests to demonstrate setting functionality

Part 7 and Resolves #126574

@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 16, 2025
@inbelic inbelic force-pushed the inbelic/rs-final-sampler branch from 28c7adf to da75749 Compare May 16, 2025 21:00
@llvmbot
Copy link
Member

llvmbot commented May 16, 2025

@llvm/pr-subscribers-clang

Author: Finn Plummer (inbelic)

Changes
  • adds the space and visibility parameters to StaticSampler
  • adds basic unit tests to demonstrate setting functionality

Part 7 and Resolves #126574


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

4 Files Affected:

  • (modified) clang/include/clang/Parse/ParseHLSLRootSignature.h (+2)
  • (modified) clang/lib/Parse/ParseHLSLRootSignature.cpp (+40)
  • (modified) clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp (+6-1)
  • (modified) llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h (+2)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 21df9d0da4a53..34d59d7fb66f4 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -121,6 +121,8 @@ class RootSignatureParser {
     std::optional<llvm::hlsl::rootsig::StaticBorderColor> BorderColor;
     std::optional<float> MinLOD;
     std::optional<float> MaxLOD;
+    std::optional<uint32_t> Space;
+    std::optional<llvm::hlsl::rootsig::ShaderVisibility> Visibility;
   };
   std::optional<ParsedStaticSamplerParams> parseStaticSamplerParams();
 
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 07bd10d00dfad..bd0f9abb5ff1b 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -411,6 +411,12 @@ std::optional<StaticSampler> RootSignatureParser::parseStaticSampler() {
   if (Params->MaxLOD.has_value())
     Sampler.MaxLOD = Params->MaxLOD.value();
 
+  if (Params->Space.has_value())
+    Sampler.Space = Params->Space.value();
+
+  if (Params->Visibility.has_value())
+    Sampler.Visibility = Params->Visibility.value();
+
   if (consumeExpectedToken(TokenKind::pu_r_paren,
                            diag::err_hlsl_unexpected_end_of_params,
                            /*param of=*/TokenKind::kw_StaticSampler))
@@ -866,6 +872,40 @@ RootSignatureParser::parseStaticSamplerParams() {
         return std::nullopt;
       Params.MaxLOD = MaxLOD;
     }
+
+    // `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 6343d9feb4411..3eede0dbed61b 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -226,7 +226,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
 TEST_F(ParseHLSLRootSignatureTest, ValidParseStaticSamplerTest) {
   const llvm::StringLiteral Source = R"cc(
     StaticSampler(s0),
-    StaticSampler(s0, maxAnisotropy = 3,
+    StaticSampler(s0, maxAnisotropy = 3, space = 4,
+      visibility = SHADER_VISIBILITY_DOMAIN,
       minLOD = 4.2f, mipLODBias = 0.23e+3,
       addressW = TEXTURE_ADDRESS_CLAMP,
       addressV = TEXTURE_ADDRESS_BORDER,
@@ -269,6 +270,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseStaticSamplerTest) {
             StaticBorderColor::OpaqueWhite);
   ASSERT_EQ(std::get<StaticSampler>(Elem).MinLOD, 0.f);
   ASSERT_EQ(std::get<StaticSampler>(Elem).MaxLOD, 3.402823466e+38f);
+  ASSERT_EQ(std::get<StaticSampler>(Elem).Space, 0u);
+  ASSERT_EQ(std::get<StaticSampler>(Elem).Visibility, ShaderVisibility::All);
 
   // Check values can be set as expected
   Elem = Elements[1];
@@ -288,6 +291,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseStaticSamplerTest) {
             StaticBorderColor::OpaqueBlackUint);
   ASSERT_EQ(std::get<StaticSampler>(Elem).MinLOD, 4.2f);
   ASSERT_EQ(std::get<StaticSampler>(Elem).MaxLOD, 9000.f);
+  ASSERT_EQ(std::get<StaticSampler>(Elem).Space, 4u);
+  ASSERT_EQ(std::get<StaticSampler>(Elem).Visibility, ShaderVisibility::Domain);
 
   ASSERT_TRUE(Consumer->isSatisfied());
 }
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 025e96ec93c2a..ee65722e68556 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -236,6 +236,8 @@ struct StaticSampler {
   StaticBorderColor BorderColor = StaticBorderColor::OpaqueWhite;
   float MinLOD = 0.f;
   float MaxLOD = 3.402823466e+38f; // FLT_MAX
+  uint32_t Space = 0;
+  ShaderVisibility Visibility = ShaderVisibility::All;
 };
 
 /// Models RootElement : RootFlags | RootConstants | RootParam

@llvmbot
Copy link
Member

llvmbot commented May 16, 2025

@llvm/pr-subscribers-hlsl

Author: Finn Plummer (inbelic)

Changes
  • adds the space and visibility parameters to StaticSampler
  • adds basic unit tests to demonstrate setting functionality

Part 7 and Resolves #126574


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

4 Files Affected:

  • (modified) clang/include/clang/Parse/ParseHLSLRootSignature.h (+2)
  • (modified) clang/lib/Parse/ParseHLSLRootSignature.cpp (+40)
  • (modified) clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp (+6-1)
  • (modified) llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h (+2)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 21df9d0da4a53..34d59d7fb66f4 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -121,6 +121,8 @@ class RootSignatureParser {
     std::optional<llvm::hlsl::rootsig::StaticBorderColor> BorderColor;
     std::optional<float> MinLOD;
     std::optional<float> MaxLOD;
+    std::optional<uint32_t> Space;
+    std::optional<llvm::hlsl::rootsig::ShaderVisibility> Visibility;
   };
   std::optional<ParsedStaticSamplerParams> parseStaticSamplerParams();
 
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 07bd10d00dfad..bd0f9abb5ff1b 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -411,6 +411,12 @@ std::optional<StaticSampler> RootSignatureParser::parseStaticSampler() {
   if (Params->MaxLOD.has_value())
     Sampler.MaxLOD = Params->MaxLOD.value();
 
+  if (Params->Space.has_value())
+    Sampler.Space = Params->Space.value();
+
+  if (Params->Visibility.has_value())
+    Sampler.Visibility = Params->Visibility.value();
+
   if (consumeExpectedToken(TokenKind::pu_r_paren,
                            diag::err_hlsl_unexpected_end_of_params,
                            /*param of=*/TokenKind::kw_StaticSampler))
@@ -866,6 +872,40 @@ RootSignatureParser::parseStaticSamplerParams() {
         return std::nullopt;
       Params.MaxLOD = MaxLOD;
     }
+
+    // `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 6343d9feb4411..3eede0dbed61b 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -226,7 +226,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
 TEST_F(ParseHLSLRootSignatureTest, ValidParseStaticSamplerTest) {
   const llvm::StringLiteral Source = R"cc(
     StaticSampler(s0),
-    StaticSampler(s0, maxAnisotropy = 3,
+    StaticSampler(s0, maxAnisotropy = 3, space = 4,
+      visibility = SHADER_VISIBILITY_DOMAIN,
       minLOD = 4.2f, mipLODBias = 0.23e+3,
       addressW = TEXTURE_ADDRESS_CLAMP,
       addressV = TEXTURE_ADDRESS_BORDER,
@@ -269,6 +270,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseStaticSamplerTest) {
             StaticBorderColor::OpaqueWhite);
   ASSERT_EQ(std::get<StaticSampler>(Elem).MinLOD, 0.f);
   ASSERT_EQ(std::get<StaticSampler>(Elem).MaxLOD, 3.402823466e+38f);
+  ASSERT_EQ(std::get<StaticSampler>(Elem).Space, 0u);
+  ASSERT_EQ(std::get<StaticSampler>(Elem).Visibility, ShaderVisibility::All);
 
   // Check values can be set as expected
   Elem = Elements[1];
@@ -288,6 +291,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseStaticSamplerTest) {
             StaticBorderColor::OpaqueBlackUint);
   ASSERT_EQ(std::get<StaticSampler>(Elem).MinLOD, 4.2f);
   ASSERT_EQ(std::get<StaticSampler>(Elem).MaxLOD, 9000.f);
+  ASSERT_EQ(std::get<StaticSampler>(Elem).Space, 4u);
+  ASSERT_EQ(std::get<StaticSampler>(Elem).Visibility, ShaderVisibility::Domain);
 
   ASSERT_TRUE(Consumer->isSatisfied());
 }
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 025e96ec93c2a..ee65722e68556 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -236,6 +236,8 @@ struct StaticSampler {
   StaticBorderColor BorderColor = StaticBorderColor::OpaqueWhite;
   float MinLOD = 0.f;
   float MaxLOD = 3.402823466e+38f; // FLT_MAX
+  uint32_t Space = 0;
+  ShaderVisibility Visibility = ShaderVisibility::All;
 };
 
 /// Models RootElement : RootFlags | RootConstants | RootParam

@inbelic inbelic linked an issue May 16, 2025 that may be closed by this pull request
4 tasks
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 Static Samplers
2 participants