[PATCH 0/1] MR68: vkd3d-shader/hlsl: Support frac() intrinsic.

[Nikolay Sivov (＠nsivov)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=34242
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/68

[PATCH 1/1] vkd3d-shader/hlsl: Support frac() intrinsic.

[Nikolay Sivov]

From: Nikolay Sivov <nsivov(a)codeweavers.com>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=34242
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>
---
 libs/vkd3d-shader/hlsl.y     | 12 ++++++++++++
 libs/vkd3d-shader/hlsl_sm1.c |  4 ++++
 2 files changed, 16 insertions(+)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index b3e38d1f..93d4ca7d 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -2502,6 +2502,17 @@ static bool intrinsic_floor(struct hlsl_ctx *ctx,
     return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_FLOOR, arg, loc);
 }
 
+static bool intrinsic_frac(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_FRACT, arg, loc);
+}
+
 static bool intrinsic_ldexp(struct hlsl_ctx *ctx,
         const struct parse_initializer *params, const struct vkd3d_shader_location *loc)
 {
@@ -2922,6 +2933,7 @@ intrinsic_functions[] =
     {"cross",                               2, true,  intrinsic_cross},
     {"dot",                                 2, true,  intrinsic_dot},
     {"floor",                               1, true,  intrinsic_floor},
+    {"frac",                                1, true,  intrinsic_frac},
     {"ldexp",                               2, true,  intrinsic_ldexp},
     {"length",                              1, true,  intrinsic_length},
     {"lerp",                                3, true,  intrinsic_lerp},
diff --git a/libs/vkd3d-shader/hlsl_sm1.c b/libs/vkd3d-shader/hlsl_sm1.c
index ba22925e..fb505473 100644
--- a/libs/vkd3d-shader/hlsl_sm1.c
+++ b/libs/vkd3d-shader/hlsl_sm1.c
@@ -658,6 +658,10 @@ static void write_sm1_expr(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *b
             write_sm1_binary_op(ctx, buffer, D3DSIO_MUL, &instr->reg, &arg1->reg, &arg2->reg);
             break;
 
+        case HLSL_OP1_FRACT:
+            write_sm1_unary_op(ctx, buffer, D3DSIO_FRC, &instr->reg, &arg1->reg, D3DSPSM_NONE);
+            break;
+
         default:
             hlsl_fixme(ctx, &instr->loc, "SM1 \"%s\" expression.", debug_hlsl_expr_op(expr->op));
             break;
-- 
GitLab

https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/68

[Henri Verbeet (＠hverbeet)]

Should we have some tests to go along with this? For example, what is the expected value of "frac(-2.25)"? GLSL defines "fract(x)" as equivalent to "x - floor(x)"; I'm not sure that necessarily matches HLSL/D3D, and this seems a nice occasion to find out.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/68#note_21427

[Nikolay Sivov (＠nsivov)]

As far as I can tell, it's translated unconditionally to 'frc' instruction for both sm4 and older. For this specific constant, it does match "x - floor(x)", giving 0.75, with floor begin -3.0. But such optimization on constant input should be done somewhere else, or do you mean we should have a test now anyway?

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/68#note_21442

[Henri Verbeet (＠hverbeet)]

> As far as I can tell, it's translated unconditionally to 'frc' instruction for both sm4 and older. For this specific constant, it does match "x - floor(x)", giving 0.75, with floor begin -3.0. But such optimization on constant input should be done somewhere else, or do you mean we should have a test now anyway?

Right, translating this to "frc" makes sense. We're also implicitly testing the GLSL and SPIR-V backends with .shader_test tests though, and I don't think we have existing coverage for frc/frac() there.

Optimising constant inputs would happen in hlsl_fold_constant_exprs(). I think handling HLSL_OP1_FRACT should be fairly straightforward, but I don't think it's a requirement for this MR.

I do think that as a matter of principle having test coverage for the different intrinsics we implement is simply a good idea. These tests don't necessarily need to be complicated; see e.g. "sqrt.shader_test" for a straightforward example.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/68#note_21449