A couple of follow-ups to !774.
From: Giovanni Mascellani gmascellani@codeweavers.com
--- tests/hlsl/discard.shader_test | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/tests/hlsl/discard.shader_test b/tests/hlsl/discard.shader_test index cf53bf482..69272b587 100644 --- a/tests/hlsl/discard.shader_test +++ b/tests/hlsl/discard.shader_test @@ -19,12 +19,6 @@ probe all rgba (1, 2, 3, 4) [require] shader model >= 5.0
-[uav 0] -format r32 float -size (2d, 2, 1) - -0.0 0.0 - [uav 1] format r32 float size (2d, 2, 1) @@ -34,7 +28,7 @@ size (2d, 2, 1) % Check that side effects stop happening after discard [pixel shader] uniform float4 x; -RWTexture2D<float> y; +RWTexture2D<float> y : register(u1);
float4 main(float4 pos : sv_position) : sv_target { @@ -51,14 +45,10 @@ float4 main(float4 pos : sv_position) : sv_target uniform 0 float4 1 2 3 4 todo(glsl) draw quad probe all rgba (1, 2, 3, 4) -if(sm<6) probe uav 1 (0, 0) r (1.0) -if(sm<6) probe uav 1 (1, 0) r (1.0) -if(sm>=6) probe uav 0 (0, 0) r (1.0) -if(sm>=6) probe uav 0 (1, 0) r (1.0) +probe uav 1 (0, 0) r (1.0) +probe uav 1 (1, 0) r (1.0) uniform 0 float4 9 8 7 6 todo(glsl) draw quad probe all rgba (1, 2, 3, 4) -if(sm<6) probe uav 1 (0, 0) r (2.0) -if(sm<6) probe uav 1 (1, 0) r (1.0) -if(sm>=6) probe uav 0 (0, 0) r (2.0) -if(sm>=6) probe uav 0 (1, 0) r (1.0) +probe uav 1 (0, 0) r (2.0) +probe uav 1 (1, 0) r (1.0)
From: Giovanni Mascellani gmascellani@codeweavers.com
--- tests/hlsl/discard.shader_test | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/tests/hlsl/discard.shader_test b/tests/hlsl/discard.shader_test index 69272b587..152a3a0d3 100644 --- a/tests/hlsl/discard.shader_test +++ b/tests/hlsl/discard.shader_test @@ -16,6 +16,30 @@ uniform 0 float4 9 8 7 6 todo(sm<4 | glsl) draw quad probe all rgba (1, 2, 3, 4)
+[require] +shader model >= 3.0 + +% Check that derivatives are still computed after discarding +% other pixels in the same quad +[pixel shader todo(sm<4)] +float4 main(float4 pos : sv_position) : sv_target +{ + if (frac((floor(pos.x) + floor(pos.y)) / 2) == 0.5) + discard; + return float4(ddx(pos.x), ddx(pos.y), ddy(pos.x), ddy(pos.y)); +} + +[test] +todo(sm<4 | glsl) draw quad +probe (0, 0) rgba (1, 0, 0, 1) +probe (1, 0) rgba (1, 2, 3, 4) +probe (0, 1) rgba (1, 2, 3, 4) +probe (1, 1) rgba (1, 0, 0, 1) +probe (2, 0) rgba (1, 0, 0, 1) +probe (3, 0) rgba (1, 2, 3, 4) +probe (2, 1) rgba (1, 2, 3, 4) +probe (3, 1) rgba (1, 0, 0, 1) + [require] shader model >= 5.0
This merge request was approved by Giovanni Mascellani.
Is the test working as intended? I would have expected this to require enabling the VK_EXT_shader_demote_to_helper_invocation for the Vulkan runner, as well as passing VKD3D_SHADER_SPIRV_EXTENSION_EXT_DEMOTE_TO_HELPER_INVOCATION in the SPIR-V target info for vkd3d_shader_compile().
On Mon Apr 15 14:01:04 2024 +0000, Henri Verbeet wrote:
Is the test working as intended? I would have expected this to require enabling the VK_EXT_shader_demote_to_helper_invocation for the Vulkan runner, as well as passing VKD3D_SHADER_SPIRV_EXTENSION_EXT_DEMOTE_TO_HELPER_INVOCATION in the SPIR-V target info for vkd3d_shader_compile().
I can see that the generates shaders have `OpKill` or `OpDemoteToHelperInvocationEXT` depending on whether they're on the Vulkan or D3D12 runner, but in practice the effect seems to be the same on all the drivers I tested. It doesn't look like that the SPIR-V specs tells a lot about what is ought to happen when derivatives are computer after `OpKill`, so I assume it's unspecified and drivers treat that as `OpDemoteToHelperInvocationEXT` anyway.
I can see that the generates shaders have `OpKill` or `OpDemoteToHelperInvocationEXT` depending on whether they're on the Vulkan or D3D12 runner, but in practice the effect seems to be the same on all the drivers I tested. It doesn't look like that the SPIR-V specs tells a lot about what is ought to happen when derivatives are computer after `OpKill`, so I assume it's unspecified and drivers treat that as `OpDemoteToHelperInvocationEXT` anyway.
I suppose, which is perhaps unfortunate. We should probably enable the extension in any case for the Vulkan runner to get defined behaviour though.