[PATCH v2 0/4] MR275: hlsl: Replace bare list pointers with hlsl_block, part 4.

[Zebediah Figura (＠zfigura)]

--
  v2: vkd3d-shader/hlsl: Use a hlsl_block to build replacement instructions in lower_int_division().

https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/275

[PATCH v2 1/4] vkd3d-shader/hlsl: Return a hlsl_block from the "primary_expr" rule.

[Zebediah Figura]

From: Zebediah Figura

---
 libs/vkd3d-shader/hlsl.y | 33 +++++++++++++++------------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 6bf87f8f..c41e4ae0 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -1117,17 +1117,17 @@ static struct hlsl_ir_function_decl *get_func_decl(struct rb_tree *funcs,
     return NULL;
 }

-static struct list *make_list(struct hlsl_ctx *ctx, struct hlsl_ir_node *node)
+static struct hlsl_block *make_block(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr)
 {
-    struct list *list;
+    struct hlsl_block *block;

-    if (!(list = make_empty_list(ctx)))
+    if (!(block = make_empty_block(ctx)))
     {
-        hlsl_free_instr(node);
+        hlsl_free_instr(instr);
         return NULL;
     }
-    list_add_tail(list, &node->entry);
-    return list;
+    hlsl_block_add_instr(block, instr);
+    return block;
 }

static unsigned int evaluate_static_expression_as_uint(struct hlsl_ctx *ctx, struct hlsl_block *block,
@@ -3684,7 +3684,7 @@ static int intrinsic_function_name_compare(const void *a, const void *b)
     return strcmp(a, func->name);
 }

-static struct list *add_call(struct hlsl_ctx *ctx, const char *name,
+static struct hlsl_block *add_call(struct hlsl_ctx *ctx, const char *name,
         struct parse_initializer *args, const struct vkd3d_shader_location *loc)
 {
     struct intrinsic_function *intrinsic;
@@ -3812,7 +3812,7 @@ static struct list *add_call(struct hlsl_ctx *ctx, const char *name,
         goto fail;
     }
     vkd3d_free(args->args);
-    return block_to_list(args->instrs);
+    return args->instrs;

fail:
     free_parse_initializer(args);
@@ -4559,7 +4559,6 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type

%type <list> declaration
 %type <list> declaration_statement
-%type <list> primary_expr
 %type <list> struct_declaration_without_vars
 %type <list> type_specs
 %type <list> variables_def
@@ -4597,6 +4596,7 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type
 %type <block> loop_statement
 %type <block> mul_expr
 %type <block> postfix_expr
+%type <block> primary_expr
 %type <block> relational_expr
 %type <block> shift_expr
 %type <block> selection_statement
@@ -6042,7 +6042,7 @@ primary_expr:

if (!(c = hlsl_new_float_constant(ctx, $1, &@1)))
                 YYABORT;
-            if (!($$ = make_list(ctx, c)))
+            if (!($$ = make_block(ctx, c)))
                 YYABORT;
         }
     | C_INTEGER
@@ -6051,7 +6051,7 @@ primary_expr:

if (!(c = hlsl_new_int_constant(ctx, $1, &@1)))
                 YYABORT;
-            if (!($$ = make_list(ctx, c)))
+            if (!($$ = make_block(ctx, c)))
                 YYABORT;
         }
     | boolean
@@ -6060,7 +6060,7 @@ primary_expr:

if (!(c = hlsl_new_bool_constant(ctx, $1, &@1)))
                 YYABORT;
-            if (!($$ = make_list(ctx, c)))
+            if (!($$ = make_block(ctx, c)))
             {
                 hlsl_free_instr(c);
                 YYABORT;
@@ -6078,12 +6078,12 @@ primary_expr:
             }
             if (!(load = hlsl_new_var_load(ctx, var, &@1)))
                 YYABORT;
-            if (!($$ = make_list(ctx, &load->node)))
+            if (!($$ = make_block(ctx, &load->node)))
                 YYABORT;
         }
     | '(' expr ')'
         {
-            $$ = block_to_list($2);
+            $$ = $2;
         }
     | var_identifier '(' func_arguments ')'
         {
@@ -6106,7 +6106,7 @@ primary_expr:
                     YYABORT;
                 if (!(load = hlsl_new_var_load(ctx, var, &@1)))
                     YYABORT;
-                if (!($$ = make_list(ctx, &load->node)))
+                if (!($$ = make_block(ctx, &load->node)))
                     YYABORT;
             }
             else
