[PATCH 0/2] MR184: Draft: vkd3d-shader/hlsl: Add support for SampleGrad() method
This one's marked as a draft, as there seems to be a blocker with the method parameters. The first commit totally works, _if_ the ddx/ddy parameters are literals - they do _not_ work when passing a variable of any kind. The test comes from tests/d3d12.c, so I'm mostly just trying to migrate that to the HLSL test suite, but it currently hits an assert before we get to the resource load (which does eventually work) and I'm not sure what's causing it: ``` vkd3d-compiler: libs/vkd3d-shader/tpf.c:3190: sm4_register_from_node: Assertion `instr->reg.allocated' failed. ``` Seems like it's surprised when we try to load from the constant buffer maybe? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184
From: Ethan Lee <flibitijibibo(a)gmail.com> Signed-off-by: Ethan Lee <flibitijibibo(a)gmail.com> --- libs/vkd3d-shader/hlsl.c | 17 ++++++++++ libs/vkd3d-shader/hlsl.h | 5 +-- libs/vkd3d-shader/hlsl.y | 71 ++++++++++++++++++++++++++++++++++++++++ libs/vkd3d-shader/tpf.c | 13 +++++++- 4 files changed, 103 insertions(+), 3 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index f0b6a5ef..7f110781 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1320,6 +1320,8 @@ struct hlsl_ir_resource_load *hlsl_new_resource_load(struct hlsl_ctx *ctx, hlsl_src_from_node(&load->coords, params->coords); hlsl_src_from_node(&load->texel_offset, params->texel_offset); hlsl_src_from_node(&load->lod, params->lod); + hlsl_src_from_node(&load->ddx, params->ddx); + hlsl_src_from_node(&load->ddy, params->ddy); return load; } @@ -1598,6 +1600,8 @@ static struct hlsl_ir_node *clone_resource_load(struct hlsl_ctx *ctx, } clone_src(map, &dst->coords, &src->coords); clone_src(map, &dst->lod, &src->lod); + clone_src(map, &dst->ddx, &src->ddx); + clone_src(map, &dst->ddy, &src->ddy); clone_src(map, &dst->texel_offset, &src->texel_offset); return &dst->node; } @@ -2384,6 +2388,7 @@ static void dump_ir_resource_load(struct vkd3d_string_buffer *buffer, const stru [HLSL_RESOURCE_SAMPLE] = "sample", [HLSL_RESOURCE_SAMPLE_LOD] = "sample_lod", [HLSL_RESOURCE_SAMPLE_LOD_BIAS] = "sample_biased", + [HLSL_RESOURCE_SAMPLE_GRAD] = "sample_grad", [HLSL_RESOURCE_GATHER_RED] = "gather_red", [HLSL_RESOURCE_GATHER_GREEN] = "gather_green", [HLSL_RESOURCE_GATHER_BLUE] = "gather_blue", @@ -2407,6 +2412,16 @@ static void dump_ir_resource_load(struct vkd3d_string_buffer *buffer, const stru vkd3d_string_buffer_printf(buffer, ", lod = "); dump_src(buffer, &load->lod); } + if (load->ddx.node) + { + vkd3d_string_buffer_printf(buffer, ", ddx = "); + dump_src(buffer, &load->ddx); + } + if (load->ddy.node) + { + vkd3d_string_buffer_printf(buffer, ", ddy = "); + dump_src(buffer, &load->ddy); + } vkd3d_string_buffer_printf(buffer, ")"); } @@ -2642,6 +2657,8 @@ static void free_ir_resource_load(struct hlsl_ir_resource_load *load) hlsl_cleanup_deref(&load->resource); hlsl_src_remove(&load->coords); hlsl_src_remove(&load->lod); + hlsl_src_remove(&load->ddx); + hlsl_src_remove(&load->ddy); hlsl_src_remove(&load->texel_offset); vkd3d_free(load); } diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 2ff18536..01897b55 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -589,6 +589,7 @@ enum hlsl_resource_load_type HLSL_RESOURCE_SAMPLE, HLSL_RESOURCE_SAMPLE_LOD, HLSL_RESOURCE_SAMPLE_LOD_BIAS, + HLSL_RESOURCE_SAMPLE_GRAD, HLSL_RESOURCE_GATHER_RED, HLSL_RESOURCE_GATHER_GREEN, HLSL_RESOURCE_GATHER_BLUE, @@ -600,7 +601,7 @@ struct hlsl_ir_resource_load struct hlsl_ir_node node; enum hlsl_resource_load_type load_type; struct hlsl_deref resource, sampler; - struct hlsl_src coords, lod, texel_offset; + struct hlsl_src coords, lod, ddx, ddy, texel_offset; }; struct hlsl_ir_resource_store @@ -798,7 +799,7 @@ struct hlsl_resource_load_params struct hlsl_type *format; enum hlsl_resource_load_type type; struct hlsl_ir_node *resource, *sampler; - struct hlsl_ir_node *coords, *lod, *texel_offset; + struct hlsl_ir_node *coords, *lod, *ddx, *ddy, *texel_offset; }; static inline struct hlsl_ir_call *hlsl_ir_call(const struct hlsl_ir_node *node) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 10deda75..69fd3fb0 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -3883,6 +3883,71 @@ static bool add_sample_lod_method_call(struct hlsl_ctx *ctx, struct list *instrs return true; } +static bool add_sample_grad_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *object, + const char *name, const struct parse_initializer *params, const struct vkd3d_shader_location *loc) +{ + const struct hlsl_type *object_type = object->data_type; + struct hlsl_resource_load_params load_params = { 0 }; + const unsigned int sampler_dim = hlsl_sampler_dim_count(object_type->sampler_dim); + const unsigned int offset_dim = hlsl_offset_dim_count(object_type->sampler_dim); + const struct hlsl_type *sampler_type; + struct hlsl_ir_resource_load *load; + + load_params.type = HLSL_RESOURCE_SAMPLE_GRAD; + + if (params->args_count < 4 || params->args_count > 5 + !!offset_dim) + { + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT, + "Wrong number of arguments to method '%s': expected from 4 to %u, but got %u.", + name, 5 + !!offset_dim, params->args_count); + return false; + } + + sampler_type = params->args[0]->data_type; + if (sampler_type->class != HLSL_CLASS_OBJECT || sampler_type->base_type != HLSL_TYPE_SAMPLER + || sampler_type->sampler_dim != HLSL_SAMPLER_DIM_GENERIC) + { + struct vkd3d_string_buffer *string; + + if ((string = hlsl_type_to_string(ctx, sampler_type))) + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, + "Wrong type for argument 0 of %s(): expected 'sampler', but got '%s'.", name, string->buffer); + hlsl_release_string_buffer(ctx, string); + return false; + } + + if (!(load_params.coords = add_implicit_conversion(ctx, instrs, params->args[1], + hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, sampler_dim), loc))) + load_params.coords = params->args[1]; + + if (!(load_params.ddx = add_implicit_conversion(ctx, instrs, params->args[2], + hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, sampler_dim), loc))) + load_params.ddx = params->args[2]; + + if (!(load_params.ddy = add_implicit_conversion(ctx, instrs, params->args[3], + hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, sampler_dim), loc))) + load_params.ddy = params->args[3]; + + if (offset_dim && params->args_count > 4) + { + if (!(load_params.texel_offset = add_implicit_conversion(ctx, instrs, params->args[4], + hlsl_get_vector_type(ctx, HLSL_TYPE_INT, offset_dim), loc))) + return false; + } + + if (params->args_count > 4 + !!offset_dim) + hlsl_fixme(ctx, loc, "Tiled resource status argument."); + + load_params.format = object_type->e.resource_format; + load_params.resource = object; + load_params.sampler = params->args[0]; + + if (!(load = hlsl_new_resource_load(ctx, &load_params, loc))) + return false; + list_add_tail(instrs, &load->node.entry); + return true; +} + static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *object, const char *name, const struct parse_initializer *params, const struct vkd3d_shader_location *loc) { @@ -3933,6 +3998,12 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl { return add_sample_lod_method_call(ctx, instrs, object, name, params, loc); } + else if (!strcmp(name, "SampleGrad") + && object_type->sampler_dim != HLSL_SAMPLER_DIM_2DMS + && object_type->sampler_dim != HLSL_SAMPLER_DIM_2DMSARRAY) + { + return add_sample_grad_method_call(ctx, instrs, object, name, params, loc); + } else { struct vkd3d_string_buffer *string; diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index d104c078..7dd676d3 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -3036,7 +3036,7 @@ struct sm4_instruction struct sm4_register reg; enum vkd3d_sm4_swizzle_type swizzle_type; unsigned int swizzle; - } srcs[4]; + } srcs[5]; unsigned int src_count; uint32_t idx[3]; @@ -3721,6 +3721,10 @@ static void write_sm4_sample(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer instr.opcode = VKD3D_SM4_OP_SAMPLE_B; break; + case HLSL_RESOURCE_SAMPLE_GRAD: + instr.opcode = VKD3D_SM4_OP_SAMPLE_GRAD; + break; + default: vkd3d_unreachable(); } @@ -3748,6 +3752,12 @@ static void write_sm4_sample(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer sm4_src_from_node(&instr.srcs[3], load->lod.node, VKD3DSP_WRITEMASK_ALL); ++instr.src_count; } + else if (load->load_type == HLSL_RESOURCE_SAMPLE_GRAD) + { + sm4_src_from_node(&instr.srcs[3], load->ddx.node, VKD3DSP_WRITEMASK_ALL); + sm4_src_from_node(&instr.srcs[4], load->ddy.node, VKD3DSP_WRITEMASK_ALL); + instr.src_count += 2; + } write_sm4_instruction(buffer, &instr); } @@ -4501,6 +4511,7 @@ static void write_sm4_resource_load(struct hlsl_ctx *ctx, case HLSL_RESOURCE_SAMPLE: case HLSL_RESOURCE_SAMPLE_LOD_BIAS: + case HLSL_RESOURCE_SAMPLE_GRAD: if (!load->sampler.var) { hlsl_fixme(ctx, &load->node.loc, "SM4 combined sample expression."); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184
From: Ethan Lee <flibitijibibo(a)gmail.com> --- tests/sampler.shader_test | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/sampler.shader_test b/tests/sampler.shader_test index 2cb70a0b..285d42b2 100644 --- a/tests/sampler.shader_test +++ b/tests/sampler.shader_test @@ -45,3 +45,34 @@ float4 main() : sv_target [test] draw quad probe all rgba (0.25, 0, 0.25, 0) + +[pixel shader] +Texture2D t; +SamplerState s; + +float4 dd; + +float4 main(float4 position : SV_POSITION) : SV_Target +{ + float2 p, ddx, ddy; + + p.x = position.x / 640.0f; + p.y = position.y / 480.0f; + ddx = float2(0, 0); // FIXME: asserts when dd.xy + ddy = float2(0, 0); // FIXME: asserts when dd.zw + return t.SampleGrad(s, p, ddx, ddy); +} + +[test] +uniform 0 float4 0.0 0.0 0.0 0.0 +draw quad +probe all rgba (0.25, 0, 0.25, 0) +uniform 0 float4 0.3 0.0 0.0 0.0 +draw quad +probe all rgba (0.25, 0, 0.25, 0) +uniform 0 float4 0.4 0.0 0.0 0.0 +draw quad +probe all rgba (0.25, 0, 0.25, 0) +uniform 0 float4 1.0 0.0 0.0 0.0 +draw quad +probe all rgba (0.25, 0, 0.25, 0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184
Pushed it a little bit harder and got it to compile this: ``` Texture2D t; SamplerState s; float4 dd; float4 main(float4 position : SV_POSITION) : SV_Target { float2 p; p.x = position.x / 640.0f; p.y = position.y / 480.0f; float2 ddx = dd.xy / float2(1, 1); float2 ddy = dd.zw / float2(1, 1); return t.SampleGrad(s, p, ddx, ddy); } ``` This just makes it so the compiler is forced to make a temp register for ddx/ddy, and that makes it happy. So temp registers work too... so why might load instructions be fussy with these last two parameters? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184#note_31314
Hi! I got this dissasembly from your test (replacing `ddx = dd.xy;` and `ddy = dd.zw;`): ``` vkd3d:trace:hlsl_dump_function Function parameters: vkd3d:trace:hlsl_dump_function in float4 position : SV_POSITION0 vkd3d:trace:hlsl_dump_function 2: float4 | <input-SV_POSITION0> vkd3d:trace:hlsl_dump_function 3: float4 | dd vkd3d:trace:hlsl_dump_function 4: float | @2.x vkd3d:trace:hlsl_dump_function 5: float | 6.40000000e+02 vkd3d:trace:hlsl_dump_function 6: float | / (@4 @5 ) vkd3d:trace:hlsl_dump_function 7: | = (p.x @6) vkd3d:trace:hlsl_dump_function 8: float | @2.y vkd3d:trace:hlsl_dump_function 9: float | 4.80000000e+02 vkd3d:trace:hlsl_dump_function 10: float | / (@8 @9 ) vkd3d:trace:hlsl_dump_function 11: | = (p.y @10) vkd3d:trace:hlsl_dump_function 12: float2 | p vkd3d:trace:hlsl_dump_function 13: float2 | @3.xy vkd3d:trace:hlsl_dump_function 14: float2 | @3.zw vkd3d:trace:hlsl_dump_function 15: float4 | sample_grad(resource = t, sampler = s, coords = @12, ddx = @13, ddy = @14) vkd3d:trace:hlsl_dump_function 16: | = (<output-SV_Target0> @15) ``` Using FIXME()'s bombardment :tm: I realized that the assertion fails because the register for instruction 13 is not allocated. This register is not allocated because `compute_livenes_recurse` is failing to inform that instructions 13 and 14 are used by sample_grad. Every time you add new fields to an instruction node type, that refer to other instruction nodes (generally hlsl_src) you have to perform certain updates to functions: - The clone_instr() in hlsl.c (well, actually one if its callees). - The compute_liveness_recurse() function in hlsl_codegen.c. (this causes the assertion failed in this case). If C had reflection we could get away with writing these functions in a way that doesn't require these updates. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184#note_31315
On Thu Apr 27 02:11:11 2023 +0000, Francisco Casas wrote:
Hi! I got this dissasembly from your test (replacing `ddx = dd.xy;` and `ddy = dd.zw;`): ``` vkd3d:trace:hlsl_dump_function Function parameters: vkd3d:trace:hlsl_dump_function in float4 position : SV_POSITION0 vkd3d:trace:hlsl_dump_function 2: float4 | <input-SV_POSITION0> vkd3d:trace:hlsl_dump_function 3: float4 | dd vkd3d:trace:hlsl_dump_function 4: float | @2.x vkd3d:trace:hlsl_dump_function 5: float | 6.40000000e+02 vkd3d:trace:hlsl_dump_function 6: float | / (@4 @5 ) vkd3d:trace:hlsl_dump_function 7: | = (p.x @6) vkd3d:trace:hlsl_dump_function 8: float | @2.y vkd3d:trace:hlsl_dump_function 9: float | 4.80000000e+02 vkd3d:trace:hlsl_dump_function 10: float | / (@8 @9 ) vkd3d:trace:hlsl_dump_function 11: | = (p.y @10) vkd3d:trace:hlsl_dump_function 12: float2 | p vkd3d:trace:hlsl_dump_function 13: float2 | @3.xy vkd3d:trace:hlsl_dump_function 14: float2 | @3.zw vkd3d:trace:hlsl_dump_function 15: float4 | sample_grad(resource = t, sampler = s, coords = @12, ddx = @13, ddy = @14) vkd3d:trace:hlsl_dump_function 16: | = (<output-SV_Target0> @15) ``` Using FIXME()'s bombardment :tm: I realized that the assertion fails because the register for instruction 13 is not allocated. This register is not allocated because `compute_livenes_recurse` is failing to inform that instructions 13 and 14 are used by sample_grad. Every time you add new fields to an instruction node type, that refer to other instruction nodes (generally hlsl_src) you have to perform certain updates to functions: - The clone_instr() in hlsl.c (well, actually one if its callees). - The compute_liveness_recurse() function in hlsl_codegen.c. (this causes the assertion failed in this case). If C had reflection we could get away with writing these functions in a way that doesn't require these updates. I should also add the corresponding dump_instr() callee for completion, but I see you updated that one.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184#note_31318
participants (3)
-
Ethan Lee -
Ethan Lee (＠flibitijibibo)
-
Francisco Casas (＠fcasas)