[PATCH v3 0/1] MR239: vkd3d-shader/codegen: Skip register allocation for unused object variables.
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- v3: vkd3d-shader/codegen: Skip register allocation for unused object variables. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/239
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 53 +++++++++++++++------ tests/register-reservations.shader_test | 61 +++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 13 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 96b4cb660..6003baa24 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2688,6 +2688,7 @@ static char get_regset_name(enum hlsl_regset regset) static void allocate_register_reservations(struct hlsl_ctx *ctx) { + struct vkd3d_string_buffer *type_string; struct hlsl_ir_var *var; LIST_FOR_EACH_ENTRY(var, &ctx->extern_vars, struct hlsl_ir_var, extern_entry) @@ -2696,29 +2697,50 @@ static void allocate_register_reservations(struct hlsl_ctx *ctx) if (!hlsl_type_is_resource(var->data_type)) continue; + if (!var->reg_reservation.reg_type) + continue; + regset = hlsl_type_get_regset(var->data_type); - if (var->reg_reservation.reg_type) + if (var->reg_reservation.reg_type != get_regset_name(regset)) { - if (var->reg_reservation.reg_type != get_regset_name(regset)) + if (var->last_read) { - struct vkd3d_string_buffer *type_string; - type_string = hlsl_type_to_string(ctx, var->data_type); hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, "Object of type '%s' must be bound to register type '%c'.", type_string->buffer, get_regset_name(regset)); hlsl_release_string_buffer(ctx, type_string); } - else - { - var->regs[regset].allocated = true; - var->regs[regset].id = var->reg_reservation.reg_index; - var->regs[regset].bind_count = var->data_type->reg_size[regset]; - TRACE("Allocated reserved %s to %c%u-%c%u.\n", var->name, var->reg_reservation.reg_type, - var->reg_reservation.reg_index, var->reg_reservation.reg_type, - var->reg_reservation.reg_index + var->regs[regset].bind_count); - } + continue; + } + + var->regs[regset].allocated = true; + var->regs[regset].id = var->reg_reservation.reg_index; + var->regs[regset].bind_count = var->data_type->reg_size[regset]; + TRACE("Allocated reserved %s to %c%u-%c%u.\n", var->name, var->reg_reservation.reg_type, + var->reg_reservation.reg_index, var->reg_reservation.reg_type, + var->reg_reservation.reg_index + var->regs[regset].bind_count); + } +} + +static void deallocate_unused_register_reservations(struct hlsl_ctx *ctx) +{ + struct hlsl_ir_var *var; + + LIST_FOR_EACH_ENTRY(var, &ctx->extern_vars, struct hlsl_ir_var, extern_entry) + { + unsigned int r; + + if (!hlsl_type_is_resource(var->data_type)) + continue; + if (!var->reg_reservation.reg_type) + continue; + if (var->last_read) + continue; + for (r = 0; r <= HLSL_REGSET_LAST_OBJECT; ++r) + { + var->regs[r].allocated = false; } } } @@ -3666,6 +3688,9 @@ static void allocate_objects(struct hlsl_ctx *ctx, enum hlsl_regset regset) const struct hlsl_ir_var *reserved_object, *last_reported = NULL; unsigned int index, i; + if (!var->last_read) + continue; + if (var->regs[regset].id < min_index) { assert(regset == HLSL_REGSET_UAVS); @@ -4130,6 +4155,8 @@ int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry allocate_semantic_registers(ctx); allocate_objects(ctx, HLSL_REGSET_SAMPLERS); + deallocate_unused_register_reservations(ctx); + if (ctx->result) return ctx->result; diff --git a/tests/register-reservations.shader_test b/tests/register-reservations.shader_test index 72f68c1e0..26f4381d0 100644 --- a/tests/register-reservations.shader_test +++ b/tests/register-reservations.shader_test @@ -69,6 +69,36 @@ draw quad probe all rgba (1.0, 1.0, 1.0, 99.0) +% Register reservation with incorrect register type. +[pixel shader] +Texture2D unused : register(s0); +Texture2D tex; + +float4 main() : sv_target +{ + return tex.Load(int3(0, 0, 0)); +} + +[test] +draw quad +probe all rgba (0.0, 0.0, 0.0, 99.0) + + +% Register reservation with incorrect register type. +[pixel shader] +sampler2D unused : register(t0); +Texture2D tex; + +float4 main() : sv_target +{ + return tex.Load(int3(0, 0, 0)); +} + + +[test] +draw quad +probe all rgba (0.0, 0.0, 0.0, 99.0) + [pixel shader] Texture2D unused[2][2] : register(t0); Texture2D tex; @@ -81,3 +111,34 @@ float4 main() : sv_target [test] draw quad probe all rgba (4.0, 4.0, 4.0, 99.0) + + +% Overlapping reservations, both overlapping objects are unused. +[pixel shader] +Texture2D tex1 : register(t0); +Texture2D tex2 : register(t0); +Texture2D tex3; + +float4 main() : sv_target +{ + return tex3.Load(int3(0, 0, 0)); +} + +[test] +draw quad +probe all rgba (1.0, 1.0, 1.0, 99.0) + + +% Overlapping reservations +[pixel shader] +Texture2D tex1 : register(t2); +Texture2D tex2 : register(t2); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +draw quad +probe all rgba (2.0, 2.0, 2.0, 99.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/239
On Tue Jun 20 13:17:56 2023 +0000, Francisco Casas wrote:
I am thinking on removing hlsl_type_get_regset(.) in future patches because it is not defined for structs (would probably reach unreacheable code here). I think it is better to do this at the end of allocate_objects(.), and just for the regset that is passed as argument. Updated to reset the flag for all regsets, for unused variables.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/239#note_36315
Approving. However, we still may want a better criteria for a variable begin used that `var->last_read`, probably something regset-wise. This compiles in native (ps_5_0), yet we would be detecting overlaps: ```hlsl struct { Texture2D tex; sampler sam; } a : register(s0); sampler sam_alone : register(s0); float4 main() : sv_target { return a.tex.Sample(sam_alone, float2(0, 0)); } ``` -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/239#note_36378
This merge request was approved by Francisco Casas. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/239
On Wed Jun 21 07:27:42 2023 +0000, Francisco Casas wrote:
Approving. However, we still may want a better criteria for a variable begin used that `var->last_read`, probably something regset-wise. This compiles in native (ps_5_0), yet we would be detecting overlaps: ```hlsl struct { Texture2D tex; sampler sam; } a : register(s0); sampler sam_alone : register(s0); float4 main() : sv_target { return a.tex.Sample(sam_alone, float2(0, 0)); } ``` Does that mean you can have only one object of distinct type in a structure, when register() is used? Or it will implicitly reserve a range [s0,s0+n)?
How do we currently track unused object in struct members? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/239#note_36416
I don't really like this v3 solution, it means "allocated" doesn't have a consistent meaning. I've submitted my preferred approach as 243. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/239#note_36516
This merge request was closed by Nikolay Sivov. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/239
participants (4)
-
Francisco Casas (＠fcasas) -
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)