@@ -6118,9 +6118,6 @@ primary_expr:

postfix_expr:
       primary_expr
-        {
-            $$ = list_to_block($1);
-        }
     | postfix_expr OP_INC
         {
             if (!add_increment(ctx, $1, false, true, &@2))

[PATCH v2 2/4] vkd3d-shader/hlsl: Return a hlsl_block from the "declaration_statement" rule.

[Zebediah Figura]

From: Zebediah Figura

---
 libs/vkd3d-shader/hlsl.y | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index c41e4ae0..b9dcb5bc 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -172,12 +172,6 @@ static struct list *make_empty_list(struct hlsl_ctx *ctx)
     return list;
 }

-static void destroy_instr_list(struct list *list)
-{
-    hlsl_free_instr_list(list);
-    vkd3d_free(list);
-}
-
 static void destroy_block(struct hlsl_block *block)
 {
     hlsl_block_cleanup(block);
@@ -4558,7 +4552,6 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type
 %token <intval> PRE_LINE

%type <list> declaration
-%type <list> declaration_statement
 %type <list> struct_declaration_without_vars
 %type <list> type_specs
 %type <list> variables_def
@@ -4585,6 +4578,7 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type
 %type <block> bitxor_expr
 %type <block> compound_statement
 %type <block> conditional_expr
+%type <block> declaration_statement
 %type <block> equality_expr
 %type <block> expr
 %type <block> expr_optional
@@ -4661,9 +4655,9 @@ hlsl_prog:
     | hlsl_prog buffer_declaration buffer_body
     | hlsl_prog declaration_statement
         {
-            if (!list_empty($2))
+            if (!list_empty(&$2->instrs))
                 hlsl_fixme(ctx, &@2, "Uniform initializer.");
-            destroy_instr_list($2);
+            destroy_block($2);
         }
     | hlsl_prog preproc_directive
     | hlsl_prog ';'
@@ -5512,10 +5506,16 @@ type:

declaration_statement:
       declaration
+        {
+            $$ = list_to_block($1);
+        }
     | struct_declaration_without_vars
+        {
+            $$ = list_to_block($1);
+        }
     | typedef
         {
-            if (!($$ = make_empty_list(ctx)))
+            if (!($$ = make_empty_block(ctx)))
                 YYABORT;
         }

@@ -5912,9 +5912,6 @@ statement_list:

statement:
       declaration_statement
-        {
-            $$ = list_to_block($1);
-        }
     | expr_statement
     | compound_statement
     | jump_statement

[PATCH v2 3/4] vkd3d-shader/hlsl: Return a hlsl_block from the "declaration" and "struct_declaration" rules.

[Zebediah Figura]

From: Zebediah Figura

---
 libs/vkd3d-shader/hlsl.y | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index b9dcb5bc..96adada7 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -2177,14 +2177,14 @@ static void declare_var(struct hlsl_ctx *ctx, struct parse_variable_def *v)
     }
 }

-static struct list *initialize_vars(struct hlsl_ctx *ctx, struct list *var_list)
+static struct hlsl_block *initialize_vars(struct hlsl_ctx *ctx, struct list *var_list)
 {
     struct parse_variable_def *v, *v_next;
-    struct list *statements_list;
+    struct hlsl_block *initializers;
     struct hlsl_ir_var *var;
     struct hlsl_type *type;

-    if (!(statements_list = make_empty_list(ctx)))
+    if (!(initializers = make_empty_block(ctx)))
     {
         LIST_FOR_EACH_ENTRY_SAFE(v, v_next, var_list, struct parse_variable_def, entry)
         {
@@ -2239,7 +2239,7 @@ static struct list *initialize_vars(struct hlsl_ctx *ctx, struct list *var_list)
             if (var->storage_modifiers & HLSL_STORAGE_STATIC)
                 hlsl_block_add_block(&ctx->static_initializers, v->initializer.instrs);
             else
-                list_move_tail(statements_list, &v->initializer.instrs->instrs);
+                hlsl_block_add_block(initializers, v->initializer.instrs);
         }
         else if (var->storage_modifiers & HLSL_STORAGE_STATIC)
         {
@@ -2277,7 +2277,7 @@ static struct list *initialize_vars(struct hlsl_ctx *ctx, struct list *var_list)
     }

vkd3d_free(var_list);
-    return statements_list;
+    return initializers;
 }

struct find_function_call_args
@@ -4551,8 +4551,6 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type
 %token <intval> C_INTEGER
 %token <intval> PRE_LINE

-%type <list> declaration
-%type <list> struct_declaration_without_vars
 %type <list> type_specs
 %type <list> variables_def
 %type <list> variables_def_typed
@@ -4578,6 +4576,7 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type
 %type <block> bitxor_expr
 %type <block> compound_statement
 %type <block> conditional_expr
+%type <block> declaration
 %type <block> declaration_statement
 %type <block> equality_expr
 %type <block> expr
@@ -4596,6 +4595,7 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type
 %type <block> selection_statement
 %type <block> statement
 %type <block> statement_list
+%type <block> struct_declaration_without_vars
 %type <block> unary_expr

%type <boolval> boolean
@@ -4725,7 +4725,7 @@ struct_declaration_without_vars:
                 hlsl_error(ctx, &@1, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER,
                         "Modifiers are not allowed on struct type declarations.");

-            if (!($$ = make_empty_list(ctx)))
+            if (!($$ = make_empty_block(ctx)))
                 YYABORT;
         }

