[PATCH v4 0/2] MR157: vkd3d-shader/hlsl: Add support for any() intrinsic.
For now, this is limited to float and bool, scalar and vector. All other types are unsupported. Fixes https://bugs.winehq.org/show_bug.cgi?id=54777 -- v4: tests: Add tests for any() intrinsic. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/157
From: Ethan Lee <flibitijibibo(a)gmail.com> For now, this is limited to float and bool, scalar and vector. All other types are unsupported. Signed-off-by: Ethan Lee <flibitijibibo(a)gmail.com> --- libs/vkd3d-shader/hlsl.y | 51 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 0ddae6ee..9ecdd37f 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -2434,6 +2434,56 @@ static bool intrinsic_all(struct hlsl_ctx *ctx, return !!add_binary_comparison_expr(ctx, params->instrs, HLSL_OP2_NEQUAL, mul, &zero->node, loc); } +static bool intrinsic_any(struct hlsl_ctx *ctx, + const struct parse_initializer *params, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *arg = params->args[0], *dot, *or; + struct hlsl_ir_constant *zero, *bfalse; + struct hlsl_ir_load *load; + unsigned int i, count; + + if (arg->data_type->class != HLSL_CLASS_VECTOR && arg->data_type->class != HLSL_CLASS_SCALAR) + { + hlsl_fixme(ctx, loc, "any() implementation for non-vector, non-scalar"); + return false; + } + + if (arg->data_type->base_type == HLSL_TYPE_FLOAT) + { + if (!(zero = hlsl_new_float_constant(ctx, 0.0f, loc))) + return false; + list_add_tail(params->instrs, &zero->node.entry); + + if (!(dot = add_binary_dot_expr(ctx, params->instrs, arg, arg, loc))) + return false; + + return !!add_binary_comparison_expr(ctx, params->instrs, HLSL_OP2_NEQUAL, dot, &zero->node, loc); + } + else if (arg->data_type->base_type == HLSL_TYPE_BOOL) + { + if (!(bfalse = hlsl_new_bool_constant(ctx, false, loc))) + return false; + list_add_tail(params->instrs, &bfalse->node.entry); + + or = &bfalse->node; + + count = hlsl_type_component_count(arg->data_type); + for (i = 0; i < count; ++i) + { + if (!(load = add_load_component(ctx, params->instrs, arg, i, loc))) + return false; + + if (!(or = add_binary_bitwise_expr(ctx, params->instrs, HLSL_OP2_BIT_OR, or, &load->node, loc))) + return false; + } + + return true; + } + + hlsl_fixme(ctx, loc, "any() implementation for non-float, non-bool"); + return false; +} + /* Find the type corresponding to the given source type, with the same * dimensions but a different base type. */ static struct hlsl_type *convert_numeric_type(const struct hlsl_ctx *ctx, @@ -3290,6 +3340,7 @@ intrinsic_functions[] = /* Note: these entries should be kept in alphabetical order. */ {"abs", 1, true, intrinsic_abs}, {"all", 1, true, intrinsic_all}, + {"any", 1, true, intrinsic_any}, {"asuint", -1, true, intrinsic_asuint}, {"clamp", 3, true, intrinsic_clamp}, {"cos", 1, true, intrinsic_cos}, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/157
From: Ethan Lee <flibitijibibo(a)gmail.com> Currently only tests float and bool, scalar and vector. Signed-off-by: Ethan Lee <flibitijibibo(a)gmail.com> --- Makefile.am | 1 + tests/any.shader_test | 96 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 tests/any.shader_test diff --git a/Makefile.am b/Makefile.am index bff65a85..8cbd9662 100644 --- a/Makefile.am +++ b/Makefile.am @@ -44,6 +44,7 @@ vkd3d_cross_tests = \ vkd3d_shader_tests = \ tests/abs.shader_test \ tests/all.shader_test \ + tests/any.shader_test \ tests/arithmetic-float.shader_test \ tests/arithmetic-float-uniform.shader_test \ tests/arithmetic-int.shader_test \ diff --git a/tests/any.shader_test b/tests/any.shader_test new file mode 100644 index 00000000..f2298d3a --- /dev/null +++ b/tests/any.shader_test @@ -0,0 +1,96 @@ +[pixel shader] +uniform float4 f; + +float4 main() : sv_target +{ + return any(f); +} + +[test] +uniform 0 float4 1.0 1.0 1.0 1.0 +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) +uniform 0 float4 1.0 0.0 0.0 0.0 +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) +uniform 0 float4 0.0 1.0 0.0 0.0 +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) +uniform 0 float4 0.0 0.0 1.0 0.0 +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) +uniform 0 float4 0.0 0.0 0.0 1.0 +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) +uniform 0 float4 0.0 0.0 0.0 0.0 +draw quad +probe all rgba (0.0, 0.0, 0.0, 0.0) +uniform 0 float4 -1.0 -1.0 -1.0 -1.0 +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) + +[pixel shader] +uniform float f; + +float4 main() : sv_target +{ + return any(f); +} + +[test] +uniform 0 float4 1.0 0.0 0.0 0.0 +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) +uniform 0 float4 0.0 0.0 0.0 0.0 +draw quad +probe all rgba (0.0, 0.0, 0.0, 0.0) +uniform 0 float4 -1.0 0.0 0.0 0.0 +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) + +[require] +shader model >= 4.0 + +[pixel shader] +uniform uint4 b; + +float4 main() : sv_target +{ + return any((bool4)b); +} + +[test] +uniform 0 uint4 1 1 1 1 +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) +uniform 0 uint4 1 0 0 0 +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) +uniform 0 uint4 0 1 0 0 +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) +uniform 0 uint4 0 0 1 0 +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) +uniform 0 uint4 0 0 0 1 +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) +uniform 0 uint4 0 0 0 0 +draw quad +probe all rgba (0.0, 0.0, 0.0, 0.0) + +[pixel shader] +uniform uint b; + +float4 main() : sv_target +{ + return any((bool)b); +} + +[test] +uniform 0 uint4 1 0 0 0 +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) +uniform 0 uint4 0 0 0 0 +draw quad +probe all rgba (0.0, 0.0, 0.0, 0.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/157
On Fri Apr 14 09:21:58 2023 +0000, Giovanni Mascellani wrote:
This is technically an ABI violation, because `true` is represented in SM4 as `0xffffffff`, not `1`. In practice it shouldn't matter, but given that it's not that hard could you change the shader to ``` uniform uint4 b; float4 main() : sv_target { return any((bool4)b); } ``` and similar for the scalar case? Done!
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/157#note_30093
This is technically an ABI violation, because `true` is represented in SM4 as `0xffffffff`, not `1`.
Is that actually the case? True is ~0u internally while in the shader, but do we know that it's required to pass it that way *to* the shader, or does the shader normalize it? It's hard to find any documentation for this... -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/157#note_30107
On Fri Apr 14 18:05:45 2023 +0000, Zebediah Figura wrote:
This is technically an ABI violation, because `true` is represented in SM4 as `0xffffffff`, not `1`. Is that actually the case? True is ~0u internally while in the shader, but do we know that it's required to pass it that way *to* the shader, or does the shader normalize it? It's hard to find any documentation for this... Sounds like that needs tests (in a separate MR of course).
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/157#note_30118
Sounds like that needs tests (in a separate MR of course).
It seems like native always compares to zero (I can get it to use movc and ine), but I'm not particularly sure how to write a convincing test for this? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/157#note_30119
On Fri Apr 14 18:56:50 2023 +0000, Zebediah Figura wrote:
Sounds like that needs tests (in a separate MR of course). It seems like native always compares to zero (I can get it to use movc and ine), but I'm not particularly sure how to write a convincing test for this? That's with the uniform declared as bool I guess? I can't think of any better test right now but it does sound like normalization is happening.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/157#note_30120
On Fri Apr 14 19:00:33 2023 +0000, Matteo Bruni wrote:
That's with the uniform declared as bool I guess? I can't think of any better test right now but it does sound like normalization is happening. Yes. I'll see if I can write something for this. If nothing else I suspect it'll prove we're doing something wrong.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/157#note_30121
On Fri Apr 14 19:02:09 2023 +0000, Zebediah Figura wrote:
Yes. I'll see if I can write something for this. If nothing else I suspect it'll prove we're doing something wrong. I tried a few examples:
``` bool4 main(int4 x : color) : SV_TARGET { return x; } ``` compiles to ``` ps_4_0 dcl_input_ps constant v0.xyzw dcl_output o0.xyzw ine o0.xyzw, v0.xyzw, l(0, 0, 0, 0) ret ``` (https://shader-playground.timjones.io/63ec9c244edae1d822a1daf5fbc79d40) So it seems that the native compiler feels an obligation to generate an actual 0xffffffff for a true, otherwise it could have just moved v0 to o0. ``` bool4 main(bool4 x : color) : SV_TARGET { return x; } ``` compiles to ``` ps_4_0 dcl_input_ps constant v0.xyzw dcl_output o0.xyzw mov o0.xyzw, v0.xyzw ret ``` (https://shader-playground.timjones.io/1647c611f02ea54b06828cae9a668fac) It seems that the native compiler trusts that a true bool is represented as 0xffffffff, otherwise it couldn't satisfy the obligation it seems to have from the test above. ``` int4 main(bool4 x : color) : SV_TARGET { return x; } ``` compiles to ``` ps_4_0 dcl_input_ps constant v0.xyzw dcl_output o0.xyzw and o0.xyzw, v0.xyzw, l(1, 1, 1, 1) ret ``` (https://shader-playground.timjones.io/c9b0ea8dd59ebb5bfe44b326816f527c) This is possibly the most convincing test: if the compiler didn't have the guarantee that true's are always 0xffffffff this would be miscompiled. So it seems that representing true as 0xffffffff is a requirement at the boundary of the shader, in both directions. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/157#note_30218
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/157
On Mon Apr 17 09:07:40 2023 +0000, Giovanni Mascellani wrote:
I tried a few examples: ``` bool4 main(int4 x : color) : SV_TARGET { return x; } ``` compiles to ``` ps_4_0 dcl_input_ps constant v0.xyzw dcl_output o0.xyzw ine o0.xyzw, v0.xyzw, l(0, 0, 0, 0) ret ``` (https://shader-playground.timjones.io/63ec9c244edae1d822a1daf5fbc79d40) So it seems that the native compiler feels an obligation to generate an actual 0xffffffff for a true, otherwise it could have just moved v0 to o0. ``` bool4 main(bool4 x : color) : SV_TARGET { return x; } ``` compiles to ``` ps_4_0 dcl_input_ps constant v0.xyzw dcl_output o0.xyzw mov o0.xyzw, v0.xyzw ret ``` (https://shader-playground.timjones.io/1647c611f02ea54b06828cae9a668fac) It seems that the native compiler trusts that a true bool is represented as 0xffffffff, otherwise it couldn't satisfy the obligation it seems to have from the test above. ``` int4 main(bool4 x : color) : SV_TARGET { return x; } ``` compiles to ``` ps_4_0 dcl_input_ps constant v0.xyzw dcl_output o0.xyzw and o0.xyzw, v0.xyzw, l(1, 1, 1, 1) ret ``` (https://shader-playground.timjones.io/c9b0ea8dd59ebb5bfe44b326816f527c) This is possibly the most convincing test: if the compiler didn't have the guarantee that true's are always 0xffffffff this would be miscompiled. So it seems that representing true as 0xffffffff is a requirement at the boundary of the shader, in both directions. That's a semantic, though. If you use a uniform instead, the code seems to change, to stop assuming that true is ~0. The same result happens for vs input semantics. This makes sense, given that a ps semantic would have been generated not by the user but by the vs.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/157#note_30257
On Mon Apr 17 19:35:45 2023 +0000, Zebediah Figura wrote:
That's a semantic, though. If you use a uniform instead, the code seems to change, to stop assuming that true is ~0. The same result happens for vs input semantics. This makes sense, given that a ps semantic would have been generated not by the user but by the vs. Ha, you're right, interesting. Then I retire my comment. On the contrary, we should probably make sure that we do the same, at some point. But that's for another MR. Or maybe we already do that?
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/157#note_30336
This will need a rebase because it conflicts with an earlier MR. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/157#note_30345
participants (6)
-
Ethan Lee -
Ethan Lee (＠flibitijibibo)
-
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)
-
Matteo Bruni (＠Mystral)
-
Zebediah Figura (＠zfigura)