[PATCH 0/1] MR368: vkd3d-shader: Add constant folding for 'rsq'.

[Nikolay Sivov (＠nsivov)]

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>

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

[PATCH 1/1] vkd3d-shader: Add constant folding for 'rsq'.

[Nikolay Sivov]

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

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>
---
 libs/vkd3d-shader/hlsl_constant_ops.c | 53 +++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/libs/vkd3d-shader/hlsl_constant_ops.c b/libs/vkd3d-shader/hlsl_constant_ops.c
index cff0ba31..f51cbd23 100644
--- a/libs/vkd3d-shader/hlsl_constant_ops.c
+++ b/libs/vkd3d-shader/hlsl_constant_ops.c
@@ -379,6 +379,55 @@ static bool fold_rcp(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst, cons
     return true;
 }
 
+static bool fold_rsq(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst, const struct hlsl_type *dst_type,
+        const struct hlsl_ir_constant *src, const struct vkd3d_shader_location *loc)
+{
+    enum hlsl_base_type type = dst_type->base_type;
+    unsigned int k;
+
+    assert(type == src->node.data_type->base_type);
+
+    for (k = 0; k < dst_type->dimx; ++k)
+    {
+        switch (type)
+        {
+            case HLSL_TYPE_FLOAT:
+            case HLSL_TYPE_HALF:
+                if (ctx->profile->major_version >= 4 && src->value.u[k].f < 0.0f)
+                {
+                    if (src->value.u[k].f < 0.0f)
+                        hlsl_warning(ctx, loc, VKD3D_SHADER_WARNING_HLSL_IMAGINARY_NUMERIC_RESULT,
+                                "Imaginary square root result.");
+                    else if (src->value.u[k].f == 0.0f)
+                        hlsl_warning(ctx, loc, VKD3D_SHADER_WARNING_HLSL_IMAGINARY_NUMERIC_RESULT,
+                                "Floating point division by zero.");
+                }
+                dst->u[k].f = 1.0f / sqrtf(src->value.u[k].f);
+                if (ctx->profile->major_version < 4 && !isfinite(dst->u[k].f))
+                {
+                    hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_NON_FINITE_RESULT,
+                            "Infinities and NaNs are not allowed by the shader model.");
+                }
+                break;
+
+            case HLSL_TYPE_DOUBLE:
+                if (src->value.u[k].d < 0.0)
+                {
+                    hlsl_warning(ctx, loc, VKD3D_SHADER_WARNING_HLSL_IMAGINARY_NUMERIC_RESULT,
+                            "Imaginary square root result.");
+                }
+                dst->u[k].d = 1.0 / sqrt(src->value.u[k].d);
+                break;
+
+            default:
+                FIXME("Fold 'rsq' for type %s.\n", debug_hlsl_type(ctx, dst_type));
+                return false;
+        }
+    }
+
+    return true;
+}
+
 static bool fold_sat(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst, const struct hlsl_type *dst_type,
         const struct hlsl_ir_constant *src)
 {
@@ -1095,6 +1144,10 @@ bool hlsl_fold_constant_exprs(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr,
             success = fold_rcp(ctx, &res, instr->data_type, arg1, &instr->loc);
             break;
 
+        case HLSL_OP1_RSQ:
+            success = fold_rsq(ctx, &res, instr->data_type, arg1, &instr->loc);
+            break;
+
         case HLSL_OP1_SAT:
             success = fold_sat(ctx, &res, instr->data_type, arg1);
             break;
-- 
GitLab

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

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl_constant_ops.c:
> +static bool fold_rsq(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst, const struct hlsl_type *dst_type,
> +        const struct hlsl_ir_constant *src, const struct vkd3d_shader_location *loc)
> +{
> +    enum hlsl_base_type type = dst_type->base_type;
> +    unsigned int k;
> +
> +    assert(type == src->node.data_type->base_type);
> +
> +    for (k = 0; k < dst_type->dimx; ++k)
> +    {
> +        switch (type)
> +        {
> +            case HLSL_TYPE_FLOAT:
> +            case HLSL_TYPE_HALF:
> +                if (ctx->profile->major_version >= 4 && src->value.u[k].f < 0.0f)

The second half looks wrong, the same checks are done inside the block.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/368#note_46934

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl_constant_ops.c:
> +                                "Floating point division by zero.");
> +                }
> +                dst->u[k].f = 1.0f / sqrtf(src->value.u[k].f);
> +                if (ctx->profile->major_version < 4 && !isfinite(dst->u[k].f))
> +                {
> +                    hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_NON_FINITE_RESULT,
> +                            "Infinities and NaNs are not allowed by the shader model.");
> +                }
> +                break;
> +
> +            case HLSL_TYPE_DOUBLE:
> +                if (src->value.u[k].d < 0.0)
> +                {
> +                    hlsl_warning(ctx, loc, VKD3D_SHADER_WARNING_HLSL_IMAGINARY_NUMERIC_RESULT,
> +                            "Imaginary square root result.");
> +                }
> Why there is no warning for zero in this case? BTW, did you manage to actually have native execute `rsqrt()` on a `double`? If I try something trivial as
float4 main(uint4 x : COLOR) : sv_target
{
    double y = 0.0;
    return rsqrt(y);
}

the warnings seem to imply that `y` is casted to `float` before being given to `rsqrt()`.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/368#note_46935

[Nikolay Sivov (＠nsivov)]

On Wed Sep 27 10:45:45 2023 +0000, Giovanni Mascellani wrote:
> Why there is no warning for zero in this case? BTW, did you manage to
> actually have native execute `rsqrt()` on a `double`? If I try something
> trivial as
> ```hlsl
> float4 main(uint4 x : COLOR) : sv_target
> {
>     double y = 0.0;
>     return rsqrt(y);
> }
> ```
> the warnings seem to imply that `y` is casted to `float` before being
> given to `rsqrt()`.
> Yes, it's better to remove anything but float/half. I don't see how to feed double to it. It's always converted.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/368#note_46941