@@ -5506,13 +5506,7 @@ type:

declaration_statement:
       declaration
-        {
-            $$ = list_to_block($1);
-        }
     | struct_declaration_without_vars
-        {
-            $$ = list_to_block($1);
-        }
     | typedef
         {
             if (!($$ = make_empty_block(ctx)))
@@ -6003,7 +5997,7 @@ loop_statement:
         }
     | attribute_list_optional KW_FOR '(' scope_start declaration expr_statement expr_optional ')' statement
         {
-            $$ = create_loop(ctx, LOOP_FOR, &$1, list_to_block($5), $6, $7, $9, &@2);
+            $$ = create_loop(ctx, LOOP_FOR, &$1, $5, $6, $7, $9, &@2);
             hlsl_pop_scope(ctx);
         }

[PATCH v2 4/4] vkd3d-shader/hlsl: Use a hlsl_block to build replacement instructions in lower_int_division().

[Zebediah Figura]

From: Zebediah Figura

---
 libs/vkd3d-shader/hlsl_codegen.c | 63 +++++++++++++++++++++++---------
 1 file changed, 45 insertions(+), 18 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index 8927e291..f97447c7 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -573,6 +573,37 @@ bool hlsl_transform_ir(struct hlsl_ctx *ctx, bool (*func)(struct hlsl_ctx *ctx,
     return progress;
 }

+typedef bool (*PFN_lower_func)(struct hlsl_ctx *, struct hlsl_ir_node *, struct hlsl_block *);
+
+static bool call_lower_func(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context)
+{
+    PFN_lower_func func = context;
+    struct hlsl_block block;
+
+    hlsl_block_init(&block);
+    if (func(ctx, instr, &block))
+    {
+        struct hlsl_ir_node *replacement = LIST_ENTRY(list_tail(&block.instrs), struct hlsl_ir_node, entry);
+
+        list_move_before(&instr->entry, &block.instrs);
+        hlsl_replace_node(instr, replacement);
+        return true;
+    }
+    else
+    {
+        hlsl_block_cleanup(&block);
+        return false;
+    }
+}
+
+/* Specific form of transform_ir() for passes which convert a single instruction
+ * to a block of one or more instructions. This helper takes care of setting up
+ * the block and calling hlsl_replace_node_with_block(). */
+static bool lower_ir(struct hlsl_ctx *ctx, PFN_lower_func func, struct hlsl_block *block)
+{
+    return hlsl_transform_ir(ctx, call_lower_func, block, func);
+}
+
 static bool transform_instr_derefs(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context)
 {
     bool res;
@@ -2299,9 +2330,9 @@ struct hlsl_ir_node *hlsl_add_conditional(struct hlsl_ctx *ctx, struct list *ins
     return &load->node;
 }

-static bool lower_int_division(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context)
+static bool lower_int_division(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, struct hlsl_block *block)
 {
-    struct hlsl_ir_node *arg1, *arg2, *xor, *and, *abs1, *abs2, *div, *neg, *cast1, *cast2, *cast3, *cond, *high_bit;
+    struct hlsl_ir_node *arg1, *arg2, *xor, *and, *abs1, *abs2, *div, *neg, *cast1, *cast2, *cast3, *high_bit;
     struct hlsl_type *type = instr->data_type, *utype;
     struct hlsl_constant_value high_bit_value;
     struct hlsl_ir_expr *expr;
@@ -2322,51 +2353,47 @@ static bool lower_int_division(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr,

if (!(xor = hlsl_new_binary_expr(ctx, HLSL_OP2_BIT_XOR, arg1, arg2)))
         return false;
-    list_add_before(&instr->entry, &xor->entry);
+    hlsl_block_add_instr(block, xor);

for (i = 0; i < type->dimx; ++i)
         high_bit_value.u[i].u = 0x80000000;
     if (!(high_bit = hlsl_new_constant(ctx, type, &high_bit_value, &instr->loc)))
         return false;
-    list_add_before(&instr->entry, &high_bit->entry);
+    hlsl_block_add_instr(block, high_bit);

if (!(and = hlsl_new_binary_expr(ctx, HLSL_OP2_BIT_AND, xor, high_bit)))
         return false;
-    list_add_before(&instr->entry, &and->entry);
+    hlsl_block_add_instr(block, and);

if (!(abs1 = hlsl_new_unary_expr(ctx, HLSL_OP1_ABS, arg1, &instr->loc)))
         return false;
-    list_add_before(&instr->entry, &abs1->entry);
+    hlsl_block_add_instr(block, abs1);

if (!(cast1 = hlsl_new_cast(ctx, abs1, utype, &instr->loc)))
         return false;
-    list_add_before(&instr->entry, &cast1->entry);
+    hlsl_block_add_instr(block, cast1);

if (!(abs2 = hlsl_new_unary_expr(ctx, HLSL_OP1_ABS, arg2, &instr->loc)))
         return false;
-    list_add_before(&instr->entry, &abs2->entry);
+    hlsl_block_add_instr(block, abs2);

if (!(cast2 = hlsl_new_cast(ctx, abs2, utype, &instr->loc)))
         return false;
-    list_add_before(&instr->entry, &cast2->entry);
+    hlsl_block_add_instr(block, cast2);

if (!(div = hlsl_new_binary_expr(ctx, HLSL_OP2_DIV, cast1, cast2)))
         return false;
-    list_add_before(&instr->entry, &div->entry);
+    hlsl_block_add_instr(block, div);

if (!(cast3 = hlsl_new_cast(ctx, div, type, &instr->loc)))
         return false;
-    list_add_before(&instr->entry, &cast3->entry);
+    hlsl_block_add_instr(block, cast3);

if (!(neg = hlsl_new_unary_expr(ctx, HLSL_OP1_NEG, cast3, &instr->loc)))
         return false;
-    list_add_before(&instr->entry, &neg->entry);
+    hlsl_block_add_instr(block, neg);

-    if (!(cond = hlsl_add_conditional(ctx, &instr->entry, and, neg, cast3)))
-        return false;
-    hlsl_replace_node(instr, cond);
-
-    return true;
+    return hlsl_add_conditional(ctx, &block->instrs, and, neg, cast3);
 }

static bool lower_int_modulus(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context)
@@ -4150,7 +4177,7 @@ int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry
     hlsl_transform_ir(ctx, lower_narrowing_casts, body, NULL);
     hlsl_transform_ir(ctx, lower_casts_to_bool, body, NULL);
     hlsl_transform_ir(ctx, lower_int_dot, body, NULL);
-    hlsl_transform_ir(ctx, lower_int_division, body, NULL);
+    lower_ir(ctx, lower_int_division, body);
     hlsl_transform_ir(ctx, lower_int_modulus, body, NULL);
     hlsl_transform_ir(ctx, lower_int_abs, body, NULL);
     hlsl_transform_ir(ctx, lower_float_modulus, body, NULL);

[Zebediah Figura (＠zfigura)]

Giovanni pointed out off-list that the new version now leaks instructions on error paths.

In order to resolve that I've shuffled things around in my local branch—4/5 is removed from this series (I'll send it separately later) and 5/5 is combined with a later patch I had that introduces a lower_ir() helper, which takes care of a bit more of the hlsl_block boilerplate. This lower_ir() helper ends up being useful for pretty much every lowering pass, of which we have quite a lot.

[PATCH v2 0/4] MR275: hlsl: Replace bare list pointers with hlsl_block, part 4. - approved

[Giovanni Mascellani (＠giomasce)]

This merge request was approved by Giovanni Mascellani.

[PATCH v2 0/4] MR275: hlsl: Replace bare list pointers with hlsl_block, part 4. - approved

[Henri Verbeet (＠hverbeet)]

This merge request was approved by Henri Verbeet.