Skip to content

[HLSL][RootSignature] Implement validation of resource ranges for RootDescriptors #140962

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jun 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -13054,6 +13054,11 @@ def err_invalid_hlsl_resource_type: Error<
def err_hlsl_spirv_only: Error<"%0 is only available for the SPIR-V target">;
def err_hlsl_vk_literal_must_contain_constant: Error<"the argument to vk::Literal must be a vk::integral_constant">;

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">;
Expand Down
2 changes: 2 additions & 0 deletions clang/include/clang/Sema/SemaHLSL.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ class SemaHLSL : public SemaBase {
SourceLocation Loc, IdentifierInfo *DeclIdent,
SmallVector<llvm::hlsl::rootsig::RootElement> &Elements);

// 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);
Expand Down
130 changes: 129 additions & 1 deletion clang/lib/Sema/SemaHLSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Twine.h"
#include "llvm/Frontend/HLSL/HLSLRootSignatureUtils.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/DXILABI.h"
#include "llvm/Support/ErrorHandling.h"
Expand Down Expand Up @@ -1068,10 +1069,138 @@ void SemaHLSL::ActOnFinishRootSignatureDecl(
SemaRef.getASTContext(), /*DeclContext=*/SemaRef.CurContext, Loc,
DeclIdent, Elements);

if (handleRootSignatureDecl(SignatureDecl, Loc))
return;

SignatureDecl->setImplicit();
SemaRef.PushOnScopeChains(SignatureDecl, SemaRef.getCurScope());
}

bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D,
SourceLocation Loc) {
// The following conducts analysis on resource ranges to detect and report
// any overlaps in resource ranges.
//
// A resource range overlaps with another resource range if they have:
// - equivalent ResourceClass (SRV, UAV, CBuffer, Sampler)
// - equivalent resource space
// - overlapping visbility
//
// The following algorithm is implemented in the following steps:
//
// 1. Collect RangeInfo from relevant RootElements:
// - RangeInfo will retain the interval, ResourceClass, Space and Visibility
// 2. Sort the RangeInfo's such that they are grouped together by
// ResourceClass and Space (GroupT defined below)
// 3. Iterate through the collected RangeInfos by their groups
// - For each group we will have a ResourceRange for each visibility
// - As we iterate through we will:
// A: Insert the current RangeInfo into the corresponding Visibility
// ResourceRange
// B: Check for overlap with any overlapping Visibility ResourceRange
using RangeInfo = llvm::hlsl::rootsig::RangeInfo;
using ResourceRange = llvm::hlsl::rootsig::ResourceRange;
using GroupT = std::pair<ResourceClass, /*Space*/ uint32_t>;

// 1. Collect RangeInfos
llvm::SmallVector<RangeInfo> Infos;
for (const llvm::hlsl::rootsig::RootElement &Elem : D->getRootElements()) {
if (const auto *Descriptor =
std::get_if<llvm::hlsl::rootsig::RootDescriptor>(&Elem)) {
RangeInfo Info;
Info.LowerBound = Descriptor->Reg.Number;
Info.UpperBound = Info.LowerBound; // use inclusive ranges []

Info.Class =
llvm::dxil::ResourceClass(llvm::to_underlying(Descriptor->Type));
Info.Space = Descriptor->Space;
Info.Visibility = Descriptor->Visibility;
Infos.push_back(Info);
}
}

// 2. Sort the RangeInfo's by their GroupT to form groupings
std::sort(Infos.begin(), Infos.end(), [](RangeInfo A, RangeInfo B) {
return std::tie(A.Class, A.Space) < std::tie(B.Class, B.Space);
});

// 3. First we will init our state to track:
if (Infos.size() == 0)
return false; // No ranges to overlap
GroupT CurGroup = {Infos[0].Class, Infos[0].Space};
bool HadOverlap = false;

// Create a ResourceRange for each Visibility
ResourceRange::MapT::Allocator Allocator;
std::array<ResourceRange, 8> Ranges = {
ResourceRange(Allocator), // All
ResourceRange(Allocator), // Vertex
ResourceRange(Allocator), // Hull
ResourceRange(Allocator), // Domain
ResourceRange(Allocator), // Geometry
ResourceRange(Allocator), // Pixel
ResourceRange(Allocator), // Amplification
ResourceRange(Allocator), // Mesh
};

// Reset the ResourceRanges for when we iterate through a new group
auto ClearRanges = [&Ranges]() {
for (ResourceRange &Range : Ranges)
Range.clear();
};

// Helper to report diagnostics
auto ReportOverlap = [this, Loc, &HadOverlap](const RangeInfo *Info,
const RangeInfo *OInfo) {
HadOverlap = true;
auto CommonVis =
Info->Visibility == llvm::hlsl::rootsig::ShaderVisibility::All
? OInfo->Visibility
: Info->Visibility;
this->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;
};

// 3: Iterate through collected RangeInfos
for (const RangeInfo &Info : Infos) {
GroupT InfoGroup = {Info.Class, Info.Space};
// Reset our ResourceRanges when we enter a new group
if (CurGroup != InfoGroup) {
ClearRanges();
CurGroup = InfoGroup;
}

// 3A: Insert range info into corresponding Visibility ResourceRange
ResourceRange &VisRange = Ranges[llvm::to_underlying(Info.Visibility)];
if (std::optional<const RangeInfo *> Overlapping = VisRange.insert(Info))
ReportOverlap(&Info, Overlapping.value());

// 3B: Check for overlap in all overlapping Visibility ResourceRanges
//
// If the range that we are inserting has ShaderVisiblity::All it needs to
// check for an overlap in all other visibility types as well.
// Otherwise, the range that is inserted needs to check that it does not
// overlap with ShaderVisibility::All.
//
// OverlapRanges will be an ArrayRef to all non-all visibility
// ResourceRanges in the former case and it will be an ArrayRef to just the
// all visiblity ResourceRange in the latter case.
ArrayRef<ResourceRange> OverlapRanges =
Info.Visibility == llvm::hlsl::rootsig::ShaderVisibility::All
? ArrayRef<ResourceRange>{Ranges}.drop_front()
: ArrayRef<ResourceRange>{Ranges}.take_front();

for (const ResourceRange &Range : OverlapRanges)
if (std::optional<const RangeInfo *> Overlapping =
Range.getOverlapping(Info))
ReportOverlap(&Info, Overlapping.value());
}

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;
Expand All @@ -1093,7 +1222,6 @@ void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) {
if (SemaRef.LookupQualifiedName(R, D->getDeclContext()))
if (auto *SignatureDecl =
dyn_cast<HLSLRootSignatureDecl>(R.getFoundDecl())) {
// Perform validation of constructs here
D->addAttr(::new (getASTContext()) RootSignatureAttr(
getASTContext(), AL, Ident, SignatureDecl));
}
Expand Down
21 changes: 21 additions & 0 deletions clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -verify

// expected-error@+1 {{resource ranges b[42;42] and b[42;42] overlap within space = 0 and visibility = All}}
[RootSignature("CBV(b42), CBV(b42)")]
void bad_root_signature_0() {}

// expected-error@+1 {{resource ranges t[0;0] and t[0;0] overlap within space = 3 and visibility = All}}
[RootSignature("SRV(t0, space = 3), SRV(t0, space = 3)")]
void bad_root_signature_1() {}

// expected-error@+1 {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}}
[RootSignature("UAV(u0, visibility = SHADER_VISIBILITY_PIXEL), UAV(u0, visibility = SHADER_VISIBILITY_PIXEL)")]
void bad_root_signature_2() {}

// expected-error@+1 {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}}
[RootSignature("UAV(u0, visibility = SHADER_VISIBILITY_ALL), UAV(u0, visibility = SHADER_VISIBILITY_PIXEL)")]
void bad_root_signature_3() {}

// expected-error@+1 {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}}
[RootSignature("UAV(u0, visibility = SHADER_VISIBILITY_PIXEL), UAV(u0, visibility = SHADER_VISIBILITY_ALL)")]
void bad_root_signature_4() {}
15 changes: 15 additions & 0 deletions clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -verify

// expected-no-diagnostics

[RootSignature("CBV(b0), CBV(b1)")]
void valid_root_signature_0() {}

[RootSignature("CBV(b0, visibility = SHADER_VISIBILITY_DOMAIN), CBV(b0, visibility = SHADER_VISIBILITY_PIXEL)")]
void valid_root_signature_1() {}

[RootSignature("CBV(b0, space = 1), CBV(b0, space = 2)")]
void valid_root_signature_2() {}

[RootSignature("CBV(b0), SRV(t0)")]
void valid_root_signature_3() {}
11 changes: 9 additions & 2 deletions llvm/include/llvm/Frontend/HLSL/HLSLRootSignatureUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,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 = ~0u;

// Interval information
uint32_t LowerBound;
uint32_t UpperBound;

// Information retained for diagnostics
llvm::dxil::ResourceClass Class;
uint32_t Space;
ShaderVisibility Visibility;
};

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

// Removes all entries of the ResourceRange
void clear();

// Insert the required (sub-)intervals such that the interval of [a;b] =
// [Info.LowerBound, Info.UpperBound] is covered and points to a valid
// RangeInfo &.
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Frontend/HLSL/HLSLRootSignatureUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,8 @@ const RangeInfo *ResourceRange::lookup(uint32_t X) const {
return Intervals.lookup(X, nullptr);
}

void ResourceRange::clear() { return Intervals.clear(); }

std::optional<const RangeInfo *> ResourceRange::insert(const RangeInfo &Info) {
uint32_t LowerBound = Info.LowerBound;
uint32_t UpperBound = Info.UpperBound;
Expand Down