[PATCH v4 0/7] MR42: vkd3d-shader/hlsl: Some fixes.
A mix of a miscellaneous fixes: * Fixes to failed asserts I have stumbled upon when implementing other features. * Checks required for properly supporting object components. * A couple of code improvements. -- v4: vkd3d-shader/hlsl: Use reg_size as component count when allocating a single register. vkd3d-shader/hlsl: Use the base type of the array elements in write_sm1_type(). vkd3d-shader/hlsl: Validate that statics don't contain both resources and numerics. vkd3d-shader/hlsl: Validate that extern structs don't contain objects SM < 5. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/42
From: Francisco Casas <fcasas(a)codeweavers.com> --- --- libs/vkd3d-shader/hlsl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 8591fe31..245393b0 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -885,7 +885,10 @@ struct hlsl_ir_store *hlsl_new_store_index(struct hlsl_ctx *ctx, const struct hl init_node(&store->node, HLSL_IR_STORE, NULL, loc); if (!init_deref(ctx, &store->lhs, lhs->var, lhs->path_len + !!idx)) + { + vkd3d_free(store); return NULL; + } for (i = 0; i < lhs->path_len; ++i) hlsl_src_from_node(&store->lhs.path[i], lhs->path[i].node); if (idx) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/42
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 10 +++++----- libs/vkd3d-shader/hlsl.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 245393b0..7fb7e6b0 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -277,9 +277,9 @@ static bool type_is_single_component(const struct hlsl_type *type) * So, this function can be called several times in sequence to obtain all the path's indexes until * the component is finally reached. */ static unsigned int traverse_path_from_component_index(struct hlsl_ctx *ctx, - struct hlsl_type **type_ptr, unsigned int *index_ptr) + const struct hlsl_type **type_ptr, unsigned int *index_ptr) { - struct hlsl_type *type = *type_ptr; + const struct hlsl_type *type = *type_ptr; unsigned int index = *index_ptr; assert(!type_is_single_component(type)); @@ -341,13 +341,13 @@ static unsigned int traverse_path_from_component_index(struct hlsl_ctx *ctx, } } -struct hlsl_type *hlsl_type_get_component_type(struct hlsl_ctx *ctx, struct hlsl_type *type, +struct hlsl_type *hlsl_type_get_component_type(struct hlsl_ctx *ctx, const struct hlsl_type *type, unsigned int index) { while (!type_is_single_component(type)) traverse_path_from_component_index(ctx, &type, &index); - return type; + return (struct hlsl_type *) type; } static bool init_deref(struct hlsl_ctx *ctx, struct hlsl_deref *deref, struct hlsl_ir_var *var, @@ -394,7 +394,7 @@ static bool init_deref_from_component_index(struct hlsl_ctx *ctx, struct hlsl_bl const struct vkd3d_shader_location *loc) { unsigned int path_len, path_index, deref_path_len, i; - struct hlsl_type *path_type; + const struct hlsl_type *path_type; struct hlsl_ir_constant *c; list_init(&block->instrs); diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index f237d6c4..724d157e 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -815,7 +815,7 @@ struct hlsl_type *hlsl_type_clone(struct hlsl_ctx *ctx, struct hlsl_type *old, unsigned int default_majority, unsigned int modifiers); unsigned int hlsl_type_component_count(const struct hlsl_type *type); 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, +struct hlsl_type *hlsl_type_get_component_type(struct hlsl_ctx *ctx, const struct hlsl_type *type, unsigned int index); bool hlsl_type_is_row_major(const struct hlsl_type *type); unsigned int hlsl_type_minor_size(const struct hlsl_type *type); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/42
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index aacaa95c..ca5e9b64 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2073,6 +2073,9 @@ static void allocate_const_registers(struct hlsl_ctx *ctx, struct hlsl_ir_functi { if (var->is_uniform && var->last_read) { + if (var->data_type->reg_size == 0) + continue; + if (var->data_type->reg_size > 4) var->reg = allocate_range(ctx, &liveness, 1, UINT_MAX, var->data_type->reg_size); else -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/42
From: Francisco Casas <fcasas(a)codeweavers.com> It is worth noting that these checks should also be included for declarations inside cbuffers, once they are implemented. --- libs/vkd3d-shader/hlsl.y | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 7ada218c..5e07415b 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1839,6 +1839,26 @@ static void initialize_var_components(struct hlsl_ctx *ctx, struct list *instrs, } } +static bool type_has_object_components(struct hlsl_type *type, bool within_struct) +{ + if (type->type == HLSL_CLASS_OBJECT) + return !within_struct; + if (type->type == HLSL_CLASS_ARRAY) + return type_has_object_components(type->e.array.type, within_struct); + + if (type->type == HLSL_CLASS_STRUCT) + { + unsigned int i; + + for (i = 0; i < type->e.record.field_count; ++i) + { + if (type_has_object_components(type->e.record.fields[i].type, false)) + return true; + } + } + return false; +} + static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_type, DWORD modifiers, struct list *var_list) { @@ -1974,6 +1994,13 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t if (!(modifiers & HLSL_STORAGE_STATIC)) var->modifiers |= HLSL_STORAGE_UNIFORM; + if (ctx->profile->major_version < 5 && (var->modifiers & HLSL_STORAGE_UNIFORM) && + type_has_object_components(var->data_type, true)) + { + hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, + "Target profile doesn't support objects as struct members in uniform variables.\n"); + } + if ((func = hlsl_get_func_decl(ctx, var->name))) { hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_REDEFINED, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/42
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 5e07415b..4c45e716 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1859,6 +1859,28 @@ static bool type_has_object_components(struct hlsl_type *type, bool within_struc return false; } +static bool type_has_numeric_components(struct hlsl_type *type) +{ + if (type->type <= HLSL_CLASS_LAST_NUMERIC) + return true; + if (type->type == HLSL_CLASS_ARRAY) + return type_has_numeric_components(type->e.array.type); + + if (type->type == HLSL_CLASS_STRUCT) + { + unsigned int i; + + for (i = 0; i < type->e.record.field_count; ++i) + { + if (type_has_numeric_components(type->e.record.fields[i].type)) + return true; + } + } + return false; +} + + + static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_type, DWORD modifiers, struct list *var_list) { @@ -2028,6 +2050,13 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t "Semantics are not allowed on local variables."); } + if ((var->modifiers & HLSL_STORAGE_STATIC) && type_has_numeric_components(var->data_type) + && type_has_object_components(var->data_type, false)) + { + hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, + "Static variables cannot have both numeric and resource components."); + } + if ((type->modifiers & HLSL_MODIFIER_CONST) && !v->initializer.args_count && !(modifiers & (HLSL_STORAGE_STATIC | HLSL_STORAGE_UNIFORM))) { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/42
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_sm1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/hlsl_sm1.c b/libs/vkd3d-shader/hlsl_sm1.c index 2219ef56..ba22925e 100644 --- a/libs/vkd3d-shader/hlsl_sm1.c +++ b/libs/vkd3d-shader/hlsl_sm1.c @@ -272,7 +272,7 @@ static void write_sm1_type(struct vkd3d_bytecode_buffer *buffer, struct hlsl_typ } } - type->bytecode_offset = put_u32(buffer, vkd3d_make_u32(sm1_class(type), sm1_base_type(type))); + type->bytecode_offset = put_u32(buffer, vkd3d_make_u32(sm1_class(type), sm1_base_type(array_type))); put_u32(buffer, vkd3d_make_u32(type->dimy, type->dimx)); put_u32(buffer, vkd3d_make_u32(array_size, field_count)); put_u32(buffer, fields_offset); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/42
From: Francisco Casas <fcasas(a)codeweavers.com> Otherwise, for instance, the added test results in: debug_hlsl_writemask: Assertion `!(writemask & ~VKD3DSP_WRITEMASK_ALL)' failed. Which happens in allocate_variable_temp_register() when the variable's type reg_size is <= 4 but its component count is larger, which may happen if it contains objects. --- v2: - Replacing "dimx" with "reg_size" on all other places where allocate_register() is called, since it was only there for historical reasons. --- libs/vkd3d-shader/hlsl_codegen.c | 8 ++++---- tests/object-references.shader_test | 25 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index ca5e9b64..87145ddb 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -1897,7 +1897,7 @@ static void allocate_variable_temp_register(struct hlsl_ctx *ctx, struct hlsl_ir var->last_read, var->data_type->reg_size); else var->reg = allocate_register(ctx, liveness, var->first_write, - var->last_read, hlsl_type_component_count(var->data_type)); + var->last_read, var->data_type->reg_size); TRACE("Allocated %s to %s (liveness %u-%u).\n", var->name, debug_register('r', var->reg, var->data_type), var->first_write, var->last_read); } @@ -1916,7 +1916,7 @@ static void allocate_temp_registers_recurse(struct hlsl_ctx *ctx, struct hlsl_bl instr->last_read, instr->data_type->reg_size); else instr->reg = allocate_register(ctx, liveness, instr->index, - instr->last_read, instr->data_type->dimx); + instr->last_read, instr->data_type->reg_size); TRACE("Allocated anonymous expression @%u to %s (liveness %u-%u).\n", instr->index, debug_register('r', instr->reg, instr->data_type), instr->index, instr->last_read); } @@ -1979,7 +1979,7 @@ static void allocate_const_registers_recurse(struct hlsl_ctx *ctx, struct hlsl_b if (reg_size > 4) constant->reg = allocate_range(ctx, liveness, 1, UINT_MAX, reg_size); else - constant->reg = allocate_register(ctx, liveness, 1, UINT_MAX, type->dimx); + constant->reg = allocate_register(ctx, liveness, 1, UINT_MAX, reg_size); TRACE("Allocated constant @%u to %s.\n", instr->index, debug_register('c', constant->reg, type)); if (!hlsl_array_reserve(ctx, (void **)&defs->values, &defs->size, @@ -2081,7 +2081,7 @@ static void allocate_const_registers(struct hlsl_ctx *ctx, struct hlsl_ir_functi else { var->reg = allocate_register(ctx, &liveness, 1, UINT_MAX, 4); - var->reg.writemask = (1u << var->data_type->dimx) - 1; + var->reg.writemask = (1u << var->data_type->reg_size) - 1; } TRACE("Allocated %s to %s.\n", var->name, debug_register('c', var->reg, var->data_type)); } diff --git a/tests/object-references.shader_test b/tests/object-references.shader_test index 6cff351a..1276dd36 100644 --- a/tests/object-references.shader_test +++ b/tests/object-references.shader_test @@ -118,3 +118,28 @@ float4 main() : sv_target { return tex[n].Load(0); } + + +[pixel shader] +Texture2D tex; +uniform float f; + +struct apple +{ + Texture2D tex1; + Texture2D tex2; + float3 aa; +}; + +float4 main() : sv_target +{ + struct apple a = {tex, tex, 1.0, 2.0, 3.0}; + + a.aa += f; + return a.aa.xyzx; +} + +[test] +uniform 0 float 10.0 +todo draw quad +probe (0, 0) rgba (11.0, 12.0, 13.0, 11.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/42
whoops, sending again because I forgot to include a test in `hlsl-invalid.shader_test` for statics with both resource and numeric components. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/42#note_12700
participants (2)
-
Francisco Casas -
Francisco Casas (＠fcasas)