diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 34b798a09c216..f2f2152b8bbbe 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -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">; diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h index 97091792ba236..7d7eae4db532c 100644 --- a/clang/include/clang/Sema/SemaHLSL.h +++ b/clang/include/clang/Sema/SemaHLSL.h @@ -134,6 +134,8 @@ class SemaHLSL : public SemaBase { SourceLocation Loc, IdentifierInfo *DeclIdent, SmallVector &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); diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 9b43ee00810b2..d003967a522a1 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/HLSLRootSignatureUtils.h" #include "llvm/Support/Casting.h" #include "llvm/Support/DXILABI.h" #include "llvm/Support/ErrorHandling.h" @@ -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; + + // 1. Collect RangeInfos + llvm::SmallVector Infos; + for (const llvm::hlsl::rootsig::RootElement &Elem : D->getRootElements()) { + if (const auto *Descriptor = + std::get_if(&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 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 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 OverlapRanges = + Info.Visibility == llvm::hlsl::rootsig::ShaderVisibility::All + ? ArrayRef{Ranges}.drop_front() + : ArrayRef{Ranges}.take_front(); + + for (const ResourceRange &Range : OverlapRanges) + if (std::optional 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; @@ -1093,7 +1222,6 @@ void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) { if (SemaRef.LookupQualifiedName(R, D->getDeclContext())) if (auto *SignatureDecl = dyn_cast(R.getFoundDecl())) { - // Perform validation of constructs here 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..e5152e72d4806 --- /dev/null +++ b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl @@ -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() {} diff --git a/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl b/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl new file mode 100644 index 0000000000000..5778fb2ae4eb9 --- /dev/null +++ b/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl @@ -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() {} diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignatureUtils.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignatureUtils.h index 25c2a9f0cc808..4769fd0559965 100644 --- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignatureUtils.h +++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignatureUtils.h @@ -71,13 +71,17 @@ class MetadataBuilder { SmallVector 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 { @@ -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 &. diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignatureUtils.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignatureUtils.cpp index a1ddb318055be..f95c141c54d8d 100644 --- a/llvm/lib/Frontend/HLSL/HLSLRootSignatureUtils.cpp +++ b/llvm/lib/Frontend/HLSL/HLSLRootSignatureUtils.cpp @@ -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 ResourceRange::insert(const RangeInfo &Info) { uint32_t LowerBound = Info.LowerBound; uint32_t UpperBound = Info.UpperBound;