[PATCH v2 0/3] MR136: vkd3d-shader/hlsl: Handle discard statement.
-- v2: tests: Add a simple test for "discard". https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/136
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/dxbc.c | 2 +- libs/vkd3d-shader/trace.c | 3 ++- libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index d99ea2e3..e0e3ff8d 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -633,7 +633,7 @@ static const struct vkd3d_sm4_opcode_info opcode_table[] = {VKD3D_SM4_OP_DEFAULT, VKD3DSIH_DEFAULT, "", ""}, {VKD3D_SM4_OP_DERIV_RTX, VKD3DSIH_DSX, "f", "f"}, {VKD3D_SM4_OP_DERIV_RTY, VKD3DSIH_DSY, "f", "f"}, - {VKD3D_SM4_OP_DISCARD, VKD3DSIH_TEXKILL, "", "u", + {VKD3D_SM4_OP_DISCARD, VKD3DSIH_DISCARD, "", "u", shader_sm4_read_conditional_op}, {VKD3D_SM4_OP_DIV, VKD3DSIH_DIV, "f", "ff"}, {VKD3D_SM4_OP_DP2, VKD3DSIH_DP2, "f", "ff"}, diff --git a/libs/vkd3d-shader/trace.c b/libs/vkd3d-shader/trace.c index 6cd2dcb2..85e38fe6 100644 --- a/libs/vkd3d-shader/trace.c +++ b/libs/vkd3d-shader/trace.c @@ -109,6 +109,7 @@ static const char * const shader_opcode_names[] = [VKD3DSIH_DEQ ] = "deq", [VKD3DSIH_DFMA ] = "dfma", [VKD3DSIH_DGE ] = "dge", + [VKD3DSIH_DISCARD ] = "discard", [VKD3DSIH_DIV ] = "div", [VKD3DSIH_DLT ] = "dlt", [VKD3DSIH_DMAX ] = "dmax", @@ -1507,7 +1508,7 @@ static void shader_dump_instruction_flags(struct vkd3d_d3d_asm_compiler *compile case VKD3DSIH_CONTINUEP: case VKD3DSIH_IF: case VKD3DSIH_RETP: - case VKD3DSIH_TEXKILL: + case VKD3DSIH_DISCARD: switch (ins->flags) { case VKD3D_SHADER_CONDITIONAL_OP_NZ: shader_addline(buffer, "_nz"); break; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 85d50cbd..1492b630 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -223,6 +223,7 @@ enum vkd3d_shader_opcode VKD3DSIH_DEQ, VKD3DSIH_DFMA, VKD3DSIH_DGE, + VKD3DSIH_DISCARD, VKD3DSIH_DIV, VKD3DSIH_DLT, VKD3DSIH_DMAX, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/136
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 14 ++++++++++++++ libs/vkd3d-shader/hlsl_sm4.c | 16 ++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index fd1eaf6e..89c71ce5 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -3933,6 +3933,7 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type %type <list> conditional_expr %type <list> declaration %type <list> declaration_statement +%type <list> discard_statement %type <list> equality_expr %type <list> expr %type <list> expr_optional @@ -5145,6 +5146,7 @@ statement: declaration_statement | expr_statement | compound_statement + | discard_statement | jump_statement | selection_statement | loop_statement @@ -5164,6 +5166,18 @@ jump_statement: YYABORT; } +discard_statement: + KW_DISCARD ';' + { + struct hlsl_ir_jump *discard; + + if (!($$ = make_empty_list(ctx))) + YYABORT; + if (!(discard = hlsl_new_jump(ctx, HLSL_IR_JUMP_DISCARD, @1))) + return false; + list_add_tail($$, &discard->node.entry); + } + selection_statement: KW_IF '(' expr ')' if_body { diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c index f81be3e9..c3b2dac2 100644 --- a/libs/vkd3d-shader/hlsl_sm4.c +++ b/libs/vkd3d-shader/hlsl_sm4.c @@ -2137,6 +2137,22 @@ static void write_sm4_jump(struct hlsl_ctx *ctx, instr.opcode = VKD3D_SM4_OP_BREAK; break; + case HLSL_IR_JUMP_DISCARD: + { + struct sm4_register *reg = &instr.srcs[0].reg; + + instr.opcode = VKD3D_SM4_OP_DISCARD | VKD3D_SM4_CONDITIONAL_NZ; + + memset(&instr.srcs[0], 0, sizeof(*instr.srcs)); + instr.srcs[0].swizzle_type = VKD3D_SM4_SWIZZLE_NONE; + instr.src_count = 1; + reg->type = VKD3D_SM4_RT_IMMCONST; + reg->dim = VKD3D_SM4_DIMENSION_SCALAR; + reg->immconst_uint[0] = ~0u; + + break; + } + case HLSL_IR_JUMP_RETURN: vkd3d_unreachable(); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/136
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- Makefile.am | 1 + tests/hlsl-discard.shader_test | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 tests/hlsl-discard.shader_test diff --git a/Makefile.am b/Makefile.am index 6ac8ffb2..17df6a5d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -74,6 +74,7 @@ vkd3d_shader_tests = \ tests/hlsl-clamp.shader_test \ tests/hlsl-comma.shader_test \ tests/hlsl-cross.shader_test \ + tests/hlsl-discard.shader_test \ tests/hlsl-dot.shader_test \ tests/hlsl-duplicate-modifiers.shader_test \ tests/hlsl-for.shader_test \ diff --git a/tests/hlsl-discard.shader_test b/tests/hlsl-discard.shader_test new file mode 100644 index 00000000..d99c49c6 --- /dev/null +++ b/tests/hlsl-discard.shader_test @@ -0,0 +1,16 @@ +[pixel shader] +uniform float4 x; + +float4 main() : sv_target +{ + if (x.x == 0.0f) discard; + return float4(1, 2, 3, 4); +} + +[test] +uniform 0 float4 1 0 0 0 +draw quad +probe all rgba (1, 2, 3, 4) +uniform 0 float4 0 0 0 0 +draw quad +probe all rgba (1, 2, 3, 4) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/136
I pushed a test. As it happens d3d12 results stand up - texture seems to be cleared, on previous versions old content is retained. That's why I used 0 for test content. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/136#note_28689
```diff @@ -633,7 +633,7 @@ static const struct vkd3d_sm4_opcode_info opcode_table[] = {VKD3D_SM4_OP_DEFAULT, VKD3DSIH_DEFAULT, "", ""}, {VKD3D_SM4_OP_DERIV_RTX, VKD3DSIH_DSX, "f", "f"}, {VKD3D_SM4_OP_DERIV_RTY, VKD3DSIH_DSY, "f", "f"}, - {VKD3D_SM4_OP_DISCARD, VKD3DSIH_TEXKILL, "", "u", + {VKD3D_SM4_OP_DISCARD, VKD3DSIH_DISCARD, "", "u", shader_sm4_read_conditional_op}, {VKD3D_SM4_OP_DIV, VKD3DSIH_DIV, "f", "ff"}, {VKD3D_SM4_OP_DP2, VKD3DSIH_DP2, "f", "ff"}, ```
This breaks "discard" handling in the SPIR-V backend. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/136#note_28699
On Mon Apr 3 14:39:41 2023 +0000, Henri Verbeet wrote:
```diff @@ -633,7 +633,7 @@ static const struct vkd3d_sm4_opcode_info
opcode_table[] =
{VKD3D_SM4_OP_DEFAULT, VKD3DSIH_DEFAULT,
"", ""},
{VKD3D_SM4_OP_DERIV_RTX, VKD3DSIH_DSX,
"f", "f"},
{VKD3D_SM4_OP_DERIV_RTY, VKD3DSIH_DSY,
"f", "f"},
- {VKD3D_SM4_OP_DISCARD, VKD3DSIH_TEXKILL, "", "u", + {VKD3D_SM4_OP_DISCARD, VKD3DSIH_DISCARD, "", "u", shader_sm4_read_conditional_op}, {VKD3D_SM4_OP_DIV, VKD3DSIH_DIV, "f", "ff"}, {VKD3D_SM4_OP_DP2, VKD3DSIH_DP2, "f", "ff"}, ``` This breaks "discard" handling in the SPIR-V backend. I see what you mean, and have two questions now. Do we need both texkill -> SpvOpKill and discard -> SpvOpKill? For example, do we convert sm1 -> spirv directly? And second questions, what happens to texkill/discard arguments when translating to spirv?
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/136#note_28707
I see what you mean, and have two questions now. Do we need both texkill -> SpvOpKill and discard -> SpvOpKill? For example, do we convert sm1 -> spirv directly? And second questions, what happens to texkill/discard arguments when translating to spirv?
We don't currently have d3dbc->spirv translation upstream, but @zfigura is working on adding that. I imagine that eventually we'll introduce a transformation pass to replace TEXKILL with an equivalent DISCARD, so that backends only need to handle DISCARD. As for the differences between (tex)kill and discard, shader_glsl_texkill() in wined3d is probably a decent reference. Note that the behaviour is slightly different between shader model 1 and shader model 2+ as well. There's also a slight difference in the allowed source registers between ps 1.4 and earlier versions, though that mostly only matters when generating bytecode. Note that the HLSL clip() intrinsic also exists. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/136#note_28719
participants (3)
-
Henri Verbeet (＠hverbeet) -
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)