[PATCH v3 0/1] MR371: vkd3d-shader/hlsl: Implement ternary operator for SM1.
This is correct for pixel shaders, vertex shaders do not have 'cmp'. -- v3: vkd3d-shader/hlsl: Implement ternary operator for SM1. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/371
From: Nikolay Sivov <nsivov(a)codeweavers.com> This is correct for pixel shaders, vertex shaders do not have 'cmp'. Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/d3dbc.c | 35 ++++++++++++++++++++ libs/vkd3d-shader/hlsl.c | 1 + libs/vkd3d-shader/hlsl.h | 5 ++- libs/vkd3d-shader/hlsl_codegen.c | 56 ++++++++++++++++++++++---------- 4 files changed, 78 insertions(+), 19 deletions(-) diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index 598b75183..6b21b434f 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -1833,6 +1833,37 @@ static void write_sm1_dp2add(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer write_sm1_instruction(ctx, buffer, &instr); } +static void write_sm1_ternary_op(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *buffer, + D3DSHADER_INSTRUCTION_OPCODE_TYPE opcode, const struct hlsl_reg *dst, const struct hlsl_reg *src1, + const struct hlsl_reg *src2, const struct hlsl_reg *src3) +{ + struct sm1_instruction instr = + { + .opcode = opcode, + + .dst.type = D3DSPR_TEMP, + .dst.writemask = dst->writemask, + .dst.reg = dst->id, + .has_dst = 1, + + .srcs[0].type = D3DSPR_TEMP, + .srcs[0].swizzle = hlsl_swizzle_from_writemask(src1->writemask), + .srcs[0].reg = src1->id, + .srcs[1].type = D3DSPR_TEMP, + .srcs[1].swizzle = hlsl_swizzle_from_writemask(src2->writemask), + .srcs[1].reg = src2->id, + .srcs[2].type = D3DSPR_TEMP, + .srcs[2].swizzle = hlsl_swizzle_from_writemask(src3->writemask), + .srcs[2].reg = src3->id, + .src_count = 3, + }; + + sm1_map_src_swizzle(&instr.srcs[0], instr.dst.writemask); + sm1_map_src_swizzle(&instr.srcs[1], instr.dst.writemask); + sm1_map_src_swizzle(&instr.srcs[2], instr.dst.writemask); + write_sm1_instruction(ctx, buffer, &instr); +} + static void write_sm1_binary_op(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *buffer, D3DSHADER_INSTRUCTION_OPCODE_TYPE opcode, const struct hlsl_reg *dst, const struct hlsl_reg *src1, const struct hlsl_reg *src2) @@ -2199,6 +2230,10 @@ static void write_sm1_expr(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *b } break; + case HLSL_OP3_CMP: + write_sm1_ternary_op(ctx, buffer, D3DSIO_CMP, &instr->reg, &arg1->reg, &arg2->reg, &arg3->reg); + break; + case HLSL_OP3_DP2ADD: write_sm1_dp2add(ctx, buffer, &instr->reg, &arg1->reg, &arg2->reg, &arg3->reg); break; diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 5fe9047bf..33ece0493 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -2485,6 +2485,7 @@ const char *debug_hlsl_expr_op(enum hlsl_ir_expr_op op) [HLSL_OP2_NEQUAL] = "!=", [HLSL_OP2_RSHIFT] = ">>", + [HLSL_OP3_CMP] = "cmp", [HLSL_OP3_DP2ADD] = "dp2add", [HLSL_OP3_MOVC] = "movc", [HLSL_OP3_TERNARY] = "ternary", diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 44cebaaf6..f4120f8a3 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -556,7 +556,10 @@ enum hlsl_ir_expr_op /* MOVC(a, b, c) returns c if a is bitwise zero and b otherwise. * TERNARY(a, b, c) returns c if a == 0 and b otherwise. * They differ for floating point numbers, because - * -0.0 == 0.0, but it is not bitwise zero. */ + * -0.0 == 0.0, but it is not bitwise zero. CMP(a, b, c) returns b + if a >= 0, and c otherwise. It's used only for SM1-SM3 targets, while + SM4+ is using MOVC in such cases. */ + HLSL_OP3_CMP, HLSL_OP3_MOVC, HLSL_OP3_TERNARY, }; diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 76572cf93..a1b80effd 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2435,7 +2435,7 @@ static bool lower_round(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, struct /* Use 'movc' 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], *replacement; + struct hlsl_ir_node *operands[HLSL_MAX_OPERANDS] = { 0 }, *replacement; struct hlsl_ir_node *zero, *cond, *first, *second; struct hlsl_constant_value zero_value = { 0 }; struct hlsl_ir_expr *expr; @@ -2452,28 +2452,49 @@ static bool lower_ternary(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, stru first = expr->operands[1].node; second = expr->operands[2].node; - if (cond->data_type->base_type == HLSL_TYPE_FLOAT) + if (ctx->profile->major_version < 4) { - if (!(zero = hlsl_new_constant(ctx, cond->data_type, &zero_value, &instr->loc))) + struct hlsl_ir_node *abs, *neg; + + if (!(abs = hlsl_new_unary_expr(ctx, HLSL_OP1_ABS, cond, &instr->loc))) return false; - hlsl_block_add_instr(block, zero); + hlsl_block_add_instr(block, abs); + + if (!(neg = hlsl_new_unary_expr(ctx, HLSL_OP1_NEG, abs, &instr->loc))) + return false; + hlsl_block_add_instr(block, neg); + + operands[0] = neg; + operands[1] = second; + operands[2] = first; + if (!(replacement = hlsl_new_expr(ctx, HLSL_OP3_CMP, operands, first->data_type, &instr->loc))) + return false; + } + else + { + if (cond->data_type->base_type == HLSL_TYPE_FLOAT) + { + if (!(zero = hlsl_new_constant(ctx, cond->data_type, &zero_value, &instr->loc))) + return false; + hlsl_block_add_instr(block, zero); + + operands[0] = zero; + operands[1] = cond; + type = cond->data_type; + type = hlsl_get_numeric_type(ctx, type->class, HLSL_TYPE_BOOL, type->dimx, type->dimy); + if (!(cond = hlsl_new_expr(ctx, HLSL_OP2_NEQUAL, operands, type, &instr->loc))) + return false; + hlsl_block_add_instr(block, cond); + } memset(operands, 0, sizeof(operands)); - operands[0] = zero; - operands[1] = cond; - type = cond->data_type; - type = hlsl_get_numeric_type(ctx, type->class, HLSL_TYPE_BOOL, type->dimx, type->dimy); - if (!(cond = hlsl_new_expr(ctx, HLSL_OP2_NEQUAL, operands, type, &instr->loc))) + operands[0] = cond; + operands[1] = first; + operands[2] = second; + if (!(replacement = hlsl_new_expr(ctx, HLSL_OP3_MOVC, operands, first->data_type, &instr->loc))) return false; - hlsl_block_add_instr(block, cond); } - memset(operands, 0, sizeof(operands)); - operands[0] = cond; - operands[1] = first; - operands[2] = second; - if (!(replacement = hlsl_new_expr(ctx, HLSL_OP3_MOVC, operands, first->data_type, &instr->loc))) - return false; hlsl_block_add_instr(block, replacement); return true; } @@ -4422,8 +4443,7 @@ int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry hlsl_transform_ir(ctx, track_object_components_usage, body, NULL); sort_synthetic_separated_samplers_first(ctx); - if (profile->major_version >= 4) - lower_ir(ctx, lower_ternary, body); + lower_ir(ctx, lower_ternary, body); if (profile->major_version < 4) { lower_ir(ctx, lower_division, body); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/371
On Thu Sep 28 09:20:45 2023 +0000, Giovanni Mascellani wrote:
I guess it's now correct, but please see also the part about renaming the helper to `write_sm1_ternary_op()` and have it accept the opcode as a parameter, like `write_sm1_binary_op()`. Ok, done. There are a few instructions that could use it later.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/371#note_47068
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/371
It looks fine now. However, please use two different force-pushes to update your MR and to rebase it on top of master. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/371#note_47071
This is correct for pixel shaders, vertex shaders do not have 'cmp'.
I don't immediately see anything preventing this from being applied to vertex shaders. Does that mean we're emitting invalid bytecode in that case? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/371#note_47110
On Thu Sep 28 18:48:28 2023 +0000, Henri Verbeet wrote:
This is correct for pixel shaders, vertex shaders do not have 'cmp'. I don't immediately see anything preventing this from being applied to vertex shaders. Does that mean we're emitting invalid bytecode in that case? I think it depends if it does not work in practice. According to current docs 'cmp' does not exist for vertex shaders, and vs_3_0 with currently compiler does not use it, on Windows. If it makes our output invalid, I think we should error out for vertex profiles.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/371#note_47123
On Thu Sep 28 18:48:28 2023 +0000, Nikolay Sivov wrote:
I think it depends if it does not work in practice. According to current docs 'cmp' does not exist for vertex shaders, and vs_3_0 with currently compiler does not use it, on Windows. If it makes our output invalid, I think we should error out for vertex profiles. I haven't tried but I'd expect it not to work, yeah.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/371#note_47200
participants (5)
-
Giovanni Mascellani (＠giomasce) -
Henri Verbeet (＠hverbeet)
-
Matteo Bruni (＠Mystral)
-
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)