[PATCH v2 0/3] MR265: vkd3d-shader/hlsl: Add some more constant folding operations.
-- v2: vkd3d-shader/hlsl: Use type width in fold_rcp(). vkd3d-shader/hlsl: Add constant folding for 'log2'. vkd3d-shader/hlsl: Add constant folding for 'sqrt'. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/265
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: 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..db5c476c 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 < 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) + { + 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
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: 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 db5c476c..ca387b5f 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 < 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) + { + 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
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_constant_ops.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/hlsl_constant_ops.c b/libs/vkd3d-shader/hlsl_constant_ops.c index ca387b5f..35ad4c2b 100644 --- a/libs/vkd3d-shader/hlsl_constant_ops.c +++ b/libs/vkd3d-shader/hlsl_constant_ops.c @@ -239,7 +239,7 @@ static bool fold_rcp(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst, cons assert(type == src->node.data_type->base_type); - for (k = 0; k < 4; ++k) + for (k = 0; k < dst_type->dimx; ++k) { switch (type) { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/265
On Wed Jul 5 18:47:51 2023 +0000, Nikolay Sivov wrote:
changed this line in [version 2 of the diff](/wine/vkd3d/-/merge_requests/265/diffs?diff_id=55734&start_sha=e39d6ca990dfcf26026688dc5dbea859dc8ec97e#42d7bc823e29607ffee11286a3147a03c3ae6ce6_287_287) Thanks, changed that and rcp() function this was copied from.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/265#note_38041
On Wed Jul 5 10:04:48 2023 +0000, Giovanni Mascellani wrote:
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. For some reason we have multiple errors instead of just a couple, so I added to that. Not sure if your comment is for my changes specifically or situation in general.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/265#note_38042
On Wed Jul 5 18:49:52 2023 +0000, Nikolay Sivov wrote:
For some reason we have multiple errors instead of just a couple, so I added to that. Not sure if your comment is for my changes specifically or situation in general. No, just a random remark. You're right that the philosophy right now is to have a lot of different errors, I just can't understand the reason. But it should do no particular harm.
Let me just point out that native doesn't return any warning for `log2(0.0)` for SM4. It just returns `-Inf`. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/265#note_38907
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/265
Let me just point out that native doesn't return any warning for `log2(0.0)` for SM4. It just returns `-Inf`.
Ah, your code does that too. Sorry, screw that. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/265#note_38909
On Thu Jul 13 14:05:09 2023 +0000, Giovanni Mascellani wrote:
Let me just point out that native doesn't return any warning for `log2(0.0)` for SM4. It just returns `-Inf`. Ah, your code does that too. Sorry, screw that. Yes, it checks for strictly negative currently.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/265#note_38911
participants (3)
-
Giovanni Mascellani (＠giomasce) -
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)