[PATCH 0/2] MR265: vkd3d-shader/hlsl: Add some more constant folding operations.

[Nikolay Sivov (＠nsivov)]

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

[PATCH 1/2] vkd3d-shader/hlsl: Add constant folding for 'sqrt'.

[Nikolay Sivov]

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

---
 libs/vkd3d-shader/hlsl_constant_ops.c    | 49 ++++++++++++++++++++++++
 libs/vkd3d-shader/vkd3d_shader_private.h |  2 +
 2 files changed, 51 insertions(+)

diff --git a/libs/vkd3d-shader/hlsl_constant_ops.c b/libs/vkd3d-shader/hlsl_constant_ops.c
index 01c438ae..aec5fae6 100644
--- a/libs/vkd3d-shader/hlsl_constant_ops.c
+++ b/libs/vkd3d-shader/hlsl_constant_ops.c
@@ -231,6 +231,51 @@ static bool fold_rcp(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst, cons
     return true;
 }
 
+static bool fold_sqrt(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 < 4; ++k)
+    {
+        switch (type)
+        {
+            case HLSL_TYPE_FLOAT:
+            case HLSL_TYPE_HALF:
+                if (ctx->profile->major_version >= 4 && src->value.u[k].f < 0.0f)
+                {
+                    hlsl_warning(ctx, loc, VKD3D_SHADER_WARNING_HLSL_IMAGINARY_NUMERIC_RESULT,
+                            "Imaginary square root result.");
+                }
+                dst->u[k].f = 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 = sqrt(src->value.u[k].d);
+                break;
+
+            default:
+                FIXME("Fold 'sqrt' for type %s.\n", debug_hlsl_type(ctx, dst_type));
+                return false;
+        }
+    }
+
+    return true;
+}
+
 static bool fold_add(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst, const struct hlsl_type *dst_type,
         const struct hlsl_ir_constant *src1, const struct hlsl_ir_constant *src2)
 {
@@ -770,6 +815,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_SQRT:
+            success = fold_sqrt(ctx, &res, instr->data_type, arg1, &instr->loc);
+            break;
+
         case HLSL_OP2_ADD:
             success = fold_add(ctx, &res, instr->data_type, arg1, arg2);
             break;
diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h
index 85fca964..655d1a97 100644
--- a/libs/vkd3d-shader/vkd3d_shader_private.h
+++ b/libs/vkd3d-shader/vkd3d_shader_private.h
@@ -133,10 +133,12 @@ enum vkd3d_shader_error
     VKD3D_SHADER_ERROR_HLSL_MISSING_ATTRIBUTE           = 5024,
     VKD3D_SHADER_ERROR_HLSL_RECURSIVE_CALL              = 5025,
     VKD3D_SHADER_ERROR_HLSL_INCONSISTENT_SAMPLER        = 5026,
+    VKD3D_SHADER_ERROR_HLSL_NON_FINITE_RESULT           = 5027,
 
     VKD3D_SHADER_WARNING_HLSL_IMPLICIT_TRUNCATION       = 5300,
     VKD3D_SHADER_WARNING_HLSL_DIVISION_BY_ZERO          = 5301,
     VKD3D_SHADER_WARNING_HLSL_UNKNOWN_ATTRIBUTE         = 5302,
+    VKD3D_SHADER_WARNING_HLSL_IMAGINARY_NUMERIC_RESULT  = 5303,
 
     VKD3D_SHADER_ERROR_GLSL_INTERNAL                    = 6000,
 
-- 
GitLab


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

[PATCH 2/2] vkd3d-shader/hlsl: Add constant folding for 'log2'.

[Nikolay Sivov]

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

---
 libs/vkd3d-shader/hlsl_constant_ops.c    | 49 ++++++++++++++++++++++++
 libs/vkd3d-shader/vkd3d_shader_private.h |  1 +
 2 files changed, 50 insertions(+)

diff --git a/libs/vkd3d-shader/hlsl_constant_ops.c b/libs/vkd3d-shader/hlsl_constant_ops.c
index aec5fae6..75e88790 100644
--- a/libs/vkd3d-shader/hlsl_constant_ops.c
+++ b/libs/vkd3d-shader/hlsl_constant_ops.c
@@ -152,6 +152,51 @@ static bool fold_cast(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst,
     return true;
 }
 
+static bool fold_log2(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 < 4; ++k)
+    {
+        switch (type)
+        {
+            case HLSL_TYPE_FLOAT:
+            case HLSL_TYPE_HALF:
+                if (ctx->profile->major_version >= 4 && src->value.u[k].f < 0.0f)
+                {
+                    hlsl_warning(ctx, loc, VKD3D_SHADER_WARNING_HLSL_NON_FINITE_RESULT,
+                            "Indefinite logarithm result.");
+                }
+                dst->u[k].f = log2f(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_NON_FINITE_RESULT,
+                            "Indefinite logarithm result.");
+                }
+                dst->u[k].d = log2(src->value.u[k].d);
+                break;
+
+            default:
+                FIXME("Fold 'log2' for type %s.\n", debug_hlsl_type(ctx, dst_type));
+                return false;
+        }
+    }
+
+    return true;
+}
+
 static bool fold_neg(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst,
         const struct hlsl_type *dst_type, const struct hlsl_ir_constant *src)
 {
@@ -807,6 +852,10 @@ bool hlsl_fold_constant_exprs(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr,
             success = fold_cast(ctx, &res, instr->data_type, arg1);
             break;
 
+        case HLSL_OP1_LOG2:
+            success = fold_log2(ctx, &res, instr->data_type, arg1, &instr->loc);
+            break;
+
         case HLSL_OP1_NEG:
             success = fold_neg(ctx, &res, instr->data_type, arg1);
             break;
diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h
index 655d1a97..c4041f2d 100644
--- a/libs/vkd3d-shader/vkd3d_shader_private.h
+++ b/libs/vkd3d-shader/vkd3d_shader_private.h
@@ -139,6 +139,7 @@ enum vkd3d_shader_error
     VKD3D_SHADER_WARNING_HLSL_DIVISION_BY_ZERO          = 5301,
     VKD3D_SHADER_WARNING_HLSL_UNKNOWN_ATTRIBUTE         = 5302,
     VKD3D_SHADER_WARNING_HLSL_IMAGINARY_NUMERIC_RESULT  = 5303,
+    VKD3D_SHADER_WARNING_HLSL_NON_FINITE_RESULT         = 5304,
 
     VKD3D_SHADER_ERROR_GLSL_INTERNAL                    = 6000,
 
-- 
GitLab

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

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl_constant_ops.c:
> return true;
>  }
>
> +static bool fold_sqrt(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 < 4; ++k)

Notice that we use `dimx` here since 717cd3f5bbbfdad8e3b3c03485625ac1a95ccf20.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/265#note_37966

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/vkd3d_shader_private.h:
> VKD3D_SHADER_ERROR_HLSL_MISSING_ATTRIBUTE           = 5024,
>      VKD3D_SHADER_ERROR_HLSL_RECURSIVE_CALL              = 5025,
>      VKD3D_SHADER_ERROR_HLSL_INCONSISTENT_SAMPLER        = 5026,
> +    VKD3D_SHADER_ERROR_HLSL_NON_FINITE_RESULT           = 5027,
>
> VKD3D_SHADER_WARNING_HLSL_IMPLICIT_TRUNCATION       = 5300,
>      VKD3D_SHADER_WARNING_HLSL_DIVISION_BY_ZERO          = 5301,
>      VKD3D_SHADER_WARNING_HLSL_UNKNOWN_ATTRIBUTE         = 5302,
> +    VKD3D_SHADER_WARNING_HLSL_IMAGINARY_NUMERIC_RESULT  = 5303,

I am not sure of what is the general philosophy for the error identifiers in vkd3d. Honestly, my general feeling is they are a bit too specific. It's good to give a specific error message when this is meant for a human, as we do. But the error identifier is likely meant for a program, and I am not sure of which circumstances would make a program behave differently depending on whether the shader compilation failed because of a "non finite result", of a "division by zero or of an "offset out of bounds". Probably the only sensible thing a program can do in any of these occasions is to just report an error to the human, for whom the natural language error message is more useful than the error identifier; for that a single "HLSL compiler failed compilation" error identifier is probably more than enough.

OTOH, if we decide that for some reason we want to be specific with error identifiers, then let's really be specific. We already have "division by zero", so "non finite result" would be a useless generalization of that. Then let's call it "square root of a negative number".

As I said, I guess that's going to be pretty useless anyway, because I can't see any reasonable way to extract useful information from the error identifier, so leaving stuff as it is is fine for me too. I'm just a bit puzzled by the inconsistency.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/265#note_37968