[PATCH 0/5] MR258: hlsl: Replace bare list pointers with hlsl_block, part 2.
From: Zebediah Figura <zfigura(a)codeweavers.com> As well as from the "statement_list" and "compound_statement" rules, which trivially pass through from "statement". --- libs/vkd3d-shader/hlsl.y | 57 ++++++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 60d6514c..cd4351a2 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -483,9 +483,8 @@ static bool attribute_list_has_duplicates(const struct parse_attribute_list *att } static struct list *create_loop(struct hlsl_ctx *ctx, enum loop_type type, const struct parse_attribute_list *attributes, struct list *init, struct list *cond, - struct list *iter, struct list *body, const struct vkd3d_shader_location *loc) + struct list *iter, struct hlsl_block *body, const struct vkd3d_shader_location *loc) { - struct hlsl_block body_block; struct hlsl_ir_node *loop; unsigned int i; @@ -525,20 +524,15 @@ static struct list *create_loop(struct hlsl_ctx *ctx, enum loop_type type, const if (!append_conditional_break(ctx, cond)) goto oom; - hlsl_block_init(&body_block); - - if (type != LOOP_DO_WHILE) - list_move_tail(&body_block.instrs, cond); - - list_move_tail(&body_block.instrs, body); - if (iter) - list_move_tail(&body_block.instrs, iter); + list_move_tail(&body->instrs, iter); if (type == LOOP_DO_WHILE) - list_move_tail(&body_block.instrs, cond); + list_move_tail(&body->instrs, cond); + else + list_move_head(&body->instrs, cond); - if (!(loop = hlsl_new_loop(ctx, &body_block, loc))) + if (!(loop = hlsl_new_loop(ctx, body, loc))) goto oom; list_add_tail(init, &loop->entry); @@ -550,7 +544,7 @@ oom: destroy_instr_list(init); destroy_instr_list(cond); destroy_instr_list(iter); - destroy_instr_list(body); + destroy_block(body); return NULL; } @@ -4328,6 +4322,7 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type char *name; DWORD modifiers; struct hlsl_ir_node *instr; + struct hlsl_block *block; struct list *list; struct parse_fields fields; struct parse_function function; @@ -4460,7 +4455,6 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type %type <list> bitand_expr %type <list> bitor_expr %type <list> bitxor_expr -%type <list> compound_statement %type <list> conditional_expr %type <list> declaration %type <list> declaration_statement @@ -4479,8 +4473,6 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type %type <list> relational_expr %type <list> selection_statement %type <list> shift_expr -%type <list> statement -%type <list> statement_list %type <list> struct_declaration %type <list> type_specs %type <list> unary_expr @@ -4501,6 +4493,10 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type %type <attr_list> attribute_list %type <attr_list> attribute_list_optional +%type <block> compound_statement +%type <block> statement +%type <block> statement_list + %type <boolval> boolean %type <buffer_type> buffer_type @@ -4813,14 +4809,14 @@ func_declaration: "Function \"%s\" is already defined.", decl->func->name); hlsl_note(ctx, &decl->loc, VKD3D_SHADER_LOG_ERROR, "\"%s\" was previously defined here.", decl->func->name); - hlsl_free_instr_list($2); + destroy_block($2); } else { size_t i; decl->has_body = true; - list_move_tail(&decl->body.instrs, $2); + hlsl_block_add_block(&decl->body, $2); vkd3d_free($2); /* Semantics are taken from whichever definition has a body. @@ -4998,7 +4994,7 @@ func_prototype: compound_statement: '{' '}' { - if (!($$ = make_empty_list(ctx))) + if (!($$ = make_empty_block(ctx))) YYABORT; } | '{' scope_start statement_list '}' @@ -5765,17 +5761,32 @@ statement_list: | statement_list statement { $$ = $1; - list_move_tail($$, $2); + hlsl_block_add_block($$, $2); vkd3d_free($2); } statement: declaration_statement + { + $$ = list_to_block($1); + } | expr_statement + { + $$ = list_to_block($1); + } | compound_statement | jump_statement + { + $$ = list_to_block($1); + } | selection_statement + { + $$ = list_to_block($1); + } | loop_statement + { + $$ = list_to_block($1); + } jump_statement: KW_RETURN expr ';' @@ -5837,13 +5848,13 @@ selection_statement: if_body: statement { - $$.then_block = list_to_block($1); + $$.then_block = $1; $$.else_block = NULL; } | statement KW_ELSE statement { - $$.then_block = list_to_block($1); - $$.else_block = list_to_block($3); + $$.then_block = $1; + $$.else_block = $3; } loop_statement: -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/258
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index cd4351a2..113c71ca 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -482,7 +482,8 @@ static bool attribute_list_has_duplicates(const struct parse_attribute_list *att return false; } -static struct list *create_loop(struct hlsl_ctx *ctx, enum loop_type type, const struct parse_attribute_list *attributes, struct list *init, struct list *cond, +static struct hlsl_block *create_loop(struct hlsl_ctx *ctx, enum loop_type type, + const struct parse_attribute_list *attributes, struct list *init, struct list *cond, struct list *iter, struct hlsl_block *body, const struct vkd3d_shader_location *loc) { struct hlsl_ir_node *loop; @@ -538,7 +539,7 @@ static struct list *create_loop(struct hlsl_ctx *ctx, enum loop_type type, const vkd3d_free(cond); vkd3d_free(body); - return init; + return list_to_block(init); oom: destroy_instr_list(init); @@ -4466,7 +4467,6 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type %type <list> jump_statement %type <list> logicand_expr %type <list> logicor_expr -%type <list> loop_statement %type <list> mul_expr %type <list> postfix_expr %type <list> primary_expr @@ -4494,6 +4494,7 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type %type <attr_list> attribute_list_optional %type <block> compound_statement +%type <block> loop_statement %type <block> statement %type <block> statement_list @@ -5784,9 +5785,6 @@ statement: $$ = list_to_block($1); } | loop_statement - { - $$ = list_to_block($1); - } jump_statement: KW_RETURN expr ';' -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/258
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 113c71ca..8800bc4e 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -4471,7 +4471,6 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type %type <list> postfix_expr %type <list> primary_expr %type <list> relational_expr -%type <list> selection_statement %type <list> shift_expr %type <list> struct_declaration %type <list> type_specs @@ -4495,6 +4494,7 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type %type <block> compound_statement %type <block> loop_statement +%type <block> selection_statement %type <block> statement %type <block> statement_list @@ -5781,9 +5781,6 @@ statement: $$ = list_to_block($1); } | selection_statement - { - $$ = list_to_block($1); - } | loop_statement jump_statement: @@ -5839,8 +5836,8 @@ selection_statement: "if condition type %s is not scalar.", string->buffer); hlsl_release_string_buffer(ctx, string); } - $$ = $3; - list_add_tail($$, &instr->entry); + $$ = list_to_block($3); + hlsl_block_add_instr($$, instr); } if_body: -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/258
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 8800bc4e..04d046d1 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -134,6 +134,11 @@ static struct hlsl_ir_node *node_from_list(struct list *list) return LIST_ENTRY(list_tail(list), struct hlsl_ir_node, entry); } +static struct hlsl_ir_node *node_from_block(struct hlsl_block *block) +{ + return LIST_ENTRY(list_tail(&block->instrs), struct hlsl_ir_node, entry); +} + static struct list *block_to_list(struct hlsl_block *block) { /* This is a temporary hack to ease the transition from lists to blocks. @@ -648,7 +653,7 @@ static struct hlsl_ir_node *get_swizzle(struct hlsl_ctx *ctx, struct hlsl_ir_nod return NULL; } -static bool add_return(struct hlsl_ctx *ctx, struct list *instrs, +static bool add_return(struct hlsl_ctx *ctx, struct hlsl_block *block, struct hlsl_ir_node *return_value, const struct vkd3d_shader_location *loc) { struct hlsl_type *return_type = ctx->cur_function->return_type; @@ -660,7 +665,7 @@ static bool add_return(struct hlsl_ctx *ctx, struct list *instrs, { struct hlsl_ir_node *store; - if (!(return_value = add_implicit_conversion(ctx, instrs, return_value, return_type, loc))) + if (!(return_value = add_implicit_conversion(ctx, block_to_list(block), return_value, return_type, loc))) return false; if (!(store = hlsl_new_simple_store(ctx, ctx->cur_function->return_var, return_value))) @@ -681,7 +686,7 @@ static bool add_return(struct hlsl_ctx *ctx, struct list *instrs, if (!(jump = hlsl_new_jump(ctx, HLSL_IR_JUMP_RETURN, NULL, loc))) return false; - list_add_tail(instrs, &jump->entry); + hlsl_block_add_instr(block, jump); return true; } @@ -4464,7 +4469,6 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type %type <list> expr_optional %type <list> expr_statement %type <list> initializer_expr -%type <list> jump_statement %type <list> logicand_expr %type <list> logicor_expr %type <list> mul_expr @@ -4493,6 +4497,7 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type %type <attr_list> attribute_list_optional %type <block> compound_statement +%type <block> jump_statement %type <block> loop_statement %type <block> selection_statement %type <block> statement @@ -5777,22 +5782,19 @@ statement: } | compound_statement | jump_statement - { - $$ = list_to_block($1); - } | selection_statement | loop_statement jump_statement: KW_RETURN expr ';' { - if (!add_return(ctx, $2, node_from_list($2), &@1)) + $$ = list_to_block($2); + if (!add_return(ctx, $$, node_from_block($$), &@1)) YYABORT; - $$ = $2; } | KW_RETURN ';' { - if (!($$ = make_empty_list(ctx))) + if (!($$ = make_empty_block(ctx))) YYABORT; if (!add_return(ctx, $$, NULL, &@1)) YYABORT; @@ -5801,16 +5803,16 @@ jump_statement: { struct hlsl_ir_node *discard, *c; - if (!($$ = make_empty_list(ctx))) + if (!($$ = make_empty_block(ctx))) YYABORT; if (!(c = hlsl_new_uint_constant(ctx, ~0u, &@1))) return false; - list_add_tail($$, &c->entry); + hlsl_block_add_instr($$, c); if (!(discard = hlsl_new_jump(ctx, HLSL_IR_JUMP_DISCARD_NZ, c, &@1))) return false; - list_add_tail($$, &discard->entry); + hlsl_block_add_instr($$, discard); } selection_statement: -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/258
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 70 ++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 04d046d1..dfcec906 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -438,19 +438,19 @@ static DWORD add_modifiers(struct hlsl_ctx *ctx, DWORD modifiers, DWORD mod, return modifiers | mod; } -static bool append_conditional_break(struct hlsl_ctx *ctx, struct list *cond_list) +static bool append_conditional_break(struct hlsl_ctx *ctx, struct hlsl_block *cond_block) { struct hlsl_ir_node *condition, *not, *iff, *jump; struct hlsl_block then_block; /* E.g. "for (i = 0; ; ++i)". */ - if (list_empty(cond_list)) + if (list_empty(&cond_block->instrs)) return true; - condition = node_from_list(cond_list); + condition = node_from_block(cond_block); if (!(not = hlsl_new_unary_expr(ctx, HLSL_OP1_LOGIC_NOT, condition, &condition->loc))) return false; - list_add_tail(cond_list, ¬->entry); + hlsl_block_add_instr(cond_block, not); hlsl_block_init(&then_block); @@ -460,7 +460,7 @@ static bool append_conditional_break(struct hlsl_ctx *ctx, struct list *cond_lis if (!(iff = hlsl_new_if(ctx, not, &then_block, NULL, &condition->loc))) return false; - list_add_tail(cond_list, &iff->entry); + hlsl_block_add_instr(cond_block, iff); return true; } @@ -488,8 +488,8 @@ static bool attribute_list_has_duplicates(const struct parse_attribute_list *att } static struct hlsl_block *create_loop(struct hlsl_ctx *ctx, enum loop_type type, - const struct parse_attribute_list *attributes, struct list *init, struct list *cond, - struct list *iter, struct hlsl_block *body, const struct vkd3d_shader_location *loc) + const struct parse_attribute_list *attributes, struct hlsl_block *init, struct hlsl_block *cond, + struct hlsl_block *iter, struct hlsl_block *body, const struct vkd3d_shader_location *loc) { struct hlsl_ir_node *loop; unsigned int i; @@ -524,32 +524,32 @@ static struct hlsl_block *create_loop(struct hlsl_ctx *ctx, enum loop_type type, } } - if (!init && !(init = make_empty_list(ctx))) + if (!init && !(init = make_empty_block(ctx))) goto oom; if (!append_conditional_break(ctx, cond)) goto oom; if (iter) - list_move_tail(&body->instrs, iter); + hlsl_block_add_block(body, iter); if (type == LOOP_DO_WHILE) - list_move_tail(&body->instrs, cond); + list_move_tail(&body->instrs, &cond->instrs); else - list_move_head(&body->instrs, cond); + list_move_head(&body->instrs, &cond->instrs); if (!(loop = hlsl_new_loop(ctx, body, loc))) goto oom; - list_add_tail(init, &loop->entry); + hlsl_block_add_instr(init, loop); vkd3d_free(cond); vkd3d_free(body); - return list_to_block(init); + return init; oom: - destroy_instr_list(init); - destroy_instr_list(cond); - destroy_instr_list(iter); + destroy_block(init); + destroy_block(cond); + destroy_block(iter); destroy_block(body); return NULL; } @@ -4465,9 +4465,6 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type %type <list> declaration %type <list> declaration_statement %type <list> equality_expr -%type <list> expr -%type <list> expr_optional -%type <list> expr_statement %type <list> initializer_expr %type <list> logicand_expr %type <list> logicor_expr @@ -4497,6 +4494,9 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type %type <attr_list> attribute_list_optional %type <block> compound_statement +%type <block> expr +%type <block> expr_optional +%type <block> expr_statement %type <block> jump_statement %type <block> loop_statement %type <block> selection_statement @@ -5523,7 +5523,7 @@ state: any_identifier '=' expr ';' { vkd3d_free($1); - hlsl_free_instr_list($3); + destroy_block($3); } state_block_start: @@ -5562,7 +5562,7 @@ arrays: unsigned int size; hlsl_clone_block(ctx, &block, &ctx->static_initializers); - list_move_tail(&block.instrs, $2); + hlsl_block_add_block(&block, $2); size = evaluate_static_expression_as_uint(ctx, &block, &@2); @@ -5777,9 +5777,6 @@ statement: $$ = list_to_block($1); } | expr_statement - { - $$ = list_to_block($1); - } | compound_statement | jump_statement | selection_statement @@ -5788,7 +5785,7 @@ statement: jump_statement: KW_RETURN expr ';' { - $$ = list_to_block($2); + $$ = $2; if (!add_return(ctx, $$, node_from_block($$), &@1)) YYABORT; } @@ -5818,7 +5815,7 @@ jump_statement: selection_statement: KW_IF '(' expr ')' if_body { - struct hlsl_ir_node *condition = node_from_list($3); + struct hlsl_ir_node *condition = node_from_block($3); struct hlsl_ir_node *instr; if (!(instr = hlsl_new_if(ctx, condition, $5.then_block, $5.else_block, &@1))) @@ -5838,7 +5835,7 @@ selection_statement: "if condition type %s is not scalar.", string->buffer); hlsl_release_string_buffer(ctx, string); } - $$ = list_to_block($3); + $$ = $3; hlsl_block_add_instr($$, instr); } @@ -5870,14 +5867,14 @@ loop_statement: } | attribute_list_optional KW_FOR '(' scope_start declaration expr_statement expr_optional ')' statement { - $$ = create_loop(ctx, LOOP_FOR, &$1, $5, $6, $7, $9, &@2); + $$ = create_loop(ctx, LOOP_FOR, &$1, list_to_block($5), $6, $7, $9, &@2); hlsl_pop_scope(ctx); } expr_optional: %empty { - if (!($$ = make_empty_list(ctx))) + if (!($$ = make_empty_block(ctx))) YYABORT; } | expr @@ -5947,7 +5944,7 @@ primary_expr: } | '(' expr ')' { - $$ = $2; + $$ = block_to_list($2); } | var_identifier '(' func_arguments ')' { @@ -6041,9 +6038,9 @@ postfix_expr: } | postfix_expr '[' expr ']' { - struct hlsl_ir_node *array = node_from_list($1), *index = node_from_list($3); + struct hlsl_ir_node *array = node_from_list($1), *index = node_from_block($3); - list_move_head($1, $3); + list_move_head($1, &$3->instrs); vkd3d_free($3); if (!add_array_access(ctx, $1, array, index, &@2)) @@ -6303,10 +6300,10 @@ conditional_expr: logicor_expr | logicor_expr '?' expr ':' assignment_expr { - struct hlsl_ir_node *cond = node_from_list($1), *first = node_from_list($3), *second = node_from_list($5); + struct hlsl_ir_node *cond = node_from_list($1), *first = node_from_block($3), *second = node_from_list($5); struct hlsl_type *common_type; - list_move_tail($1, $3); + list_move_tail($1, &$3->instrs); list_move_tail($1, $5); vkd3d_free($3); vkd3d_free($5); @@ -6392,9 +6389,12 @@ assign_op: expr: assignment_expr + { + $$ = list_to_block($1); + } | expr ',' assignment_expr { $$ = $1; - list_move_tail($$, $3); + list_move_tail(&$$->instrs, $3); vkd3d_free($3); } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/258
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
"Function \"%s\" is already defined.", decl->func->name); hlsl_note(ctx, &decl->loc, VKD3D_SHADER_LOG_ERROR, "\"%s\" was previously defined here.", decl->func->name); - hlsl_free_instr_list($2); + destroy_block($2); } else { size_t i;
decl->has_body = true; - list_move_tail(&decl->body.instrs, $2); + hlsl_block_add_block(&decl->body, $2); vkd3d_free($2);
I don't remember what's your position on this, but I would use `destroy_block()` to keep the abstraction. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/258#note_37279
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
| statement_list statement { $$ = $1; - list_move_tail($$, $2); + hlsl_block_add_block($$, $2); vkd3d_free($2);
`destroy_block()` here too. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/258#note_37280
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
- list_move_tail(&body_block.instrs, iter); + list_move_tail(&body->instrs, iter);
if (type == LOOP_DO_WHILE) - list_move_tail(&body_block.instrs, cond); + list_move_tail(&body->instrs, cond); + else + list_move_head(&body->instrs, cond);
- if (!(loop = hlsl_new_loop(ctx, &body_block, loc))) + if (!(loop = hlsl_new_loop(ctx, body, loc))) goto oom; list_add_tail(init, &loop->entry);
vkd3d_free(cond); vkd3d_free(body); `destroy_block()` here too.
Also, shouldn't you free `iter` too? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/258#note_37281
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
+ hlsl_block_add_block(body, iter);
if (type == LOOP_DO_WHILE) - list_move_tail(&body->instrs, cond); + list_move_tail(&body->instrs, &cond->instrs); else - list_move_head(&body->instrs, cond); + list_move_head(&body->instrs, &cond->instrs);
if (!(loop = hlsl_new_loop(ctx, body, loc))) goto oom; - list_add_tail(init, &loop->entry); + hlsl_block_add_instr(init, loop);
vkd3d_free(cond); vkd3d_free(body); `destroy_block()` here too, but by now I suspect you're using `vkd3d_free()` intentionally. There are a couple of other instances in this commit, at any rate.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/258#note_37282
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/258
On Thu Jun 29 09:14:18 2023 +0000, Giovanni Mascellani wrote:
`destroy_block()` here too, but by now I suspect you're using `vkd3d_free()` intentionally. There are a couple of other instances in this commit, at any rate. Eh... kind of? When I wrote these I didn't really have anything in mind besides transforming list operations to blocks. All the instances of vkd3d_free() should be with already empty lists, so it kind of makes sense as-is. But on the other hand destroy_block() is free type safety, so... meh.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/258#note_37352
participants (3)
-
Giovanni Mascellani (＠giomasce) -
Zebediah Figura
-
Zebediah Figura (＠zfigura)