[PATCH v3 0/3] MR136: vkd3d-shader/hlsl: Handle discard statement.
-- v3: tests: Add a simple test for "discard". vkd3d-shader/hlsl: Handle discard statement. vkd3d-shader/trace: Add separate id 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/spirv.c | 1 + libs/vkd3d-shader/trace.c | 3 ++- libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 4 files changed, 5 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/spirv.c b/libs/vkd3d-shader/spirv.c index ddd77ffb..865ea20e 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -7858,6 +7858,7 @@ static int spirv_compiler_emit_control_flow_instruction(struct spirv_compiler *c spirv_compiler_emit_retc(compiler, instruction); break; + case VKD3DSIH_DISCARD: case VKD3DSIH_TEXKILL: spirv_compiler_emit_kill(compiler, instruction); break; diff --git a/libs/vkd3d-shader/trace.c b/libs/vkd3d-shader/trace.c index d17a2819..9438bfac 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", @@ -1505,9 +1506,9 @@ static void shader_dump_instruction_flags(struct vkd3d_d3d_asm_compiler *compile { case VKD3DSIH_BREAKP: case VKD3DSIH_CONTINUEP: + case VKD3DSIH_DISCARD: case VKD3DSIH_IF: case VKD3DSIH_RETP: - case VKD3DSIH_TEXKILL: 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 d1a4aa1d..c1c0c670 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> Signed-off-by: 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 afcc4575..81323ef9 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 3fc44d9c..fa803089 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> Signed-off-by: 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 77f44568..1858cbea 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
We might need to throw away HLSL statements in the same block after a discard. In theory it wouldn't matter, but it does for derivatives. I have some half-completed tests I need to find and finish. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/136#note_29043
```diff +[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) ```
It's a bit unfortunate that simply ignoring "discard" would result in the test passing. It would seem helpful to either add support for clears to the shader runner (e.g., "clear (0, 1, 0, 0)") and use those here, or to pass the output colour as a uniform to the shader above and then use different colours for the two draws. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/136#note_29434
On Mon Apr 10 11:14:35 2023 +0000, Henri Verbeet wrote:
```diff +[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) ```
It's a bit unfortunate that simply ignoring "discard" would result in the test passing. It would seem helpful to either add support for clears to the shader runner (e.g., "clear (0, 1, 0, 0)") and use those here, or to pass the output colour as a uniform to the shader above and then use different colours for the two draws. I see now where confusion came from and why results differ in d3d12 - we have an explicit ClearRenderTargetView() with zero color. That's why with active discard previous contents are not preserved.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/136#note_29446
participants (4)
-
Henri Verbeet (＠hverbeet) -
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)