[PATCH 5/6] d3dcompiler: Store initializers inside the hlsl_ir_var structure.
Zebediah Figura
zfigura at codeweavers.com
Wed Jun 17 10:47:01 CDT 2020
On 6/17/20 10:29 AM, Matteo Bruni wrote:
> 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.
It would work, I think, and I agree it'd be simpler than this patch. The
one place this approach gets hairy is with uniform initializers (which
translate to default values). We could parse that in declare_vars() and
store it as a flat value in hlsl_ir_var without difficulty, but we'd
need to be able to run constant folding over it (and maybe other passes?
Of course, I don't even have examples of anything that needs uniform
initializers.)
> 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).
Yes, and they act like C static locals, which is to say, like static
globals.
>
>> + | 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.
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20200617/d43b3067/attachment.sig>
More information about the wine-devel
mailing list