llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: None (joaosaffran) <details> <summary>Changes</summary> DXC checks if registers are correctly bound to root signature descriptors. This implements the same check. Closes: [126645](https://github.com/llvm/llvm-project/issues/126645) --- Full diff: https://github.com/llvm/llvm-project/pull/146785.diff 7 Files Affected: - (added) clang/test/SemaHLSL/RootSignature-Validation-Fail.hlsl (+35) - (added) clang/test/SemaHLSL/RootSignature-Validation.hlsl (+33) - (modified) llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp (+133-3) - (modified) llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h (+119) - (modified) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll (+2-2) - (modified) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll (+2-2) - (modified) llvm/test/CodeGen/DirectX/llc-pipeline.ll (+1) ``````````diff diff --git a/clang/test/SemaHLSL/RootSignature-Validation-Fail.hlsl b/clang/test/SemaHLSL/RootSignature-Validation-Fail.hlsl new file mode 100644 index 0000000000000..b590ed67e7085 --- /dev/null +++ b/clang/test/SemaHLSL/RootSignature-Validation-Fail.hlsl @@ -0,0 +1,35 @@ +// RUN: not %clang_dxc -T cs_6_6 -E CSMain %s 2>&1 | FileCheck %s + +// CHECK: error: register cbuffer (space=665, register=3) is not defined in Root Signature +// CHECK: error: register srv (space=0, register=0) is not defined in Root Signature +// CHECK: error: register uav (space=0, register=4294967295) is not defined in Root Signature + + +#define ROOT_SIGNATURE \ + "CBV(b3, space=666, visibility=SHADER_VISIBILITY_ALL), " \ + "DescriptorTable(SRV(t0, space=0, numDescriptors=1), visibility=SHADER_VISIBILITY_VERTEX), " \ + "DescriptorTable(Sampler(s0, numDescriptors=2), visibility=SHADER_VISIBILITY_ALL), " \ + "DescriptorTable(UAV(u0, numDescriptors=unbounded), visibility=SHADER_VISIBILITY_ALL)" + +cbuffer CB : register(b3, space665) { + float a; +} + +StructuredBuffer<int> In : register(t0, space0); +RWStructuredBuffer<int> Out : register(u0); + +RWBuffer<float> UAV : register(u4294967295); + +RWBuffer<float> UAV1 : register(u2), UAV2 : register(u4); + +RWBuffer<float> UAV3 : register(space0); + + + +// Compute Shader for UAV testing +[numthreads(8, 8, 1)] +[RootSignature(ROOT_SIGNATURE)] +void CSMain(uint id : SV_GroupID) +{ + Out[0] = a + id + In[0] + UAV[0] + UAV1[0] + UAV3[0]; +} diff --git a/clang/test/SemaHLSL/RootSignature-Validation.hlsl b/clang/test/SemaHLSL/RootSignature-Validation.hlsl new file mode 100644 index 0000000000000..5a7f5baf00619 --- /dev/null +++ b/clang/test/SemaHLSL/RootSignature-Validation.hlsl @@ -0,0 +1,33 @@ +// RUN: %clang_dxc -T cs_6_6 -E CSMain %s 2>&1 + +// expected-no-diagnostics + + +#define ROOT_SIGNATURE \ + "CBV(b3, space=1, visibility=SHADER_VISIBILITY_ALL), " \ + "DescriptorTable(SRV(t0, space=0, numDescriptors=1), visibility=SHADER_VISIBILITY_ALL), " \ + "DescriptorTable(Sampler(s0, numDescriptors=2), visibility=SHADER_VISIBILITY_VERTEX), " \ + "DescriptorTable(UAV(u0, numDescriptors=unbounded), visibility=SHADER_VISIBILITY_ALL)" + +cbuffer CB : register(b3, space1) { + float a; +} + +StructuredBuffer<int> In : register(t0, space0); +RWStructuredBuffer<int> Out : register(u0); + +RWBuffer<float> UAV : register(u4294967294); + +RWBuffer<float> UAV1 : register(u2), UAV2 : register(u4); + +RWBuffer<float> UAV3 : register(space0); + + + +// Compute Shader for UAV testing +[numthreads(8, 8, 1)] +[RootSignature(ROOT_SIGNATURE)] +void CSMain(uint id : SV_GroupID) +{ + Out[0] = a + id + In[0] + UAV[0] + UAV1[0] + UAV3[0]; +} diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp index 398dcbb8d1737..a52a04323514c 100644 --- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "DXILPostOptimizationValidation.h" +#include "DXILRootSignature.h" #include "DXILShaderFlags.h" #include "DirectX.h" #include "llvm/ADT/SmallString.h" @@ -84,8 +85,60 @@ static void reportOverlappingBinding(Module &M, DXILResourceMap &DRM) { } } +static void reportRegNotBound(Module &M, Twine Type, + ResourceInfo::ResourceBinding Binding) { + SmallString<128> Message; + raw_svector_ostream OS(Message); + OS << "register " << Type << " (space=" << Binding.Space + << ", register=" << Binding.LowerBound << ")" + << " is not defined in Root Signature"; + M.getContext().diagnose(DiagnosticInfoGeneric(Message)); +} + +static dxbc::ShaderVisibility +tripleToVisibility(llvm::Triple::EnvironmentType ET) { + assert((ET == Triple::Pixel || ET == Triple::Vertex || + ET == Triple::Geometry || ET == Triple::Hull || + ET == Triple::Domain || ET == Triple::Mesh || + ET == Triple::Compute) && + "Invalid Triple to shader stage conversion"); + + switch (ET) { + case Triple::Pixel: + return dxbc::ShaderVisibility::Pixel; + case Triple::Vertex: + return dxbc::ShaderVisibility::Vertex; + case Triple::Geometry: + return dxbc::ShaderVisibility::Geometry; + case Triple::Hull: + return dxbc::ShaderVisibility::Hull; + case Triple::Domain: + return dxbc::ShaderVisibility::Domain; + case Triple::Mesh: + return dxbc::ShaderVisibility::Mesh; + case Triple::Compute: + return dxbc::ShaderVisibility::All; + default: + llvm_unreachable("Invalid triple to shader stage conversion"); + } +} + +std::optional<mcdxbc::RootSignatureDesc> +getRootSignature(RootSignatureBindingInfo &RSBI, + dxil::ModuleMetadataInfo &MMI) { + if (MMI.EntryPropertyVec.size() == 0) + return std::nullopt; + std::optional<mcdxbc::RootSignatureDesc> RootSigDesc = + RSBI.getDescForFunction(MMI.EntryPropertyVec[0].Entry); + if (!RootSigDesc) + return std::nullopt; + return RootSigDesc; +} + static void reportErrors(Module &M, DXILResourceMap &DRM, - DXILResourceBindingInfo &DRBI) { + DXILResourceBindingInfo &DRBI, + RootSignatureBindingInfo &RSBI, + dxil::ModuleMetadataInfo &MMI) { if (DRM.hasInvalidCounterDirection()) reportInvalidDirection(M, DRM); @@ -94,14 +147,83 @@ static void reportErrors(Module &M, DXILResourceMap &DRM, assert(!DRBI.hasImplicitBinding() && "implicit bindings should be handled in " "DXILResourceImplicitBinding pass"); + + if (auto RSD = getRootSignature(RSBI, MMI)) { + + RootSignatureBindingValidation Validation; + Validation.addRsBindingInfo(*RSD, tripleToVisibility(MMI.ShaderProfile)); + + for (const ResourceInfo &CBuf : DRM.cbuffers()) { + ResourceInfo::ResourceBinding Binding = CBuf.getBinding(); + if (!Validation.checkCRegBinding(Binding)) + reportRegNotBound(M, "cbuffer", Binding); + } + + for (const ResourceInfo &SRV : DRM.srvs()) { + ResourceInfo::ResourceBinding Binding = SRV.getBinding(); + if (!Validation.checkTRegBinding(Binding)) + reportRegNotBound(M, "srv", Binding); + } + + for (const ResourceInfo &UAV : DRM.uavs()) { + ResourceInfo::ResourceBinding Binding = UAV.getBinding(); + if (!Validation.checkURegBinding(Binding)) + reportRegNotBound(M, "uav", Binding); + } + + for (const ResourceInfo &Sampler : DRM.samplers()) { + ResourceInfo::ResourceBinding Binding = Sampler.getBinding(); + if (!Validation.checkSamplerBinding(Binding)) + reportRegNotBound(M, "sampler", Binding); + } + } } } // namespace +void RootSignatureBindingValidation::addRsBindingInfo( + mcdxbc::RootSignatureDesc &RSD, dxbc::ShaderVisibility Visibility) { + for (size_t I = 0; I < RSD.ParametersContainer.size(); I++) { + const auto &[Type, Loc] = + RSD.ParametersContainer.getTypeAndLocForParameter(I); + + const auto &Header = RSD.ParametersContainer.getHeader(I); + switch (Type) { + case llvm::to_underlying(dxbc::RootParameterType::SRV): + case llvm::to_underlying(dxbc::RootParameterType::UAV): + case llvm::to_underlying(dxbc::RootParameterType::CBV): { + dxbc::RTS0::v2::RootDescriptor Desc = + RSD.ParametersContainer.getRootDescriptor(Loc); + + if (Header.ShaderVisibility == + llvm::to_underlying(dxbc::ShaderVisibility::All) || + Header.ShaderVisibility == llvm::to_underlying(Visibility)) + addRange(Desc, Type); + break; + } + case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable): { + const mcdxbc::DescriptorTable &Table = + RSD.ParametersContainer.getDescriptorTable(Loc); + + for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) { + if (Header.ShaderVisibility == + llvm::to_underlying(dxbc::ShaderVisibility::All) || + Header.ShaderVisibility == llvm::to_underlying(Visibility)) + addRange(Range); + } + break; + } + } + } +} + PreservedAnalyses DXILPostOptimizationValidation::run(Module &M, ModuleAnalysisManager &MAM) { DXILResourceMap &DRM = MAM.getResult<DXILResourceAnalysis>(M); DXILResourceBindingInfo &DRBI = MAM.getResult<DXILResourceBindingAnalysis>(M); - reportErrors(M, DRM, DRBI); + RootSignatureBindingInfo &RSBI = MAM.getResult<RootSignatureAnalysis>(M); + ModuleMetadataInfo &MMI = MAM.getResult<DXILMetadataAnalysis>(M); + + reportErrors(M, DRM, DRBI, RSBI, MMI); return PreservedAnalyses::all(); } @@ -113,7 +235,12 @@ class DXILPostOptimizationValidationLegacy : public ModulePass { getAnalysis<DXILResourceWrapperPass>().getResourceMap(); DXILResourceBindingInfo &DRBI = getAnalysis<DXILResourceBindingWrapperPass>().getBindingInfo(); - reportErrors(M, DRM, DRBI); + RootSignatureBindingInfo &RSBI = + getAnalysis<RootSignatureAnalysisWrapper>().getRSInfo(); + dxil::ModuleMetadataInfo &MMI = + getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata(); + + reportErrors(M, DRM, DRBI, RSBI, MMI); return false; } StringRef getPassName() const override { @@ -125,10 +252,13 @@ class DXILPostOptimizationValidationLegacy : public ModulePass { void getAnalysisUsage(llvm::AnalysisUsage &AU) const override { AU.addRequired<DXILResourceWrapperPass>(); AU.addRequired<DXILResourceBindingWrapperPass>(); + AU.addRequired<RootSignatureAnalysisWrapper>(); + AU.addRequired<DXILMetadataAnalysisWrapperPass>(); AU.addPreserved<DXILResourceWrapperPass>(); AU.addPreserved<DXILResourceBindingWrapperPass>(); AU.addPreserved<DXILMetadataAnalysisWrapperPass>(); AU.addPreserved<ShaderFlagsAnalysisWrapper>(); + AU.addPreserved<RootSignatureAnalysisWrapper>(); } }; char DXILPostOptimizationValidationLegacy::ID = 0; diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h index cb5e624514272..0fa0285425d7e 100644 --- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h @@ -14,10 +14,129 @@ #ifndef LLVM_LIB_TARGET_DIRECTX_DXILPOSTOPTIMIZATIONVALIDATION_H #define LLVM_LIB_TARGET_DIRECTX_DXILPOSTOPTIMIZATIONVALIDATION_H +#include "DXILRootSignature.h" +#include "llvm/ADT/IntervalMap.h" +#include "llvm/Analysis/DXILResource.h" #include "llvm/IR/PassManager.h" namespace llvm { +static uint64_t combineUint32ToUint64(uint32_t High, uint32_t Low) { + return (static_cast<uint64_t>(High) << 32) | Low; +} + +class RootSignatureBindingValidation { + using MapT = + llvm::IntervalMap<uint64_t, dxil::ResourceInfo::ResourceBinding, + sizeof(llvm::dxil::ResourceInfo::ResourceBinding), + llvm::IntervalMapInfo<uint64_t>>; + +private: + MapT::Allocator Allocator; + MapT CRegBindingsMap; + MapT TRegBindingsMap; + MapT URegBindingsMap; + MapT SamplersBindingsMap; + + void addRange(const dxbc::RTS0::v2::RootDescriptor &Desc, uint32_t Type) { + assert((Type == llvm::to_underlying(dxbc::RootParameterType::CBV) || + Type == llvm::to_underlying(dxbc::RootParameterType::SRV) || + Type == llvm::to_underlying(dxbc::RootParameterType::UAV)) && + "Invalid Type in add Range Method"); + + llvm::dxil::ResourceInfo::ResourceBinding Binding; + Binding.LowerBound = Desc.ShaderRegister; + Binding.Space = Desc.RegisterSpace; + Binding.Size = 1; + + uint64_t LowRange = + combineUint32ToUint64(Binding.Space, Binding.LowerBound); + uint64_t HighRange = combineUint32ToUint64( + Binding.Space, Binding.LowerBound + Binding.Size - 1); + + assert(LowRange <= HighRange && "Invalid range configuration"); + + switch (Type) { + + case llvm::to_underlying(dxbc::RootParameterType::CBV): + CRegBindingsMap.insert(LowRange, HighRange, Binding); + break; + case llvm::to_underlying(dxbc::RootParameterType::SRV): + TRegBindingsMap.insert(LowRange, HighRange, Binding); + break; + case llvm::to_underlying(dxbc::RootParameterType::UAV): + URegBindingsMap.insert(LowRange, HighRange, Binding); + break; + } + } + + void addRange(const dxbc::RTS0::v2::DescriptorRange &Range) { + + llvm::dxil::ResourceInfo::ResourceBinding Binding; + Binding.LowerBound = Range.BaseShaderRegister; + Binding.Space = Range.RegisterSpace; + Binding.Size = Range.NumDescriptors; + + uint64_t LowRange = + combineUint32ToUint64(Binding.Space, Binding.LowerBound); + uint64_t HighRange = combineUint32ToUint64( + Binding.Space, Binding.LowerBound + Binding.Size - 1); + + assert(LowRange <= HighRange && "Invalid range configuration"); + + switch (Range.RangeType) { + case llvm::to_underlying(dxbc::DescriptorRangeType::CBV): + CRegBindingsMap.insert(LowRange, HighRange, Binding); + break; + case llvm::to_underlying(dxbc::DescriptorRangeType::SRV): + TRegBindingsMap.insert(LowRange, HighRange, Binding); + break; + case llvm::to_underlying(dxbc::DescriptorRangeType::UAV): + URegBindingsMap.insert(LowRange, HighRange, Binding); + break; + case llvm::to_underlying(dxbc::DescriptorRangeType::Sampler): + SamplersBindingsMap.insert(LowRange, HighRange, Binding); + break; + } + } + +public: + RootSignatureBindingValidation() + : Allocator(), CRegBindingsMap(Allocator), TRegBindingsMap(Allocator), + URegBindingsMap(Allocator), SamplersBindingsMap(Allocator) {} + + void addRsBindingInfo(mcdxbc::RootSignatureDesc &RSD, + dxbc::ShaderVisibility Visibility); + + bool checkCRegBinding(dxil::ResourceInfo::ResourceBinding Binding) { + return CRegBindingsMap.overlaps( + combineUint32ToUint64(Binding.Space, Binding.LowerBound), + combineUint32ToUint64(Binding.Space, + Binding.LowerBound + Binding.Size - 1)); + } + + bool checkTRegBinding(dxil::ResourceInfo::ResourceBinding Binding) { + return TRegBindingsMap.overlaps( + combineUint32ToUint64(Binding.Space, Binding.LowerBound), + combineUint32ToUint64(Binding.Space, + Binding.LowerBound + Binding.Size - 1)); + } + + bool checkURegBinding(dxil::ResourceInfo::ResourceBinding Binding) { + return URegBindingsMap.overlaps( + combineUint32ToUint64(Binding.Space, Binding.LowerBound), + combineUint32ToUint64(Binding.Space, + Binding.LowerBound + Binding.Size - 1)); + } + + bool checkSamplerBinding(dxil::ResourceInfo::ResourceBinding Binding) { + return SamplersBindingsMap.overlaps( + combineUint32ToUint64(Binding.Space, Binding.LowerBound), + combineUint32ToUint64(Binding.Space, + Binding.LowerBound + Binding.Size - 1)); + } +}; + class DXILPostOptimizationValidation : public PassInfoMixin<DXILPostOptimizationValidation> { public: diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll index 9d89dbdd9107b..053721de1eb1f 100644 --- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll +++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll @@ -13,7 +13,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } !2 = !{ ptr @main, !3, i32 1 } ; function, root signature !3 = !{ !5 } ; list of root signature elements !5 = !{ !"DescriptorTable", i32 0, !6, !7 } -!6 = !{ !"Sampler", i32 0, i32 1, i32 0, i32 -1, i32 1 } +!6 = !{ !"Sampler", i32 1, i32 1, i32 0, i32 -1, i32 1 } !7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 3 } @@ -33,7 +33,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } ; DXC-NEXT: RangesOffset: 44 ; DXC-NEXT: Ranges: ; DXC-NEXT: - RangeType: 3 -; DXC-NEXT: NumDescriptors: 0 +; DXC-NEXT: NumDescriptors: 1 ; DXC-NEXT: BaseShaderRegister: 1 ; DXC-NEXT: RegisterSpace: 0 ; DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295 diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll index b516d66180247..8e9b4b43b11a6 100644 --- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll +++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll @@ -16,7 +16,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } !2 = !{ ptr @main, !3, i32 2 } ; function, root signature !3 = !{ !5 } ; list of root signature elements !5 = !{ !"DescriptorTable", i32 0, !6, !7 } -!6 = !{ !"SRV", i32 0, i32 1, i32 0, i32 -1, i32 4 } +!6 = !{ !"SRV", i32 1, i32 1, i32 0, i32 -1, i32 4 } !7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 2 } ; DXC: - Name: RTS0 @@ -35,7 +35,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } ; DXC-NEXT: RangesOffset: 44 ; DXC-NEXT: Ranges: ; DXC-NEXT: - RangeType: 0 -; DXC-NEXT: NumDescriptors: 0 +; DXC-NEXT: NumDescriptors: 1 ; DXC-NEXT: BaseShaderRegister: 1 ; DXC-NEXT: RegisterSpace: 0 ; DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295 diff --git a/llvm/test/CodeGen/DirectX/llc-pipeline.ll b/llvm/test/CodeGen/DirectX/llc-pipeline.ll index 2b29fd30a7a56..8d75249dc6ecb 100644 --- a/llvm/test/CodeGen/DirectX/llc-pipeline.ll +++ b/llvm/test/CodeGen/DirectX/llc-pipeline.ll @@ -31,6 +31,7 @@ ; CHECK-NEXT: DXIL Module Metadata analysis ; CHECK-NEXT: DXIL Shader Flag Analysis ; CHECK-NEXT: DXIL Translate Metadata +; CHECK-NEXT: DXIL Root Signature Analysis ; CHECK-NEXT: DXIL Post Optimization Validation ; CHECK-NEXT: DXIL Op Lowering ; CHECK-NEXT: DXIL Prepare Module `````````` </details> https://github.com/llvm/llvm-project/pull/146785 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
