[PATCH 5/6] d3dcompiler: Store initializers inside the hlsl_ir_var structure.
Matteo Bruni
matteo.mystral at gmail.com
Wed Jun 17 10:29:04 CDT 2020
On Thu, Jun 11, 2020 at 11:45 PM Zebediah Figura <z.figura12 at gmail.com> wrote:
>
> This is necessary so that global variable initializers can work correctly (both static and extern).
>
> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
> ---
> dlls/d3dcompiler_43/d3dcompiler_private.h | 1 +
> dlls/d3dcompiler_43/hlsl.y | 155 ++++++++++++----------
> dlls/d3dcompiler_43/utils.c | 4 +
> 3 files changed, 93 insertions(+), 67 deletions(-)
>
> diff --git a/dlls/d3dcompiler_43/d3dcompiler_private.h b/dlls/d3dcompiler_43/d3dcompiler_private.h
> index 9feaee0d8ac..bc8734f0148 100644
> --- a/dlls/d3dcompiler_43/d3dcompiler_private.h
> +++ b/dlls/d3dcompiler_43/d3dcompiler_private.h
> @@ -706,6 +706,7 @@ struct hlsl_ir_var
> unsigned int modifiers;
> const struct reg_reservation *reg_reservation;
> struct list scope_entry, param_entry;
> + struct list initializer;
>
> unsigned int first_write, last_read;
> };
> diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y
> index 393f8d52fbd..734289f66a5 100644
> --- a/dlls/d3dcompiler_43/hlsl.y
> +++ b/dlls/d3dcompiler_43/hlsl.y
> @@ -525,6 +525,7 @@ static struct hlsl_ir_var *new_var(const char *name, struct hlsl_type *type, con
> var->semantic = semantic;
> var->modifiers = modifiers;
> var->reg_reservation = reg_reservation;
> + list_init(&var->initializer);
> return var;
> }
>
> @@ -770,30 +771,18 @@ static void free_parse_variable_def(struct parse_variable_def *v)
> d3dcompiler_free(v);
> }
>
> -static struct list *declare_vars(struct hlsl_type *basic_type, DWORD modifiers, struct list *var_list)
> +static BOOL declare_vars(struct hlsl_type *basic_type, DWORD modifiers, struct list *var_list)
> {
> struct hlsl_type *type;
> struct parse_variable_def *v, *v_next;
> struct hlsl_ir_var *var;
> - struct hlsl_ir_node *assignment;
> BOOL ret, local = TRUE;
> - struct list *statements_list = d3dcompiler_alloc(sizeof(*statements_list));
>
> if (basic_type->type == HLSL_CLASS_MATRIX)
> assert(basic_type->modifiers & HLSL_MODIFIERS_MAJORITY_MASK);
>
> - if (!statements_list)
> - {
> - ERR("Out of memory.\n");
> - LIST_FOR_EACH_ENTRY_SAFE(v, v_next, var_list, struct parse_variable_def, entry)
> - free_parse_variable_def(v);
> - d3dcompiler_free(var_list);
> - return NULL;
> - }
> - list_init(statements_list);
> -
> if (!var_list)
> - return statements_list;
> + return TRUE;
>
> LIST_FOR_EACH_ENTRY_SAFE(v, v_next, var_list, struct parse_variable_def, entry)
> {
> @@ -808,6 +797,7 @@ static struct list *declare_vars(struct hlsl_type *basic_type, DWORD modifiers,
> continue;
> }
> debug_dump_decl(type, modifiers, v->name, v->loc.line);
> + list_init(&var->initializer);
>
> if (hlsl_ctx.cur_scope == hlsl_ctx.globals)
> {
> @@ -835,7 +825,6 @@ static struct list *declare_vars(struct hlsl_type *basic_type, DWORD modifiers,
> if (v->initializer.args_count)
> {
> unsigned int size = initializer_size(&v->initializer);
> - struct hlsl_ir_load *load;
>
> TRACE("Variable with initializer.\n");
> if (type->type <= HLSL_CLASS_LAST_NUMERIC
> @@ -862,7 +851,7 @@ static struct list *declare_vars(struct hlsl_type *basic_type, DWORD modifiers,
>
> if (type->type == HLSL_CLASS_STRUCT)
> {
> - struct_var_initializer(statements_list, var, &v->initializer);
> + struct_var_initializer(&var->initializer, var, &v->initializer);
> d3dcompiler_free(v);
> continue;
> }
> @@ -888,19 +877,16 @@ static struct list *declare_vars(struct hlsl_type *basic_type, DWORD modifiers,
> continue;
> }
>
> - list_move_tail(statements_list, v->initializer.instrs);
> + list_move_tail(&var->initializer, v->initializer.instrs);
> d3dcompiler_free(v->initializer.instrs);
> -
> - load = new_var_load(var, var->loc);
> - list_add_tail(statements_list, &load->node.entry);
> - assignment = make_assignment(&load->node, ASSIGN_OP_ASSIGN, v->initializer.args[0]);
> d3dcompiler_free(v->initializer.args);
> - list_add_tail(statements_list, &assignment->entry);
> +
> + implicit_conversion(node_from_list(&var->initializer), var->data_type, &var->loc);
> }
> d3dcompiler_free(v);
> }
> d3dcompiler_free(var_list);
> - return statements_list;
> + return TRUE;
> }
>
> static BOOL add_struct_field(struct list *fields, struct hlsl_struct_field *field)
> @@ -1282,6 +1268,20 @@ static struct hlsl_ir_function_decl *new_func_decl(struct hlsl_type *return_type
> return decl;
> }
>
> +/* Append an instruction assigning "var" to its own initializer. */
> +static BOOL add_assignment_to_initializer(struct hlsl_ir_var *var)
> +{
> + struct hlsl_ir_assignment *assign;
> +
> + if (!list_empty(&var->initializer))
> + {
> + if (!(assign = make_simple_assignment(var, node_from_list(&var->initializer))))
> + return FALSE;
> + list_add_tail(&var->initializer, &assign->node.entry);
> + }
> + return TRUE;
> +}
> +
> %}
>
> %locations
> @@ -1416,9 +1416,6 @@ static struct hlsl_ir_function_decl *new_func_decl(struct hlsl_type *return_type
> %type <boolval> boolean
> %type <type> base_type
> %type <type> type
> -%type <list> declaration_statement
> -%type <list> declaration
> -%type <list> struct_declaration
> %type <type> struct_spec
> %type <type> named_struct_spec
> %type <type> unnamed_struct_spec
> @@ -1566,7 +1563,8 @@ struct_declaration: var_modifiers struct_spec variables_def_optional ';'
>
> if (!(type = apply_type_modifiers($2, &modifiers, get_location(&@1))))
> YYABORT;
> - $$ = declare_vars(type, modifiers, $3);
> + if (!declare_vars(type, modifiers, $3))
> + YYABORT;
> }
>
> struct_spec: named_struct_spec
> @@ -1908,18 +1906,11 @@ base_type:
> d3dcompiler_free($2);
> }
>
> -declaration_statement: declaration
> - | struct_declaration
> - | typedef
> - {
> - $$ = d3dcompiler_alloc(sizeof(*$$));
> - if (!$$)
> - {
> - ERR("Out of memory\n");
> - YYABORT;
> - }
> - list_init($$);
> - }
> +declaration_statement:
> +
> + declaration
> + | struct_declaration
> + | typedef
>
> typedef_type: type
> | struct_spec
> @@ -1967,7 +1958,8 @@ declaration: var_modifiers type variables_def ';'
>
> if (!(type = apply_type_modifiers($2, &modifiers, get_location(&@1))))
> YYABORT;
> - $$ = declare_vars(type, modifiers, $3);
> + if (!declare_vars(type, modifiers, $3))
> + YYABORT;
> }
>
> variables_def_optional: /* Empty */
> @@ -2148,12 +2140,29 @@ statement_list: statement
> d3dcompiler_free($2);
> }
>
> -statement: declaration_statement
> - | expr_statement
> - | compound_statement
> - | jump_statement
> - | selection_statement
> - | loop_statement
> +statement:
> +
> + declaration_statement
> + {
> + struct hlsl_ir_var *var;
> +
> + /* Append any initializers made by this declaration_statement to the
> + * instruction list. */
> + if (!($$ = d3dcompiler_alloc(sizeof(*$$))))
> + YYABORT;
> + list_init($$);
> + LIST_FOR_EACH_ENTRY(var, &hlsl_ctx.cur_scope->vars, struct hlsl_ir_var, scope_entry)
> + {
> + if (!add_assignment_to_initializer(var))
> + YYABORT;
> + list_move_tail($$, &var->initializer);
> + }
> + }
I see what you're doing but I'm a bit bothered by the part where you
store the initializers into struct hlsl_ir_var. I don't know that it
is a fair "complaint", it's certainly mostly stylistical. But hear my
variation and see if it makes any sense to you.
Initializers could be temporarily stored in a special list inside
hlsl_ctx. Here in declaration_statement and other places where you
process initializers, you list_move_head() from that list to $$ and
that's it. Local static initializers should probably be stored into a
separate list, together with global initializers (assuming static
locals are a thing in HLSL, I don't recall off the top of my head).
> + | expr_statement
> + | compound_statement
> + | jump_statement
> + | selection_statement
> + | loop_statement
>
> jump_statement: KW_RETURN expr ';'
> {
> @@ -2206,27 +2215,39 @@ if_body: statement
> $$.else_instrs = $3;
> }
>
> -loop_statement: KW_WHILE '(' expr ')' statement
> - {
> - $$ = create_loop(LOOP_WHILE, NULL, $3, NULL, $5, get_location(&@1));
> - }
> - | KW_DO statement KW_WHILE '(' expr ')' ';'
> - {
> - $$ = create_loop(LOOP_DO_WHILE, NULL, $5, NULL, $2, get_location(&@1));
> - }
> - | KW_FOR '(' scope_start expr_statement expr_statement expr ')' statement
> - {
> - $$ = create_loop(LOOP_FOR, $4, $5, $6, $8, get_location(&@1));
> - pop_scope(&hlsl_ctx);
> - }
> - | KW_FOR '(' scope_start declaration expr_statement expr ')' statement
> - {
> - if (!$4)
> - hlsl_report_message(get_location(&@4), HLSL_LEVEL_WARNING,
> - "no expressions in for loop initializer");
> - $$ = create_loop(LOOP_FOR, $4, $5, $6, $8, get_location(&@1));
> - pop_scope(&hlsl_ctx);
> - }
> +loop_statement:
> +
> + KW_WHILE '(' expr ')' statement
> + {
> + $$ = create_loop(LOOP_WHILE, NULL, $3, NULL, $5, get_location(&@1));
> + }
> + | KW_DO statement KW_WHILE '(' expr ')' ';'
> + {
> + $$ = create_loop(LOOP_DO_WHILE, NULL, $5, NULL, $2, get_location(&@1));
> + }
> + | KW_FOR '(' scope_start expr_statement expr_statement expr ')' statement
> + {
> + $$ = create_loop(LOOP_FOR, $4, $5, $6, $8, get_location(&@1));
> + pop_scope(&hlsl_ctx);
> + }
> + | KW_FOR '(' scope_start declaration expr_statement expr ')' statement
> + {
> + struct hlsl_ir_var *var;
> +
> + $$ = create_loop(LOOP_FOR, NULL, $5, $6, $8, get_location(&@1));
> +
> + /* Prepend any initializers made by this declaration to the
> + * instruction list. Walk through the list in reverse, so that the
> + * initializers are executed in the order that they were declared. */
> + LIST_FOR_EACH_ENTRY_REV(var, &hlsl_ctx.cur_scope->vars, struct hlsl_ir_var, scope_entry)
> + {
> + if (!add_assignment_to_initializer(var))
> + YYABORT;
> + list_move_head($$, &var->initializer);
> + }
If I'm reading this correctly, here the temporary list idea would
avoid the need to go in reverse, which might be a bit less surprising
overall.
More information about the wine-devel
mailing list