[PATCH 0/1] MR669: vkd3d-shader/hlsl: Implement ternary operator for older vertex profiles.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56333 Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/669
From: Nikolay Sivov <nsivov(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56333 Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/d3dbc.c | 4 +++ libs/vkd3d-shader/hlsl.c | 1 + libs/vkd3d-shader/hlsl.h | 1 + libs/vkd3d-shader/hlsl_codegen.c | 42 +++++++++++++++++++++++++++++--- 4 files changed, 45 insertions(+), 3 deletions(-) diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index 27f5c8104..a697a2a80 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -2329,6 +2329,10 @@ static void write_sm1_expr(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *b } break; + case HLSL_OP2_SLT: + write_sm1_binary_op(ctx, buffer, D3DSIO_SLT, &instr->reg, &arg1->reg, &arg2->reg); + break; + case HLSL_OP3_CMP: write_sm1_ternary_op(ctx, buffer, D3DSIO_CMP, &instr->reg, &arg1->reg, &arg2->reg, &arg3->reg); break; diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 353e34d21..eb766b4d9 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -2610,6 +2610,7 @@ const char *debug_hlsl_expr_op(enum hlsl_ir_expr_op op) [HLSL_OP2_MUL] = "*", [HLSL_OP2_NEQUAL] = "!=", [HLSL_OP2_RSHIFT] = ">>", + [HLSL_OP2_SLT] = "slt", [HLSL_OP3_CMP] = "cmp", [HLSL_OP3_DP2ADD] = "dp2add", diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 5b73857b9..f6cf2fe4a 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -591,6 +591,7 @@ enum hlsl_ir_expr_op HLSL_OP2_MUL, HLSL_OP2_NEQUAL, HLSL_OP2_RSHIFT, + HLSL_OP2_SLT, /* DP2ADD(a, b, c) computes the scalar product of a.xy and b.xy, * then adds c. */ diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 307f86f55..e7fc02d05 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2903,7 +2903,7 @@ static bool lower_floor(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, struct return true; } -/* Use 'movc' for the ternary operator. */ +/* Use movc/cmp/slt for the ternary operator. */ static bool lower_ternary(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, struct hlsl_block *block) { struct hlsl_ir_node *operands[HLSL_MAX_OPERANDS] = { 0 }, *replacement; @@ -2949,8 +2949,44 @@ static bool lower_ternary(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, stru } else if (ctx->profile->major_version < 4 && ctx->profile->type == VKD3D_SHADER_TYPE_VERTEX) { - hlsl_fixme(ctx, &instr->loc, "Ternary operator is not implemented for %s profile.", ctx->profile->name); - return false; + struct hlsl_ir_node *neg, *slt, *sum, *mul, *cond2; + + /* Expression used here is "slt(<cond>) * (first - second) + second". */ + + if (ctx->profile->major_version == 3) + { + if (!(cond2 = hlsl_new_unary_expr(ctx, HLSL_OP1_ABS, cond, &instr->loc))) + return false; + } + else + { + if (!(cond2 = hlsl_new_binary_expr(ctx, HLSL_OP2_MUL, cond, cond))) + return false; + } + hlsl_block_add_instr(block, cond2); + + if (!(neg = hlsl_new_unary_expr(ctx, HLSL_OP1_NEG, cond2, &instr->loc))) + return false; + hlsl_block_add_instr(block, neg); + + if (!(slt = hlsl_new_binary_expr(ctx, HLSL_OP2_SLT, neg, cond2))) + return false; + hlsl_block_add_instr(block, slt); + + if (!(neg = hlsl_new_unary_expr(ctx, HLSL_OP1_NEG, second, &instr->loc))) + return false; + hlsl_block_add_instr(block, neg); + + if (!(sum = hlsl_new_binary_expr(ctx, HLSL_OP2_ADD, first, neg))) + return false; + hlsl_block_add_instr(block, sum); + + if (!(mul = hlsl_new_binary_expr(ctx, HLSL_OP2_MUL, slt, sum))) + return false; + hlsl_block_add_instr(block, mul); + + if (!(replacement = hlsl_new_binary_expr(ctx, HLSL_OP2_ADD, mul, second))) + return false; } else { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/669
"slt" is only valid for vertex shaders. We've been rather lax about this so far, which isn't great. But we definitely shouldn't be using VS-only instructions in a pixel shader. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/669#note_62332
On Thu Feb 22 20:10:06 2024 +0000, Zebediah Figura wrote:
"slt" is only valid for vertex shaders. We've been rather lax about this so far, which isn't great. But we definitely shouldn't be using VS-only instructions in a pixel shader. Yes, exactly. It's used in place of 'cmp'. There is a profile type check, it's not used for pixel shaders.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/669#note_62336
On Thu Feb 22 20:10:06 2024 +0000, Nikolay Sivov wrote:
Yes, exactly. It's used in place of 'cmp'. There is a profile type check, it's not used for pixel shaders. Ugh, I'm sorry, I really need to read instead of blindly assuming things.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/669#note_62337
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/669
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/669
A couple of months ago [there was a request on wine-devel(https://www.winehq.org/mailman3/hyperkitty/list/wine-devel(a)winehq.org/thre...) to make it easier for testers to have a list of the applications that it's sensible to test in view of a vkd3d release. No definitive decision was taken, but the request makes sense to me and I wrote a proposal. Nothing happened since then, but since this commit is specifically for fixing an application bug it might be a good opportunity to try to set a good practice. Would you consider adding something like this to your commit message? ``` Impacted-Application: MotorM4X ``` -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/669#note_62400
On Fri Feb 23 11:55:28 2024 +0000, Giovanni Mascellani wrote:
A couple of months ago [there was a request on wine-devel(https://www.winehq.org/mailman3/hyperkitty/list/wine-devel(a)winehq.org/thre...) to make it easier for testers to have a list of the applications that it's sensible to test in view of a vkd3d release. No definitive decision was taken, but the request makes sense to me and I wrote a proposal. Nothing happened since then, but since this commit is specifically for fixing an application bug it might be a good opportunity to try to set a good practice. Would you consider adding something like this to your commit message? ``` Impacted-Application: MotorM4X ``` I think a bug link gives more information.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/669#note_62416
On Fri Feb 23 11:55:27 2024 +0000, Nikolay Sivov wrote:
I think a bug link gives more information. But it makes it harder to find the interesting information for the tester and it's not easy to grep. They're not mutually exclusive.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/669#note_62417
On Fri Feb 23 12:04:21 2024 +0000, Giovanni Mascellani wrote:
But it makes it harder to find the interesting information for the tester and it's not easy to grep. They're not mutually exclusive. @giomasce Maybe these things could just be saved somewhere for release notes, even if that's outside the repo. As long as these things are visible when a new release is made, that should be good enough I think. IMHO it doesn't seem like a good idea to be putting these things inside commit messages.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/669#note_62418
On Fri Feb 23 13:24:45 2024 +0000, Gijs Vermeulen wrote:
@giomasce Maybe these things could just be saved somewhere for release notes, even if that's outside the repo. As long as these things are visible when a new release is made, that should be good enough I think. IMHO it doesn't seem like a good idea to be putting these things inside commit messages. The advantage of commit messages is that they already exist and require no additional maintenance. I am not aware of other places with the same features. Also commit messages allow to link an application with potentially problematic commits.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/669#note_62419
This merge request was approved by Henri Verbeet. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/669
participants (6)
-
Gijs Vermeulen (＠gverm) -
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)
-
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)