From: Francisco Casas Subject: [PATCH vkd3d] vkd3d-shader/hlsl: Support array initializers. Message-Id: <20211229145135.123336-1-fcasas@codeweavers.com> Date: Wed, 29 Dec 2021 11:51:35 -0300 The new function initialize_var_recursive() "pops" arguments from the parse_variable_def initializer to initialize the members of var for both struct and array initialization recursively. initializer_i is used to keep track of the index of the next input parameter to read. This approach should scale well for nested arrays and structs. Signed-off-by: Francisco Casas --- The new definition of free_parse_variable_def() is unorthodox, but I think it helps illustrate how the parse_variable_def memory should be free on the different scenarios. The exercise of writting it this way allowed to find a memory leak. Ignoring inner braces on initializers is still pending, this could be achieved through a flattening method or at the parser level. Signed-off-by: Francisco Casas --- Makefile.am | 4 + libs/vkd3d-shader/hlsl.y | 207 +++++++++--------- .../hlsl-array-initializer-local.shader_test | 21 ++ .../hlsl-array-initializer-static.shader_test | 21 ++ 4 files changed, 155 insertions(+), 98 deletions(-) create mode 100644 tests/hlsl-array-initializer-local.shader_test create mode 100644 tests/hlsl-array-initializer-static.shader_test diff --git a/Makefile.am b/Makefile.am index 20fee06d..91e396ee 100644 --- a/Makefile.am +++ b/Makefile.am @@ -61,6 +61,8 @@ vkd3d_shader_tests = \ tests/cast-to-uint.shader_test \ tests/conditional.shader_test \ tests/hlsl-array-dimension.shader_test \ + tests/hlsl-array-initializer-local.shader_test \ + tests/hlsl-array-initializer-static.shader_test \ tests/hlsl-bool-cast.shader_test \ tests/hlsl-clamp.shader_test \ tests/hlsl-comma.shader_test \ @@ -289,6 +291,8 @@ XFAIL_TESTS = \ tests/cast-to-uint.shader_test \ tests/conditional.shader_test \ tests/hlsl-array-dimension.shader_test \ + tests/hlsl-array-initializer-local.shader_test \ + tests/hlsl-array-initializer-static.shader_test \ tests/hlsl-bool-cast.shader_test \ tests/hlsl-duplicate-modifiers.shader_test \ tests/hlsl-for.shader_test \ diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 636882c4..52c32878 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -904,7 +904,8 @@ static bool expr_compatible_data_types(struct hlsl_type *t1, struct hlsl_type *t static enum hlsl_base_type expr_common_base_type(enum hlsl_base_type t1, enum hlsl_base_type t2) { - if (t1 > HLSL_TYPE_LAST_SCALAR || t2 > HLSL_TYPE_LAST_SCALAR) { + if (t1 > HLSL_TYPE_LAST_SCALAR || t2 > HLSL_TYPE_LAST_SCALAR) + { FIXME("Unexpected base type.\n"); return HLSL_TYPE_FLOAT; } @@ -1278,71 +1279,113 @@ static bool add_increment(struct hlsl_ctx *ctx, struct list *instrs, bool decrem return true; } -static void struct_var_initializer(struct hlsl_ctx *ctx, struct list *list, struct hlsl_ir_var *var, - struct parse_initializer *initializer) +static void initialize_var_recursive(struct hlsl_ctx *ctx, struct hlsl_ir_var *var, unsigned int reg_offset, + struct hlsl_type *type, struct parse_variable_def *v, unsigned int *initializer_i, struct list *instrs) { - struct hlsl_type *type = var->data_type; - struct hlsl_struct_field *field; - unsigned int i = 0; - - if (initializer_size(initializer) != hlsl_type_component_count(type)) + if (type->type <= HLSL_CLASS_LAST_NUMERIC) { - hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT, - "Expected %u components in initializer, but got %u.", - hlsl_type_component_count(type), initializer_size(initializer)); - free_parse_initializer(initializer); - return; - } + unsigned int writemask_offset = 0; + unsigned int components_read = 0; - list_move_tail(list, initializer->instrs); - vkd3d_free(initializer->instrs); + if (type->type == HLSL_CLASS_MATRIX) + hlsl_fixme(ctx, &var->loc, "Matrix initializer\n"); - LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry) - { - struct hlsl_ir_node *node = initializer->args[i]; - struct hlsl_ir_store *store; - struct hlsl_ir_constant *c; + while (components_read < hlsl_type_component_count(type)) + { + struct hlsl_ir_store *store; + struct hlsl_ir_constant *c; + struct hlsl_ir_node *node; + unsigned int width; - if (i++ >= initializer->args_count) - break; + assert(*initializer_i < v->initializer.args_count); + node = v->initializer.args[*initializer_i]; + *initializer_i += 1; - if (hlsl_type_component_count(field->type) == hlsl_type_component_count(node->data_type)) - { - if (!(c = hlsl_new_uint_constant(ctx, field->reg_offset, node->loc))) - break; - list_add_tail(list, &c->node.entry); + width = hlsl_type_component_count(node->data_type); + components_read += width; - if (!(store = hlsl_new_store(ctx, var, &c->node, node, 0, node->loc))) - break; - list_add_tail(list, &store->node.entry); + if (components_read > hlsl_type_component_count(type)) + { + hlsl_error(ctx, &node->loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT, + "Initializer argument has spare components.\n"); + return; + } + + if (!(node = add_implicit_conversion(ctx, instrs, node, + hlsl_get_vector_type(ctx, type->base_type, width), &node->loc))) + return; + + if (!(c = hlsl_new_uint_constant(ctx, reg_offset, node->loc))) + return; + list_add_tail(instrs, &c->node.entry); + + if (!(store = hlsl_new_store(ctx, var, &c->node, node, + ((1u << width) - 1) << writemask_offset, node->loc))) + return; + list_add_tail(instrs, &store->node.entry); + + writemask_offset += width; } - else + } + else if (type->type == HLSL_CLASS_STRUCT) + { + struct hlsl_struct_field *field; + + LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry) { - hlsl_fixme(ctx, &node->loc, "Implicit cast in structure initializer."); + initialize_var_recursive(ctx, var, reg_offset + field->reg_offset, field->type, v, + initializer_i, instrs); } + return; } + else if (type->type == HLSL_CLASS_ARRAY) + { + for (int i = 0; i < type->e.array.elements_count; i++) + { + struct hlsl_type *elem_type = type->e.array.type; + unsigned int elem_offset = i * elem_type->reg_size; - vkd3d_free(initializer->args); + initialize_var_recursive(ctx, var, reg_offset + elem_offset, elem_type, v, + initializer_i, instrs); + } + return; + } + else if (type->type == HLSL_CLASS_OBJECT) + { + hlsl_fixme(ctx, &var->loc, "Initializers for objects not supported yet.\n"); + return; + } } -static void free_parse_variable_def(struct parse_variable_def *v) +static void free_parse_variable_def(struct parse_variable_def *v, int freeness) { - free_parse_initializer(&v->initializer); - vkd3d_free(v->arrays.sizes); - vkd3d_free(v->name); - vkd3d_free((void *)v->semantic.name); - vkd3d_free(v); + /* 0 : If v's names and instructions were passed to another struct. */ + /* 1 : If only v's names were passed to another struct. */ + /* 2 : If v still retains the ownership of all its members. */ + if (freeness == 0) + { + vkd3d_free(v->initializer.args); + vkd3d_free(v->initializer.instrs); + } + if (freeness == 1) + { + free_parse_initializer(&v->initializer); + } + if (freeness == 2) + { + free_parse_initializer(&v->initializer); + vkd3d_free(v->name); + vkd3d_free((void *)v->semantic.name); + } + vkd3d_free(v->arrays.sizes); + vkd3d_free(v); } static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_type, DWORD modifiers, struct list *var_list) { struct parse_variable_def *v, *v_next; - struct hlsl_ir_function_decl *func; struct list *statements_list; - struct hlsl_ir_var *var; - struct hlsl_type *type; - bool local = true; if (basic_type->type == HLSL_CLASS_MATRIX) assert(basic_type->modifiers & HLSL_MODIFIERS_MAJORITY_MASK); @@ -1350,7 +1393,7 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t if (!(statements_list = make_empty_list(ctx))) { LIST_FOR_EACH_ENTRY_SAFE(v, v_next, var_list, struct parse_variable_def, entry) - free_parse_variable_def(v); + free_parse_variable_def(v,2); vkd3d_free(var_list); return NULL; } @@ -1360,9 +1403,12 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t LIST_FOR_EACH_ENTRY_SAFE(v, v_next, var_list, struct parse_variable_def, entry) { + struct hlsl_type *type = basic_type; + struct hlsl_ir_function_decl *func; + struct hlsl_ir_var *var; + bool local = true; unsigned int i; - type = basic_type; for (i = 0; i < v->arrays.count; ++i) type = hlsl_new_array_type(ctx, type, v->arrays.sizes[i]); @@ -1371,7 +1417,7 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t if (!(var = hlsl_new_var(ctx, v->name, type, v->loc, &v->semantic, modifiers, &v->reg_reservation))) { - free_parse_variable_def(v); + free_parse_variable_def(v,2); continue; } @@ -1424,7 +1470,7 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_MISSING_INITIALIZER, "Const variable \"%s\" is missing an initializer.", var->name); hlsl_free_var(var); - vkd3d_free(v); + free_parse_variable_def(v,1); continue; } @@ -1436,78 +1482,43 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t "Variable \"%s\" was already declared in this scope.", var->name); hlsl_note(ctx, &old->loc, VKD3D_SHADER_LOG_ERROR, "\"%s\" was previously declared here.", old->name); hlsl_free_var(var); - vkd3d_free(v); + free_parse_variable_def(v,1); continue; } if (v->initializer.args_count) { unsigned int size = initializer_size(&v->initializer); - struct hlsl_ir_load *load; - if (type->type <= HLSL_CLASS_LAST_NUMERIC - && type->dimx * type->dimy != size && size != 1) - { - if (size < type->dimx * type->dimy) - { - hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT, - "Expected %u components in numeric initializer, but got %u.", - type->dimx * type->dimy, size); - free_parse_initializer(&v->initializer); - vkd3d_free(v); - continue; - } - } - if ((type->type == HLSL_CLASS_STRUCT || type->type == HLSL_CLASS_ARRAY) - && hlsl_type_component_count(type) != size) + if (hlsl_type_component_count(type) != size && (size != 1 || type->type > HLSL_CLASS_LAST_NUMERIC)) { hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT, "Expected %u components in initializer, but got %u.", hlsl_type_component_count(type), size); - free_parse_initializer(&v->initializer); - vkd3d_free(v); + free_parse_variable_def(v,1); continue; } - if (type->type == HLSL_CLASS_STRUCT) + if(var->data_type->type <= HLSL_CLASS_LAST_NUMERIC) { - struct_var_initializer(ctx, statements_list, var, &v->initializer); - vkd3d_free(v); - continue; - } - if (type->type > HLSL_CLASS_LAST_NUMERIC) - { - FIXME("Initializers for non scalar/struct variables not supported yet.\n"); - free_parse_initializer(&v->initializer); - vkd3d_free(v); - continue; - } - if (v->arrays.count) - { - hlsl_fixme(ctx, &v->loc, "Array initializer."); - free_parse_initializer(&v->initializer); - vkd3d_free(v); - continue; + struct hlsl_ir_load *load; + + load = hlsl_new_var_load(ctx, var, var->loc); + list_add_tail(v->initializer.instrs, &load->node.entry); + add_assignment(ctx, v->initializer.instrs, &load->node, ASSIGN_OP_ASSIGN, v->initializer.args[0]); } - if (v->initializer.args_count > 1) + else { - hlsl_fixme(ctx, &v->loc, "Complex initializer."); - free_parse_initializer(&v->initializer); - vkd3d_free(v); - continue; - } - - load = hlsl_new_var_load(ctx, var, var->loc); - list_add_tail(v->initializer.instrs, &load->node.entry); - add_assignment(ctx, v->initializer.instrs, &load->node, ASSIGN_OP_ASSIGN, v->initializer.args[0]); - vkd3d_free(v->initializer.args); + unsigned int initializer_i = 0; + initialize_var_recursive(ctx, var, 0, var->data_type, v, &initializer_i, v->initializer.instrs); + } if (modifiers & HLSL_STORAGE_STATIC) list_move_tail(&ctx->static_initializers, v->initializer.instrs); else list_move_tail(statements_list, v->initializer.instrs); - vkd3d_free(v->initializer.instrs); } - vkd3d_free(v); + + free_parse_variable_def(v,0); } vkd3d_free(var_list); return statements_list; diff --git a/tests/hlsl-array-initializer-local.shader_test b/tests/hlsl-array-initializer-local.shader_test new file mode 100644 index 00000000..e9310b98 --- /dev/null +++ b/tests/hlsl-array-initializer-local.shader_test @@ -0,0 +1,21 @@ +[pixel shader] +float4 main() : SV_TARGET +{ + float4 array_loc[2][4] = { + 11, 12, 13, 14, + 21, 22, 23, 24, + 31, 32, 33, 34, + 41, 42, 43, 44, + 51, 52, 53, 54, + 61, 62, 63, 64, + 71, 72, 73, 74, + 81, 82, 83, 84, + }; + + return array_loc[1][2]; +} + +[test] +draw quad +probe all rgba (71, 72, 73, 74) + diff --git a/tests/hlsl-array-initializer-static.shader_test b/tests/hlsl-array-initializer-static.shader_test new file mode 100644 index 00000000..f103dd9a --- /dev/null +++ b/tests/hlsl-array-initializer-static.shader_test @@ -0,0 +1,21 @@ +[pixel shader] +static const float4 array_st[4][2] = { + 11, 12, 13, 14, + 21, 22, 23, 24, + 31, 32, 33, 34, + 41, 42, 43, 44, + 51, 52, 53, 54, + 61, 62, 63, 64, + 71, 72, 73, 74, + 81, 82, 83, 84, +}; + +float4 main() : SV_TARGET +{ + return array_st[2][1]; +} + +[test] +draw quad +probe all rgba (61, 62, 63, 64) + -- 2.25.1