[HLSL][RootSignature] Implement validation of resource ranges for RootDescriptors
#140962
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-hlsl Author: Finn Plummer (inbelic) ChangesAs was established previously, we created a structure to model a resource range and to detect an overlap in a given set of these. However, a resource range only overlaps with another resource range if they have:
For instance, the following don't overlap even though they have the same register range:
The first two clauses are naturally modelled by a mapping, This mapping is defined in the structure of
Part 2 of #129942 Full diff: https://github.com/llvm/llvm-project/pull/140962.diff 6 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f0bd5a1174020..b1026e733ec37 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12967,6 +12967,11 @@ def err_hlsl_expect_arg_const_int_one_or_neg_one: Error<
def err_invalid_hlsl_resource_type: Error<
"invalid __hlsl_resource_t type attributes">;
+def err_hlsl_resource_range_overlap: Error<
+ "resource ranges %select{t|u|b|s}0[%1;%2] and %select{t|u|b|s}3[%4;%5] "
+ "overlap within space = %6 and visibility = "
+ "%select{All|Vertex|Hull|Domain|Geometry|Pixel|Amplification|Mesh}7">;
+
// Layout randomization diagnostics.
def err_non_designated_init_used : Error<
"a randomized struct can only be initialized with a designated initializer">;
diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h
index 15182bb27bbdf..a161236d8baa0 100644
--- a/clang/include/clang/Sema/SemaHLSL.h
+++ b/clang/include/clang/Sema/SemaHLSL.h
@@ -119,6 +119,8 @@ class SemaHLSL : public SemaBase {
bool IsCompAssign);
void emitLogicalOperatorFixIt(Expr *LHS, Expr *RHS, BinaryOperatorKind Opc);
+ // Returns true when D is invalid and a diagnostic was produced
+ bool handleRootSignatureDecl(HLSLRootSignatureDecl *D, SourceLocation Loc);
void handleRootSignatureAttr(Decl *D, const ParsedAttr &AL);
void handleNumThreadsAttr(Decl *D, const ParsedAttr &AL);
void handleWaveSizeAttr(Decl *D, const ParsedAttr &AL);
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index e6daa67fcee95..cc53f25c766b0 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -39,6 +39,7 @@
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Twine.h"
+#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/DXILABI.h"
#include "llvm/Support/ErrorHandling.h"
@@ -951,6 +952,101 @@ void SemaHLSL::emitLogicalOperatorFixIt(Expr *LHS, Expr *RHS,
<< NewFnName << FixItHint::CreateReplacement(FullRange, OS.str());
}
+namespace {
+
+// A resource range overlaps with another resource range if they have:
+// - equivalent ResourceClass (SRV, UAV, CBuffer, Sampler)
+// - equivalent resource space
+// - overlapping visbility
+class ResourceRanges {
+public:
+ // KeyT: 32-lsb denotes resource space, and 32-msb denotes resource type enum
+ using KeyT = uint64_t;
+
+ constexpr static KeyT getKey(const llvm::hlsl::rootsig::RangeInfo &Info) {
+ uint64_t SpacePacked = (uint64_t)Info.Space;
+ uint64_t ClassPacked = (uint64_t)llvm::to_underlying(Info.Class);
+ return (ClassPacked << 32) | SpacePacked;
+ }
+
+ static const unsigned NumVisEnums = 8;
+ // (unsigned)llvm::hlsl::rootsig::ShaderVisibility::NumEnums;
+
+private:
+ llvm::hlsl::rootsig::ResourceRange::IMap::Allocator Allocator;
+
+ using MapT = llvm::SmallDenseMap<KeyT, llvm::hlsl::rootsig::ResourceRange>;
+
+ MapT RangeMaps[NumVisEnums];
+
+public:
+ // Returns std::nullopt if there was no collision. Otherwise, it will
+ // return the RangeInfo of the collision
+ std::optional<const llvm::hlsl::rootsig::RangeInfo *> addRange(const llvm::hlsl::rootsig::RangeInfo &Info) {
+ MapT &VisRangeMap = RangeMaps[llvm::to_underlying(Info.Vis)];
+ auto [It, _] = VisRangeMap.insert({getKey(Info), llvm::hlsl::rootsig::ResourceRange(Allocator)});
+ auto Res = It->second.insert(Info);
+ if (Res.has_value())
+ return Res;
+
+ MutableArrayRef<MapT> Maps =
+ Info.Vis == llvm::hlsl::rootsig::ShaderVisibility::All
+ ? MutableArrayRef<MapT>{RangeMaps}.drop_front()
+ : MutableArrayRef<MapT>{RangeMaps}.take_front();
+
+ for (MapT &CurMap : Maps) {
+ auto CurIt = CurMap.find(getKey(Info));
+ if (CurIt != CurMap.end())
+ if (auto Overlapping = CurIt->second.getOverlapping(Info))
+ return Overlapping;
+ }
+
+ return std::nullopt;
+ }
+};
+
+} // namespace
+
+bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D,
+ SourceLocation Loc) {
+ auto Elements = D->getRootElements();
+
+ // First we will go through and collect our range info
+ llvm::SmallVector<llvm::hlsl::rootsig::RangeInfo> Infos;
+ for (const auto &Elem : Elements) {
+ if (const auto *Param =
+ std::get_if<llvm::hlsl::rootsig::RootParam>(&Elem)) {
+ llvm::hlsl::rootsig::RangeInfo Info;
+ Info.LowerBound = Param->Reg.Number;
+ Info.UpperBound = Info.LowerBound; // use inclusive ranges []
+
+ Info.Class = Param->Type;
+ Info.Space = Param->Space;
+ Info.Vis = Param->Visibility;
+ Infos.push_back(Info);
+ }
+ }
+
+ // Iterate through info and attempt to insert corresponding range
+ ResourceRanges Ranges;
+ bool HadOverlap = false;
+ for (const llvm::hlsl::rootsig::RangeInfo &Info : Infos)
+ if (auto MaybeOverlappingInfo = Ranges.addRange(Info)) {
+ const llvm::hlsl::rootsig::RangeInfo *OInfo = MaybeOverlappingInfo.value();
+ auto CommonVis = Info.Vis == llvm::hlsl::rootsig::ShaderVisibility::All
+ ? OInfo->Vis : Info.Vis;
+
+ Diag(Loc, diag::err_hlsl_resource_range_overlap)
+ << llvm::to_underlying(Info.Class) << Info.LowerBound << Info.UpperBound
+ << llvm::to_underlying(OInfo->Class) << OInfo->LowerBound << OInfo->UpperBound
+ << Info.Space << CommonVis;
+
+ HadOverlap = true;
+ }
+
+ return HadOverlap;
+}
+
void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) {
if (AL.getNumArgs() != 1) {
Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << AL << 1;
@@ -973,6 +1069,8 @@ void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) {
if (auto *SignatureDecl =
dyn_cast<HLSLRootSignatureDecl>(R.getFoundDecl())) {
// Perform validation of constructs here
+ if (handleRootSignatureDecl(SignatureDecl, AL.getLoc()))
+ return;
D->addAttr(::new (getASTContext()) RootSignatureAttr(
getASTContext(), AL, Ident, SignatureDecl));
}
diff --git a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl
new file mode 100644
index 0000000000000..a4b5b9c77a248
--- /dev/null
+++ b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -verify
+
+#define Overlap0 "CBV(b42), CBV(b42)"
+
+[RootSignature(Overlap0)] // expected-error {{resource ranges b[42;42] and b[42;42] overlap within space = 0 and visibility = All}}
+void bad_root_signature_0() {}
+
+#define Overlap1 "SRV(t0, space = 3), SRV(t0, space = 3)"
+
+[RootSignature(Overlap1)] // expected-error {{resource ranges t[0;0] and t[0;0] overlap within space = 3 and visibility = All}}
+void bad_root_signature_1() {}
+
+#define Overlap2 "UAV(u0, visibility = SHADER_VISIBILITY_PIXEL), UAV(u0, visibility = SHADER_VISIBILITY_PIXEL)"
+
+[RootSignature(Overlap2)] // expected-error {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}}
+void bad_root_signature_2() {}
+
+#define Overlap3 "UAV(u0, visibility = SHADER_VISIBILITY_ALL), UAV(u0, visibility = SHADER_VISIBILITY_PIXEL)"
+
+[RootSignature(Overlap3)] // expected-error {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}}
+void bad_root_signature_3() {}
+
+#define Overlap4 "UAV(u0, visibility = SHADER_VISIBILITY_PIXEL), UAV(u0, visibility = SHADER_VISIBILITY_ALL)"
+
+[RootSignature(Overlap4)] // expected-error {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}}
+void bad_root_signature_4() {}
diff --git a/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl b/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl
new file mode 100644
index 0000000000000..3145b5b332dba
--- /dev/null
+++ b/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -verify
+// expected-no-diagnostics
+
+#define NoOverlap0 "CBV(b0), CBV(b1)"
+
+[RootSignature(NoOverlap0)]
+void valid_root_signature_0() {}
+
+#define NoOverlap1 "CBV(b0, visibility = SHADER_VISIBILITY_DOMAIN), CBV(b0, visibility = SHADER_VISIBILITY_PIXEL)"
+
+[RootSignature(NoOverlap1)]
+void valid_root_signature_1() {}
+
+#define NoOverlap2 "CBV(b0, space = 1), CBV(b0, space = 2)"
+
+[RootSignature(NoOverlap2)]
+void valid_root_signature_2() {}
+
+#define NoOverlap3 "CBV(b0), SRV(t0)"
+
+[RootSignature(NoOverlap3)]
+void valid_root_signature_3() {}
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 3e5054566052b..ec6289e8d858c 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -199,13 +199,17 @@ class MetadataBuilder {
SmallVector<Metadata *> GeneratedMetadata;
};
-// RangeInfo holds the information to correctly construct a ResourceRange
-// and retains this information to be used for displaying a better diagnostic
struct RangeInfo {
const static uint32_t Unbounded = static_cast<uint32_t>(-1);
+ // Interval information
uint32_t LowerBound;
uint32_t UpperBound;
+
+ // Information retained for diagnostics
+ llvm::dxil::ResourceClass Class;
+ uint32_t Space;
+ ShaderVisibility Vis;
};
class ResourceRange {
|
|
@llvm/pr-subscribers-clang Author: Finn Plummer (inbelic) ChangesAs was established previously, we created a structure to model a resource range and to detect an overlap in a given set of these. However, a resource range only overlaps with another resource range if they have:
For instance, the following don't overlap even though they have the same register range:
The first two clauses are naturally modelled by a mapping, This mapping is defined in the structure of
Part 2 of #129942 Full diff: https://github.com/llvm/llvm-project/pull/140962.diff 6 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f0bd5a1174020..b1026e733ec37 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12967,6 +12967,11 @@ def err_hlsl_expect_arg_const_int_one_or_neg_one: Error<
def err_invalid_hlsl_resource_type: Error<
"invalid __hlsl_resource_t type attributes">;
+def err_hlsl_resource_range_overlap: Error<
+ "resource ranges %select{t|u|b|s}0[%1;%2] and %select{t|u|b|s}3[%4;%5] "
+ "overlap within space = %6 and visibility = "
+ "%select{All|Vertex|Hull|Domain|Geometry|Pixel|Amplification|Mesh}7">;
+
// Layout randomization diagnostics.
def err_non_designated_init_used : Error<
"a randomized struct can only be initialized with a designated initializer">;
diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h
index 15182bb27bbdf..a161236d8baa0 100644
--- a/clang/include/clang/Sema/SemaHLSL.h
+++ b/clang/include/clang/Sema/SemaHLSL.h
@@ -119,6 +119,8 @@ class SemaHLSL : public SemaBase {
bool IsCompAssign);
void emitLogicalOperatorFixIt(Expr *LHS, Expr *RHS, BinaryOperatorKind Opc);
+ // Returns true when D is invalid and a diagnostic was produced
+ bool handleRootSignatureDecl(HLSLRootSignatureDecl *D, SourceLocation Loc);
void handleRootSignatureAttr(Decl *D, const ParsedAttr &AL);
void handleNumThreadsAttr(Decl *D, const ParsedAttr &AL);
void handleWaveSizeAttr(Decl *D, const ParsedAttr &AL);
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index e6daa67fcee95..cc53f25c766b0 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -39,6 +39,7 @@
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Twine.h"
+#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/DXILABI.h"
#include "llvm/Support/ErrorHandling.h"
@@ -951,6 +952,101 @@ void SemaHLSL::emitLogicalOperatorFixIt(Expr *LHS, Expr *RHS,
<< NewFnName << FixItHint::CreateReplacement(FullRange, OS.str());
}
+namespace {
+
+// A resource range overlaps with another resource range if they have:
+// - equivalent ResourceClass (SRV, UAV, CBuffer, Sampler)
+// - equivalent resource space
+// - overlapping visbility
+class ResourceRanges {
+public:
+ // KeyT: 32-lsb denotes resource space, and 32-msb denotes resource type enum
+ using KeyT = uint64_t;
+
+ constexpr static KeyT getKey(const llvm::hlsl::rootsig::RangeInfo &Info) {
+ uint64_t SpacePacked = (uint64_t)Info.Space;
+ uint64_t ClassPacked = (uint64_t)llvm::to_underlying(Info.Class);
+ return (ClassPacked << 32) | SpacePacked;
+ }
+
+ static const unsigned NumVisEnums = 8;
+ // (unsigned)llvm::hlsl::rootsig::ShaderVisibility::NumEnums;
+
+private:
+ llvm::hlsl::rootsig::ResourceRange::IMap::Allocator Allocator;
+
+ using MapT = llvm::SmallDenseMap<KeyT, llvm::hlsl::rootsig::ResourceRange>;
+
+ MapT RangeMaps[NumVisEnums];
+
+public:
+ // Returns std::nullopt if there was no collision. Otherwise, it will
+ // return the RangeInfo of the collision
+ std::optional<const llvm::hlsl::rootsig::RangeInfo *> addRange(const llvm::hlsl::rootsig::RangeInfo &Info) {
+ MapT &VisRangeMap = RangeMaps[llvm::to_underlying(Info.Vis)];
+ auto [It, _] = VisRangeMap.insert({getKey(Info), llvm::hlsl::rootsig::ResourceRange(Allocator)});
+ auto Res = It->second.insert(Info);
+ if (Res.has_value())
+ return Res;
+
+ MutableArrayRef<MapT> Maps =
+ Info.Vis == llvm::hlsl::rootsig::ShaderVisibility::All
+ ? MutableArrayRef<MapT>{RangeMaps}.drop_front()
+ : MutableArrayRef<MapT>{RangeMaps}.take_front();
+
+ for (MapT &CurMap : Maps) {
+ auto CurIt = CurMap.find(getKey(Info));
+ if (CurIt != CurMap.end())
+ if (auto Overlapping = CurIt->second.getOverlapping(Info))
+ return Overlapping;
+ }
+
+ return std::nullopt;
+ }
+};
+
+} // namespace
+
+bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D,
+ SourceLocation Loc) {
+ auto Elements = D->getRootElements();
+
+ // First we will go through and collect our range info
+ llvm::SmallVector<llvm::hlsl::rootsig::RangeInfo> Infos;
+ for (const auto &Elem : Elements) {
+ if (const auto *Param =
+ std::get_if<llvm::hlsl::rootsig::RootParam>(&Elem)) {
+ llvm::hlsl::rootsig::RangeInfo Info;
+ Info.LowerBound = Param->Reg.Number;
+ Info.UpperBound = Info.LowerBound; // use inclusive ranges []
+
+ Info.Class = Param->Type;
+ Info.Space = Param->Space;
+ Info.Vis = Param->Visibility;
+ Infos.push_back(Info);
+ }
+ }
+
+ // Iterate through info and attempt to insert corresponding range
+ ResourceRanges Ranges;
+ bool HadOverlap = false;
+ for (const llvm::hlsl::rootsig::RangeInfo &Info : Infos)
+ if (auto MaybeOverlappingInfo = Ranges.addRange(Info)) {
+ const llvm::hlsl::rootsig::RangeInfo *OInfo = MaybeOverlappingInfo.value();
+ auto CommonVis = Info.Vis == llvm::hlsl::rootsig::ShaderVisibility::All
+ ? OInfo->Vis : Info.Vis;
+
+ Diag(Loc, diag::err_hlsl_resource_range_overlap)
+ << llvm::to_underlying(Info.Class) << Info.LowerBound << Info.UpperBound
+ << llvm::to_underlying(OInfo->Class) << OInfo->LowerBound << OInfo->UpperBound
+ << Info.Space << CommonVis;
+
+ HadOverlap = true;
+ }
+
+ return HadOverlap;
+}
+
void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) {
if (AL.getNumArgs() != 1) {
Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << AL << 1;
@@ -973,6 +1069,8 @@ void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) {
if (auto *SignatureDecl =
dyn_cast<HLSLRootSignatureDecl>(R.getFoundDecl())) {
// Perform validation of constructs here
+ if (handleRootSignatureDecl(SignatureDecl, AL.getLoc()))
+ return;
D->addAttr(::new (getASTContext()) RootSignatureAttr(
getASTContext(), AL, Ident, SignatureDecl));
}
diff --git a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl
new file mode 100644
index 0000000000000..a4b5b9c77a248
--- /dev/null
+++ b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -verify
+
+#define Overlap0 "CBV(b42), CBV(b42)"
+
+[RootSignature(Overlap0)] // expected-error {{resource ranges b[42;42] and b[42;42] overlap within space = 0 and visibility = All}}
+void bad_root_signature_0() {}
+
+#define Overlap1 "SRV(t0, space = 3), SRV(t0, space = 3)"
+
+[RootSignature(Overlap1)] // expected-error {{resource ranges t[0;0] and t[0;0] overlap within space = 3 and visibility = All}}
+void bad_root_signature_1() {}
+
+#define Overlap2 "UAV(u0, visibility = SHADER_VISIBILITY_PIXEL), UAV(u0, visibility = SHADER_VISIBILITY_PIXEL)"
+
+[RootSignature(Overlap2)] // expected-error {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}}
+void bad_root_signature_2() {}
+
+#define Overlap3 "UAV(u0, visibility = SHADER_VISIBILITY_ALL), UAV(u0, visibility = SHADER_VISIBILITY_PIXEL)"
+
+[RootSignature(Overlap3)] // expected-error {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}}
+void bad_root_signature_3() {}
+
+#define Overlap4 "UAV(u0, visibility = SHADER_VISIBILITY_PIXEL), UAV(u0, visibility = SHADER_VISIBILITY_ALL)"
+
+[RootSignature(Overlap4)] // expected-error {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}}
+void bad_root_signature_4() {}
diff --git a/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl b/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl
new file mode 100644
index 0000000000000..3145b5b332dba
--- /dev/null
+++ b/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -verify
+// expected-no-diagnostics
+
+#define NoOverlap0 "CBV(b0), CBV(b1)"
+
+[RootSignature(NoOverlap0)]
+void valid_root_signature_0() {}
+
+#define NoOverlap1 "CBV(b0, visibility = SHADER_VISIBILITY_DOMAIN), CBV(b0, visibility = SHADER_VISIBILITY_PIXEL)"
+
+[RootSignature(NoOverlap1)]
+void valid_root_signature_1() {}
+
+#define NoOverlap2 "CBV(b0, space = 1), CBV(b0, space = 2)"
+
+[RootSignature(NoOverlap2)]
+void valid_root_signature_2() {}
+
+#define NoOverlap3 "CBV(b0), SRV(t0)"
+
+[RootSignature(NoOverlap3)]
+void valid_root_signature_3() {}
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 3e5054566052b..ec6289e8d858c 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -199,13 +199,17 @@ class MetadataBuilder {
SmallVector<Metadata *> GeneratedMetadata;
};
-// RangeInfo holds the information to correctly construct a ResourceRange
-// and retains this information to be used for displaying a better diagnostic
struct RangeInfo {
const static uint32_t Unbounded = static_cast<uint32_t>(-1);
+ // Interval information
uint32_t LowerBound;
uint32_t UpperBound;
+
+ // Information retained for diagnostics
+ llvm::dxil::ResourceClass Class;
+ uint32_t Space;
+ ShaderVisibility Vis;
};
class ResourceRange {
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
inbelic
force-pushed
the
inbelic/rs-verify-ranges
branch
from
630dae0 to
bcc056e
Compare
Icohedron
reviewed
May 26, 2025
6 tasks
inbelic
commented
May 29, 2025
inbelic
added a commit
that referenced
this pull request
Jun 17, 2025
…140957) A resource range consists of a closed interval, `[a;b]`, denoting which shader registers it is bound to. For instance: - `CBV(b1)` corresponds to the resource range of `[1;1]` - `CBV(b0, numDescriptors = 3)` likewise to `[0;2]` We want to provide an error diagnostic when there is an overlap in the required registers (an overlap in the resource ranges). The goal of this pr is to implement a structure to model a set of resource ranges and provide an api to detect any overlap over a set of resource ranges. `ResourceRange` models this by implementing an `IntervalMap` to denote a mapping from an interval of registers back to a resource range. It allows for a new `ResourceRange` to be added to the mapping and it will report if and what the first overlap is. For the context of how this will be used in validation of a `RootSignatureDecl` please see the proceeding pull request here: #140962. - Implements `ResourceRange` as an `IntervalMap` - Adds unit testing of the various `insert` scenarios Note: it was also considered to implement this as an `IntervalTree`, this would allow reporting of a diagnostic for each overlap that is encountered, as opposed to just the first. However, error generation of just reporting the first error is already rather verbose, and adding the additional diagnostics only made this worse. Part 1 of #129942
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jun 17, 2025
…ervalMap` (#140957) A resource range consists of a closed interval, `[a;b]`, denoting which shader registers it is bound to. For instance: - `CBV(b1)` corresponds to the resource range of `[1;1]` - `CBV(b0, numDescriptors = 3)` likewise to `[0;2]` We want to provide an error diagnostic when there is an overlap in the required registers (an overlap in the resource ranges). The goal of this pr is to implement a structure to model a set of resource ranges and provide an api to detect any overlap over a set of resource ranges. `ResourceRange` models this by implementing an `IntervalMap` to denote a mapping from an interval of registers back to a resource range. It allows for a new `ResourceRange` to be added to the mapping and it will report if and what the first overlap is. For the context of how this will be used in validation of a `RootSignatureDecl` please see the proceeding pull request here: llvm/llvm-project#140962. - Implements `ResourceRange` as an `IntervalMap` - Adds unit testing of the various `insert` scenarios Note: it was also considered to implement this as an `IntervalTree`, this would allow reporting of a diagnostic for each overlap that is encountered, as opposed to just the first. However, error generation of just reporting the first error is already rather verbose, and adding the additional diagnostics only made this worse. Part 1 of llvm/llvm-project#129942
fschlimb
pushed a commit
to fschlimb/llvm-project
that referenced
this pull request
Jun 18, 2025
…lvm#140957) A resource range consists of a closed interval, `[a;b]`, denoting which shader registers it is bound to. For instance: - `CBV(b1)` corresponds to the resource range of `[1;1]` - `CBV(b0, numDescriptors = 3)` likewise to `[0;2]` We want to provide an error diagnostic when there is an overlap in the required registers (an overlap in the resource ranges). The goal of this pr is to implement a structure to model a set of resource ranges and provide an api to detect any overlap over a set of resource ranges. `ResourceRange` models this by implementing an `IntervalMap` to denote a mapping from an interval of registers back to a resource range. It allows for a new `ResourceRange` to be added to the mapping and it will report if and what the first overlap is. For the context of how this will be used in validation of a `RootSignatureDecl` please see the proceeding pull request here: llvm#140962. - Implements `ResourceRange` as an `IntervalMap` - Adds unit testing of the various `insert` scenarios Note: it was also considered to implement this as an `IntervalTree`, this would allow reporting of a diagnostic for each overlap that is encountered, as opposed to just the first. However, error generation of just reporting the first error is already rather verbose, and adding the additional diagnostics only made this worse. Part 1 of llvm#129942
- this will improve readability and efficiency as we will be able to free all ResourceRanges after they are used
inbelic
force-pushed
the
inbelic/rs-verify-ranges
branch
from
5c25e86 to
75402c3
Compare
inbelic
commented
Jun 19, 2025
bogner
reviewed
Jun 19, 2025
bogner
approved these changes
Jun 20, 2025
Icohedron
approved these changes
Jun 20, 2025
6 tasks
Jaddyen
pushed a commit
to Jaddyen/llvm-project
that referenced
this pull request
Jun 23, 2025
…otDescriptors` (llvm#140962) As was established [previously](llvm#140957), we created a structure to model a resource range and to detect an overlap in a given set of these. However, a resource range only overlaps with another resource range if they have: - equivalent ResourceClass (SRV, UAV, CBuffer, Sampler) - equivalent resource name-space - overlapping shader visibility For instance, the following don't overlap even though they have the same register range: - `CBV(b0)` and `SRV(t0)` (different resource class) - `CBV(b0, space = 0)` and `CBV(b0, space = 1)` (different space) - `CBV(b0, visibility = Pixel)` and `CBV(b0, visibility = Domain)` (non-overlapping visibility) The first two clauses are naturally modelled by grouping all the `RangeInfo`s that have the equivalent `ResourceClass` and `Space` values together and check if there is any overlap on a `ResourceRange` for all these `RangeInfo`s. However, `Visibility` is not quite as easily mapped (`Visibility = All` would overlap with any other visibility). So we will instead need to track a `ResourceRange` for each of the `Visibility` types in a group. Then we can determine when inserting a range of the same group if it would overlap with any overlapping visibilities. The collection of `RangeInfo` for `RootDescriptor`s, sorting of the `RangeInfo`s into the groups and finally the insertion of each point into their respective `ResourceRange`s are implemented. Furthermore, we integrate this into `SemaHLSL` to provide a diagnostic for each entry function that uses the invalid root signature. - Implements collection of `RangeInfo` for `RootDescriptors` - Implements resource range validation in `SemaHLSL` - Add diagnostic testing of error production in `RootSignature-resource-ranges-err.hlsl` - Add testing to ensure no errors are raised in valid root signatures `RootSignature-resource-ranges.hlsl` Part 2 of llvm#129942 A final pr will be produced to integrate the analysis of `DescriptorTable`, `StaticSampler` and `RootConstants` by defining how to construct the `RangeInfo` from their elements respectively.
inbelic
added a commit
that referenced
this pull request
Jun 24, 2025
…ootElement`s (#145109) As implemented previously #140962. We now have a validation pass to ensure that there is no overlap in the register ranges of the associated resources. However, in the previous pr, for the sake of brevity, we only "collected `RangeInfo`" for Root Descriptors. This means the analysis is not run on any other `RootElement` type. This pr simply implements the collection of `RangeInfo` for the remaining types so that the analysis is run account for all `RootElement` types. Additionally, we improve the diagnostics message to display `unbounded` ranges. Part 3 of and Resolves #129942.
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jun 24, 2025
…emaining `RootElement`s (#145109) As implemented previously llvm/llvm-project#140962. We now have a validation pass to ensure that there is no overlap in the register ranges of the associated resources. However, in the previous pr, for the sake of brevity, we only "collected `RangeInfo`" for Root Descriptors. This means the analysis is not run on any other `RootElement` type. This pr simply implements the collection of `RangeInfo` for the remaining types so that the analysis is run account for all `RootElement` types. Additionally, we improve the diagnostics message to display `unbounded` ranges. Part 3 of and Resolves llvm/llvm-project#129942.
DrSergei
pushed a commit
to DrSergei/llvm-project
that referenced
this pull request
Jun 24, 2025
…ootElement`s (llvm#145109) As implemented previously llvm#140962. We now have a validation pass to ensure that there is no overlap in the register ranges of the associated resources. However, in the previous pr, for the sake of brevity, we only "collected `RangeInfo`" for Root Descriptors. This means the analysis is not run on any other `RootElement` type. This pr simply implements the collection of `RangeInfo` for the remaining types so that the analysis is run account for all `RootElement` types. Additionally, we improve the diagnostics message to display `unbounded` ranges. Part 3 of and Resolves llvm#129942.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As was established previously, we created a structure to model a resource range and to detect an overlap in a given set of these.
However, a resource range only overlaps with another resource range if they have:
For instance, the following don't overlap even though they have the same register range:
CBV(b0)andSRV(t0)(different resource class)CBV(b0, space = 0)andCBV(b0, space = 1)(different space)CBV(b0, visibility = Pixel)andCBV(b0, visibility = Domain)(non-overlapping visibility)The first two clauses are naturally modelled by grouping all the
RangeInfos that have the equivalentResourceClassandSpacevalues together and check if there is any overlap on aResourceRangefor all theseRangeInfos. However,Visibilityis not quite as easily mapped (Visibility = Allwould overlap with any other visibility). So we will instead need to track aResourceRangefor each of theVisibilitytypes in a group. Then we can determine when inserting a range of the same group if it would overlap with any overlapping visibilities.The collection of
RangeInfoforRootDescriptors, sorting of theRangeInfos into the groups and finally the insertion of each point into their respectiveResourceRanges are implemented. Furthermore, we integrate this intoSemaHLSLto provide a diagnostic for each entry function that uses the invalid root signature.RangeInfoforRootDescriptorsSemaHLSLRootSignature-resource-ranges-err.hlslRootSignature-resource-ranges.hlslPart 2 of #129942
A final pr will be produced to integrate the analysis of
DescriptorTable,StaticSamplerandRootConstantsby defining how to construct theRangeInfofrom their elements respectively.