Skip to content

Commit e6ee2c7

Browse files
authored
[HLSL][RootSignature] Implement validation of resource ranges for RootDescriptors (#140962)
As was established [previously](#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 #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.
1 parent a911543 commit e6ee2c7

File tree

7 files changed

+183
-3
lines changed

7 files changed

+183
-3
lines changed

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13054,6 +13054,11 @@ def err_invalid_hlsl_resource_type: Error<
1305413054
def err_hlsl_spirv_only: Error<"%0 is only available for the SPIR-V target">;
1305513055
def err_hlsl_vk_literal_must_contain_constant: Error<"the argument to vk::Literal must be a vk::integral_constant">;
1305613056

13057+
def err_hlsl_resource_range_overlap: Error<
13058+
"resource ranges %select{t|u|b|s}0[%1;%2] and %select{t|u|b|s}3[%4;%5] "
13059+
"overlap within space = %6 and visibility = "
13060+
"%select{All|Vertex|Hull|Domain|Geometry|Pixel|Amplification|Mesh}7">;
13061+
1305713062
// Layout randomization diagnostics.
1305813063
def err_non_designated_init_used : Error<
1305913064
"a randomized struct can only be initialized with a designated initializer">;

clang/include/clang/Sema/SemaHLSL.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,8 @@ class SemaHLSL : public SemaBase {
134134
SourceLocation Loc, IdentifierInfo *DeclIdent,
135135
SmallVector<llvm::hlsl::rootsig::RootElement> &Elements);
136136

137+
// Returns true when D is invalid and a diagnostic was produced
138+
bool handleRootSignatureDecl(HLSLRootSignatureDecl *D, SourceLocation Loc);
137139
void handleRootSignatureAttr(Decl *D, const ParsedAttr &AL);
138140
void handleNumThreadsAttr(Decl *D, const ParsedAttr &AL);
139141
void handleWaveSizeAttr(Decl *D, const ParsedAttr &AL);

clang/lib/Sema/SemaHLSL.cpp

Lines changed: 129 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
#include "llvm/ADT/StringExtras.h"
4040
#include "llvm/ADT/StringRef.h"
4141
#include "llvm/ADT/Twine.h"
42+
#include "llvm/Frontend/HLSL/HLSLRootSignatureUtils.h"
4243
#include "llvm/Support/Casting.h"
4344
#include "llvm/Support/DXILABI.h"
4445
#include "llvm/Support/ErrorHandling.h"
@@ -1068,10 +1069,138 @@ void SemaHLSL::ActOnFinishRootSignatureDecl(
10681069
SemaRef.getASTContext(), /*DeclContext=*/SemaRef.CurContext, Loc,
10691070
DeclIdent, Elements);
10701071

1072+
if (handleRootSignatureDecl(SignatureDecl, Loc))
1073+
return;
1074+
10711075
SignatureDecl->setImplicit();
10721076
SemaRef.PushOnScopeChains(SignatureDecl, SemaRef.getCurScope());
10731077
}
10741078

1079+
bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D,
1080+
SourceLocation Loc) {
1081+
// The following conducts analysis on resource ranges to detect and report
1082+
// any overlaps in resource ranges.
1083+
//
1084+
// A resource range overlaps with another resource range if they have:
1085+
// - equivalent ResourceClass (SRV, UAV, CBuffer, Sampler)
1086+
// - equivalent resource space
1087+
// - overlapping visbility
1088+
//
1089+
// The following algorithm is implemented in the following steps:
1090+
//
1091+
// 1. Collect RangeInfo from relevant RootElements:
1092+
// - RangeInfo will retain the interval, ResourceClass, Space and Visibility
1093+
// 2. Sort the RangeInfo's such that they are grouped together by
1094+
// ResourceClass and Space (GroupT defined below)
1095+
// 3. Iterate through the collected RangeInfos by their groups
1096+
// - For each group we will have a ResourceRange for each visibility
1097+
// - As we iterate through we will:
1098+
// A: Insert the current RangeInfo into the corresponding Visibility
1099+
// ResourceRange
1100+
// B: Check for overlap with any overlapping Visibility ResourceRange
1101+
using RangeInfo = llvm::hlsl::rootsig::RangeInfo;
1102+
using ResourceRange = llvm::hlsl::rootsig::ResourceRange;
1103+
using GroupT = std::pair<ResourceClass, /*Space*/ uint32_t>;
1104+
1105+
// 1. Collect RangeInfos
1106+
llvm::SmallVector<RangeInfo> Infos;
1107+
for (const llvm::hlsl::rootsig::RootElement &Elem : D->getRootElements()) {
1108+
if (const auto *Descriptor =
1109+
std::get_if<llvm::hlsl::rootsig::RootDescriptor>(&Elem)) {
1110+
RangeInfo Info;
1111+
Info.LowerBound = Descriptor->Reg.Number;
1112+
Info.UpperBound = Info.LowerBound; // use inclusive ranges []
1113+
1114+
Info.Class =
1115+
llvm::dxil::ResourceClass(llvm::to_underlying(Descriptor->Type));
1116+
Info.Space = Descriptor->Space;
1117+
Info.Visibility = Descriptor->Visibility;
1118+
Infos.push_back(Info);
1119+
}
1120+
}
1121+
1122+
// 2. Sort the RangeInfo's by their GroupT to form groupings
1123+
std::sort(Infos.begin(), Infos.end(), [](RangeInfo A, RangeInfo B) {
1124+
return std::tie(A.Class, A.Space) < std::tie(B.Class, B.Space);
1125+
});
1126+
1127+
// 3. First we will init our state to track:
1128+
if (Infos.size() == 0)
1129+
return false; // No ranges to overlap
1130+
GroupT CurGroup = {Infos[0].Class, Infos[0].Space};
1131+
bool HadOverlap = false;
1132+
1133+
// Create a ResourceRange for each Visibility
1134+
ResourceRange::MapT::Allocator Allocator;
1135+
std::array<ResourceRange, 8> Ranges = {
1136+
ResourceRange(Allocator), // All
1137+
ResourceRange(Allocator), // Vertex
1138+
ResourceRange(Allocator), // Hull
1139+
ResourceRange(Allocator), // Domain
1140+
ResourceRange(Allocator), // Geometry
1141+
ResourceRange(Allocator), // Pixel
1142+
ResourceRange(Allocator), // Amplification
1143+
ResourceRange(Allocator), // Mesh
1144+
};
1145+
1146+
// Reset the ResourceRanges for when we iterate through a new group
1147+
auto ClearRanges = [&Ranges]() {
1148+
for (ResourceRange &Range : Ranges)
1149+
Range.clear();
1150+
};
1151+
1152+
// Helper to report diagnostics
1153+
auto ReportOverlap = [this, Loc, &HadOverlap](const RangeInfo *Info,
1154+
const RangeInfo *OInfo) {
1155+
HadOverlap = true;
1156+
auto CommonVis =
1157+
Info->Visibility == llvm::hlsl::rootsig::ShaderVisibility::All
1158+
? OInfo->Visibility
1159+
: Info->Visibility;
1160+
this->Diag(Loc, diag::err_hlsl_resource_range_overlap)
1161+
<< llvm::to_underlying(Info->Class) << Info->LowerBound
1162+
<< Info->UpperBound << llvm::to_underlying(OInfo->Class)
1163+
<< OInfo->LowerBound << OInfo->UpperBound << Info->Space << CommonVis;
1164+
};
1165+
1166+
// 3: Iterate through collected RangeInfos
1167+
for (const RangeInfo &Info : Infos) {
1168+
GroupT InfoGroup = {Info.Class, Info.Space};
1169+
// Reset our ResourceRanges when we enter a new group
1170+
if (CurGroup != InfoGroup) {
1171+
ClearRanges();
1172+
CurGroup = InfoGroup;
1173+
}
1174+
1175+
// 3A: Insert range info into corresponding Visibility ResourceRange
1176+
ResourceRange &VisRange = Ranges[llvm::to_underlying(Info.Visibility)];
1177+
if (std::optional<const RangeInfo *> Overlapping = VisRange.insert(Info))
1178+
ReportOverlap(&Info, Overlapping.value());
1179+
1180+
// 3B: Check for overlap in all overlapping Visibility ResourceRanges
1181+
//
1182+
// If the range that we are inserting has ShaderVisiblity::All it needs to
1183+
// check for an overlap in all other visibility types as well.
1184+
// Otherwise, the range that is inserted needs to check that it does not
1185+
// overlap with ShaderVisibility::All.
1186+
//
1187+
// OverlapRanges will be an ArrayRef to all non-all visibility
1188+
// ResourceRanges in the former case and it will be an ArrayRef to just the
1189+
// all visiblity ResourceRange in the latter case.
1190+
ArrayRef<ResourceRange> OverlapRanges =
1191+
Info.Visibility == llvm::hlsl::rootsig::ShaderVisibility::All
1192+
? ArrayRef<ResourceRange>{Ranges}.drop_front()
1193+
: ArrayRef<ResourceRange>{Ranges}.take_front();
1194+
1195+
for (const ResourceRange &Range : OverlapRanges)
1196+
if (std::optional<const RangeInfo *> Overlapping =
1197+
Range.getOverlapping(Info))
1198+
ReportOverlap(&Info, Overlapping.value());
1199+
}
1200+
1201+
return HadOverlap;
1202+
}
1203+
10751204
void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) {
10761205
if (AL.getNumArgs() != 1) {
10771206
Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << AL << 1;
@@ -1093,7 +1222,6 @@ void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) {
10931222
if (SemaRef.LookupQualifiedName(R, D->getDeclContext()))
10941223
if (auto *SignatureDecl =
10951224
dyn_cast<HLSLRootSignatureDecl>(R.getFoundDecl())) {
1096-
// Perform validation of constructs here
10971225
D->addAttr(::new (getASTContext()) RootSignatureAttr(
10981226
getASTContext(), AL, Ident, SignatureDecl));
10991227
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -verify
2+
3+
// expected-error@+1 {{resource ranges b[42;42] and b[42;42] overlap within space = 0 and visibility = All}}
4+
[RootSignature("CBV(b42), CBV(b42)")]
5+
void bad_root_signature_0() {}
6+
7+
// expected-error@+1 {{resource ranges t[0;0] and t[0;0] overlap within space = 3 and visibility = All}}
8+
[RootSignature("SRV(t0, space = 3), SRV(t0, space = 3)")]
9+
void bad_root_signature_1() {}
10+
11+
// expected-error@+1 {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}}
12+
[RootSignature("UAV(u0, visibility = SHADER_VISIBILITY_PIXEL), UAV(u0, visibility = SHADER_VISIBILITY_PIXEL)")]
13+
void bad_root_signature_2() {}
14+
15+
// expected-error@+1 {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}}
16+
[RootSignature("UAV(u0, visibility = SHADER_VISIBILITY_ALL), UAV(u0, visibility = SHADER_VISIBILITY_PIXEL)")]
17+
void bad_root_signature_3() {}
18+
19+
// expected-error@+1 {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}}
20+
[RootSignature("UAV(u0, visibility = SHADER_VISIBILITY_PIXEL), UAV(u0, visibility = SHADER_VISIBILITY_ALL)")]
21+
void bad_root_signature_4() {}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -verify
2+
3+
// expected-no-diagnostics
4+
5+
[RootSignature("CBV(b0), CBV(b1)")]
6+
void valid_root_signature_0() {}
7+
8+
[RootSignature("CBV(b0, visibility = SHADER_VISIBILITY_DOMAIN), CBV(b0, visibility = SHADER_VISIBILITY_PIXEL)")]
9+
void valid_root_signature_1() {}
10+
11+
[RootSignature("CBV(b0, space = 1), CBV(b0, space = 2)")]
12+
void valid_root_signature_2() {}
13+
14+
[RootSignature("CBV(b0), SRV(t0)")]
15+
void valid_root_signature_3() {}

llvm/include/llvm/Frontend/HLSL/HLSLRootSignatureUtils.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,17 @@ class MetadataBuilder {
7171
SmallVector<Metadata *> GeneratedMetadata;
7272
};
7373

74-
// RangeInfo holds the information to correctly construct a ResourceRange
75-
// and retains this information to be used for displaying a better diagnostic
7674
struct RangeInfo {
7775
const static uint32_t Unbounded = ~0u;
7876

77+
// Interval information
7978
uint32_t LowerBound;
8079
uint32_t UpperBound;
80+
81+
// Information retained for diagnostics
82+
llvm::dxil::ResourceClass Class;
83+
uint32_t Space;
84+
ShaderVisibility Visibility;
8185
};
8286

8387
class ResourceRange {
@@ -98,6 +102,9 @@ class ResourceRange {
98102
// Return the mapped RangeInfo at X or nullptr if no mapping exists
99103
const RangeInfo *lookup(uint32_t X) const;
100104

105+
// Removes all entries of the ResourceRange
106+
void clear();
107+
101108
// Insert the required (sub-)intervals such that the interval of [a;b] =
102109
// [Info.LowerBound, Info.UpperBound] is covered and points to a valid
103110
// RangeInfo &.

llvm/lib/Frontend/HLSL/HLSLRootSignatureUtils.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,8 @@ const RangeInfo *ResourceRange::lookup(uint32_t X) const {
509509
return Intervals.lookup(X, nullptr);
510510
}
511511

512+
void ResourceRange::clear() { return Intervals.clear(); }
513+
512514
std::optional<const RangeInfo *> ResourceRange::insert(const RangeInfo &Info) {
513515
uint32_t LowerBound = Info.LowerBound;
514516
uint32_t UpperBound = Info.UpperBound;

0 commit comments

Comments
 (0)