[PATCH v2 0/2] MR76: vkd3d-shader/hlsl: Support rsqrt() and distance() intrinsics.
-- v2: vkd3d-shader/hlsl: Support distance() intrinsic. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/76
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 12 ++++++++++++ tests/sqrt.shader_test | 13 +++++++++++++ 2 files changed, 25 insertions(+) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index a4de0edd..71eedfa5 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -2778,6 +2778,17 @@ static bool intrinsic_round(struct hlsl_ctx *ctx, return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_ROUND, arg, loc); } +static bool intrinsic_rsqrt(struct hlsl_ctx *ctx, + const struct parse_initializer *params, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *arg; + + if (!(arg = intrinsic_float_convert_arg(ctx, params, params->args[0], loc))) + return false; + + return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_RSQ, arg, loc); +} + static bool intrinsic_saturate(struct hlsl_ctx *ctx, const struct parse_initializer *params, const struct vkd3d_shader_location *loc) { @@ -2976,6 +2987,7 @@ intrinsic_functions[] = {"normalize", 1, true, intrinsic_normalize}, {"pow", 2, true, intrinsic_pow}, {"round", 1, true, intrinsic_round}, + {"rsqrt", 1, true, intrinsic_rsqrt}, {"saturate", 1, true, intrinsic_saturate}, {"sin", 1, true, intrinsic_sin}, {"smoothstep", 3, true, intrinsic_smoothstep}, diff --git a/tests/sqrt.shader_test b/tests/sqrt.shader_test index 5d048b4f..78d89d38 100644 --- a/tests/sqrt.shader_test +++ b/tests/sqrt.shader_test @@ -10,3 +10,16 @@ float4 main() : sv_target uniform 0 float4 1.0 9.0 32.3 46.5 draw quad probe all rgba (1.0, 3.0, 5.683309, 6.819091) 1 + +[pixel shader] +uniform float4 f; + +float4 main() : sv_target +{ + return rsqrt(f); +} + +[test] +uniform 0 float4 1.0 9.0 4.0 16.0 +draw quad +probe all rgba (1.0, 0.33333333, 0.5, 0.25) 1 -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/76
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- Makefile.am | 1 + libs/vkd3d-shader/hlsl.y | 21 +++++++++++++++++++++ tests/distance.shader_test | 14 ++++++++++++++ 3 files changed, 36 insertions(+) create mode 100644 tests/distance.shader_test diff --git a/Makefile.am b/Makefile.am index a2625a5c..c45f2c4a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -59,6 +59,7 @@ vkd3d_shader_tests = \ tests/cbuffer.shader_test \ tests/compute.shader_test \ tests/conditional.shader_test \ + tests/distance.shader_test \ tests/exp.shader_test \ tests/floor.shader_test \ tests/frac.shader_test \ diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 71eedfa5..213322dc 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -2485,6 +2485,26 @@ static bool intrinsic_cross(struct hlsl_ctx *ctx, return !!add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_ADD, mul2, mul1_neg, loc); } +static bool intrinsic_distance(struct hlsl_ctx *ctx, + const struct parse_initializer *params, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *neg, *add, *dot; + + if (!elementwise_intrinsic_float_convert_args(ctx, params, loc)) + return false; + + if (!(neg = add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_NEG, params->args[1], loc))) + return false; + + if (!(add = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_ADD, params->args[0], neg, loc))) + return false; + + if (!(dot = add_binary_dot_expr(ctx, params->instrs, add, add, loc))) + return false; + + return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_SQRT, dot, loc); +} + static bool intrinsic_dot(struct hlsl_ctx *ctx, const struct parse_initializer *params, const struct vkd3d_shader_location *loc) { @@ -2973,6 +2993,7 @@ intrinsic_functions[] = {"clamp", 3, true, intrinsic_clamp}, {"cos", 1, true, intrinsic_cos}, {"cross", 2, true, intrinsic_cross}, + {"distance", 2, true, intrinsic_distance}, {"dot", 2, true, intrinsic_dot}, {"exp", 1, true, intrinsic_exp}, {"exp2", 1, true, intrinsic_exp2}, diff --git a/tests/distance.shader_test b/tests/distance.shader_test new file mode 100644 index 00000000..39c23eaf --- /dev/null +++ b/tests/distance.shader_test @@ -0,0 +1,14 @@ +[pixel shader] +uniform float4 x; +uniform float4 y; + +float4 main() : sv_target +{ + return distance(x, y); +} + +[test] +uniform 0 float4 -2.0 3.0 4.0 0.1 +uniform 1 float4 2.0 -1.0 4.0 5.0 +draw quad +probe all rgba (8.3666, 8.3666, 8.3666, 8.3666) 1 -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/76
On Mon Jan 30 11:51:29 2023 +0000, Nikolay Sivov wrote:
Also, please add some tests for `distance()` too. Ideally every intrinsic should have at least a basic smoke test. Sure, I'll add something. This is blocked by a missing dot() actually, so it should get in first.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/76#note_22303
On Mon Jan 30 11:58:11 2023 +0000, Giovanni Mascellani wrote:
Those `.shader_test` files are meant to test both the compiler and the executor. So it's useful to have something that tests what happen to `sqrt(-1)`, in order to ensure that to conversion to SPIR-V and then the execution through a Vulkan driver does what the shader expects. What Giovanni said, although it's kind of a mix of both reasons. We don't want to match native compiler output exactly, but it's also useful to test the lower level compilers (for our purposes, dxbc -> spirv/glsl, but Mesa has also expressed interest in running our tests.)
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/76#note_22331
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/hlsl.y:
return !!add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_ADD, mul2, mul1_neg, loc); }
+static bool intrinsic_distance(struct hlsl_ctx *ctx, + const struct parse_initializer *params, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *neg, *add, *dot; + + if (!elementwise_intrinsic_float_convert_args(ctx, params, loc)) + return false;
`distance()` seems to not be subjected to the type conversion rules of element-wise intrinsics (which makes sense, because it isn't one). For instance, using `distance(a, b)` with `a : float4x2` and `b : float2x4` isn't allowed. It probably makes sense to remove these lines because the `add_binary_arithmetic_expr()` call should be taking care of converting to the right type checks, already. Still, I think it would be good to add tests for some combinations of matrices, vectors, and scalar, to check that this is indeed the case. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/76#note_22500
participants (4)
-
Francisco Casas (＠fcasas) -
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)