-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[DirectX] Validating Root flags are denying shader stage #153287
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 25 commits
b1e34ff
90c2578
6e20bdf
84a4c4b
4400e2e
eb425c5
ffcff83
51ff280
e5812ce
d186ebd
5c35c32
9c09f21
9169be0
65089ce
5910271
311a2e5
901bd1d
aba77f9
22319f9
4c86232
6641d66
c9986ed
a0916e1
1f8e5b6
1c2a864
8cccaf3
b3bc8b8
18f9e9c
83ee5fe
fdcd3e3
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 |
---|---|---|
|
@@ -160,6 +160,40 @@ tripleToVisibility(llvm::Triple::EnvironmentType ET) { | |
} | ||
} | ||
|
||
static void reportIfDeniedShaderStageAccess(Module &M, dxbc::RootFlags Flags, | ||
dxbc::RootFlags Mask) { | ||
if ((Flags & Mask) != Mask) | ||
return; | ||
|
||
SmallString<128> Message; | ||
raw_svector_ostream OS(Message); | ||
OS << "Shader has root bindings but root signature uses a DENY flag to " | ||
"disallow root binding access to the shader stage."; | ||
M.getContext().diagnose(DiagnosticInfoGeneric(Message)); | ||
} | ||
|
||
inbelic marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
static dxbc::RootFlags | ||
getEnvironmentDenyFlagMask(Triple::EnvironmentType ShaderProfile) { | ||
switch (ShaderProfile) { | ||
case Triple::Pixel: | ||
return dxbc::RootFlags::DenyPixelShaderRootAccess; | ||
case Triple::Vertex: | ||
return dxbc::RootFlags::DenyVertexShaderRootAccess; | ||
case Triple::Geometry: | ||
return dxbc::RootFlags::DenyGeometryShaderRootAccess; | ||
case Triple::Hull: | ||
return dxbc::RootFlags::DenyHullShaderRootAccess; | ||
case Triple::Domain: | ||
return dxbc::RootFlags::DenyDomainShaderRootAccess; | ||
case Triple::Mesh: | ||
return dxbc::RootFlags::DenyMeshShaderRootAccess; | ||
case Triple::Amplification: | ||
return dxbc::RootFlags::DenyAmplificationShaderRootAccess; | ||
default: | ||
llvm_unreachable("Invalid triple to shader stage conversion"); | ||
} | ||
} | ||
|
||
static void validateRootSignature(Module &M, | ||
const mcdxbc::RootSignatureDesc &RSD, | ||
dxil::ModuleMetadataInfo &MMI, | ||
|
@@ -225,7 +259,9 @@ static void validateRootSignature(Module &M, | |
Builder.findOverlapping(ReportedBinding); | ||
reportOverlappingRegisters(M, ReportedBinding, Overlaping); | ||
}); | ||
|
||
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. That is intentional, I think it improves readability. |
||
const hlsl::BoundRegs &BoundRegs = Builder.takeBoundRegs(); | ||
bool HasBindings = false; | ||
inbelic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for (const ResourceInfo &RI : DRM) { | ||
const ResourceInfo::ResourceBinding &Binding = RI.getBinding(); | ||
const dxil::ResourceTypeInfo &RTI = DRTM[RI.getHandleTy()]; | ||
|
@@ -236,21 +272,33 @@ static void validateRootSignature(Module &M, | |
BoundRegs.findBoundReg(RC, Binding.Space, Binding.LowerBound, | ||
Binding.LowerBound + Binding.Size - 1); | ||
|
||
if (Reg != nullptr) { | ||
const auto *ParamInfo = | ||
static_cast<const mcdxbc::RootParameterInfo *>(Reg->Cookie); | ||
if (!Reg) { | ||
reportRegNotBound(M, RC, Binding); | ||
continue; | ||
} | ||
|
||
if (RC != ResourceClass::SRV && RC != ResourceClass::UAV) | ||
continue; | ||
const auto *ParamInfo = | ||
static_cast<const mcdxbc::RootParameterInfo *>(Reg->Cookie); | ||
|
||
const bool IsRootSRVOrUAV = | ||
RC == ResourceClass::SRV || RC == ResourceClass::UAV; | ||
const bool IsDescriptorTable = | ||
ParamInfo->Type == dxbc::RootParameterType::DescriptorTable; | ||
const bool IsRawOrStructuredBuffer = | ||
RK != ResourceKind::RawBuffer && RK != ResourceKind::StructuredBuffer; | ||
if (IsRootSRVOrUAV && !IsDescriptorTable && IsRawOrStructuredBuffer) { | ||
|
||
reportInvalidHandleTyError(M, RC, Binding); | ||
continue; | ||
} | ||
|
||
if (ParamInfo->Type == dxbc::RootParameterType::DescriptorTable) | ||
continue; | ||
HasBindings = true; | ||
} | ||
|
||
if (RK != ResourceKind::RawBuffer && RK != ResourceKind::StructuredBuffer) | ||
reportInvalidHandleTyError(M, RC, Binding); | ||
} else { | ||
reportRegNotBound(M, RC, Binding); | ||
} | ||
if (HasBindings && MMI.ShaderProfile != Triple::Compute) { | ||
const dxbc::RootFlags Flags = dxbc::RootFlags(RSD.Flags); | ||
const dxbc::RootFlags Mask = getEnvironmentDenyFlagMask(MMI.ShaderProfile); | ||
|
||
|
||
reportIfDeniedShaderStageAccess(M, Flags, Mask); | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
; RUN: opt -S -passes='dxil-post-optimization-validation' %s | ||
; This is a valid case where no resource is being used | ||
target triple = "dxil-pc-shadermodel6.6-pixel" | ||
|
||
define void @CSMain() #0 { | ||
entry: | ||
ret void | ||
} | ||
attributes #0 = { noinline nounwind "exp-shader"="cs" "hlsl.numthreads"="1,2,1" "hlsl.shader"="geometry" } | ||
|
||
!dx.rootsignatures = !{!0} | ||
|
||
!0 = !{ptr @CSMain, !1, i32 2} | ||
!1 = !{!2, !3, !4} | ||
!2 = !{!"RootConstants", i32 0, i32 2, i32 0, i32 4} | ||
!3 = !{ !"RootFlags", i32 294 } ; 294 = deny_pixel/hull/vertex/amplification_shader_root_access | ||
!4 = !{ !"RootSRV", i32 0, i32 1, i32 0, i32 0 } |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
; RUN: not opt -S -passes='dxil-post-optimization-validation' %s 2>&1 | FileCheck %s | ||
inbelic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
; CHECK: error: Shader has root bindings but root signature uses a DENY flag to disallow root binding access to the shader stage. | ||
target triple = "dxil-pc-shadermodel6.6-pixel" | ||
|
||
%__cblayout_CB = type <{ float }> | ||
|
||
@CB.str = private unnamed_addr constant [3 x i8] c"CB\00", align 1 | ||
|
||
define void @CSMain() "hlsl.shader"="compute" { | ||
entry: | ||
%CB = tail call target("dx.CBuffer", target("dx.Layout", %__cblayout_CB, 4, 0)) @llvm.dx.resource.handlefrombinding(i32 0, i32 2, i32 1, i32 0, ptr nonnull @CB.str) | ||
ret void | ||
} | ||
|
||
!dx.rootsignatures = !{!0} | ||
|
||
!0 = !{ptr @CSMain, !1, i32 2} | ||
!1 = !{!2, !3} | ||
!2 = !{!"RootConstants", i32 0, i32 2, i32 0, i32 4} | ||
!3 = !{!"RootFlags", i32 294} ; 294 = deny_pixel/hull/vertex/amplification_shader_root_access |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
; RUN: not opt -S -passes='dxil-post-optimization-validation' %s 2>&1 | FileCheck %s | ||
|
||
; CHECK: error: Shader has root bindings but root signature uses a DENY flag to disallow root binding access to the shader stage. | ||
target triple = "dxil-pc-shadermodel6.6-pixel" | ||
|
||
@SB.str = private unnamed_addr constant [3 x i8] c"SB\00", align 1 | ||
|
||
define void @CSMain() "hlsl.shader"="pixel" { | ||
entry: | ||
%SB = tail call target("dx.RawBuffer", i32, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 0, i32 1, i32 0, ptr nonnull @SB.str) | ||
ret void | ||
} | ||
|
||
!dx.rootsignatures = !{!0} | ||
|
||
!0 = !{ptr @CSMain, !1, i32 2} | ||
!1 = !{!2, !3} | ||
!2 = !{!"DescriptorTable", i32 0, !4} | ||
!4 = !{!"SRV", i32 1, i32 0, i32 0, i32 -1, i32 4} | ||
!3 = !{!"RootFlags", i32 32} ; 32 = deny_pixel_shader_root_access |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
; RUN: not opt -S -passes='dxil-post-optimization-validation' %s 2>&1 | FileCheck %s | ||
|
||
; CHECK: error: Shader has root bindings but root signature uses a DENY flag to disallow root binding access to the shader stage. | ||
target triple = "dxil-pc-shadermodel6.6-pixel" | ||
|
||
@SB.str = private unnamed_addr constant [3 x i8] c"SB\00", align 1 | ||
|
||
define void @CSMain() "hlsl.shader"="pixel" { | ||
entry: | ||
%SB = tail call target("dx.RawBuffer", i32, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 0, i32 1, i32 0, ptr nonnull @SB.str) | ||
ret void | ||
} | ||
|
||
!dx.rootsignatures = !{!0} | ||
|
||
!0 = !{ptr @CSMain, !1, i32 2} | ||
!1 = !{!2, !3} | ||
!2 = !{!"RootSRV", i32 0, i32 0, i32 0, i32 4} | ||
!3 = !{!"RootFlags", i32 32} ; 32 = deny_pixel_shader_root_access |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
; RUN: opt -S -passes='dxil-post-optimization-validation' %s | ||
; Valid scenario where shader stage is not blocked from accessing root bindings | ||
target triple = "dxil-pc-shadermodel6.6-geometry" | ||
|
||
%__cblayout_CB = type <{ float }> | ||
|
||
@CB.str = private unnamed_addr constant [3 x i8] c"CB\00", align 1 | ||
|
||
define void @CSMain() "hlsl.shader"="geometry" { | ||
entry: | ||
%CB = tail call target("dx.CBuffer", target("dx.Layout", %__cblayout_CB, 4, 0)) @llvm.dx.resource.handlefrombinding(i32 0, i32 2, i32 1, i32 0, ptr nonnull @CB.str) | ||
ret void | ||
} | ||
attributes #0 = { noinline nounwind "exp-shader"="cs" "hlsl.numthreads"="1,2,1" "hlsl.shader"="geometry" } | ||
|
||
!dx.rootsignatures = !{!0} | ||
|
||
!0 = !{ptr @CSMain, !1, i32 2} | ||
!1 = !{!2, !3} | ||
!2 = !{ !"RootFlags", i32 294 } ; 294 = deny_pixel/hull/vertex/amplification_shader_root_access | ||
!3 = !{ !"RootCBV", i32 0, i32 2, i32 0, i32 0 } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this check here, because I think it simplifies the code. If this is not here, I need to make the same check in all statements of the switch with slightly different masks.