[clang] [llvm] [Sema][Parse][HLSL] Implement front-end rootsignature validations (PR #156754)
https://github.com/inbelic closed https://github.com/llvm/llvm-project/pull/156754 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Sema][Parse][HLSL] Implement front-end rootsignature validations (PR #156754)
https://github.com/joaosaffran approved this pull request. LGTM, small nit https://github.com/llvm/llvm-project/pull/156754 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Sema][Parse][HLSL] Implement front-end rootsignature validations (PR #156754)
@@ -37,8 +37,18 @@ bool RootSignatureParser::parse() {
// Iterate as many RootSignatureElements as possible, until we hit the
// end of the stream
bool HadError = false;
+ bool HasRootFlags = false;
while (!peekExpectedToken(TokenKind::end_of_stream)) {
if (tryConsumeExpectedToken(TokenKind::kw_RootFlags)) {
+ if (HasRootFlags) {
+reportDiag(diag::err_hlsl_rootsig_repeat_param)
+<< TokenKind::kw_RootFlags;
+HadError = true;
+skipUntilExpectedToken(RootElementKeywords);
+continue;
+ }
+ HasRootFlags = true;
joaosaffran wrote:
nit: This doesn't seem related to this change, is it checking if we are
defining multiple `RootFlags` in a root signature?
https://github.com/llvm/llvm-project/pull/156754
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Sema][Parse][HLSL] Implement front-end rootsignature validations (PR #156754)
https://github.com/joaosaffran edited https://github.com/llvm/llvm-project/pull/156754 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Sema][Parse][HLSL] Implement front-end rootsignature validations (PR #156754)
https://github.com/farzonl approved this pull request. https://github.com/llvm/llvm-project/pull/156754 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Sema][Parse][HLSL] Implement front-end rootsignature validations (PR #156754)
@@ -1331,12 +1331,48 @@ bool SemaHLSL::handleRootSignatureElements(
std::get_if(&Elem)) {
assert(UnboundClauses.size() == Table->NumClauses &&
"Number of unbound elements must match the number of clauses");
+ bool HasSampler = false;
+ bool HasNonSampler = false;
farzonl wrote:
can you change the naming to something like `HasAny*`?
https://github.com/llvm/llvm-project/pull/156754
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Sema][Parse][HLSL] Implement front-end rootsignature validations (PR #156754)
llvmbot wrote:
@llvm/pr-subscribers-hlsl
Author: Finn Plummer (inbelic)
Changes
This pr implements the following validations:
1. Check that descriptor tables don't mix Sample and non-Sampler resources
2. Ensure that descriptor ranges don't append onto an unbounded range
3. Ensure that descriptor ranges don't overflow
4. Adds a missing validation to ensure that only a single `RootFlags` parameter
is provided
Resolves: https://github.com/llvm/llvm-project/issues/153868.
---
Full diff: https://github.com/llvm/llvm-project/pull/156754.diff
9 Files Affected:
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3)
- (modified) clang/lib/Parse/ParseHLSLRootSignature.cpp (+10)
- (modified) clang/lib/Sema/SemaHLSL.cpp (+37-2)
- (modified) clang/test/SemaHLSL/RootSignature-err.hlsl (+6-2)
- (modified) clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl (+25)
- (modified) clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl (+3)
- (modified) clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp (+82-11)
- (modified) llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h (+4)
- (modified) llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp (+15)
``diff
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c934fed2c7462..8bb47e3a4d46d 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -13184,6 +13184,9 @@ def err_hlsl_vk_literal_must_contain_constant:
Error<"the argument to vk::Litera
def err_hlsl_invalid_rootsig_value : Error<"value must be in the range [%0,
%1]">;
def err_hlsl_invalid_rootsig_flag : Error< "invalid flags for version 1.%0">;
+def err_hlsl_invalid_mixed_resources: Error< "sampler and non-sampler resource
mixed in descriptor table">;
+def err_hlsl_appending_onto_unbound: Error<"offset appends to unbounded
descriptor range">;
+def err_hlsl_offset_overflow: Error<"descriptor range offset overflows [%0,
%1]">;
def subst_hlsl_format_ranges: TextSubstitution<
"%select{t|u|b|s}0[%1;%select{%3]|unbounded)}2">;
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp
b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 1af72f8b1c934..7dd0c3e90886b 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -37,8 +37,18 @@ bool RootSignatureParser::parse() {
// Iterate as many RootSignatureElements as possible, until we hit the
// end of the stream
bool HadError = false;
+ bool HasRootFlags = false;
while (!peekExpectedToken(TokenKind::end_of_stream)) {
if (tryConsumeExpectedToken(TokenKind::kw_RootFlags)) {
+ if (HasRootFlags) {
+reportDiag(diag::err_hlsl_rootsig_repeat_param)
+<< TokenKind::kw_RootFlags;
+HadError = true;
+skipUntilExpectedToken(RootElementKeywords);
+continue;
+ }
+ HasRootFlags = true;
+
SourceLocation ElementLoc = getTokenLocation(CurToken);
auto Flags = parseRootFlags();
if (!Flags.has_value()) {
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 1e5ec952c1ecf..4cf08eac6d171 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -1331,12 +1331,47 @@ bool SemaHLSL::handleRootSignatureElements(
std::get_if(&Elem)) {
assert(UnboundClauses.size() == Table->NumClauses &&
"Number of unbound elements must match the number of clauses");
+ bool HasSampler = false;
+ bool HasNonSampler = false;
+ uint32_t Offset = 0;
for (const auto &[Clause, ClauseElem] : UnboundClauses) {
-uint32_t LowerBound(Clause->Reg.Number);
+SourceLocation Loc = RootSigElem.getLocation();
+if (Clause->Type == llvm::dxil::ResourceClass::Sampler)
+ HasSampler = true;
+else
+ HasNonSampler = true;
+
+if (HasSampler && HasNonSampler)
+ Diag(Loc, diag::err_hlsl_invalid_mixed_resources);
+
// Relevant error will have already been reported above and needs to be
-// fixed before we can conduct range analysis, so shortcut error return
+// fixed before we can conduct further analysis, so shortcut error
+// return
if (Clause->NumDescriptors == 0)
return true;
+
+if (Clause->Offset !=
+llvm::hlsl::rootsig::DescriptorTableOffsetAppend) {
+ // Manually specified the offset
+ Offset = Clause->Offset;
+}
+
+uint64_t NextOffset =
+llvm::hlsl::rootsig::nextOffset(Offset, Clause->NumDescriptors);
+
+if (!llvm::hlsl::rootsig::verifyBoundOffset(Offset)) {
+ // Trying to append onto unbound offset
+ Diag(Loc, diag::err_hlsl_appending_onto_unbound);
+} else if (!llvm::hlsl::rootsig::verifyNoOverflowedOffset(NextOffset -
+
[clang] [llvm] [Sema][Parse][HLSL] Implement front-end rootsignature validations (PR #156754)
https://github.com/inbelic updated
https://github.com/llvm/llvm-project/pull/156754
>From 30e3907799ec8d0e11bb4a0b2cf5927931f3b682 Mon Sep 17 00:00:00 2001
From: Finn Plummer
Date: Tue, 2 Sep 2025 14:05:25 -0700
Subject: [PATCH 1/7] [SemaHLSL] add validation of mixed resources
---
clang/include/clang/Basic/DiagnosticSemaKinds.td | 1 +
clang/lib/Sema/SemaHLSL.cpp | 14 ++
clang/test/SemaHLSL/RootSignature-err.hlsl | 6 +-
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c934fed2c7462..279c6b3122fe0 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -13184,6 +13184,7 @@ def err_hlsl_vk_literal_must_contain_constant:
Error<"the argument to vk::Litera
def err_hlsl_invalid_rootsig_value : Error<"value must be in the range [%0,
%1]">;
def err_hlsl_invalid_rootsig_flag : Error< "invalid flags for version 1.%0">;
+def err_hlsl_invalid_mixed_resources: Error< "sampler and non-sampler resource
mixed in descriptor table">;
def subst_hlsl_format_ranges: TextSubstitution<
"%select{t|u|b|s}0[%1;%select{%3]|unbounded)}2">;
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 1e5ec952c1ecf..32b2b4b7688bb 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -1241,6 +1241,8 @@ bool SemaHLSL::handleRootSignatureElements(
<< /*version minor*/ VersionEnum;
};
+ bool HasSampler = false;
+ bool HasNonSampler = false;
// Iterate through the elements and do basic validations
for (const hlsl::RootSignatureElement &RootSigElem : Elements) {
SourceLocation Loc = RootSigElem.getLocation();
@@ -1287,6 +1289,18 @@ bool SemaHLSL::handleRootSignatureElements(
Version, llvm::to_underlying(Clause->Type),
llvm::to_underlying(Clause->Flags)))
ReportFlagError(Loc);
+
+ if (Clause->Type == llvm::dxil::ResourceClass::Sampler)
+HasSampler = true;
+ else
+HasNonSampler = true;
+
+ if (HasSampler && HasNonSampler)
+Diag(Loc, diag::err_hlsl_invalid_mixed_resources);
+} else if (std::holds_alternative(
+ Elem)) {
+ HasSampler = false;
+ HasNonSampler = false;
}
}
diff --git a/clang/test/SemaHLSL/RootSignature-err.hlsl
b/clang/test/SemaHLSL/RootSignature-err.hlsl
index ccfa093baeb87..1a03e68bb36cd 100644
--- a/clang/test/SemaHLSL/RootSignature-err.hlsl
+++ b/clang/test/SemaHLSL/RootSignature-err.hlsl
@@ -189,4 +189,8 @@ void basic_validation_5() {}
// expected-error@+1 {{value must be in the range [-16.00, 15.99]}}
[RootSignature("StaticSampler(s0, mipLODBias = 15.990001)")]
-void basic_validation_6() {}
\ No newline at end of file
+void basic_validation_6() {}
+
+// expected-error@+1 {{sampler and non-sampler resource mixed in descriptor
table}}
+[RootSignature("DescriptorTable(Sampler(s0), CBV(b0))")]
+void mixed_resource_table() {}
>From 06930216082ade0a6c2c9fd71ce0571b2bc5ec7c Mon Sep 17 00:00:00 2001
From: Finn Plummer
Date: Tue, 2 Sep 2025 14:14:35 -0700
Subject: [PATCH 2/7] correct invalid descriptor tables in example
---
clang/test/SemaHLSL/RootSignature-err.hlsl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/SemaHLSL/RootSignature-err.hlsl
b/clang/test/SemaHLSL/RootSignature-err.hlsl
index 1a03e68bb36cd..89c684cd8d11f 100644
--- a/clang/test/SemaHLSL/RootSignature-err.hlsl
+++ b/clang/test/SemaHLSL/RootSignature-err.hlsl
@@ -179,7 +179,7 @@ void basic_validation_3() {}
// expected-error@+2 {{value must be in the range [1, 4294967294]}}
// expected-error@+1 {{value must be in the range [1, 4294967294]}}
-[RootSignature("DescriptorTable(UAV(u0, numDescriptors = 0), Sampler(s0,
numDescriptors = 0))")]
+[RootSignature("DescriptorTable(UAV(u0, numDescriptors = 0)),
DescriptorTable(Sampler(s0, numDescriptors = 0))")]
void basic_validation_4() {}
// expected-error@+2 {{value must be in the range [0, 16]}}
>From 5cc3365c93fe172450f27d275b2389aff254fd2f Mon Sep 17 00:00:00 2001
From: Finn Plummer
Date: Tue, 2 Sep 2025 14:45:39 -0700
Subject: [PATCH 3/7] [ParseHLSL] add error for multiple root flags
---
clang/lib/Parse/ParseHLSLRootSignature.cpp| 10
.../Parse/ParseHLSLRootSignatureTest.cpp | 23 +++
2 files changed, 33 insertions(+)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp
b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 1af72f8b1c934..7dd0c3e90886b 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -37,8 +37,18 @@ bool RootSignatureParser::parse() {
// Iterate as many RootSignatureElements as possible, until we hit the
// end of the stream
bool HadError = false;
+ bool HasRootFlags = false;
while (!peekExpectedToken(TokenKind::end_
[clang] [llvm] [Sema][Parse][HLSL] Implement front-end rootsignature validations (PR #156754)
https://github.com/inbelic edited https://github.com/llvm/llvm-project/pull/156754 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Sema][Parse][HLSL] Implement front-end rootsignature validations (PR #156754)
https://github.com/inbelic created
https://github.com/llvm/llvm-project/pull/156754
This pr implements the following validations:
1. Check that descriptor tables don't mix Sample and non-Sampler resources
2. Ensure that descriptor ranges don't append onto an unbounded range
3. Ensure that descriptor ranges don't overflow
4. Ensure that only a single `RootFlags` parameter is provided
Resolves: https://github.com/llvm/llvm-project/issues/153868.
>From 30e3907799ec8d0e11bb4a0b2cf5927931f3b682 Mon Sep 17 00:00:00 2001
From: Finn Plummer
Date: Tue, 2 Sep 2025 14:05:25 -0700
Subject: [PATCH 1/6] [SemaHLSL] add validation of mixed resources
---
clang/include/clang/Basic/DiagnosticSemaKinds.td | 1 +
clang/lib/Sema/SemaHLSL.cpp | 14 ++
clang/test/SemaHLSL/RootSignature-err.hlsl | 6 +-
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c934fed2c7462..279c6b3122fe0 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -13184,6 +13184,7 @@ def err_hlsl_vk_literal_must_contain_constant:
Error<"the argument to vk::Litera
def err_hlsl_invalid_rootsig_value : Error<"value must be in the range [%0,
%1]">;
def err_hlsl_invalid_rootsig_flag : Error< "invalid flags for version 1.%0">;
+def err_hlsl_invalid_mixed_resources: Error< "sampler and non-sampler resource
mixed in descriptor table">;
def subst_hlsl_format_ranges: TextSubstitution<
"%select{t|u|b|s}0[%1;%select{%3]|unbounded)}2">;
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 1e5ec952c1ecf..32b2b4b7688bb 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -1241,6 +1241,8 @@ bool SemaHLSL::handleRootSignatureElements(
<< /*version minor*/ VersionEnum;
};
+ bool HasSampler = false;
+ bool HasNonSampler = false;
// Iterate through the elements and do basic validations
for (const hlsl::RootSignatureElement &RootSigElem : Elements) {
SourceLocation Loc = RootSigElem.getLocation();
@@ -1287,6 +1289,18 @@ bool SemaHLSL::handleRootSignatureElements(
Version, llvm::to_underlying(Clause->Type),
llvm::to_underlying(Clause->Flags)))
ReportFlagError(Loc);
+
+ if (Clause->Type == llvm::dxil::ResourceClass::Sampler)
+HasSampler = true;
+ else
+HasNonSampler = true;
+
+ if (HasSampler && HasNonSampler)
+Diag(Loc, diag::err_hlsl_invalid_mixed_resources);
+} else if (std::holds_alternative(
+ Elem)) {
+ HasSampler = false;
+ HasNonSampler = false;
}
}
diff --git a/clang/test/SemaHLSL/RootSignature-err.hlsl
b/clang/test/SemaHLSL/RootSignature-err.hlsl
index ccfa093baeb87..1a03e68bb36cd 100644
--- a/clang/test/SemaHLSL/RootSignature-err.hlsl
+++ b/clang/test/SemaHLSL/RootSignature-err.hlsl
@@ -189,4 +189,8 @@ void basic_validation_5() {}
// expected-error@+1 {{value must be in the range [-16.00, 15.99]}}
[RootSignature("StaticSampler(s0, mipLODBias = 15.990001)")]
-void basic_validation_6() {}
\ No newline at end of file
+void basic_validation_6() {}
+
+// expected-error@+1 {{sampler and non-sampler resource mixed in descriptor
table}}
+[RootSignature("DescriptorTable(Sampler(s0), CBV(b0))")]
+void mixed_resource_table() {}
>From 06930216082ade0a6c2c9fd71ce0571b2bc5ec7c Mon Sep 17 00:00:00 2001
From: Finn Plummer
Date: Tue, 2 Sep 2025 14:14:35 -0700
Subject: [PATCH 2/6] correct invalid descriptor tables in example
---
clang/test/SemaHLSL/RootSignature-err.hlsl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/SemaHLSL/RootSignature-err.hlsl
b/clang/test/SemaHLSL/RootSignature-err.hlsl
index 1a03e68bb36cd..89c684cd8d11f 100644
--- a/clang/test/SemaHLSL/RootSignature-err.hlsl
+++ b/clang/test/SemaHLSL/RootSignature-err.hlsl
@@ -179,7 +179,7 @@ void basic_validation_3() {}
// expected-error@+2 {{value must be in the range [1, 4294967294]}}
// expected-error@+1 {{value must be in the range [1, 4294967294]}}
-[RootSignature("DescriptorTable(UAV(u0, numDescriptors = 0), Sampler(s0,
numDescriptors = 0))")]
+[RootSignature("DescriptorTable(UAV(u0, numDescriptors = 0)),
DescriptorTable(Sampler(s0, numDescriptors = 0))")]
void basic_validation_4() {}
// expected-error@+2 {{value must be in the range [0, 16]}}
>From 5cc3365c93fe172450f27d275b2389aff254fd2f Mon Sep 17 00:00:00 2001
From: Finn Plummer
Date: Tue, 2 Sep 2025 14:45:39 -0700
Subject: [PATCH 3/6] [ParseHLSL] add error for multiple root flags
---
clang/lib/Parse/ParseHLSLRootSignature.cpp| 10
.../Parse/ParseHLSLRootSignatureTest.cpp | 23 +++
2 files changed, 33 insertions(+)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp
b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 1af72f8b1c934..7dd0c
