https://github.com/inbelic updated
https://github.com/llvm/llvm-project/pull/147350
>From b8644c7d9a44f9480cdbe0b3c46f0899cdcffc28 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienf...@gmail.com>
Date: Sat, 5 Jul 2025 01:41:26 +0000
Subject: [PATCH 1/7] fix for root elements
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 33 ++++++++-----------
clang/test/SemaHLSL/RootSignature-err.hlsl | 4 +++
.../Parse/ParseHLSLRootSignatureTest.cpp | 25 ++++++++++++++
3 files changed, 43 insertions(+), 19 deletions(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp
b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index cf86c62f3b671..7956f1f7fdbfa 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -25,44 +25,41 @@ RootSignatureParser::RootSignatureParser(
Lexer(Signature->getString()), PP(PP), CurToken(0) {}
bool RootSignatureParser::parse() {
- // Iterate as many RootElements as possible
- do {
+ // Iterate as many RootSignatureElements as possible, until we hit the
+ // end of the stream
+ while (!peekExpectedToken(TokenKind::end_of_stream)) {
if (tryConsumeExpectedToken(TokenKind::kw_RootFlags)) {
auto Flags = parseRootFlags();
if (!Flags.has_value())
return true;
Elements.push_back(*Flags);
- }
-
- if (tryConsumeExpectedToken(TokenKind::kw_RootConstants)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_RootConstants)) {
auto Constants = parseRootConstants();
if (!Constants.has_value())
return true;
Elements.push_back(*Constants);
- }
-
- if (tryConsumeExpectedToken(TokenKind::kw_DescriptorTable)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_DescriptorTable)) {
auto Table = parseDescriptorTable();
if (!Table.has_value())
return true;
Elements.push_back(*Table);
- }
-
- if (tryConsumeExpectedToken(
- {TokenKind::kw_CBV, TokenKind::kw_SRV, TokenKind::kw_UAV})) {
+ } else if (tryConsumeExpectedToken(
+ {TokenKind::kw_CBV, TokenKind::kw_SRV, TokenKind::kw_UAV}))
{
auto Descriptor = parseRootDescriptor();
if (!Descriptor.has_value())
return true;
Elements.push_back(*Descriptor);
- }
-
- if (tryConsumeExpectedToken(TokenKind::kw_StaticSampler)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_StaticSampler)) {
auto Sampler = parseStaticSampler();
if (!Sampler.has_value())
return true;
Elements.push_back(*Sampler);
}
- } while (tryConsumeExpectedToken(TokenKind::pu_comma));
+
+ // ',' denotes another element, otherwise, expected to be at end of stream
+ if (!tryConsumeExpectedToken(TokenKind::pu_comma))
+ break;
+ }
return consumeExpectedToken(TokenKind::end_of_stream,
diag::err_hlsl_unexpected_end_of_params,
@@ -252,9 +249,7 @@ std::optional<DescriptorTable>
RootSignatureParser::parseDescriptorTable() {
return std::nullopt;
Elements.push_back(*Clause);
Table.NumClauses++;
- }
-
- if (tryConsumeExpectedToken(TokenKind::kw_visibility)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_visibility)) {
if (Visibility.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
diff --git a/clang/test/SemaHLSL/RootSignature-err.hlsl
b/clang/test/SemaHLSL/RootSignature-err.hlsl
index 118fc38daf3f2..50807d611e91a 100644
--- a/clang/test/SemaHLSL/RootSignature-err.hlsl
+++ b/clang/test/SemaHLSL/RootSignature-err.hlsl
@@ -34,3 +34,7 @@ void bad_root_signature_5() {}
// expected-error@+1 {{expected ')' to denote end of parameters, or, another
valid parameter of RootConstants}}
[RootSignature(MultiLineRootSignature)]
void bad_root_signature_6() {}
+
+// expected-error@+1 {{expected end of stream to denote end of parameters, or,
another valid parameter of RootSignature}}
+[RootSignature("RootFlags() RootConstants(b0, num32BitConstants = 1))")]
+void bad_root_signature_7() {}
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index ff1697f1bbb9a..fd452b29ebf00 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -1238,4 +1238,29 @@ TEST_F(ParseHLSLRootSignatureTest,
InvalidNonZeroFlagsTest) {
ASSERT_TRUE(Consumer->isSatisfied());
}
+TEST_F(ParseHLSLRootSignatureTest, InvalidRootElementMissingCommaTest) {
+ // This test will check that an error is produced when there is a missing
+ // comma between parameters
+ const llvm::StringLiteral Source = R"cc(
+ RootFlags()
+ RootConstants(num32BitConstants = 1, b0)
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
} // anonymous namespace
>From 24ed530a19fa49bb6363ec0cf2325a85400030e6 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienf...@gmail.com>
Date: Sat, 5 Jul 2025 01:52:21 +0000
Subject: [PATCH 2/7] fix for descriptor table
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 18 ++++++++-----
.../Parse/ParseHLSLRootSignatureTest.cpp | 27 +++++++++++++++++++
2 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp
b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 7956f1f7fdbfa..23355567e871b 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -240,16 +240,18 @@ std::optional<DescriptorTable>
RootSignatureParser::parseDescriptorTable() {
DescriptorTable Table;
std::optional<llvm::dxbc::ShaderVisibility> Visibility;
- // Iterate as many Clauses as possible
- do {
+ // Iterate as many Clauses as possible, until we hit ')'
+ while (!peekExpectedToken(TokenKind::pu_r_paren)) {
if (tryConsumeExpectedToken({TokenKind::kw_CBV, TokenKind::kw_SRV,
TokenKind::kw_UAV, TokenKind::kw_Sampler})) {
+ // DescriptorTableClause - CBV, SRV, UAV, or Sampler
auto Clause = parseDescriptorTableClause();
if (!Clause.has_value())
return std::nullopt;
Elements.push_back(*Clause);
Table.NumClauses++;
} else if (tryConsumeExpectedToken(TokenKind::kw_visibility)) {
+ // visibility = SHADER_VISIBILITY
if (Visibility.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -262,17 +264,21 @@ std::optional<DescriptorTable>
RootSignatureParser::parseDescriptorTable() {
if (!Visibility.has_value())
return std::nullopt;
}
- } while (tryConsumeExpectedToken(TokenKind::pu_comma));
- // Fill in optional visibility
- if (Visibility.has_value())
- Table.Visibility = Visibility.value();
+ // ',' denotes another element, otherwise, expected to be at ')'
+ if (!tryConsumeExpectedToken(TokenKind::pu_comma))
+ break;
+ }
if (consumeExpectedToken(TokenKind::pu_r_paren,
diag::err_hlsl_unexpected_end_of_params,
/*param of=*/TokenKind::kw_DescriptorTable))
return std::nullopt;
+ // Fill in optional visibility
+ if (Visibility.has_value())
+ Table.Visibility = Visibility.value();
+
return Table;
}
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index fd452b29ebf00..5da2aea0a63c0 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -1263,4 +1263,31 @@ TEST_F(ParseHLSLRootSignatureTest,
InvalidRootElementMissingCommaTest) {
ASSERT_TRUE(Consumer->isSatisfied());
}
+TEST_F(ParseHLSLRootSignatureTest, InvalidDescriptorTableMissingCommaTest) {
+ // This test will check that an error is produced when there is a missing
+ // comma between parameters
+ const llvm::StringLiteral Source = R"cc(
+ DescriptorTable(
+ CBV(b0)
+ visibility = SHADER_VISIBILITY_ALL
+ )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
} // anonymous namespace
>From 4f210fac40caf9531320bfb5dc3d952a3ed541c6 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienf...@gmail.com>
Date: Mon, 7 Jul 2025 16:30:59 +0000
Subject: [PATCH 3/7] fix for root constants
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 38 +++++++++----------
.../Parse/ParseHLSLRootSignatureTest.cpp | 27 +++++++++++++
2 files changed, 45 insertions(+), 20 deletions(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp
b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 23355567e871b..c73025613e819 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -136,6 +136,11 @@ std::optional<RootConstants>
RootSignatureParser::parseRootConstants() {
if (!Params.has_value())
return std::nullopt;
+ if (consumeExpectedToken(TokenKind::pu_r_paren,
+ diag::err_hlsl_unexpected_end_of_params,
+ /*param of=*/TokenKind::kw_RootConstants))
+ return std::nullopt;
+
// Check mandatory parameters where provided
if (!Params->Num32BitConstants.has_value()) {
reportDiag(diag::err_hlsl_rootsig_missing_param)
@@ -159,11 +164,6 @@ std::optional<RootConstants>
RootSignatureParser::parseRootConstants() {
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))
- return std::nullopt;
-
return Constants;
}
@@ -429,9 +429,9 @@ RootSignatureParser::parseRootConstantParams() {
"Expects to only be invoked starting at given token");
ParsedConstantParams Params;
- do {
- // `num32BitConstants` `=` POS_INT
+ while (!peekExpectedToken(TokenKind::pu_r_paren)) {
if (tryConsumeExpectedToken(TokenKind::kw_num32BitConstants)) {
+ // `num32BitConstants` `=` POS_INT
if (Params.Num32BitConstants.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -444,10 +444,8 @@ RootSignatureParser::parseRootConstantParams() {
if (!Num32BitConstants.has_value())
return std::nullopt;
Params.Num32BitConstants = Num32BitConstants;
- }
-
- // `b` POS_INT
- if (tryConsumeExpectedToken(TokenKind::bReg)) {
+ } else if (tryConsumeExpectedToken(TokenKind::bReg)) {
+ // `b` POS_INT
if (Params.Reg.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -456,10 +454,8 @@ RootSignatureParser::parseRootConstantParams() {
if (!Reg.has_value())
return std::nullopt;
Params.Reg = Reg;
- }
-
- // `space` `=` POS_INT
- if (tryConsumeExpectedToken(TokenKind::kw_space)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_space)) {
+ // `space` `=` POS_INT
if (Params.Space.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -472,10 +468,8 @@ RootSignatureParser::parseRootConstantParams() {
if (!Space.has_value())
return std::nullopt;
Params.Space = Space;
- }
-
- // `visibility` `=` SHADER_VISIBILITY
- if (tryConsumeExpectedToken(TokenKind::kw_visibility)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_visibility)) {
+ // `visibility` `=` SHADER_VISIBILITY
if (Params.Visibility.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -489,7 +483,11 @@ RootSignatureParser::parseRootConstantParams() {
return std::nullopt;
Params.Visibility = Visibility;
}
- } while (tryConsumeExpectedToken(TokenKind::pu_comma));
+
+ // ',' denotes another element, otherwise, expected to be at ')'
+ if (!tryConsumeExpectedToken(TokenKind::pu_comma))
+ break;
+ }
return Params;
}
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 5da2aea0a63c0..8d9e8a120b6fc 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -1290,4 +1290,31 @@ TEST_F(ParseHLSLRootSignatureTest,
InvalidDescriptorTableMissingCommaTest) {
ASSERT_TRUE(Consumer->isSatisfied());
}
+TEST_F(ParseHLSLRootSignatureTest, InvalidRootConstantParamsCommaTest) {
+ // This test will check that an error is produced when there is a missing
+ // comma between parameters
+ const llvm::StringLiteral Source = R"cc(
+ RootConstants(
+ num32BitConstants = 1
+ b0
+ )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
} // anonymous namespace
>From 15844909e7f88d4fce582ebaa97eb418ea55f6cf Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienf...@gmail.com>
Date: Mon, 7 Jul 2025 16:54:53 +0000
Subject: [PATCH 4/7] fix for root descriptors
---
.../clang/Parse/ParseHLSLRootSignature.h | 3 +-
clang/lib/Parse/ParseHLSLRootSignature.cpp | 43 +++++++++----------
.../Parse/ParseHLSLRootSignatureTest.cpp | 27 ++++++++++++
3 files changed, 50 insertions(+), 23 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h
b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 007c3f7ba1e9c..0c6594e44d51b 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -99,7 +99,8 @@ class RootSignatureParser {
std::optional<llvm::dxbc::RootDescriptorFlags> Flags;
};
std::optional<ParsedRootDescriptorParams>
- parseRootDescriptorParams(RootSignatureToken::Kind RegType);
+ parseRootDescriptorParams(RootSignatureToken::Kind DescType,
+ RootSignatureToken::Kind RegType);
struct ParsedClauseParams {
std::optional<llvm::hlsl::rootsig::Register> Reg;
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp
b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index c73025613e819..15e01eed78f7c 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -199,10 +199,15 @@ std::optional<RootDescriptor>
RootSignatureParser::parseRootDescriptor() {
}
Descriptor.setDefaultFlags(Version);
- auto Params = parseRootDescriptorParams(ExpectedReg);
+ auto Params = parseRootDescriptorParams(DescriptorKind, ExpectedReg);
if (!Params.has_value())
return std::nullopt;
+ if (consumeExpectedToken(TokenKind::pu_r_paren,
+ diag::err_hlsl_unexpected_end_of_params,
+ /*param of=*/DescriptorKind))
+ return std::nullopt;
+
// Check mandatory parameters were provided
if (!Params->Reg.has_value()) {
reportDiag(diag::err_hlsl_rootsig_missing_param) << ExpectedReg;
@@ -221,11 +226,6 @@ std::optional<RootDescriptor>
RootSignatureParser::parseRootDescriptor() {
if (Params->Flags.has_value())
Descriptor.Flags = Params->Flags.value();
- if (consumeExpectedToken(TokenKind::pu_r_paren,
- diag::err_hlsl_unexpected_end_of_params,
- /*param of=*/TokenKind::kw_RootConstants))
- return std::nullopt;
-
return Descriptor;
}
@@ -493,14 +493,15 @@ RootSignatureParser::parseRootConstantParams() {
}
std::optional<RootSignatureParser::ParsedRootDescriptorParams>
-RootSignatureParser::parseRootDescriptorParams(TokenKind RegType) {
+RootSignatureParser::parseRootDescriptorParams(TokenKind DescType,
+ TokenKind RegType) {
assert(CurToken.TokKind == TokenKind::pu_l_paren &&
"Expects to only be invoked starting at given token");
ParsedRootDescriptorParams Params;
- do {
- // ( `b` | `t` | `u`) POS_INT
+ while (!peekExpectedToken(TokenKind::pu_r_paren)) {
if (tryConsumeExpectedToken(RegType)) {
+ // ( `b` | `t` | `u`) POS_INT
if (Params.Reg.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -509,10 +510,8 @@ RootSignatureParser::parseRootDescriptorParams(TokenKind
RegType) {
if (!Reg.has_value())
return std::nullopt;
Params.Reg = Reg;
- }
-
- // `space` `=` POS_INT
- if (tryConsumeExpectedToken(TokenKind::kw_space)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_space)) {
+ // `space` `=` POS_INT
if (Params.Space.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -525,10 +524,8 @@ RootSignatureParser::parseRootDescriptorParams(TokenKind
RegType) {
if (!Space.has_value())
return std::nullopt;
Params.Space = Space;
- }
-
- // `visibility` `=` SHADER_VISIBILITY
- if (tryConsumeExpectedToken(TokenKind::kw_visibility)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_visibility)) {
+ // `visibility` `=` SHADER_VISIBILITY
if (Params.Visibility.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -541,10 +538,8 @@ RootSignatureParser::parseRootDescriptorParams(TokenKind
RegType) {
if (!Visibility.has_value())
return std::nullopt;
Params.Visibility = Visibility;
- }
-
- // `flags` `=` ROOT_DESCRIPTOR_FLAGS
- if (tryConsumeExpectedToken(TokenKind::kw_flags)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_flags)) {
+ // `flags` `=` ROOT_DESCRIPTOR_FLAGS
if (Params.Flags.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -558,7 +553,11 @@ RootSignatureParser::parseRootDescriptorParams(TokenKind
RegType) {
return std::nullopt;
Params.Flags = Flags;
}
- } while (tryConsumeExpectedToken(TokenKind::pu_comma));
+
+ // ',' denotes another element, otherwise, expected to be at ')'
+ if (!tryConsumeExpectedToken(TokenKind::pu_comma))
+ break;
+ }
return Params;
}
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 8d9e8a120b6fc..c0c345b6ad58d 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -1317,4 +1317,31 @@ TEST_F(ParseHLSLRootSignatureTest,
InvalidRootConstantParamsCommaTest) {
ASSERT_TRUE(Consumer->isSatisfied());
}
+TEST_F(ParseHLSLRootSignatureTest, InvalidRootDescriptorParamsCommaTest) {
+ // This test will check that an error is produced when there is a missing
+ // comma between parameters
+ const llvm::StringLiteral Source = R"cc(
+ CBV(
+ b0
+ flags = 0
+ )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
} // anonymous namespace
>From 8dae33e6b70ac79cc2b6be266c2324d49443b91c Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienf...@gmail.com>
Date: Mon, 7 Jul 2025 17:12:31 +0000
Subject: [PATCH 5/7] fix for descriptor table clauses
---
.../clang/Parse/ParseHLSLRootSignature.h | 3 +-
clang/lib/Parse/ParseHLSLRootSignature.cpp | 48 +++++++++----------
.../Parse/ParseHLSLRootSignatureTest.cpp | 29 +++++++++++
3 files changed, 53 insertions(+), 27 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h
b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 0c6594e44d51b..fc71ad5e3722c 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -110,7 +110,8 @@ class RootSignatureParser {
std::optional<llvm::dxbc::DescriptorRangeFlags> Flags;
};
std::optional<ParsedClauseParams>
- parseDescriptorTableClauseParams(RootSignatureToken::Kind RegType);
+ parseDescriptorTableClauseParams(RootSignatureToken::Kind DescType,
+ RootSignatureToken::Kind RegType);
struct ParsedStaticSamplerParams {
std::optional<llvm::hlsl::rootsig::Register> Reg;
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp
b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 15e01eed78f7c..9088da892f8dc 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -320,10 +320,15 @@ RootSignatureParser::parseDescriptorTableClause() {
}
Clause.setDefaultFlags(Version);
- auto Params = parseDescriptorTableClauseParams(ExpectedReg);
+ auto Params = parseDescriptorTableClauseParams(ParamKind, ExpectedReg);
if (!Params.has_value())
return std::nullopt;
+ if (consumeExpectedToken(TokenKind::pu_r_paren,
+ diag::err_hlsl_unexpected_end_of_params,
+ /*param of=*/ParamKind))
+ return std::nullopt;
+
// Check mandatory parameters were provided
if (!Params->Reg.has_value()) {
reportDiag(diag::err_hlsl_rootsig_missing_param) << ExpectedReg;
@@ -345,11 +350,6 @@ RootSignatureParser::parseDescriptorTableClause() {
if (Params->Flags.has_value())
Clause.Flags = Params->Flags.value();
- if (consumeExpectedToken(TokenKind::pu_r_paren,
- diag::err_hlsl_unexpected_end_of_params,
- /*param of=*/ParamKind))
- return std::nullopt;
-
return Clause;
}
@@ -563,14 +563,15 @@ RootSignatureParser::parseRootDescriptorParams(TokenKind
DescType,
}
std::optional<RootSignatureParser::ParsedClauseParams>
-RootSignatureParser::parseDescriptorTableClauseParams(TokenKind RegType) {
+RootSignatureParser::parseDescriptorTableClauseParams(TokenKind DescType,
+ TokenKind RegType) {
assert(CurToken.TokKind == TokenKind::pu_l_paren &&
"Expects to only be invoked starting at given token");
ParsedClauseParams Params;
- do {
- // ( `b` | `t` | `u` | `s`) POS_INT
+ while (!peekExpectedToken(TokenKind::pu_r_paren)) {
if (tryConsumeExpectedToken(RegType)) {
+ // ( `b` | `t` | `u` | `s`) POS_INT
if (Params.Reg.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -579,10 +580,8 @@
RootSignatureParser::parseDescriptorTableClauseParams(TokenKind RegType) {
if (!Reg.has_value())
return std::nullopt;
Params.Reg = Reg;
- }
-
- // `numDescriptors` `=` POS_INT | unbounded
- if (tryConsumeExpectedToken(TokenKind::kw_numDescriptors)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_numDescriptors)) {
+ // `numDescriptors` `=` POS_INT | unbounded
if (Params.NumDescriptors.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -601,10 +600,8 @@
RootSignatureParser::parseDescriptorTableClauseParams(TokenKind RegType) {
}
Params.NumDescriptors = NumDescriptors;
- }
-
- // `space` `=` POS_INT
- if (tryConsumeExpectedToken(TokenKind::kw_space)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_space)) {
+ // `space` `=` POS_INT
if (Params.Space.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -617,10 +614,8 @@
RootSignatureParser::parseDescriptorTableClauseParams(TokenKind RegType) {
if (!Space.has_value())
return std::nullopt;
Params.Space = Space;
- }
-
- // `offset` `=` POS_INT | DESCRIPTOR_RANGE_OFFSET_APPEND
- if (tryConsumeExpectedToken(TokenKind::kw_offset)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_offset)) {
+ // `offset` `=` POS_INT | DESCRIPTOR_RANGE_OFFSET_APPEND
if (Params.Offset.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -639,10 +634,8 @@
RootSignatureParser::parseDescriptorTableClauseParams(TokenKind RegType) {
}
Params.Offset = Offset;
- }
-
- // `flags` `=` DESCRIPTOR_RANGE_FLAGS
- if (tryConsumeExpectedToken(TokenKind::kw_flags)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_flags)) {
+ // `flags` `=` DESCRIPTOR_RANGE_FLAGS
if (Params.Flags.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -657,7 +650,10 @@
RootSignatureParser::parseDescriptorTableClauseParams(TokenKind RegType) {
Params.Flags = Flags;
}
- } while (tryConsumeExpectedToken(TokenKind::pu_comma));
+ // ',' denotes another element, otherwise, expected to be at ')'
+ if (!tryConsumeExpectedToken(TokenKind::pu_comma))
+ break;
+ }
return Params;
}
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index c0c345b6ad58d..601ee74388edd 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -1344,4 +1344,33 @@ TEST_F(ParseHLSLRootSignatureTest,
InvalidRootDescriptorParamsCommaTest) {
ASSERT_TRUE(Consumer->isSatisfied());
}
+TEST_F(ParseHLSLRootSignatureTest, InvalidDescriptorClauseParamsCommaTest) {
+ // This test will check that an error is produced when there is a missing
+ // comma between parameters
+ const llvm::StringLiteral Source = R"cc(
+ DescriptorTable(
+ UAV(
+ u0
+ flags = 0
+ )
+ )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
} // anonymous namespace
>From 1643d251d01563fee60fc3ffc011de84ae408660 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienf...@gmail.com>
Date: Mon, 7 Jul 2025 17:18:09 +0000
Subject: [PATCH 6/7] fix for static samplers
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 92 ++++++++-----------
.../Parse/ParseHLSLRootSignatureTest.cpp | 27 ++++++
2 files changed, 63 insertions(+), 56 deletions(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp
b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 9088da892f8dc..5cc1dc598334e 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -367,6 +367,11 @@ std::optional<StaticSampler>
RootSignatureParser::parseStaticSampler() {
if (!Params.has_value())
return std::nullopt;
+ if (consumeExpectedToken(TokenKind::pu_r_paren,
+ diag::err_hlsl_unexpected_end_of_params,
+ /*param of=*/TokenKind::kw_StaticSampler))
+ return std::nullopt;
+
// Check mandatory parameters were provided
if (!Params->Reg.has_value()) {
reportDiag(diag::err_hlsl_rootsig_missing_param) << TokenKind::sReg;
@@ -412,11 +417,6 @@ std::optional<StaticSampler>
RootSignatureParser::parseStaticSampler() {
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))
- return std::nullopt;
-
return Sampler;
}
@@ -664,9 +664,9 @@ RootSignatureParser::parseStaticSamplerParams() {
"Expects to only be invoked starting at given token");
ParsedStaticSamplerParams Params;
- do {
- // `s` POS_INT
+ while (!peekExpectedToken(TokenKind::pu_r_paren)) {
if (tryConsumeExpectedToken(TokenKind::sReg)) {
+ // `s` POS_INT
if (Params.Reg.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -675,10 +675,8 @@ RootSignatureParser::parseStaticSamplerParams() {
if (!Reg.has_value())
return std::nullopt;
Params.Reg = Reg;
- }
-
- // `filter` `=` FILTER
- if (tryConsumeExpectedToken(TokenKind::kw_filter)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_filter)) {
+ // `filter` `=` FILTER
if (Params.Filter.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -691,10 +689,8 @@ RootSignatureParser::parseStaticSamplerParams() {
if (!Filter.has_value())
return std::nullopt;
Params.Filter = Filter;
- }
-
- // `addressU` `=` TEXTURE_ADDRESS
- if (tryConsumeExpectedToken(TokenKind::kw_addressU)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_addressU)) {
+ // `addressU` `=` TEXTURE_ADDRESS
if (Params.AddressU.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -707,10 +703,8 @@ RootSignatureParser::parseStaticSamplerParams() {
if (!AddressU.has_value())
return std::nullopt;
Params.AddressU = AddressU;
- }
-
- // `addressV` `=` TEXTURE_ADDRESS
- if (tryConsumeExpectedToken(TokenKind::kw_addressV)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_addressV)) {
+ // `addressV` `=` TEXTURE_ADDRESS
if (Params.AddressV.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -723,10 +717,8 @@ RootSignatureParser::parseStaticSamplerParams() {
if (!AddressV.has_value())
return std::nullopt;
Params.AddressV = AddressV;
- }
-
- // `addressW` `=` TEXTURE_ADDRESS
- if (tryConsumeExpectedToken(TokenKind::kw_addressW)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_addressW)) {
+ // `addressW` `=` TEXTURE_ADDRESS
if (Params.AddressW.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -739,10 +731,8 @@ RootSignatureParser::parseStaticSamplerParams() {
if (!AddressW.has_value())
return std::nullopt;
Params.AddressW = AddressW;
- }
-
- // `mipLODBias` `=` NUMBER
- if (tryConsumeExpectedToken(TokenKind::kw_mipLODBias)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_mipLODBias)) {
+ // `mipLODBias` `=` NUMBER
if (Params.MipLODBias.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -755,10 +745,8 @@ RootSignatureParser::parseStaticSamplerParams() {
if (!MipLODBias.has_value())
return std::nullopt;
Params.MipLODBias = MipLODBias;
- }
-
- // `maxAnisotropy` `=` POS_INT
- if (tryConsumeExpectedToken(TokenKind::kw_maxAnisotropy)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_maxAnisotropy)) {
+ // `maxAnisotropy` `=` POS_INT
if (Params.MaxAnisotropy.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -771,10 +759,8 @@ RootSignatureParser::parseStaticSamplerParams() {
if (!MaxAnisotropy.has_value())
return std::nullopt;
Params.MaxAnisotropy = MaxAnisotropy;
- }
-
- // `comparisonFunc` `=` COMPARISON_FUNC
- if (tryConsumeExpectedToken(TokenKind::kw_comparisonFunc)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_comparisonFunc)) {
+ // `comparisonFunc` `=` COMPARISON_FUNC
if (Params.CompFunc.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -787,10 +773,8 @@ RootSignatureParser::parseStaticSamplerParams() {
if (!CompFunc.has_value())
return std::nullopt;
Params.CompFunc = CompFunc;
- }
-
- // `borderColor` `=` STATIC_BORDER_COLOR
- if (tryConsumeExpectedToken(TokenKind::kw_borderColor)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_borderColor)) {
+ // `borderColor` `=` STATIC_BORDER_COLOR
if (Params.BorderColor.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -803,10 +787,8 @@ RootSignatureParser::parseStaticSamplerParams() {
if (!BorderColor.has_value())
return std::nullopt;
Params.BorderColor = BorderColor;
- }
-
- // `minLOD` `=` NUMBER
- if (tryConsumeExpectedToken(TokenKind::kw_minLOD)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_minLOD)) {
+ // `minLOD` `=` NUMBER
if (Params.MinLOD.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -819,10 +801,8 @@ RootSignatureParser::parseStaticSamplerParams() {
if (!MinLOD.has_value())
return std::nullopt;
Params.MinLOD = MinLOD;
- }
-
- // `maxLOD` `=` NUMBER
- if (tryConsumeExpectedToken(TokenKind::kw_maxLOD)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_maxLOD)) {
+ // `maxLOD` `=` NUMBER
if (Params.MaxLOD.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -835,10 +815,8 @@ RootSignatureParser::parseStaticSamplerParams() {
if (!MaxLOD.has_value())
return std::nullopt;
Params.MaxLOD = MaxLOD;
- }
-
- // `space` `=` POS_INT
- if (tryConsumeExpectedToken(TokenKind::kw_space)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_space)) {
+ // `space` `=` POS_INT
if (Params.Space.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -851,10 +829,8 @@ RootSignatureParser::parseStaticSamplerParams() {
if (!Space.has_value())
return std::nullopt;
Params.Space = Space;
- }
-
- // `visibility` `=` SHADER_VISIBILITY
- if (tryConsumeExpectedToken(TokenKind::kw_visibility)) {
+ } else if (tryConsumeExpectedToken(TokenKind::kw_visibility)) {
+ // `visibility` `=` SHADER_VISIBILITY
if (Params.Visibility.has_value()) {
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
return std::nullopt;
@@ -868,7 +844,11 @@ RootSignatureParser::parseStaticSamplerParams() {
return std::nullopt;
Params.Visibility = Visibility;
}
- } while (tryConsumeExpectedToken(TokenKind::pu_comma));
+
+ // ',' denotes another element, otherwise, expected to be at ')'
+ if (!tryConsumeExpectedToken(TokenKind::pu_comma))
+ break;
+ }
return Params;
}
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 601ee74388edd..e82dcadebba3f 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -1373,4 +1373,31 @@ TEST_F(ParseHLSLRootSignatureTest,
InvalidDescriptorClauseParamsCommaTest) {
ASSERT_TRUE(Consumer->isSatisfied());
}
+TEST_F(ParseHLSLRootSignatureTest, InvalidStaticSamplerCommaTest) {
+ // This test will check that an error is produced when there is a missing
+ // comma between parameters
+ const llvm::StringLiteral Source = R"cc(
+ StaticSampler(
+ s0
+ maxLOD = 3
+ )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
} // anonymous namespace
>From b9cf61471f5b00ae684071d1b7b7cff4e665fc67 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienf...@gmail.com>
Date: Tue, 8 Jul 2025 18:28:36 +0000
Subject: [PATCH 7/7] rebase-fix: remove unneeded params
- these are lingering from the improve diag related changes
---
clang/include/clang/Parse/ParseHLSLRootSignature.h | 6 ++----
clang/lib/Parse/ParseHLSLRootSignature.cpp | 10 ++++------
2 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h
b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index fc71ad5e3722c..007c3f7ba1e9c 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -99,8 +99,7 @@ class RootSignatureParser {
std::optional<llvm::dxbc::RootDescriptorFlags> Flags;
};
std::optional<ParsedRootDescriptorParams>
- parseRootDescriptorParams(RootSignatureToken::Kind DescType,
- RootSignatureToken::Kind RegType);
+ parseRootDescriptorParams(RootSignatureToken::Kind RegType);
struct ParsedClauseParams {
std::optional<llvm::hlsl::rootsig::Register> Reg;
@@ -110,8 +109,7 @@ class RootSignatureParser {
std::optional<llvm::dxbc::DescriptorRangeFlags> Flags;
};
std::optional<ParsedClauseParams>
- parseDescriptorTableClauseParams(RootSignatureToken::Kind DescType,
- RootSignatureToken::Kind RegType);
+ parseDescriptorTableClauseParams(RootSignatureToken::Kind RegType);
struct ParsedStaticSamplerParams {
std::optional<llvm::hlsl::rootsig::Register> Reg;
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp
b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 5cc1dc598334e..dc5f6faefbab4 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -199,7 +199,7 @@ std::optional<RootDescriptor>
RootSignatureParser::parseRootDescriptor() {
}
Descriptor.setDefaultFlags(Version);
- auto Params = parseRootDescriptorParams(DescriptorKind, ExpectedReg);
+ auto Params = parseRootDescriptorParams(ExpectedReg);
if (!Params.has_value())
return std::nullopt;
@@ -320,7 +320,7 @@ RootSignatureParser::parseDescriptorTableClause() {
}
Clause.setDefaultFlags(Version);
- auto Params = parseDescriptorTableClauseParams(ParamKind, ExpectedReg);
+ auto Params = parseDescriptorTableClauseParams(ExpectedReg);
if (!Params.has_value())
return std::nullopt;
@@ -493,8 +493,7 @@ RootSignatureParser::parseRootConstantParams() {
}
std::optional<RootSignatureParser::ParsedRootDescriptorParams>
-RootSignatureParser::parseRootDescriptorParams(TokenKind DescType,
- TokenKind RegType) {
+RootSignatureParser::parseRootDescriptorParams(TokenKind RegType) {
assert(CurToken.TokKind == TokenKind::pu_l_paren &&
"Expects to only be invoked starting at given token");
@@ -563,8 +562,7 @@ RootSignatureParser::parseRootDescriptorParams(TokenKind
DescType,
}
std::optional<RootSignatureParser::ParsedClauseParams>
-RootSignatureParser::parseDescriptorTableClauseParams(TokenKind DescType,
- TokenKind RegType) {
+RootSignatureParser::parseDescriptorTableClauseParams(TokenKind RegType) {
assert(CurToken.TokKind == TokenKind::pu_l_paren &&
"Expects to only be invoked starting at given token");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
