[PATCH 0/5] MR342: vkd3d-shader/hlsl: Stop using hlsl_type_get_regset().
As has been pointed out a couple of times, mainly by @zfigura, hlsl_type_get_regset() is not defined for structs. So it is responsibility of the programmer to ensure that the data type in question belongs to a single regset before calling that function. Turns out that when we call this function we either have to handle the possibility of the type spanning across multiple regsets, or we want the regset of a single component or vector, so this MR replaces the calls to hlsl_type_get_regset() with either a for loop that goes through all the pertaining regsets, hlsl_type_get_component_offset(), or hlsl_type_get_regset(). hlsl_type_get_regset() is still needed in the implementation of these functions, so it is downgraded to a static function for hlsl.c instead of being completely removed. The tests in 3/5 (structs with multiple register reservations) are to explain why it makes sense to iterate over the regsets in 4/5 even if we don't have a proper implementation for register reservations on structs that span across mutiple regsets yet. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/342
From: Francisco Casas <fcasas(a)codeweavers.com> Components only span across a single regset, so instead of expecting the regset as input for the offset, hlsl_type_get_component_offset() can actually retrieve it. --- libs/vkd3d-shader/hlsl.c | 26 +++++++++++++++----------- libs/vkd3d-shader/hlsl.h | 2 +- libs/vkd3d-shader/tpf.c | 3 +-- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 09580970..772905cd 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -452,11 +452,11 @@ struct hlsl_type *hlsl_type_get_component_type(struct hlsl_ctx *ctx, struct hlsl } unsigned int hlsl_type_get_component_offset(struct hlsl_ctx *ctx, struct hlsl_type *type, - enum hlsl_regset regset, unsigned int index) + unsigned int index, enum hlsl_regset *regset) { + unsigned int offset[HLSL_REGSET_LAST + 1] = {0}; struct hlsl_type *next_type; - unsigned int offset = 0; - unsigned int idx; + unsigned int idx, r; while (!type_is_single_component(type)) { @@ -468,19 +468,22 @@ unsigned int hlsl_type_get_component_offset(struct hlsl_ctx *ctx, struct hlsl_ty case HLSL_CLASS_SCALAR: case HLSL_CLASS_VECTOR: case HLSL_CLASS_MATRIX: - if (regset == HLSL_REGSET_NUMERIC) - offset += idx; + offset[HLSL_REGSET_NUMERIC] += idx; break; case HLSL_CLASS_STRUCT: - offset += type->e.record.fields[idx].reg_offset[regset]; + for (r = 0; r <= HLSL_REGSET_LAST; ++r) + offset[r] += type->e.record.fields[idx].reg_offset[r]; break; case HLSL_CLASS_ARRAY: - if (regset == HLSL_REGSET_NUMERIC) - offset += idx * align(type->e.array.type->reg_size[regset], 4); - else - offset += idx * type->e.array.type->reg_size[regset]; + for (r = 0; r <= HLSL_REGSET_LAST; ++r) + { + if (r == HLSL_REGSET_NUMERIC) + offset[r] += idx * align(type->e.array.type->reg_size[r], 4); + else + offset[r] += idx * type->e.array.type->reg_size[r]; + } break; case HLSL_CLASS_OBJECT: @@ -493,7 +496,8 @@ unsigned int hlsl_type_get_component_offset(struct hlsl_ctx *ctx, struct hlsl_ty type = next_type; } - return offset; + *regset = hlsl_type_get_regset(type); + return offset[*regset]; } static bool init_deref(struct hlsl_ctx *ctx, struct hlsl_deref *deref, struct hlsl_ir_var *var, diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 04e681a9..995bfa6e 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -1228,7 +1228,7 @@ unsigned int hlsl_type_get_array_element_reg_size(const struct hlsl_type *type, struct hlsl_type *hlsl_type_get_component_type(struct hlsl_ctx *ctx, struct hlsl_type *type, unsigned int index); unsigned int hlsl_type_get_component_offset(struct hlsl_ctx *ctx, struct hlsl_type *type, - enum hlsl_regset regset, unsigned int index); + unsigned int index, enum hlsl_regset *regset); bool hlsl_type_is_row_major(const struct hlsl_type *type); unsigned int hlsl_type_minor_size(const struct hlsl_type *type); unsigned int hlsl_type_major_size(const struct hlsl_type *type); diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 045fb6c5..018ad25e 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -3202,8 +3202,7 @@ static struct extern_resource *sm4_get_extern_resources(struct hlsl_ctx *ctx, un if (!hlsl_type_is_resource(component_type)) continue; - regset = hlsl_type_get_regset(component_type); - regset_offset = hlsl_type_get_component_offset(ctx, var->data_type, regset, k); + regset_offset = hlsl_type_get_component_offset(ctx, var->data_type, k, ®set); if (regset_offset > var->regs[regset].allocation_size) continue; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/342
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/tpf.c | 51 ++++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 018ad25e..afb86130 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -3248,38 +3248,43 @@ static struct extern_resource *sm4_get_extern_resources(struct hlsl_ctx *ctx, un } else { + unsigned int r; + if (!hlsl_type_is_resource(var->data_type)) continue; - regset = hlsl_type_get_regset(var->data_type); - if (!var->regs[regset].allocated) - continue; - if (!(hlsl_array_reserve(ctx, (void **)&extern_resources, &capacity, *count + 1, - sizeof(*extern_resources)))) + for (r = 0; r <= HLSL_REGSET_LAST; ++r) { - sm4_free_extern_resources(extern_resources, *count); - *count = 0; - return NULL; - } + if (!var->regs[r].allocated) + continue; - if (!(name = hlsl_strdup(ctx, string_skip_tag(var->name)))) - { - sm4_free_extern_resources(extern_resources, *count); - *count = 0; - return NULL; - } + if (!(hlsl_array_reserve(ctx, (void **)&extern_resources, &capacity, *count + 1, + sizeof(*extern_resources)))) + { + sm4_free_extern_resources(extern_resources, *count); + *count = 0; + return NULL; + } + + if (!(name = hlsl_strdup(ctx, string_skip_tag(var->name)))) + { + sm4_free_extern_resources(extern_resources, *count); + *count = 0; + return NULL; + } - extern_resources[*count].var = var; + extern_resources[*count].var = var; - extern_resources[*count].name = name; - extern_resources[*count].data_type = var->data_type; - extern_resources[*count].is_user_packed = !!var->reg_reservation.reg_type; + extern_resources[*count].name = name; + extern_resources[*count].data_type = var->data_type; + extern_resources[*count].is_user_packed = !!var->reg_reservation.reg_type; - extern_resources[*count].regset = regset; - extern_resources[*count].id = var->regs[regset].id; - extern_resources[*count].bind_count = var->bind_count[regset]; + extern_resources[*count].regset = r; + extern_resources[*count].id = var->regs[r].id; + extern_resources[*count].bind_count = var->bind_count[r]; - ++*count; + ++*count; + } } } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/342
From: Francisco Casas <fcasas(a)codeweavers.com> --- I doubt we will need to implement support for register reservations for structs in the near future, since it would require an implementation capable of handling many register() entries. Nevertheless, I added the tests to ilustrate why the next patch is a step in the right direction. --- tests/hlsl/register-reservations.shader_test | 73 ++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/tests/hlsl/register-reservations.shader_test b/tests/hlsl/register-reservations.shader_test index 7e109eca..13e2fac2 100644 --- a/tests/hlsl/register-reservations.shader_test +++ b/tests/hlsl/register-reservations.shader_test @@ -156,3 +156,76 @@ float4 main() : sv_target [test] draw quad probe all rgba (2.0, 2.0, 2.0, 99.0) + + +[require] +shader model >= 5.0 + +[texture 0] +size (2, 2) +0.0 0.0 0.0 99.0 +0.0 0.0 0.0 99.0 +0.0 0.0 0.0 99.0 +0.0 0.0 0.0 99.0 + +[texture 1] +size (2, 2) +1.0 1.0 1.0 99.0 +1.0 1.0 1.0 99.0 +0.0 0.0 0.0 99.0 +0.0 0.0 0.0 99.0 + +[sampler 0] +filter point point point +address clamp clamp clamp + +[sampler 1] +filter linear linear linear +address clamp clamp clamp + +% If a struct spans many object register sets and one is reserved, all other used object register sets must be. +[pixel shader fail todo] +struct +{ + Texture2D tex; + sampler sam; +} foo : register(t1); + +float4 main() : sv_target +{ + return foo.tex.Sample(foo.sam, float2(0.4, 0)); +} + + +[pixel shader todo] +struct +{ + Texture2D tex; + sampler sam; +} foo : register(t1) : register(s1); + +float4 main() : sv_target +{ + return foo.tex.Sample(foo.sam, float2(0, 0.5)); +} + +[test] +todo draw quad +todo probe all rgba (0.5, 0.5, 0.5, 99.0) + + +[pixel shader] +struct +{ + Texture2D tex; + sampler sam; // unused. +} foo : register(t1); + +float4 main() : sv_target +{ + return foo.tex.Load(int3(0, 0, 0)); +} + +[test] +draw quad +todo probe all rgba (1.0, 1.0, 1.0, 99.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/342
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 46 +++++++++++++++++++------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 132f44a2..9f8a02e6 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2944,31 +2944,39 @@ static void allocate_register_reservations(struct hlsl_ctx *ctx) LIST_FOR_EACH_ENTRY(var, &ctx->extern_vars, struct hlsl_ir_var, extern_entry) { - enum hlsl_regset regset; + unsigned int r; if (!hlsl_type_is_resource(var->data_type)) continue; - regset = hlsl_type_get_regset(var->data_type); - if (var->reg_reservation.reg_type && var->regs[regset].allocation_size) + if (var->reg_reservation.reg_type) { - if (var->reg_reservation.reg_type != get_regset_name(regset)) + for (r = 0; r <= HLSL_REGSET_LAST_OBJECT; ++r) { - 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; - 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].allocation_size); + if (var->regs[r].allocation_size > 0) + { + if (var->reg_reservation.reg_type != get_regset_name(r)) + { + struct vkd3d_string_buffer *type_string; + + /* We can throw this error because resources can only span across a single + * regset, but we have to check for multiple regsets if we support register + * reservations for structs for SM5. */ + 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(r)); + hlsl_release_string_buffer(ctx, type_string); + } + else + { + var->regs[r].allocated = true; + var->regs[r].id = var->reg_reservation.reg_index; + 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[r].allocation_size); + } + } } } } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/342
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 22 ++++++++++++++++++---- libs/vkd3d-shader/hlsl.h | 2 +- libs/vkd3d-shader/hlsl_codegen.c | 11 ++++++----- libs/vkd3d-shader/tpf.c | 4 ++-- 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 772905cd..b8d636e1 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -216,13 +216,15 @@ bool hlsl_type_is_resource(const struct hlsl_type *type) return false; } -enum hlsl_regset hlsl_type_get_regset(const struct hlsl_type *type) +/* Only intended to be used for derefs (after copies have been lowered to components or vectors) or + * resources, since for both their data types span across a single regset. */ +static enum hlsl_regset type_get_regset(const struct hlsl_type *type) { if (type->class <= HLSL_CLASS_LAST_NUMERIC) return HLSL_REGSET_NUMERIC; if (type->class == HLSL_CLASS_ARRAY) - return hlsl_type_get_regset(type->e.array.type); + return type_get_regset(type->e.array.type); if (type->class == HLSL_CLASS_OBJECT) { @@ -245,6 +247,18 @@ enum hlsl_regset hlsl_type_get_regset(const struct hlsl_type *type) vkd3d_unreachable(); } +enum hlsl_regset hlsl_deref_get_regset(struct hlsl_ctx *ctx, const struct hlsl_deref *deref) +{ + struct hlsl_type *type; + + if (deref->data_type) + type = deref->data_type; + else + type = hlsl_deref_get_type(ctx, deref); + + return type_get_regset(type); +} + unsigned int hlsl_type_get_sm4_offset(const struct hlsl_type *type, unsigned int offset) { /* Align to the next vec4 boundary if: @@ -324,7 +338,7 @@ static void hlsl_type_calculate_reg_size(struct hlsl_ctx *ctx, struct hlsl_type { if (hlsl_type_is_resource(type)) { - enum hlsl_regset regset = hlsl_type_get_regset(type); + enum hlsl_regset regset = type_get_regset(type); type->reg_size[regset] = 1; } @@ -496,7 +510,7 @@ unsigned int hlsl_type_get_component_offset(struct hlsl_ctx *ctx, struct hlsl_ty type = next_type; } - *regset = hlsl_type_get_regset(type); + *regset = type_get_regset(type); return offset[*regset]; } diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 995bfa6e..34e2e043 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -1234,7 +1234,6 @@ unsigned int hlsl_type_minor_size(const struct hlsl_type *type); unsigned int hlsl_type_major_size(const struct hlsl_type *type); unsigned int hlsl_type_element_count(const struct hlsl_type *type); bool hlsl_type_is_resource(const struct hlsl_type *type); -enum hlsl_regset hlsl_type_get_regset(const struct hlsl_type *type); unsigned int hlsl_type_get_sm4_offset(const struct hlsl_type *type, unsigned int offset); bool hlsl_types_are_equal(const struct hlsl_type *t1, const struct hlsl_type *t2); @@ -1247,6 +1246,7 @@ unsigned int hlsl_map_swizzle(unsigned int swizzle, unsigned int writemask); unsigned int hlsl_swizzle_from_writemask(unsigned int writemask); struct hlsl_type *hlsl_deref_get_type(struct hlsl_ctx *ctx, const struct hlsl_deref *deref); +enum hlsl_regset hlsl_deref_get_regset(struct hlsl_ctx *ctx, const struct hlsl_deref *deref); bool hlsl_component_index_range_from_deref(struct hlsl_ctx *ctx, const struct hlsl_deref *deref, unsigned int *start, unsigned int *count); bool hlsl_regset_index_from_deref(struct hlsl_ctx *ctx, const struct hlsl_deref *deref, diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 9f8a02e6..b10cf201 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -97,7 +97,7 @@ static struct hlsl_ir_node *new_offset_from_path_index(struct hlsl_ctx *ctx, str static struct hlsl_ir_node *new_offset_instr_from_deref(struct hlsl_ctx *ctx, struct hlsl_block *block, const struct hlsl_deref *deref, const struct vkd3d_shader_location *loc) { - enum hlsl_regset regset = hlsl_type_get_regset(deref->data_type); + enum hlsl_regset regset = hlsl_deref_get_regset(ctx, deref); struct hlsl_ir_node *offset = NULL; struct hlsl_type *type; unsigned int i; @@ -2135,7 +2135,7 @@ static bool lower_combined_samples(struct hlsl_ctx *ctx, struct hlsl_ir_node *in return false; } - assert(hlsl_type_get_regset(load->resource.var->data_type) == HLSL_REGSET_SAMPLERS); + assert(hlsl_deref_get_regset(ctx, &load->resource) == HLSL_REGSET_SAMPLERS); if (!(name = hlsl_get_string_buffer(ctx))) return false; @@ -3315,7 +3315,7 @@ static bool track_object_components_sampler_dim(struct hlsl_ctx *ctx, struct hls load = hlsl_ir_resource_load(instr); var = load->resource.var; - regset = hlsl_type_get_regset(hlsl_deref_get_type(ctx, &load->resource)); + regset = hlsl_deref_get_regset(ctx, &load->resource); if (!hlsl_regset_index_from_deref(ctx, &load->resource, regset, &index)) return false; @@ -3360,7 +3360,8 @@ static bool track_object_components_usage(struct hlsl_ctx *ctx, struct hlsl_ir_n load = hlsl_ir_resource_load(instr); var = load->resource.var; - regset = hlsl_type_get_regset(hlsl_deref_get_type(ctx, &load->resource)); + regset = hlsl_deref_get_regset(ctx, &load->resource); + if (!hlsl_regset_index_from_deref(ctx, &load->resource, regset, &index)) return false; @@ -4167,7 +4168,7 @@ bool hlsl_offset_from_deref(struct hlsl_ctx *ctx, const struct hlsl_deref *deref return false; *offset = hlsl_ir_constant(offset_node)->value.u[0].u; - regset = hlsl_type_get_regset(deref->data_type); + regset = hlsl_deref_get_regset(ctx, deref); size = deref->var->data_type->reg_size[regset]; if (*offset >= size) diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index afb86130..a53db9dd 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -3628,7 +3628,7 @@ static void sm4_register_from_deref(struct hlsl_ctx *ctx, struct sm4_register *r if (var->is_uniform) { - enum hlsl_regset regset = hlsl_type_get_regset(data_type); + enum hlsl_regset regset = hlsl_deref_get_regset(ctx, deref); if (regset == HLSL_REGSET_TEXTURES) { @@ -4373,7 +4373,7 @@ static void write_sm4_ld(const struct tpf_writer *tpf, const struct hlsl_ir_node const struct hlsl_type *resource_type = hlsl_deref_get_type(tpf->ctx, resource); bool multisampled = resource_type->base_type == HLSL_TYPE_TEXTURE && (resource_type->sampler_dim == HLSL_SAMPLER_DIM_2DMS || resource_type->sampler_dim == HLSL_SAMPLER_DIM_2DMSARRAY); - bool uav = (hlsl_type_get_regset(resource_type) == HLSL_REGSET_UAVS); + bool uav = (hlsl_deref_get_regset(tpf->ctx, resource) == HLSL_REGSET_UAVS); unsigned int coords_writemask = VKD3DSP_WRITEMASK_ALL; struct sm4_instruction instr; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/342
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/342
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/342
This merge request was approved by Henri Verbeet. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/342
participants (5)
-
Francisco Casas -
Francisco Casas (＠fcasas)
-
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)
-
Zebediah Figura (＠zfigura)