-
Notifications
You must be signed in to change notification settings - Fork 14.1k
[DirectX] Implement Shader Flag Analysis for UAVsAtEveryStage
#137085
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
Conversation
@llvm/pr-subscribers-backend-directx Author: Deric C. (Icohedron) ChangesFixes #112272 Full diff: https://github.com/llvm/llvm-project/pull/137085.diff 13 Files Affected:
diff --git a/llvm/lib/Target/DirectX/DXILShaderFlags.cpp b/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
index 282b9dcf6de2b..aa140330c61bb 100644
--- a/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
+++ b/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
@@ -265,6 +265,26 @@ void ModuleShaderFlags::initialize(Module &M, DXILResourceTypeMap &DRTM,
NumUAVs += UAV.getBinding().Size;
if (NumUAVs > 8)
CombinedSFMask.Max64UAVs = true;
+
+ // Set UAVsAtEveryStage flag based on the presence of UAVs, the shader
+ // model version, and the shader environment
+ if (!DRM.uavs().empty()) {
+ if (MMDI.ValidatorVersion < VersionTuple(1, 8))
+ CombinedSFMask.UAVsAtEveryStage =
+ MMDI.ShaderProfile != Triple::EnvironmentType::Compute &&
+ MMDI.ShaderProfile != Triple::EnvironmentType::Pixel;
+ else // MMDI.ValidatorVersion >= VersionTuple(1, 8)
+ switch (MMDI.ShaderProfile) {
+ default:
+ break;
+ case Triple::EnvironmentType::Vertex:
+ case Triple::EnvironmentType::Hull:
+ case Triple::EnvironmentType::Domain:
+ case Triple::EnvironmentType::Geometry:
+ CombinedSFMask.UAVsAtEveryStage = true;
+ break;
+ }
+ }
}
void ComputedShaderFlags::print(raw_ostream &OS) const {
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-sm6_5.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-sm6_5.ll
index 5b978d67866be..b476cab236cb3 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-sm6_5.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-sm6_5.ll
@@ -29,5 +29,10 @@ define void @test() "hlsl.export" {
ret void
}
+; Set dx.valver and dx.resmayalias to prevent flags ResMayNotAlias and
+; UAVsAtEveryStage from being set, as to not distract from the flag that is
+; actually being tested
!llvm.module.flags = !{!0}
+!dx.valver = !{!1}
!0 = !{i32 1, !"dx.resmayalias", i32 1}
+!1 = !{i32 1, i32 8}
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-sm6_6.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-sm6_6.ll
index 4b901a78e6ea4..6c2b82a85188e 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-sm6_6.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-sm6_6.ll
@@ -29,5 +29,10 @@ define void @test() "hlsl.export" {
ret void
}
+; Set dx.valver and dx.resmayalias to prevent flags ResMayNotAlias and
+; UAVsAtEveryStage from being set, as to not distract from the flag that is
+; actually being tested
!llvm.module.flags = !{!0}
+!dx.valver = !{!1}
!0 = !{i32 1, !"dx.resmayalias", i32 1}
+!1 = !{i32 1, i32 8}
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs.ll
index c002ff2851452..9045f75ef33c1 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs.ll
@@ -56,5 +56,10 @@ define void @test() "hlsl.export" {
ret void
}
+; Set dx.valver and dx.resmayalias to prevent flags ResMayNotAlias and
+; UAVsAtEveryStage from being set, as to not distract from the flag that is
+; actually being tested
!llvm.module.flags = !{!0}
+!dx.valver = !{!1}
!0 = !{i32 1, !"dx.resmayalias", i32 1}
+!1 = !{i32 1, i32 8}
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-alias-0.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-alias-0.ll
index d15b5c7b61984..6996ccc59c204 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-alias-0.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-alias-0.ll
@@ -40,4 +40,9 @@ define float @loadSRV() #0 {
; But if it does, ensure that it has no effect.
!0 = !{i32 1, !"dx.resmayalias", i32 0}
+; Set dx.valver to prevent the flag UAVsAtEveryStage from being set, as to not
+; distract from the flag that is actually being tested
+!dx.valver = !{!1}
+!1 = !{i32 1, i32 8}
+
attributes #0 = { convergent norecurse nounwind "hlsl.export"}
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-alias-1.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-alias-1.ll
index edd3250a2db0d..c4f9ab498bddf 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-alias-1.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-alias-1.ll
@@ -35,7 +35,11 @@ define float @loadSRV() #0 {
}
!llvm.module.flags = !{!0}
-
!0 = !{i32 1, !"dx.resmayalias", i32 1}
+; Set dx.valver to prevent the flag UAVsAtEveryStage from being set, as to not
+; distract from the flag that is actually being tested
+!dx.valver = !{!1}
+!1 = !{i32 1, i32 8}
+
attributes #0 = { convergent norecurse nounwind "hlsl.export"}
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.6.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.6.ll
index da7c4c619790c..e2f1155b26063 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.6.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.6.ll
@@ -34,4 +34,9 @@ define float @loadSRV() #0 {
ret float %val
}
+; Set dx.valver to prevent the flag UAVsAtEveryStage from being set, as to not
+; distract from the flag that is actually being tested
+!dx.valver = !{!0}
+!0 = !{i32 1, i32 8}
+
attributes #0 = { convergent norecurse nounwind "hlsl.export"}
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.7.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.7.ll
index 87a76162f734e..c6358473ff2e4 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.7.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.7.ll
@@ -35,4 +35,9 @@ define float @loadSRV() #0 {
ret float %val
}
+; Set dx.valver to prevent the flag UAVsAtEveryStage from being set, as to not
+; distract from the flag that is actually being tested
+!dx.valver = !{!0}
+!0 = !{i32 1, i32 8}
+
attributes #0 = { convergent norecurse nounwind "hlsl.export"}
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.8.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.8.ll
index a309d8ecea56c..10eb7a878e5c4 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.8.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.8.ll
@@ -35,4 +35,9 @@ define float @loadSRV() #0 {
ret float %val
}
+; Set dx.valver to prevent the flag UAVsAtEveryStage from being set, as to not
+; distract from the flag that is actually being tested
+!dx.valver = !{!0}
+!0 = !{i32 1, i32 8}
+
attributes #0 = { convergent norecurse nounwind "hlsl.export"}
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/typed-uav-load-additional-formats.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/typed-uav-load-additional-formats.ll
index 1bb8a4d78eb16..dd42bb73e9bdd 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/typed-uav-load-additional-formats.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/typed-uav-load-additional-formats.ll
@@ -43,8 +43,13 @@ define void @noload(<4 x float> %val) #0 {
ret void
}
+; Set dx.valver and dx.resmayalias to prevent flags ResMayNotAlias and
+; UAVsAtEveryStage from being set, as to not distract from the flag that is
+; actually being tested
!llvm.module.flags = !{!0}
+!dx.valver = !{!1}
!0 = !{i32 1, !"dx.resmayalias", i32 1}
+!1 = !{i32 1, i32 8}
attributes #0 = { convergent norecurse nounwind "hlsl.export"}
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/uavs-at-every-stage-lib-valver1.7.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/uavs-at-every-stage-lib-valver1.7.ll
new file mode 100644
index 0000000000000..5db3b32deee62
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/uavs-at-every-stage-lib-valver1.7.ll
@@ -0,0 +1,39 @@
+; RUN: opt -S --passes="print-dx-shader-flags" 2>&1 %s | FileCheck %s
+; RUN: llc %s --filetype=obj -o - | obj2yaml | FileCheck %s --check-prefix=DXC
+
+; This test ensures that a library shader with a UAV gets the module and
+; shader feature flag UAVsAtEveryStage when the DXIL validator version is < 1.8
+
+target triple = "dxil-pc-shadermodel6.5-library"
+
+; CHECK: Combined Shader Flags for Module
+; CHECK-NEXT: Shader Flags Value: 0x00010000
+
+; CHECK: Note: shader requires additional functionality:
+; CHECK: UAVs at every shader stage
+
+; CHECK: Function test : 0x00000000
+define void @test() "hlsl.export" {
+ ; RWBuffer<float> Buf : register(u0, space0)
+ %buf0 = call target("dx.TypedBuffer", float, 1, 0, 1)
+ @llvm.dx.resource.handlefrombinding.tdx.TypedBuffer_f32_1_0t(
+ i32 0, i32 0, i32 1, i32 0, i1 false)
+ ret void
+}
+
+!dx.valver = !{!1}
+!1 = !{i32 1, i32 7}
+
+; Set dx.resmayalias to prevent the shader flag ResMayNotAlias from being set,
+; as to not distract from the shader flag that is actually being tested
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"dx.resmayalias", i32 1}
+
+; DXC: - Name: SFI0
+; DXC-NEXT: Size: 8
+; DXC-NEXT: Flags:
+; DXC-NOT: {{[A-Za-z]+: +true}}
+; DXC: UAVsAtEveryStage: true
+; DXC-NOT: {{[A-Za-z]+: +true}}
+; DXC: NextUnusedBit: false
+; DXC: ...
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/uavs-at-every-stage-lib-valver1.8.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/uavs-at-every-stage-lib-valver1.8.ll
new file mode 100644
index 0000000000000..98c2e833f4aee
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/uavs-at-every-stage-lib-valver1.8.ll
@@ -0,0 +1,29 @@
+; RUN: opt -S --passes="print-dx-shader-flags" 2>&1 %s | FileCheck %s
+
+; This test ensures that a library shader with a UAV does not get the module and
+; shader feature flag UAVsAtEveryStage when the DXIL validator version is >= 1.8
+
+target triple = "dxil-pc-shadermodel6.5-library"
+
+; CHECK: Combined Shader Flags for Module
+; CHECK-NEXT: Shader Flags Value: 0x00000000
+
+; CHECK-NOT: Note: shader requires additional functionality:
+; CHECK-NOT: UAVs at every shader stage
+
+; CHECK: Function test : 0x00000000
+define void @test() "hlsl.export" {
+ ; RWBuffer<float> Buf : register(u0, space0)
+ %buf0 = call target("dx.TypedBuffer", float, 1, 0, 1)
+ @llvm.dx.resource.handlefrombinding.tdx.TypedBuffer_f32_1_0t(
+ i32 0, i32 0, i32 1, i32 0, i1 false)
+ ret void
+}
+
+!dx.valver = !{!1}
+!1 = !{i32 1, i32 8}
+
+; Set dx.resmayalias to prevent the shader flag ResMayNotAlias from being set,
+; as to not distract from the shader flag that is actually being tested
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"dx.resmayalias", i32 1}
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/uavs-at-every-stage-vs.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/uavs-at-every-stage-vs.ll
new file mode 100644
index 0000000000000..4aeddf08f6a8b
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/uavs-at-every-stage-vs.ll
@@ -0,0 +1,40 @@
+; RUN: opt -S --passes="print-dx-shader-flags" 2>&1 %s | FileCheck %s
+; TODO: Remove this comment and add 'RUN' to the line below once vertex shaders are supported by llc
+; llc %s --filetype=obj -o - | obj2yaml | FileCheck %s --check-prefix=DXC
+
+; This test ensures that a Vertex shader with a UAV gets the module and
+; shader feature flag UAVsAtEveryStage
+
+target triple = "dxil-pc-shadermodel6.5-vertex"
+
+; CHECK: Combined Shader Flags for Module
+; CHECK-NEXT: Shader Flags Value: 0x00010000
+
+; CHECK: Note: shader requires additional functionality:
+; CHECK: UAVs at every shader stage
+
+; CHECK: Function test : 0x00000000
+define void @test() "hlsl.export" {
+ ; RWBuffer<float> Buf : register(u0, space0)
+ %buf0 = call target("dx.TypedBuffer", float, 1, 0, 1)
+ @llvm.dx.resource.handlefrombinding.tdx.TypedBuffer_f32_1_0t(
+ i32 0, i32 0, i32 1, i32 0, i1 false)
+ ret void
+}
+
+!dx.valver = !{!1}
+!1 = !{i32 1, i32 8}
+
+; Set dx.resmayalias to prevent the shader flag ResMayNotAlias from being set,
+; as to not distract from the shader flag that is actually being tested
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"dx.resmayalias", i32 1}
+
+; DXC: - Name: SFI0
+; DXC-NEXT: Size: 8
+; DXC-NEXT: Flags:
+; DXC-NOT: {{[A-Za-z]+: +true}}
+; DXC-NEXT: UAVsAtEveryStage: true
+; DXC-NOT: {{[A-Za-z]+: +true}}
+; DXC: NextUnusedBit: false
+; DXC: ...
|
llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-sm6_5.ll
Outdated
Show resolved
Hide resolved
llvm/test/CodeGen/DirectX/ShaderFlags/uavs-at-every-stage-lib-valver1.7.ll
Outdated
Show resolved
Hide resolved
- Move the logic for setting UAVsAtEveryStage out into its own function to not increase the length of ModuleShaderFlags::initialize - Use one switch statement to handle both special cases with regards to DXIL validator version Co-authored-by: Justin Bogner <[email protected]>
✅ With the latest revision this PR passed the C/C++ code formatter. |
case Triple::EnvironmentType::Geometry: | ||
case Triple::EnvironmentType::Hull: | ||
case Triple::EnvironmentType::Domain: | ||
return true; |
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.
Shouldnt this be only if the version is greater than 1.8, based on the issue description?
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.
If the version is less than 1.8, they would still get the UAVsAtEveryStage flag because they are not Compute or Pixel
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 think the real potential issue is if in the future we get new shader stage, the UAVsAtEveryStage flag will not get assigned to the new shader stage when the DXIL validator version is < 1.8
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.
@bogner Do you think the hasUAVsAtEveryStage
function should be this instead?
bool hasUAVsAtEveryStage(DXILResourceMap &DRM, const ModuleMetadataInfo &MMDI) {
if (DRM.uavs().empty())
return false;
switch (MMDI.ShaderProfile) {
default:
return MMDI.ValidatorVersion < VersionTuple(1, 8);
case Triple::EnvironmentType::Compute:
case Triple::EnvironmentType::Pixel:
return false;
case Triple::EnvironmentType::Vertex:
case Triple::EnvironmentType::Geometry:
case Triple::EnvironmentType::Hull:
case Triple::EnvironmentType::Domain:
return true;
}
}
This is so that we don't have to edit the switch every time a new shader stage gets added.
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.
This is one of the places where the fact that we're using the triple environment rather than a dedicated enum is kind of annoying. If we had a dedicated enum then we would get a -Wcovered-switch
warning if that came up and my answer to this question would unambiguously be no.
However, I still don't think that this would be an improvement. Consider:
- If we get another shader stage then by definition the validator version is greater than 1.8, since a new shader stage couldn't be added without bumping the DXIL version.
- Whether or not UAVsAtEveryStage should be set for the new shader stage probably depends on what that stage is, so we'd probably want to modify this anyway.
- If we have a triple that doesn't have a shader stage as its environment (ie, it isn't a
dxil-
triple at all), then calling this function doesn't make any sense. We could arguably make thedefault
casellvm_unreachable()
and just crash if that were to happen.
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/16887 Here is the relevant piece of the build log for the reference
|
…#137085) Fixes llvm#112272 In addition to the implementation of the UAVsAtEveryStage shader flag analysis, several unrelated tests have had the `dx.valver` module metadata defined to avoid setting the UAVsAtEveryStage shader flag in them. Example: ``` !dx.valver = !{!0} !0 = !{i32 1, i32 8} ``` --------- Co-authored-by: Justin Bogner <[email protected]>
…#137085) Fixes llvm#112272 In addition to the implementation of the UAVsAtEveryStage shader flag analysis, several unrelated tests have had the `dx.valver` module metadata defined to avoid setting the UAVsAtEveryStage shader flag in them. Example: ``` !dx.valver = !{!0} !0 = !{i32 1, i32 8} ``` --------- Co-authored-by: Justin Bogner <[email protected]>
…#137085) Fixes llvm#112272 In addition to the implementation of the UAVsAtEveryStage shader flag analysis, several unrelated tests have had the `dx.valver` module metadata defined to avoid setting the UAVsAtEveryStage shader flag in them. Example: ``` !dx.valver = !{!0} !0 = !{i32 1, i32 8} ``` --------- Co-authored-by: Justin Bogner <[email protected]>
…#137085) Fixes llvm#112272 In addition to the implementation of the UAVsAtEveryStage shader flag analysis, several unrelated tests have had the `dx.valver` module metadata defined to avoid setting the UAVsAtEveryStage shader flag in them. Example: ``` !dx.valver = !{!0} !0 = !{i32 1, i32 8} ``` --------- Co-authored-by: Justin Bogner <[email protected]>
…#137085) Fixes llvm#112272 In addition to the implementation of the UAVsAtEveryStage shader flag analysis, several unrelated tests have had the `dx.valver` module metadata defined to avoid setting the UAVsAtEveryStage shader flag in them. Example: ``` !dx.valver = !{!0} !0 = !{i32 1, i32 8} ``` --------- Co-authored-by: Justin Bogner <[email protected]>
Fixes #112272
In addition to the implementation of the UAVsAtEveryStage shader flag analysis, several unrelated tests have had the
dx.valver
module metadata defined to avoid setting the UAVsAtEveryStage shader flag in them.Example: