-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[DirectX] Add Range Overlap validation to DXILPostOptimizationValidation.cpp
#148919
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
Changes from 9 commits
831dc1c
e0949e0
4c38beb
4bb0368
79ec21d
a281de3
6720815
4d34b01
356c4b7
dec12d9
1cb111a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,12 +15,14 @@ | |
| #include "llvm/ADT/SmallString.h" | ||
| #include "llvm/Analysis/DXILMetadataAnalysis.h" | ||
| #include "llvm/Analysis/DXILResource.h" | ||
| #include "llvm/BinaryFormat/DXContainer.h" | ||
| #include "llvm/Frontend/HLSL/RootSignatureValidations.h" | ||
| #include "llvm/IR/DiagnosticInfo.h" | ||
| #include "llvm/IR/Instructions.h" | ||
| #include "llvm/IR/IntrinsicsDirectX.h" | ||
| #include "llvm/IR/Module.h" | ||
| #include "llvm/InitializePasses.h" | ||
| #include "llvm/Support/DXILABI.h" | ||
|
|
||
| #define DEBUG_TYPE "dxil-post-optimization-validation" | ||
|
|
||
|
|
@@ -295,6 +297,105 @@ getRootSignature(RootSignatureBindingInfo &RSBI, | |
| return RootSigDesc; | ||
| } | ||
|
|
||
| static void | ||
| reportOverlappingRegisters(Module &M, | ||
| llvm::hlsl::rootsig::OverlappingRanges Overlap) { | ||
| const llvm::hlsl::rootsig::RangeInfo *Info = Overlap.A; | ||
| const llvm::hlsl::rootsig::RangeInfo *OInfo = Overlap.B; | ||
| SmallString<128> Message; | ||
| raw_svector_ostream OS(Message); | ||
| OS << "register " << ResourceClassToString(Info->Class) | ||
| << " (space=" << Info->Space << ", register=" << Info->LowerBound << ")" | ||
| << " is overlapping with" | ||
| << " register " << ResourceClassToString(OInfo->Class) | ||
| << " (space=" << OInfo->Space << ", register=" << OInfo->LowerBound << ")" | ||
| << ", verify your root signature definition."; | ||
|
|
||
| M.getContext().diagnose(DiagnosticInfoGeneric(Message)); | ||
| } | ||
|
|
||
| static bool reportOverlappingRanges(Module &M, | ||
| const mcdxbc::RootSignatureDesc &RSD) { | ||
| using namespace llvm::hlsl::rootsig; | ||
|
|
||
| llvm::SmallVector<RangeInfo> Infos; | ||
|
|
||
| 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::Constants32Bit): { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than using That's definitely the better pattern to use for cases like this.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I filed an issue to clean up this pattern across the DX backend. We really shouldn't be writing switch statements like this. The issue is #150676. |
||
| dxbc::RTS0::v1::RootConstants Const = | ||
| RSD.ParametersContainer.getConstant(Loc); | ||
|
|
||
| RangeInfo Info; | ||
| Info.Space = Const.RegisterSpace; | ||
| Info.LowerBound = Const.ShaderRegister; | ||
| Info.UpperBound = Info.LowerBound; | ||
| Info.Class = ParameterToResourceClass(Type); | ||
| Info.Visibility = (dxbc::ShaderVisibility)Header.ShaderVisibility; | ||
|
|
||
| Infos.push_back(Info); | ||
| break; | ||
| } | ||
| 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); | ||
|
|
||
| RangeInfo Info; | ||
| Info.Space = Desc.RegisterSpace; | ||
| Info.LowerBound = Desc.ShaderRegister; | ||
| Info.UpperBound = Info.LowerBound; | ||
| Info.Class = ParameterToResourceClass(Type); | ||
| Info.Visibility = (dxbc::ShaderVisibility)Header.ShaderVisibility; | ||
|
|
||
| Infos.push_back(Info); | ||
| 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) { | ||
| assert(Range.NumDescriptors > 0); | ||
| RangeInfo Info; | ||
| Info.Space = Range.RegisterSpace; | ||
| Info.LowerBound = Range.BaseShaderRegister; | ||
joaosaffran marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Info.UpperBound = Info.LowerBound + ((Range.NumDescriptors == ~0U) | ||
| ? Range.NumDescriptors | ||
| : Range.NumDescriptors - 1); | ||
| Info.Class = RangeToResourceClass(Range.RangeType); | ||
bob80905 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Info.Visibility = (dxbc::ShaderVisibility)Header.ShaderVisibility; | ||
|
|
||
| Infos.push_back(Info); | ||
| } | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| for (const dxbc::RTS0::v1::StaticSampler &Sampler : RSD.StaticSamplers) { | ||
| RangeInfo Info; | ||
| Info.Space = Sampler.RegisterSpace; | ||
| Info.LowerBound = Sampler.ShaderRegister; | ||
| Info.UpperBound = Info.LowerBound; | ||
| Info.Class = ResourceClass::Sampler; | ||
| Info.Visibility = (dxbc::ShaderVisibility)Sampler.ShaderVisibility; | ||
|
|
||
| Infos.push_back(Info); | ||
| } | ||
|
|
||
| llvm::SmallVector<OverlappingRanges> Overlaps = | ||
| llvm::hlsl::rootsig::findOverlappingRanges(Infos); | ||
| for (OverlappingRanges Overlap : Overlaps) | ||
| reportOverlappingRegisters(M, Overlap); | ||
|
|
||
| return Overlaps.size() > 0; | ||
| } | ||
|
|
||
| static void reportInvalidHandleTy( | ||
| Module &M, const llvm::ArrayRef<dxil::ResourceInfo::ResourceBinding> &RDs, | ||
| const iterator_range<SmallVectorImpl<dxil::ResourceInfo>::iterator> | ||
|
|
@@ -359,6 +460,8 @@ static void reportErrors(Module &M, DXILResourceMap &DRM, | |
| "DXILResourceImplicitBinding pass"); | ||
|
|
||
| if (auto RSD = getRootSignature(RSBI, MMI)) { | ||
| if (reportOverlappingRanges(M, *RSD)) | ||
| return; | ||
| dxbc::ShaderVisibility Visibility = tripleToVisibility(MMI.ShaderProfile); | ||
| llvm::hlsl::rootsig::RootSignatureBindingValidation Validation = | ||
| initRSBindingValidation(*RSD, Visibility); | ||
|
|
||
joaosaffran marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| ; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s | ||
| ; CHECK: error: register CBuffer (space=0, register=0) is overlapping with register CBuffer (space=0, register=2), verify your root signature definition | ||
|
|
||
| define void @CSMain() "hlsl.shader"="compute" { | ||
| entry: | ||
| ret void | ||
| } | ||
|
|
||
| ; RootConstants(num32BitConstants=4, b2), DescriptorTable(CBV(b0, numDescriptors=3)) | ||
joaosaffran marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| !dx.rootsignatures = !{!0} | ||
| !0 = !{ptr @CSMain, !1, i32 2} | ||
| !1 = !{!2, !3} | ||
| !2 = !{!"RootConstants", i32 0, i32 2, i32 0, i32 4} | ||
| !3 = !{!"DescriptorTable", i32 0, !4} | ||
| !4 = !{!"CBV", i32 3, i32 0, i32 0, i32 -1, i32 4} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| ; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s | ||
| ; CHECK: error: register UAV (space=0, register=2) is overlapping with register UAV (space=0, register=0), verify your root signature definition | ||
|
|
||
| define void @CSMain() "hlsl.shader"="compute" { | ||
| entry: | ||
| ret void | ||
| } | ||
|
|
||
| ; DescriptorTable(UAV(u0, numDescriptors=unbounded), visibility = SHADER_VISIBILITY_HULL), DescriptorTable(UAV(u2, numDescriptors=4)) | ||
| !dx.rootsignatures = !{!0} | ||
| !0 = !{ptr @CSMain, !1, i32 2} | ||
| !1 = !{!2, !4} | ||
| !2 = !{!"DescriptorTable", i32 2, !3} | ||
| !3 = !{!"UAV", i32 -1, i32 0, i32 0, i32 -1, i32 2} | ||
| !4 = !{!"DescriptorTable", i32 0, !5} | ||
| !5 = !{!"UAV", i32 4, i32 2, i32 0, i32 -1, i32 2} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| ; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s | ||
| ; CHECK: error: register Sampler (space=0, register=42) is overlapping with register Sampler (space=0, register=42), verify your root signature definition. | ||
|
|
||
| define void @CSMain() "hlsl.shader"="compute" { | ||
| entry: | ||
| ret void | ||
| } | ||
|
|
||
| ; RootConstants(num32BitConstants=4, b2), DescriptorTable(CBV(b0, numDescriptors=3)) | ||
| !dx.rootsignatures = !{!0} | ||
| !0 = !{ptr @CSMain, !1, i32 2} | ||
| !1 = !{!2, !3} | ||
| !2 = !{ !"StaticSampler", i32 4, i32 2, i32 3, i32 5, float 0x3FF6CCCCC0000000, i32 9, i32 3, i32 2, float -1.280000e+02, float 1.280000e+02, i32 42, i32 0, i32 0 } | ||
joaosaffran marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| !3 = !{ !"StaticSampler", i32 4, i32 2, i32 3, i32 5, float 0x3FF6CCCCC0000000, i32 9, i32 3, i32 2, float -1.280000e+02, float 1.280000e+02, i32 42, i32 0, i32 0 } | ||
Uh oh!
There was an error while loading. Please reload this page.