https://github.com/inbelic updated
https://github.com/llvm/llvm-project/pull/138002
>From 15857bf8e1303e2325b48e417e7abd26aa77910e Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienf...@gmail.com>
Date: Wed, 30 Apr 2025 17:53:11 +0000
Subject: [PATCH 1/2] [HLSL][RootSignature] Add mandatory parameters for
RootConstants
- defines the `parseRootConstantParams` function and adds handling for
the mandatory arguments of `num32BitConstants` and `bReg`
- adds corresponding unit tests
Part two of implementing
---
.../clang/Parse/ParseHLSLRootSignature.h | 10 ++-
clang/lib/Parse/ParseHLSLRootSignature.cpp | 68 ++++++++++++++++++-
.../Parse/ParseHLSLRootSignatureTest.cpp | 14 +++-
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 5 +-
4 files changed, 89 insertions(+), 8 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h
b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index efa735ea03d94..0f05b05ed4df6 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -77,8 +77,14 @@ class RootSignatureParser {
parseDescriptorTableClause();
/// Parameter arguments (eg. `bReg`, `space`, ...) can be specified in any
- /// order and only exactly once. `ParsedClauseParams` denotes the current
- /// state of parsed params
+ /// order and only exactly once. The following methods define a
+ /// `Parsed.*Params` struct to denote the current state of parsed params
+ struct ParsedConstantParams {
+ std::optional<llvm::hlsl::rootsig::Register> Reg;
+ std::optional<uint32_t> Num32BitConstants;
+ };
+ std::optional<ParsedConstantParams> parseRootConstantParams();
+
struct ParsedClauseParams {
std::optional<llvm::hlsl::rootsig::Register> Reg;
std::optional<uint32_t> NumDescriptors;
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp
b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 48d3e38b0519d..2ce8e6e5cca98 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -57,6 +57,27 @@ std::optional<RootConstants>
RootSignatureParser::parseRootConstants() {
RootConstants Constants;
+ auto Params = parseRootConstantParams();
+ if (!Params.has_value())
+ return std::nullopt;
+
+ // Check mandatory parameters were provided
+ if (!Params->Num32BitConstants.has_value()) {
+ getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_missing_param)
+ << TokenKind::kw_num32BitConstants;
+ return std::nullopt;
+ }
+
+ Constants.Num32BitConstants = Params->Num32BitConstants.value();
+
+ if (!Params->Reg.has_value()) {
+ getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_missing_param)
+ << TokenKind::bReg;
+ return std::nullopt;
+ }
+
+ Constants.Reg = Params->Reg.value();
+
if (consumeExpectedToken(TokenKind::pu_r_paren,
diag::err_hlsl_unexpected_end_of_params,
/*param of=*/TokenKind::kw_RootConstants))
@@ -187,14 +208,55 @@ RootSignatureParser::parseDescriptorTableClause() {
return Clause;
}
+// Parameter arguments (eg. `bReg`, `space`, ...) can be specified in any
+// order and only exactly once. The following methods will parse through as
+// many arguments as possible reporting an error if a duplicate is seen.
+std::optional<RootSignatureParser::ParsedConstantParams>
+RootSignatureParser::parseRootConstantParams() {
+ assert(CurToken.TokKind == TokenKind::pu_l_paren &&
+ "Expects to only be invoked starting at given token");
+
+ ParsedConstantParams Params;
+ do {
+ // `num32BitConstants` `=` POS_INT
+ if (tryConsumeExpectedToken(TokenKind::kw_num32BitConstants)) {
+ if (Params.Num32BitConstants.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 Num32BitConstants = parseUIntParam();
+ if (!Num32BitConstants.has_value())
+ return std::nullopt;
+ Params.Num32BitConstants = Num32BitConstants;
+ }
+
+ // `b` POS_INT
+ if (tryConsumeExpectedToken(TokenKind::bReg)) {
+ if (Params.Reg.has_value()) {
+ getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param)
+ << CurToken.TokKind;
+ return std::nullopt;
+ }
+ auto Reg = parseRegister();
+ if (!Reg.has_value())
+ return std::nullopt;
+ Params.Reg = Reg;
+ }
+ } while (tryConsumeExpectedToken(TokenKind::pu_comma));
+
+ return Params;
+}
+
std::optional<RootSignatureParser::ParsedClauseParams>
RootSignatureParser::parseDescriptorTableClauseParams(TokenKind RegType) {
assert(CurToken.TokKind == TokenKind::pu_l_paren &&
"Expects to only be invoked starting at given token");
- // Parameter arguments (eg. `bReg`, `space`, ...) can be specified in any
- // order and only exactly once. Parse through as many arguments as possible
- // reporting an error if a duplicate is seen.
ParsedClauseParams Params;
do {
// ( `b` | `t` | `u` | `s`) POS_INT
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 0a7d8ac86cc5f..336868b579866 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -254,7 +254,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidSamplerFlagsTest) {
TEST_F(ParseHLSLRootSignatureTest, ValidParseRootConsantsTest) {
const llvm::StringLiteral Source = R"cc(
- RootConstants()
+ RootConstants(num32BitConstants = 1, b0),
+ RootConstants(b42, num32BitConstants = 4294967295)
)cc";
TrivialModuleLoader ModLoader;
@@ -270,10 +271,19 @@ TEST_F(ParseHLSLRootSignatureTest,
ValidParseRootConsantsTest) {
ASSERT_FALSE(Parser.parse());
- ASSERT_EQ(Elements.size(), 1u);
+ ASSERT_EQ(Elements.size(), 2u);
RootElement Elem = Elements[0];
ASSERT_TRUE(std::holds_alternative<RootConstants>(Elem));
+ ASSERT_EQ(std::get<RootConstants>(Elem).Num32BitConstants, 1u);
+ ASSERT_EQ(std::get<RootConstants>(Elem).Reg.ViewType, RegisterType::BReg);
+ ASSERT_EQ(std::get<RootConstants>(Elem).Reg.Number, 0u);
+
+ Elem = Elements[1];
+ ASSERT_TRUE(std::holds_alternative<RootConstants>(Elem));
+ ASSERT_EQ(std::get<RootConstants>(Elem).Num32BitConstants, 4294967295u);
+ ASSERT_EQ(std::get<RootConstants>(Elem).Reg.ViewType, RegisterType::BReg);
+ ASSERT_EQ(std::get<RootConstants>(Elem).Reg.Number, 42u);
ASSERT_TRUE(Consumer->isSatisfied());
}
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 05735fa75b318..a3f98a9f1944f 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -55,7 +55,10 @@ struct Register {
};
// Models the parameter values of root constants
-struct RootConstants {};
+struct RootConstants {
+ uint32_t Num32BitConstants;
+ Register Reg;
+};
// Models the end of a descriptor table and stores its visibility
struct DescriptorTable {
>From f8011807d233d1d0d1f9d4ce08bad0c559cde74a Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienf...@gmail.com>
Date: Wed, 30 Apr 2025 18:08:31 +0000
Subject: [PATCH 2/2] [HLSL][RootSignature] Add optional parameter for
RootConstants
- extends `parseRootConstantParams` and the struct to include the
optional parameters of a RootConstant
- adds corresponding unit tests
---
.../clang/Parse/ParseHLSLRootSignature.h | 2 +
clang/lib/Parse/ParseHLSLRootSignature.cpp | 41 +++++++++++++++++++
.../Parse/ParseHLSLRootSignatureTest.cpp | 8 +++-
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 2 +
4 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h
b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 0f05b05ed4df6..2ac2083983741 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -82,6 +82,8 @@ class RootSignatureParser {
struct ParsedConstantParams {
std::optional<llvm::hlsl::rootsig::Register> Reg;
std::optional<uint32_t> Num32BitConstants;
+ std::optional<uint32_t> Space;
+ std::optional<llvm::hlsl::rootsig::ShaderVisibility> Visibility;
};
std::optional<ParsedConstantParams> parseRootConstantParams();
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp
b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 2ce8e6e5cca98..a5006b77a6e44 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -78,6 +78,13 @@ std::optional<RootConstants>
RootSignatureParser::parseRootConstants() {
Constants.Reg = Params->Reg.value();
+ // Fill in optional parameters
+ if (Params->Visibility.has_value())
+ Constants.Visibility = Params->Visibility.value();
+
+ if (Params->Space.has_value())
+ Constants.Space = Params->Space.value();
+
if (consumeExpectedToken(TokenKind::pu_r_paren,
diag::err_hlsl_unexpected_end_of_params,
/*param of=*/TokenKind::kw_RootConstants))
@@ -247,6 +254,40 @@ RootSignatureParser::parseRootConstantParams() {
return std::nullopt;
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 336868b579866..150eb3e6e54ef 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -255,7 +255,9 @@ TEST_F(ParseHLSLRootSignatureTest, ValidSamplerFlagsTest) {
TEST_F(ParseHLSLRootSignatureTest, ValidParseRootConsantsTest) {
const llvm::StringLiteral Source = R"cc(
RootConstants(num32BitConstants = 1, b0),
- RootConstants(b42, num32BitConstants = 4294967295)
+ RootConstants(b42, space = 3, num32BitConstants = 4294967295,
+ visibility = SHADER_VISIBILITY_HULL
+ )
)cc";
TrivialModuleLoader ModLoader;
@@ -278,12 +280,16 @@ TEST_F(ParseHLSLRootSignatureTest,
ValidParseRootConsantsTest) {
ASSERT_EQ(std::get<RootConstants>(Elem).Num32BitConstants, 1u);
ASSERT_EQ(std::get<RootConstants>(Elem).Reg.ViewType, RegisterType::BReg);
ASSERT_EQ(std::get<RootConstants>(Elem).Reg.Number, 0u);
+ ASSERT_EQ(std::get<RootConstants>(Elem).Space, 0u);
+ ASSERT_EQ(std::get<RootConstants>(Elem).Visibility, ShaderVisibility::All);
Elem = Elements[1];
ASSERT_TRUE(std::holds_alternative<RootConstants>(Elem));
ASSERT_EQ(std::get<RootConstants>(Elem).Num32BitConstants, 4294967295u);
ASSERT_EQ(std::get<RootConstants>(Elem).Reg.ViewType, RegisterType::BReg);
ASSERT_EQ(std::get<RootConstants>(Elem).Reg.Number, 42u);
+ ASSERT_EQ(std::get<RootConstants>(Elem).Space, 3u);
+ ASSERT_EQ(std::get<RootConstants>(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 a3f98a9f1944f..8b8324df18bb3 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -58,6 +58,8 @@ struct Register {
struct RootConstants {
uint32_t Num32BitConstants;
Register Reg;
+ uint32_t Space = 0;
+ ShaderVisibility Visibility = ShaderVisibility::All;
};
// Models the end of a descriptor table and stores its visibility
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
