[DirectX] Fix shader flag version-checking logic to match DXC #136787
+92
−28
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-backend-directx Author: Deric C. (Icohedron) ChangesThis PR primarily fixes the version-checking logic of the shader flags Primary changes:
Additional changes included for consistency:
Full diff: https://github.com/llvm/llvm-project/pull/136787.diff 9 Files Affected:
diff --git a/llvm/lib/Target/DirectX/DXILShaderFlags.cpp b/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
index 282b9dcf6de2b..10bee7256f4d6 100644
--- a/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
+++ b/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
@@ -132,9 +132,10 @@ void ModuleShaderFlags::updateFunctionFlags(ComputedShaderFlags &CSF,
case Intrinsic::dx_resource_handlefrombinding: {
dxil::ResourceTypeInfo &RTI = DRTM[cast<TargetExtType>(II->getType())];
- // Set ResMayNotAlias if DXIL version >= 1.8 and function uses UAVs
+ // Set ResMayNotAlias if DXIL validator version >= 1.8 and the function
+ // uses UAVs
if (!CSF.ResMayNotAlias && CanSetResMayNotAlias &&
- MMDI.DXILVersion >= VersionTuple(1, 8) && RTI.isUAV())
+ MMDI.ValidatorVersion >= VersionTuple(1, 8) && RTI.isUAV())
CSF.ResMayNotAlias = true;
switch (RTI.getResourceKind()) {
@@ -208,15 +209,15 @@ void ModuleShaderFlags::initialize(Module &M, DXILResourceTypeMap &DRTM,
continue;
}
- // Set ResMayNotAlias to true if DXIL version < 1.8 and there are UAVs
- // present globally.
- if (CanSetResMayNotAlias && MMDI.DXILVersion < VersionTuple(1, 8))
+ // Set ResMayNotAlias to true if DXIL validator version < 1.8 and there
+ // are UAVs present globally.
+ if (CanSetResMayNotAlias && MMDI.ValidatorVersion < VersionTuple(1, 8))
SCCSF.ResMayNotAlias = !DRM.uavs().empty();
// Set UseNativeLowPrecision using dx.nativelowprec module metadata
if (auto *NativeLowPrec = mdconst::extract_or_null<ConstantInt>(
M.getModuleFlag("dx.nativelowprec")))
- if (MMDI.DXILVersion >= VersionTuple(1, 2) &&
+ if (MMDI.ShaderModelVersion >= VersionTuple(6, 2) &&
NativeLowPrec->getValue() != 0)
SCCSF.UseNativeLowPrecision = true;
@@ -259,9 +260,9 @@ void ModuleShaderFlags::initialize(Module &M, DXILResourceTypeMap &DRTM,
// Set the Max64UAVs flag if the number of UAVs is > 8
uint32_t NumUAVs = 0;
for (auto &UAV : DRM.uavs())
- if (MMDI.DXILVersion < VersionTuple(1, 6))
+ if (MMDI.ValidatorVersion < VersionTuple(1, 6))
NumUAVs++;
- else // MMDI.DXILVersion >= VersionTuple(1, 6)
+ else // MMDI.ValidatorVersion >= VersionTuple(1, 6)
NumUAVs += UAV.getBinding().Size;
if (NumUAVs > 8)
CombinedSFMask.Max64UAVs = true;
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-valver1.5.ll
similarity index 82%
rename from llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-sm6_5.ll
rename to llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-valver1.5.ll
index 5b978d67866be..a157c124f9dfc 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-sm6_5.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-valver1.5.ll
@@ -1,13 +1,13 @@
; RUN: opt -S --passes="print-dx-shader-flags" 2>&1 %s | FileCheck %s
; This test makes sure that resource arrays only add 1 to the count of the
-; number of UAVs for setting the shader flag '64 UAV slots' when the shader
-; model version is < 6.6
+; number of UAVs for setting the shader flag '64 UAV slots' when the validator
+; version is < 1.6
; Note: there is no feature flag here (only a module flag), so we don't have an
; object test.
-target triple = "dxil-pc-shadermodel6.5-library"
+target triple = "dxil-pc-shadermodel6.7-library"
; CHECK: Combined Shader Flags for Module
; CHECK-NEXT: Shader Flags Value: 0x00000000
@@ -29,5 +29,10 @@ define void @test() "hlsl.export" {
ret void
}
+; Set validator version to 1.5
+!dx.valver = !{!1}
+!1 = !{i32 1, i32 5}
+
+; Set this flag to 1 to prevent the ResMayNotAlias flag from being set
!llvm.module.flags = !{!0}
!0 = !{i32 1, !"dx.resmayalias", i32 1}
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-valver1.6.ll
similarity index 82%
rename from llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-sm6_6.ll
rename to llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-valver1.6.ll
index 4b901a78e6ea4..2b5f3edd47b12 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-sm6_6.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/max-64-uavs-array-valver1.6.ll
@@ -2,12 +2,12 @@
; This test makes sure that resource arrays sizes are accounted for when
; counting the number of UAVs for setting the shader flag '64 UAV slots' when
-; the shader model version is >= 6.6
+; the validator version is >= 1.6
; Note: there is no feature flag here (only a module flag), so we don't have an
; object test.
-target triple = "dxil-pc-shadermodel6.6-library"
+target triple = "dxil-pc-shadermodel6.7-library"
; CHECK: Combined Shader Flags for Module
; CHECK-NEXT: Shader Flags Value: 0x00008000
@@ -29,5 +29,10 @@ define void @test() "hlsl.export" {
ret void
}
+; Set validator version to 1.6
+!dx.valver = !{!1}
+!1 = !{i32 1, i32 6}
+
+; Set this flag to 1 to prevent the ResMayNotAlias flag from being set
!llvm.module.flags = !{!0}
!0 = !{i32 1, !"dx.resmayalias", i32 1}
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..77d80470d6501 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-alias-0.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-alias-0.ll
@@ -35,9 +35,11 @@ define float @loadSRV() #0 {
}
!llvm.module.flags = !{!0}
+!dx.valver = !{!1}
; dx.resmayalias should never appear with a value of 0.
; But if it does, ensure that it has no effect.
!0 = !{i32 1, !"dx.resmayalias", i32 0}
+!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-sm6.6.ll
similarity index 99%
rename from llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.6.ll
rename to llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-sm6.6.ll
index da7c4c619790c..e7828c6569912 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.6.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-sm6.6.ll
@@ -1,7 +1,7 @@
; RUN: opt -S --passes="print-dx-shader-flags" 2>&1 %s | FileCheck %s
; This test checks to ensure the behavior of the DXIL shader flag analysis
-; for the flag ResMayNotAlias is correct when the DXIL Version is 1.6. The
+; for the flag ResMayNotAlias is correct when the DXIL Version is < 1.7. The
; ResMayNotAlias flag (0x20000000) should not be set at all.
target triple = "dxil-pc-shadermodel6.6-library"
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-sm6.7.ll
similarity index 99%
rename from llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.7.ll
rename to llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-sm6.7.ll
index 87a76162f734e..da75a3dfc7cd5 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.7.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-sm6.7.ll
@@ -1,7 +1,7 @@
; RUN: opt -S --passes="print-dx-shader-flags" 2>&1 %s | FileCheck %s
; This test checks to ensure the behavior of the DXIL shader flag analysis
-; for the flag ResMayNotAlias is correct when the DXIL Version is 1.7. The
+; for the flag ResMayNotAlias is correct when the DXIL Version is >= 1.7. The
; ResMayNotAlias flag (0x20000000) should be set on all functions if there are
; one or more UAVs present globally in the module.
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-valver1.8.ll
similarity index 82%
rename from llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.8.ll
rename to llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-valver1.8.ll
index a309d8ecea56c..a5be9f5bc69a9 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-shadermodel6.8.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/res-may-not-alias-valver1.8.ll
@@ -1,11 +1,11 @@
; RUN: opt -S --passes="print-dx-shader-flags" 2>&1 %s | FileCheck %s
-; This test checks to ensure the behavior of the DXIL shader flag analysis
-; for the flag ResMayNotAlias is correct when the DXIL Version is 1.8. The
-; ResMayNotAlias flag (0x20000000) should only be set when a function uses a
+; This test checks to ensure the behavior of the DXIL shader flag analysis for
+; the flag ResMayNotAlias is correct when the DXIL validator version is >= 1.8.
+; The ResMayNotAlias flag (0x20000000) should only be set when a function uses a
; UAV.
-target triple = "dxil-pc-shadermodel6.8-library"
+target triple = "dxil-pc-shadermodel6.7-library"
; CHECK: Combined Shader Flags for Module
; CHECK-NEXT: Shader Flags Value: 0x200000010
@@ -35,4 +35,7 @@ define float @loadSRV() #0 {
ret float %val
}
+!dx.valver = !{!1}
+!1 = !{i32 1, i32 8}
+
attributes #0 = { convergent norecurse nounwind "hlsl.export"}
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/use-native-low-precision-0.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/use-native-low-precision-0.ll
new file mode 100644
index 0000000000000..4c9040706189d
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/use-native-low-precision-0.ll
@@ -0,0 +1,37 @@
+; RUN: opt -S --passes="print-dx-shader-flags" 2>&1 %s | FileCheck %s
+
+; Check that when the dx.nativelowprec module flag is set to 0, the module-level
+; shader flag UseNativeLowPrecision is not set
+
+target triple = "dxil-pc-shadermodel6.7-library"
+
+;CHECK: ; Combined Shader Flags for Module
+;CHECK-NEXT: ; Shader Flags Value: 0x00000020
+;CHECK-NEXT: ;
+;CHECK-NEXT: ; Note: shader requires additional functionality:
+;CHECK-NEXT: ; Note: extra DXIL module flags:
+;CHECK-NEXT: ; D3D11_1_SB_GLOBAL_FLAG_ENABLE_MINIMUM_PRECISION
+;CHECK-NOT: ; Native 16bit types enabled
+;CHECK-NEXT: ;
+;CHECK-NEXT: ; Shader Flags for Module Functions
+
+;CHECK-LABEL: ; Function add_i16 : 0x00000020
+define i16 @add_i16(i16 %a, i16 %b) {
+ %sum = add i16 %a, %b
+ ret i16 %sum
+}
+
+;CHECK-LABEL: ; Function add_i32 : 0x00000000
+define i32 @add_i32(i32 %a, i32 %b) {
+ %sum = add i32 %a, %b
+ ret i32 %sum
+}
+
+;CHECK-LABEL: ; Function add_half : 0x00000020
+define half @add_half(half %a, half %b) {
+ %sum = fadd half %a, %b
+ ret half %sum
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"dx.nativelowprec", i32 0}
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/use-native-low-precision.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/use-native-low-precision-1.ll
similarity index 100%
rename from llvm/test/CodeGen/DirectX/ShaderFlags/use-native-low-precision.ll
rename to llvm/test/CodeGen/DirectX/ShaderFlags/use-native-low-precision-1.ll
|
|
Can you link to the updated docs here that specify the required logic? I think you had a pr up somewhere for that. Edit: aha here it is microsoft/DirectXShaderCompiler#7295. Is this blocked on that pr getting approved and merged? |
|
No, the PRs are not blocking each other. They are both supposed to reflect how DXC works as currently written in the source code. |
inbelic
approved these changes
Apr 23, 2025
farzonl
approved these changes
Apr 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR primarily fixes the version-checking logic of the shader flags
ResMayNotAliasandMax64UAVsto correctly match DXC's behavior.Primary changes:
The logic for determining the presence of UAVs for the
ResMayNotAliasshader flag checked against the DXIL Version when it should have been checking against the DXIL Validator Version. (See DXC: DxilShaderFlags.cpp#L484)The logic for counting UAVs for the
Max64UAVsshader flag checked against the DXIL Version when it should have been checking against the DXIL Validator Version. (See DXC: DxilModule.cpp#L327)Tests have been modified to test the corrected behaviors for these two flags
Additional changes included for consistency:
The logic for setting
UseNativeLowPrecisionnow checks against Shader Model version instead of DXIL version to be consistent with the code comments from DXC (DxilShaderFlags.h#L280).An additional test has been added to ensure that the module flag "dx.nativelowprec" set to 0 does not apply the
UseNativeLowPrecisionshader flagRelated shader flag tests were renamed to be more consistent, and some comments were edited for clarification
Add obj2yaml tests for the
Max64UAVsshader flag