[PATCH v3 0/1] MR211: vkd3d-shader/hlsl: Support clip() intrinsic.
-- v3: vkd3d-shader/hlsl: Support clip() intrinsic. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/211
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- Makefile.am | 1 + libs/vkd3d-shader/d3dbc.c | 35 ++++++++++++++- libs/vkd3d-shader/hlsl.c | 17 ++++++-- libs/vkd3d-shader/hlsl.h | 3 +- libs/vkd3d-shader/hlsl.y | 74 ++++++++++++++++++++++++++++++-- libs/vkd3d-shader/hlsl_codegen.c | 2 +- tests/hlsl-clip.shader_test | 22 ++++++++++ 7 files changed, 144 insertions(+), 10 deletions(-) create mode 100644 tests/hlsl-clip.shader_test diff --git a/Makefile.am b/Makefile.am index 954b0ffd..1220fc09 100644 --- a/Makefile.am +++ b/Makefile.am @@ -79,6 +79,7 @@ vkd3d_shader_tests = \ tests/hlsl-attributes.shader_test \ tests/hlsl-bool-cast.shader_test \ tests/hlsl-clamp.shader_test \ + tests/hlsl-clip.shader_test \ tests/hlsl-comma.shader_test \ tests/hlsl-cross.shader_test \ tests/hlsl-d3dcolor-to-ubyte4.shader_test \ diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index c35f8ca0..8ec47a34 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -349,7 +349,7 @@ static const struct vkd3d_sm1_opcode_info ps_opcode_table[] = /* Texture */ {VKD3D_SM1_OP_TEXCOORD, 1, 0, VKD3DSIH_TEXCOORD, {0, 0}, { 1, 3}}, {VKD3D_SM1_OP_TEXCOORD, 1, 1, VKD3DSIH_TEXCOORD, {1 ,4}, { 1, 4}}, - {VKD3D_SM1_OP_TEXKILL, 1, 0, VKD3DSIH_TEXKILL, {1 ,0}, { 3, 0}}, + {VKD3D_SM1_OP_TEXKILL, 0, 1, VKD3DSIH_TEXKILL, {1 ,0}, { 3, 0}}, {VKD3D_SM1_OP_TEX, 1, 0, VKD3DSIH_TEX, {0, 0}, { 1, 3}}, {VKD3D_SM1_OP_TEX, 1, 1, VKD3DSIH_TEX, {1, 4}, { 1, 4}}, {VKD3D_SM1_OP_TEX, 1, 2, VKD3DSIH_TEX, {2, 0}, {~0u, ~0u}}, @@ -1834,6 +1834,35 @@ static void write_sm1_expr(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *b } } +static void write_sm1_jump(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *buffer, const struct hlsl_ir_node *instr) +{ + const struct hlsl_ir_jump *jump = hlsl_ir_jump(instr); + + switch (jump->type) + { + case HLSL_IR_JUMP_DISCARD: + { + struct hlsl_reg *src = &jump->arg.node->reg; + + struct sm1_instruction instr = + { + .opcode = VKD3D_SM1_OP_TEXKILL, + + .srcs[0].type = D3DSPR_TEMP, + .srcs[0].swizzle = hlsl_swizzle_from_writemask(src->writemask), + .srcs[0].reg = src->id, + .src_count = 1, + }; + + write_sm1_instruction(ctx, buffer, &instr); + break; + } + + default: + hlsl_fixme(ctx, &jump->node.loc, "Jump type %s.\n", hlsl_jump_type_to_string(jump->type)); + } +} + static void write_sm1_load(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *buffer, const struct hlsl_ir_node *instr) { const struct hlsl_ir_load *load = hlsl_ir_load(instr); @@ -2028,6 +2057,10 @@ static void write_sm1_instructions(struct hlsl_ctx *ctx, struct vkd3d_bytecode_b write_sm1_expr(ctx, buffer, instr); break; + case HLSL_IR_JUMP: + write_sm1_jump(ctx, buffer, instr); + break; + case HLSL_IR_LOAD: write_sm1_load(ctx, buffer, instr); break; diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 5bca84ba..61888b0b 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1429,7 +1429,7 @@ struct hlsl_ir_node *hlsl_new_index(struct hlsl_ctx *ctx, struct hlsl_ir_node *v } struct hlsl_ir_node *hlsl_new_jump(struct hlsl_ctx *ctx, enum hlsl_ir_jump_type type, - const struct vkd3d_shader_location *loc) + struct hlsl_ir_node *arg, const struct vkd3d_shader_location *loc) { struct hlsl_ir_jump *jump; @@ -1437,6 +1437,7 @@ struct hlsl_ir_node *hlsl_new_jump(struct hlsl_ctx *ctx, enum hlsl_ir_jump_type return NULL; init_node(&jump->node, HLSL_IR_JUMP, NULL, loc); jump->type = type; + hlsl_src_from_node(&jump->arg, arg); return &jump->node; } @@ -1587,9 +1588,16 @@ static struct hlsl_ir_node *clone_if(struct hlsl_ctx *ctx, struct clone_instr_ma return dst; } -static struct hlsl_ir_node *clone_jump(struct hlsl_ctx *ctx, struct hlsl_ir_jump *src) +static struct hlsl_ir_node *clone_jump(struct hlsl_ctx *ctx, struct clone_instr_map *map, struct hlsl_ir_jump *src) { - return hlsl_new_jump(ctx, src->type, &src->node.loc); + struct hlsl_ir_jump *jump; + + if (!(jump = hlsl_alloc(ctx, sizeof(*jump)))) + return NULL; + init_node(&jump->node, HLSL_IR_JUMP, NULL, &src->node.loc); + jump->type = src->type; + clone_src(map, &jump->arg, &src->arg); + return &jump->node; } static struct hlsl_ir_node *clone_load(struct hlsl_ctx *ctx, struct clone_instr_map *map, struct hlsl_ir_load *src) @@ -1729,7 +1737,7 @@ static struct hlsl_ir_node *clone_instr(struct hlsl_ctx *ctx, return clone_index(ctx, map, hlsl_ir_index(instr)); case HLSL_IR_JUMP: - return clone_jump(ctx, hlsl_ir_jump(instr)); + return clone_jump(ctx, map, hlsl_ir_jump(instr)); case HLSL_IR_LOAD: return clone_load(ctx, map, hlsl_ir_load(instr)); @@ -2697,6 +2705,7 @@ static void free_ir_if(struct hlsl_ir_if *if_node) static void free_ir_jump(struct hlsl_ir_jump *jump) { + hlsl_src_remove(&jump->arg); vkd3d_free(jump); } diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 24d431e4..ea88fb16 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -565,6 +565,7 @@ struct hlsl_ir_jump { struct hlsl_ir_node node; enum hlsl_ir_jump_type type; + struct hlsl_src arg; }; struct hlsl_ir_swizzle @@ -1118,7 +1119,7 @@ struct hlsl_ir_node *hlsl_new_if(struct hlsl_ctx *ctx, struct hlsl_ir_node *cond struct hlsl_block *then_block, struct hlsl_block *else_block, const struct vkd3d_shader_location *loc); struct hlsl_ir_node *hlsl_new_int_constant(struct hlsl_ctx *ctx, int32_t n, const struct vkd3d_shader_location *loc); struct hlsl_ir_node *hlsl_new_jump(struct hlsl_ctx *ctx, - enum hlsl_ir_jump_type type, const struct vkd3d_shader_location *loc); + enum hlsl_ir_jump_type type, struct hlsl_ir_node *arg, const struct vkd3d_shader_location *loc); void hlsl_init_simple_deref_from_var(struct hlsl_deref *deref, struct hlsl_ir_var *var); diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index dae1851c..55394113 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -421,7 +421,7 @@ static bool append_conditional_break(struct hlsl_ctx *ctx, struct list *cond_lis hlsl_block_init(&then_block); - if (!(jump = hlsl_new_jump(ctx, HLSL_IR_JUMP_BREAK, &condition->loc))) + if (!(jump = hlsl_new_jump(ctx, HLSL_IR_JUMP_BREAK, NULL, &condition->loc))) return false; hlsl_block_add_instr(&then_block, jump); @@ -656,7 +656,7 @@ static bool add_return(struct hlsl_ctx *ctx, struct list *instrs, hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RETURN, "Void functions cannot return a value."); } - if (!(jump = hlsl_new_jump(ctx, HLSL_IR_JUMP_RETURN, loc))) + if (!(jump = hlsl_new_jump(ctx, HLSL_IR_JUMP_RETURN, NULL, loc))) return false; list_add_tail(instrs, &jump->entry); @@ -2539,6 +2539,73 @@ static bool intrinsic_clamp(struct hlsl_ctx *ctx, return !!add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MIN, max, params->args[2], loc); } +static bool intrinsic_clip(struct hlsl_ctx *ctx, + const struct parse_initializer *params, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *arg, *zero, *bool_false, *or, *cmp, *load, *discard, *iff; + struct hlsl_block then_block; + unsigned int i, count; + + if (!elementwise_intrinsic_float_convert_args(ctx, params, loc)) + return false; + + arg = params->args[0]; + + if (ctx->profile->major_version >= 4) + { + if (!(zero = hlsl_new_float_constant(ctx, 0.0f, loc))) + return false; + list_add_tail(params->instrs, &zero->entry); + + if (!(cmp = add_binary_comparison_expr(ctx, params->instrs, HLSL_OP2_LESS, arg, zero, loc))) + return false; + + if (!(bool_false = hlsl_new_bool_constant(ctx, false, loc))) + return false; + list_add_tail(params->instrs, &bool_false->entry); + + or = bool_false; + + count = hlsl_type_component_count(cmp->data_type); + for (i = 0; i < count; ++i) + { + if (!(load = add_load_component(ctx, params->instrs, cmp, i, loc))) + return false; + + if (!(or = add_binary_logical_expr(ctx, params->instrs, HLSL_OP2_LOGIC_OR, or, load, loc))) + return false; + } + + hlsl_block_init(&then_block); + + if (!(discard = hlsl_new_jump(ctx, HLSL_IR_JUMP_DISCARD, NULL, loc))) + return false; + hlsl_block_add_instr(&then_block, discard); + + if (!(iff = hlsl_new_if(ctx, or, &then_block, NULL, loc))) + return false; + list_add_tail(params->instrs, &iff->entry); + } + else if (hlsl_type_component_count(arg->data_type) > 4) + { + struct vkd3d_string_buffer *string; + + if ((string = hlsl_type_to_string(ctx, arg->data_type))) + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, + "Argument type cannot exceed 4 components, got type \"%s\".", string->buffer); + hlsl_release_string_buffer(ctx, string); + return false; + } + else + { + if (!(discard = hlsl_new_jump(ctx, HLSL_IR_JUMP_DISCARD, arg, loc))) + return false; + list_add_tail(params->instrs, &discard->entry); + } + + return true; +} + static bool intrinsic_cos(struct hlsl_ctx *ctx, const struct parse_initializer *params, const struct vkd3d_shader_location *loc) { @@ -3482,6 +3549,7 @@ intrinsic_functions[] = {"asfloat", 1, true, intrinsic_asfloat}, {"asuint", -1, true, intrinsic_asuint}, {"clamp", 3, true, intrinsic_clamp}, + {"clip", 1, true, intrinsic_clip}, {"cos", 1, true, intrinsic_cos}, {"cross", 2, true, intrinsic_cross}, {"ddx", 1, true, intrinsic_ddx}, @@ -5626,7 +5694,7 @@ discard_statement: if (!($$ = make_empty_list(ctx))) YYABORT; - if (!(discard = hlsl_new_jump(ctx, HLSL_IR_JUMP_DISCARD, &@1))) + if (!(discard = hlsl_new_jump(ctx, HLSL_IR_JUMP_DISCARD, NULL, &@1))) return false; list_add_tail($$, &discard->entry); } diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index bbb5223b..520744b4 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -655,7 +655,7 @@ static void insert_early_return_break(struct hlsl_ctx *ctx, return; list_add_after(&cf_instr->entry, &load->node.entry); - if (!(jump = hlsl_new_jump(ctx, HLSL_IR_JUMP_BREAK, &cf_instr->loc))) + if (!(jump = hlsl_new_jump(ctx, HLSL_IR_JUMP_BREAK, NULL, &cf_instr->loc))) return; hlsl_block_add_instr(&then_block, jump); diff --git a/tests/hlsl-clip.shader_test b/tests/hlsl-clip.shader_test new file mode 100644 index 00000000..f9859e1b --- /dev/null +++ b/tests/hlsl-clip.shader_test @@ -0,0 +1,22 @@ +[pixel shader] +uniform float4 x; + +float4 main() : sv_target +{ + clip(x); + return x; +} + +[test] +uniform 0 float4 1 2 3 4 +draw quad +probe all rgba (1, 2, 3, 4) +uniform 0 float4 9 8 7 6 +draw quad +probe all rgba (9, 8, 7, 6) +uniform 0 float4 -1 8 7 6 +draw quad +probe all rgba (9, 8, 7, 6) +uniform 0 float4 9 0 7 6 +draw quad +probe all rgba (9, 0, 7, 6) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/211
On Wed May 24 20:23:01 2023 +0000, Zebediah Figura wrote:
It does, and I was considering saying that we should make it a HLSL op accordingly. On reflection, though, I decided we probably don't, but rather we probably want to do a "raising" peephole. I wouldn't be surprised if there are programs out there that spell out clip() as something like "if (any(x < 0)) discard". Then again, if I write that out in a shader, native d3dcompiler doesn't seem capable of optimizing that all the way down to texkill()... I pushed something to implement clip() for pre-sm4 profiles using `texkill`.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/211#note_33847
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/d3dbc.c:
/* Texture */ {VKD3D_SM1_OP_TEXCOORD, 1, 0, VKD3DSIH_TEXCOORD, {0, 0}, { 1, 3}}, {VKD3D_SM1_OP_TEXCOORD, 1, 1, VKD3DSIH_TEXCOORD, {1 ,4}, { 1, 4}}, - {VKD3D_SM1_OP_TEXKILL, 1, 0, VKD3DSIH_TEXKILL, {1 ,0}, { 3, 0}}, + {VKD3D_SM1_OP_TEXKILL, 0, 1, VKD3DSIH_TEXKILL, {1 ,0}, { 3, 0}},
Conceptually backwards though it may seem, this really is a destination parameter. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/211#note_34005
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl.y:
return !!add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MIN, max, params->args[2], loc); }
+static bool intrinsic_clip(struct hlsl_ctx *ctx, + const struct parse_initializer *params, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *arg, *zero, *bool_false, *or, *cmp, *load, *discard, *iff; + struct hlsl_block then_block; + unsigned int i, count; + + if (!elementwise_intrinsic_float_convert_args(ctx, params, loc)) + return false; + + arg = params->args[0]; + + if (ctx->profile->major_version >= 4)
The error may need to be specific to sm1, but I don't think we want to emit different IR. Rather, this should be a lowering pass, like everything else. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/211#note_34006
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl.c:
return dst; }
-static struct hlsl_ir_node *clone_jump(struct hlsl_ctx *ctx, struct hlsl_ir_jump *src) +static struct hlsl_ir_node *clone_jump(struct hlsl_ctx *ctx, struct clone_instr_map *map, struct hlsl_ir_jump *src) { - return hlsl_new_jump(ctx, src->type, &src->node.loc); + struct hlsl_ir_jump *jump; + + if (!(jump = hlsl_alloc(ctx, sizeof(*jump)))) + return NULL; + init_node(&jump->node, HLSL_IR_JUMP, NULL, &src->node.loc); + jump->type = src->type; + clone_src(map, &jump->arg, &src->arg); + return &jump->node;
It should be possible to still use hlsl_new_jump() here. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/211#note_34007
Note that sm4 discard instructions are also inherently conditional, but keyed on either zero or nonzero instead. Accordingly we may want different variations, like HLSL_JUMP_DISCARD_NEGATIVE and HLSL_JUMP_DISCARD_ZERO, and then lower clip() to something that uses sm4 conditional discard. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/211#note_34008
On Fri May 26 21:24:14 2023 +0000, Zebediah Figura wrote:
The error may need to be specific to sm1, but I don't think we want to emit different IR. Rather, this should be a lowering pass, like everything else. Why is it a problem for IR to differ? There is already a precedent in D3DCOLORtoUBYTE4(). For SM4 input has to be computed to a bool, for older versions we pass floats in. Do you mean that we could reference the argument of JUMP_DISCARD as is, and run a pass for SM4+ to produce this float comparison condition?
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/211#note_34029
On Sat May 27 09:38:47 2023 +0000, Zebediah Figura wrote:
Note that sm4 discard instructions are also inherently conditional, but keyed on either zero or nonzero instead. Accordingly we may want different variations, like HLSL_JUMP_DISCARD_NEGATIVE and HLSL_JUMP_DISCARD_ZERO, and then lower clip() to something that uses sm4 conditional discard. I think they always use discard_nz variant, but I'm not entirely sure.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/211#note_34030
participants (3)
-
Nikolay Sivov -
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)