[PATCH v3 0/5] MR250: vkd3d-shader/hlsl: Declare vars individually when parsing declarations.

[Francisco Casas (＠fcasas)]

In a declaration with multiple variables, the variables must be created
before the initializer of the next variable is parsed. This is required
for initializers such as:

```
float a = 1, b = a, c = b + 1;
```

The solution is to parse the type information in the same rule as the first variable ('a' in this case) so that it can be declared before the parser reaches the initializer for the following variables.

To initialize the following variables, the type information is passed along from the first variable's struct parse_variable_def to the next and so on.

---

Solves https://bugs.winehq.org/show_bug.cgi?id=54484, pointed out by @nsivov.

--
  v3: vkd3d-shader/hlsl: Declare vars individually when parsing struct declarations.
      vkd3d-shader/hlsl: Declare vars individually when parsing regular declarations.
      vkd3d-shader/hlsl: Split declare_vars().

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

[PATCH v3 1/5] tests: Test semantics in uniform variables.

[Francisco Casas]

From: Francisco Casas

---
 Makefile.am                              |  1 +
 tests/hlsl-uniform-semantics.shader_test | 28 ++++++++++++++++++++++++
 2 files changed, 29 insertions(+)
 create mode 100644 tests/hlsl-uniform-semantics.shader_test

diff --git a/Makefile.am b/Makefile.am
index 92c6d0792..69833e9ce 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -131,6 +131,7 @@ vkd3d_shader_tests = \
    tests/hlsl-transpose.shader_test \
    tests/hlsl-trunc.shader_test \
    tests/hlsl-type-names.shader_test \
+	tests/hlsl-uniform-semantics.shader_test \
    tests/hlsl-vector-indexing.shader_test \
    tests/hlsl-vector-indexing-uniform.shader_test \
    tests/lit.shader_test \
diff --git a/tests/hlsl-uniform-semantics.shader_test b/tests/hlsl-uniform-semantics.shader_test
new file mode 100644
index 000000000..1125117fe
--- /dev/null
+++ b/tests/hlsl-uniform-semantics.shader_test
@@ -0,0 +1,28 @@
+% Semantics can be used in uniforms even though they are ignored.
+
+[pixel shader]
+float a : SEMANTIC;
+
+float4 main() : sv_target
+{
+    return a;
+}
+
+[test]
+uniform 0 float 3.5
+draw quad
+probe all rgba (3.5, 3.5, 3.5, 3.5)
+
+
+[pixel shader]
+float2 a : sv_position;
+
+float4 main() : sv_target
+{
+    return float4(a, a);
+}
+
+[test]
+uniform 0 float4 4.0 5.0 6.0 7.0
+draw quad
+probe all rgba (4.0, 5.0, 4.0, 5.0)

[PATCH v3 2/5] tests: Test multiple variable initializers.

[Francisco Casas]

From: Francisco Casas

---
 Makefile.am                              |  1 +
 tests/hlsl-initializer-multi.shader_test | 52 ++++++++++++++++++++++++
 2 files changed, 53 insertions(+)
 create mode 100644 tests/hlsl-initializer-multi.shader_test

diff --git a/Makefile.am b/Makefile.am
index 69833e9ce..827f427c6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -97,6 +97,7 @@ vkd3d_shader_tests = \
    tests/hlsl-initializer-invalid-arg-count.shader_test \
    tests/hlsl-initializer-local-array.shader_test \
    tests/hlsl-initializer-matrix.shader_test \
+	tests/hlsl-initializer-multi.shader_test \
    tests/hlsl-initializer-nested.shader_test \
    tests/hlsl-initializer-numeric.shader_test \
    tests/hlsl-initializer-objects.shader_test \
diff --git a/tests/hlsl-initializer-multi.shader_test b/tests/hlsl-initializer-multi.shader_test
new file mode 100644
index 000000000..9c45a5330
--- /dev/null
+++ b/tests/hlsl-initializer-multi.shader_test
@@ -0,0 +1,52 @@
+[pixel shader todo]
+float4 main() : sv_target
+{
+    float a = 2.0, b = a + 1.0, c = b;
+
+    return float4(a, b, c, 0);
+}
+
+[test]
+todo draw quad
+probe all rgba (2, 3, 3, 0)
+
+
+[pixel shader fail]
+float4 main() : sv_target
+{
+    float a = 2.0, b = c + 1.0, c = b;
+
+    return float4(0);
+}
+
+
+[pixel shader todo]
+float4 main() : sv_target
+{
+    struct apple {
+        float a;
+        int b;
+    } apple1 = {7.2, 8.1}, apple2 = apple1;
+
+    return float4(apple1.a, apple1.b, apple2.a, apple2.b);
+}
+
+[test]
+todo draw quad
+probe all rgba (7.2, 8.0, 7.2, 8.0)
+
+
+[pixel shader todo]
+float4 main() : sv_target
+{
+    struct apple {
+        float a;
+        int b;
+    } apple1 = {5.2, 9.1}, apples[2] = {apple1, apple1};
+
+    return float4(apples[0].a, apples[0].b, apples[1].a, apples[1].b);
+}
+
+[test]
+todo draw quad
+probe all rgba (5.2, 9.0, 5.2, 9.0)

[PATCH v3 3/5] vkd3d-shader/hlsl: Split declare_vars().

[Francisco Casas]

From: Francisco Casas

Basically, declare_vars() is separated in three functions:

1. check_invalid_in_out_modifiers(), which is to be called once per
   declaration and emits an error when in or out modifiers are used for
   these non-parameter variables.

2. declare_var(), which now handles one variable at the time and doesn't
   free any memory.

3. initialize_vars(), which takes care of preparing the initialization
   instructions of several variables and frees their struct
   parse_variable_def, using exclusively free_parse_variable_def().

This allows to declare variables individually before the initializer of
the next variable in the same declaration is parsed, which is used in
the following patches.

Also, simplifies memory management.
---
 libs/vkd3d-shader/hlsl.y | 388 ++++++++++++++++++++++-----------------
 1 file changed, 218 insertions(+), 170 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 0e07fe578..ba189994e 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -1924,211 +1924,233 @@ static bool type_has_numeric_components(struct hlsl_type *type)
     return false;
 }

-static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_type,
-        unsigned int modifiers, const struct vkd3d_shader_location *modifiers_loc, struct list *var_list)
+static void check_invalid_in_out_modifiers(struct hlsl_ctx *ctx, unsigned int modifiers,
+        const struct vkd3d_shader_location *loc)
+{
+    modifiers &= (HLSL_STORAGE_IN | HLSL_STORAGE_OUT);
+    if (modifiers)
+    {
+        struct vkd3d_string_buffer *string;
+
+        if ((string = hlsl_modifiers_to_string(ctx, modifiers)))
+            hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER,
+                    "Modifiers '%s' are not allowed on non-parameter variables.", string->buffer);
+        hlsl_release_string_buffer(ctx, string);
+    }
+}
+
+static void declare_var(struct hlsl_ctx *ctx, struct hlsl_type *basic_type,
+        unsigned int modifiers, const struct vkd3d_shader_location *modifiers_loc, struct parse_variable_def *v)
 {
-    struct parse_variable_def *v, *v_next;
     struct hlsl_ir_function_decl *func;
-    unsigned int invalid_modifiers;
-    struct list *statements_list;
+    struct hlsl_semantic new_semantic;
+    bool unbounded_res_array = false;
     struct hlsl_ir_var *var;
     struct hlsl_type *type;
     bool local = true;
+    char *var_name;
+    unsigned int i;
+
+    assert(basic_type);

if (basic_type->class == HLSL_CLASS_MATRIX)
         assert(basic_type->modifiers & HLSL_MODIFIERS_MAJORITY_MASK);

-    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);
-        vkd3d_free(var_list);
-        return NULL;
-    }
-
-    if (!var_list)
-        return statements_list;
+    type = basic_type;

-    invalid_modifiers = modifiers & (HLSL_STORAGE_IN | HLSL_STORAGE_OUT);
-    if (invalid_modifiers)
+    if (shader_is_sm_5_1(ctx) && type->class == HLSL_CLASS_OBJECT)
     {
-        struct vkd3d_string_buffer *string;
-
-        if ((string = hlsl_modifiers_to_string(ctx, invalid_modifiers)))
-            hlsl_error(ctx, modifiers_loc, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER,
-                    "Modifiers '%s' are not allowed on non-parameter variables.", string->buffer);
-        hlsl_release_string_buffer(ctx, string);
+        for (i = 0; i < v->arrays.count; ++i)
+            unbounded_res_array |= (v->arrays.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT);
     }

-    LIST_FOR_EACH_ENTRY_SAFE(v, v_next, var_list, struct parse_variable_def, entry)
+    if (unbounded_res_array)
     {
-        bool unbounded_res_array = false;
-        unsigned int i;
-
-        type = basic_type;
-
-        if (shader_is_sm_5_1(ctx) && type->class == HLSL_CLASS_OBJECT)
+        if (v->arrays.count == 1)
         {
-            for (i = 0; i < v->arrays.count; ++i)
-                unbounded_res_array |= (v->arrays.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT);
+            hlsl_fixme(ctx, &v->loc, "Unbounded resource arrays.");
+            return;
         }
-
-        if (unbounded_res_array)
+        else
         {
-            if (v->arrays.count == 1)
-            {
-                hlsl_fixme(ctx, &v->loc, "Unbounded resource arrays.");
-                free_parse_variable_def(v);
-                continue;
-            }
-            else
-            {
-                hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                        "Unbounded resource arrays cannot be multi-dimensional.");
-            }
+            hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                    "Unbounded resource arrays cannot be multi-dimensional.");
         }
-        else
+    }
+    else
+    {
+        for (i = 0; i < v->arrays.count; ++i)
         {
-            for (i = 0; i < v->arrays.count; ++i)
+            if (v->arrays.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT)
             {
-                if (v->arrays.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT)
-                {
-                    unsigned int size = initializer_size(&v->initializer);
-                    unsigned int elem_components = hlsl_type_component_count(type);
-
-                    if (i < v->arrays.count - 1)
-                    {
-                        hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                                "Only innermost array size can be implicit.");
-                        free_parse_initializer(&v->initializer);
-                        v->initializer.args_count = 0;
-                    }
-                    else if (elem_components == 0)
-                    {
-                        hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                                "Cannot declare an implicit size array of a size 0 type.");
-                        free_parse_initializer(&v->initializer);
-                        v->initializer.args_count = 0;
-                    }
-                    else if (size == 0)
-                    {
-                        hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                                "Implicit size arrays need to be initialized.");
-                        free_parse_initializer(&v->initializer);
-                        v->initializer.args_count = 0;
+                unsigned int size = initializer_size(&v->initializer);
+                unsigned int elem_components = hlsl_type_component_count(type);

-                    }
-                    else if (size % elem_components != 0)
-                    {
-                        hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT,
-                                "Cannot initialize implicit size array with %u components, expected a multiple of %u.",
-                                size, elem_components);
-                        free_parse_initializer(&v->initializer);
-                        v->initializer.args_count = 0;
-                    }
-                    else
-                    {
-                        v->arrays.sizes[i] = size / elem_components;
-                    }
+                if (i < v->arrays.count - 1)
+                {
+                    hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                            "Only innermost array size can be implicit.");
+                    v->initializer.args_count = 0;
+                }
+                else if (elem_components == 0)
+                {
+                    hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                            "Cannot declare an implicit size array of a size 0 type.");
+                    v->initializer.args_count = 0;
+                }
+                else if (size == 0)
+                {
+                    hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                            "Implicit size arrays need to be initialized.");
+                    v->initializer.args_count = 0;
+                }
+                else if (size % elem_components != 0)
+                {
+                    hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT,
+                            "Cannot initialize implicit size array with %u components, expected a multiple of %u.",
+                            size, elem_components);
+                    v->initializer.args_count = 0;
+                }
+                else
+                {
+                    v->arrays.sizes[i] = size / elem_components;
                 }
-                type = hlsl_new_array_type(ctx, type, v->arrays.sizes[i]);
             }
+            type = hlsl_new_array_type(ctx, type, v->arrays.sizes[i]);
         }
-        vkd3d_free(v->arrays.sizes);
-
-        if (!(var = hlsl_new_var(ctx, v->name, type, &v->loc, &v->semantic, modifiers, &v->reg_reservation)))
-        {
-            free_parse_variable_def(v);
-            continue;
-        }
+    }

-        var->buffer = ctx->cur_buffer;
+    if (!(var_name = vkd3d_strdup(v->name)))
+        return;

-        if (var->buffer == ctx->globals_buffer)
+    new_semantic = v->semantic;
+    if (v->semantic.name)
+    {
+        if (!(new_semantic.name = vkd3d_strdup(v->semantic.name)))
         {
-            if (var->reg_reservation.offset_type)
-                hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION,
-                        "packoffset() is only allowed inside constant buffer declarations.");
+            vkd3d_free(var_name);
+            return;
         }
+    }

-        if (ctx->cur_scope == ctx->globals)
-        {
-            local = false;
+    if (!(var = hlsl_new_var(ctx, var_name, type, &v->loc, &new_semantic, modifiers, &v->reg_reservation)))
+    {
+        hlsl_cleanup_semantic(&new_semantic);
+        vkd3d_free(var_name);
+        return;
+    }

-            if ((modifiers & HLSL_STORAGE_UNIFORM) && (modifiers & HLSL_STORAGE_STATIC))
-                hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER,
-                        "Variable '%s' is declared as both "uniform" and "static".", var->name);
+    var->buffer = ctx->cur_buffer;

-            /* Mark it as uniform. We need to do this here since synthetic
-             * variables also get put in the global scope, but shouldn't be
-             * considered uniforms, and we have no way of telling otherwise. */
-            if (!(modifiers & HLSL_STORAGE_STATIC))
-                var->storage_modifiers |= HLSL_STORAGE_UNIFORM;
+    if (var->buffer == ctx->globals_buffer)
+    {
+        if (var->reg_reservation.offset_type)
+            hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION,
+                    "packoffset() is only allowed inside constant buffer declarations.");
+    }

-            if (ctx->profile->major_version < 5 && (var->storage_modifiers & HLSL_STORAGE_UNIFORM) &&
-                    type_has_object_components(var->data_type, true))
-            {
-                hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                        "Target profile doesn't support objects as struct members in uniform variables.\n");
-            }
+    if (ctx->cur_scope == ctx->globals)
+    {
+        local = false;

-            if ((func = hlsl_get_func_decl(ctx, var->name)))
-            {
-                hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_REDEFINED,
-                        "'%s' is already defined as a function.", var->name);
-                hlsl_note(ctx, &func->loc, VKD3D_SHADER_LOG_ERROR,
-                        "'%s' was previously defined here.", var->name);
-            }
-        }
-        else
-        {
-            static const unsigned int invalid = HLSL_STORAGE_EXTERN | HLSL_STORAGE_SHARED
-                    | HLSL_STORAGE_GROUPSHARED | HLSL_STORAGE_UNIFORM;
+        if ((modifiers & HLSL_STORAGE_UNIFORM) && (modifiers & HLSL_STORAGE_STATIC))
+            hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER,
+                    "Variable '%s' is declared as both "uniform" and "static".", var->name);

-            if (modifiers & invalid)
-            {
-                struct vkd3d_string_buffer *string;
+        /* Mark it as uniform. We need to do this here since synthetic
+            * variables also get put in the global scope, but shouldn't be
+            * considered uniforms, and we have no way of telling otherwise. */
+        if (!(modifiers & HLSL_STORAGE_STATIC))
+            var->storage_modifiers |= HLSL_STORAGE_UNIFORM;

-                if ((string = hlsl_modifiers_to_string(ctx, modifiers & invalid)))
-                    hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER,
-                            "Modifiers '%s' are not allowed on local variables.", string->buffer);
-                hlsl_release_string_buffer(ctx, string);
-            }
-            if (var->semantic.name)
-                hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SEMANTIC,
-                        "Semantics are not allowed on local variables.");
+        if (ctx->profile->major_version < 5 && (var->storage_modifiers & HLSL_STORAGE_UNIFORM) &&
+                type_has_object_components(var->data_type, true))
+        {
+            hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                    "Target profile doesn't support objects as struct members in uniform variables.\n");
         }

-        if ((var->storage_modifiers & HLSL_STORAGE_STATIC) && type_has_numeric_components(var->data_type)
-                && type_has_object_components(var->data_type, false))
+        if ((func = hlsl_get_func_decl(ctx, var->name)))
         {
-            hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
-                    "Static variables cannot have both numeric and resource components.");
+            hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_REDEFINED,
+                    "'%s' is already defined as a function.", var->name);
+            hlsl_note(ctx, &func->loc, VKD3D_SHADER_LOG_ERROR,
+                    "'%s' was previously defined here.", var->name);
         }
+    }
+    else
+    {
+        static const unsigned int invalid = HLSL_STORAGE_EXTERN | HLSL_STORAGE_SHARED
+                | HLSL_STORAGE_GROUPSHARED | HLSL_STORAGE_UNIFORM;

-        if ((type->modifiers & HLSL_MODIFIER_CONST) && !v->initializer.args_count
-                && !(modifiers & (HLSL_STORAGE_STATIC | HLSL_STORAGE_UNIFORM)))
+        if (modifiers & invalid)
         {
-            hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_MISSING_INITIALIZER,
-                    "Const variable "%s" is missing an initializer.", var->name);
-            hlsl_free_var(var);
-            free_parse_initializer(&v->initializer);
-            vkd3d_free(v);
-            continue;
+            struct vkd3d_string_buffer *string;
+
+            if ((string = hlsl_modifiers_to_string(ctx, modifiers & invalid)))
+                hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER,
+                        "Modifiers '%s' are not allowed on local variables.", string->buffer);
+            hlsl_release_string_buffer(ctx, string);
         }
+        if (var->semantic.name)
+            hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SEMANTIC,
+                    "Semantics are not allowed on local variables.");
+    }
+
+    if ((var->storage_modifiers & HLSL_STORAGE_STATIC) && type_has_numeric_components(var->data_type)
+            && type_has_object_components(var->data_type, false))
+    {
+        hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                "Static variables cannot have both numeric and resource components.");
+    }
+
+    if ((type->modifiers & HLSL_MODIFIER_CONST) && !v->initializer.args_count
+            && !(modifiers & (HLSL_STORAGE_STATIC | HLSL_STORAGE_UNIFORM)))
+    {
+        hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_MISSING_INITIALIZER,
+                "Const variable "%s" is missing an initializer.", var->name);
+        hlsl_free_var(var);
+        return;
+    }
+
+    if (!hlsl_add_var(ctx, var, local))
+    {
+        struct hlsl_ir_var *old = hlsl_get_var(ctx->cur_scope, var->name);
+
+        hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_REDEFINED,
+                "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);
+        return;
+    }
+}
+
+static struct list *initialize_vars(struct hlsl_ctx *ctx, struct list *var_list)
+{
+    struct parse_variable_def *v, *v_next;
+    struct list *statements_list;
+    struct hlsl_ir_var *var;
+    struct hlsl_type *type;

-        if (!hlsl_add_var(ctx, var, local))
+    if (!(statements_list = make_empty_list(ctx)))
+    {
+        LIST_FOR_EACH_ENTRY_SAFE(v, v_next, var_list, struct parse_variable_def, entry)
         {
-            struct hlsl_ir_var *old = hlsl_get_var(ctx->cur_scope, var->name);
+            free_parse_variable_def(v);
+        }
+        vkd3d_free(var_list);
+        return NULL;
+    }

-            hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_REDEFINED,
-                    "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);
-            free_parse_initializer(&v->initializer);
-            vkd3d_free(v);
+    LIST_FOR_EACH_ENTRY_SAFE(v, v_next, var_list, struct parse_variable_def, entry)
+    {
+        /* If this fails, the variable failed to be declared. */
+        if (!(var = hlsl_get_var(ctx->cur_scope, v->name)))
+        {
+            free_parse_variable_def(v);
             continue;
         }
+        type = var->data_type;

if (v->initializer.args_count)
         {
@@ -2143,8 +2165,7 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t
                     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);
                     continue;
                 }

@@ -2163,12 +2184,10 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t
                 add_assignment(ctx, v->initializer.instrs, &load->node, ASSIGN_OP_ASSIGN, v->initializer.args[0]);
             }

-            if (modifiers & HLSL_STORAGE_STATIC)
+            if (var->storage_modifiers & HLSL_STORAGE_STATIC)
                 list_move_tail(&ctx->static_initializers.instrs, v->initializer.instrs);
             else
                 list_move_tail(statements_list, v->initializer.instrs);
-            vkd3d_free(v->initializer.args);
-            vkd3d_free(v->initializer.instrs);
         }
         else if (var->storage_modifiers & HLSL_STORAGE_STATIC)
         {
@@ -2178,32 +2197,33 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t

if (type_has_object_components(var->data_type, false))
             {
-                vkd3d_free(v);
+                free_parse_variable_def(v);
                 continue;
             }

if (!(zero = hlsl_new_uint_constant(ctx, 0, &var->loc)))
             {
-                vkd3d_free(v);
+                free_parse_variable_def(v);
                 continue;
             }
             hlsl_block_add_instr(&ctx->static_initializers, zero);

if (!(cast = add_cast(ctx, &ctx->static_initializers.instrs, zero, var->data_type, &var->loc)))
             {
-                vkd3d_free(v);
+                free_parse_variable_def(v);
                 continue;
             }

if (!(store = hlsl_new_simple_store(ctx, var, cast)))
             {
-                vkd3d_free(v);
+                free_parse_variable_def(v);
                 continue;
             }
             hlsl_block_add_instr(&ctx->static_initializers, store);
         }
-        vkd3d_free(v);
+        free_parse_variable_def(v);
     }
+
     vkd3d_free(var_list);
     return statements_list;
 }
@@ -4564,6 +4584,7 @@ preproc_directive:
 struct_declaration:
       var_modifiers struct_spec variables_def_optional ';'
         {
+            struct parse_variable_def *v, *v_next;
             struct hlsl_type *type;
             unsigned int modifiers = $1;

@@ -4579,7 +4600,24 @@ struct_declaration:

if (!(type = apply_type_modifiers(ctx, $2, &modifiers, true, &@1)))
                 YYABORT;
-            $$ = declare_vars(ctx, type, modifiers, &@1, $3);
+
+            check_invalid_in_out_modifiers(ctx, modifiers, &@1);
+
+            if ($3)
+            {
+                LIST_FOR_EACH_ENTRY_SAFE(v, v_next, $3, struct parse_variable_def, entry)
+                {
+                    declare_var(ctx, type, modifiers, &@1, v);
+                }
+
+                if (!($$ = initialize_vars(ctx, $3)))
+                    YYABORT;
+            }
+            else
+            {
+                if (!($$ = make_empty_list(ctx)))
+                    YYABORT;
+            }
         }

struct_spec:
@@ -5418,12 +5456,22 @@ type_spec:
 declaration:
       var_modifiers type variables_def ';'
         {
+            struct parse_variable_def *v, *v_next;
             struct hlsl_type *type;
             unsigned int modifiers = $1;

if (!(type = apply_type_modifiers(ctx, $2, &modifiers, true, &@1)))
                 YYABORT;
-            $$ = declare_vars(ctx, type, modifiers, &@1, $3);
+
+            check_invalid_in_out_modifiers(ctx, modifiers, &@1);
+
+            LIST_FOR_EACH_ENTRY_SAFE(v, v_next, $3, struct parse_variable_def, entry)
+            {
+                declare_var(ctx, type, modifiers, &@1, v);
+            }
+
+            if (!($$ = initialize_vars(ctx, $3)))
+                YYABORT;
         }

variables_def_optional:

[PATCH v3 4/5] vkd3d-shader/hlsl: Declare vars individually when parsing regular declarations.

[Francisco Casas]

From: Francisco Casas

In a declaration with multiple variables, the variables must be created
before the initializer of the next variable is parsed. This is required
for initializers such as:

float a = 1, b = a, c = b + 1;

A requisite for this is that the type information is parsed in the same
rule as the first variable (as a variable_def_typed) so it is
immediately available to declare the first variable. Then, the next
untyped variable declaration is parsed, and the type from the first
variable can be used to declare the second, before the third is parsed,
and so on.
---
 libs/vkd3d-shader/hlsl.y                 | 79 ++++++++++++++++++------
 tests/hlsl-initializer-multi.shader_test |  4 +-
 2 files changed, 62 insertions(+), 21 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index ba189994e..0a468110b 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -73,6 +73,10 @@ struct parse_variable_def
     struct hlsl_semantic semantic;
     struct hlsl_reg_reservation reg_reservation;
     struct parse_initializer initializer;
+
+    struct hlsl_type *basic_type;
+    unsigned int modifiers;
+    struct vkd3d_shader_location modifiers_loc;
 };

struct parse_function
@@ -1939,11 +1943,12 @@ static void check_invalid_in_out_modifiers(struct hlsl_ctx *ctx, unsigned int mo
     }
 }

-static void declare_var(struct hlsl_ctx *ctx, struct hlsl_type *basic_type,
-        unsigned int modifiers, const struct vkd3d_shader_location *modifiers_loc, struct parse_variable_def *v)
+static void declare_var(struct hlsl_ctx *ctx, struct parse_variable_def *v)
 {
+    struct hlsl_type *basic_type = v->basic_type;
     struct hlsl_ir_function_decl *func;
     struct hlsl_semantic new_semantic;
+    uint32_t modifiers = v->modifiers;
     bool unbounded_res_array = false;
     struct hlsl_ir_var *var;
     struct hlsl_type *type;
@@ -4451,6 +4456,7 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type
 %type <list> unary_expr
 %type <list> variables_def
 %type <list> variables_def_optional
+%type <list> variables_def_typed

%token <name> VAR_IDENTIFIER
 %token <name> NEW_IDENTIFIER
@@ -4513,6 +4519,7 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type
 %type <variable_def> type_spec
 %type <variable_def> variable_decl
 %type <variable_def> variable_def
+%type <variable_def> variable_def_typed

%%

@@ -4607,7 +4614,11 @@ struct_declaration:
             {
                 LIST_FOR_EACH_ENTRY_SAFE(v, v_next, $3, struct parse_variable_def, entry)
                 {
-                    declare_var(ctx, type, modifiers, &@1, v);
+                    v->basic_type = type;
+                    v->modifiers = modifiers;
+                    v->modifiers_loc = @1;
+
+                    declare_var(ctx, v);
                 }

if (!($$ = initialize_vars(ctx, $3)))
@@ -5454,23 +5465,9 @@ type_spec:
         }

declaration:
-      var_modifiers type variables_def ';'
+      variables_def_typed ';'
         {
-            struct parse_variable_def *v, *v_next;
-            struct hlsl_type *type;
-            unsigned int modifiers = $1;
-
-            if (!(type = apply_type_modifiers(ctx, $2, &modifiers, true, &@1)))
-                YYABORT;
-
-            check_invalid_in_out_modifiers(ctx, modifiers, &@1);
-
-            LIST_FOR_EACH_ENTRY_SAFE(v, v_next, $3, struct parse_variable_def, entry)
-            {
-                declare_var(ctx, type, modifiers, &@1, v);
-            }
-
-            if (!($$ = initialize_vars(ctx, $3)))
+            if (!($$ = initialize_vars(ctx, $1)))
                 YYABORT;
         }

@@ -5494,6 +5491,33 @@ variables_def:
             list_add_tail($$, &$3->entry);
         }

+variables_def_typed:
+      variable_def_typed
+        {
+            if (!($$ = make_empty_list(ctx)))
+                YYABORT;
+            list_add_head($$, &$1->entry);
+
+            declare_var(ctx, $1);
+        }
+    | variables_def_typed ',' variable_def
+        {
+            struct parse_variable_def *head_def;
+
+            assert(!list_empty($1));
+            head_def = LIST_ENTRY(list_head($1), struct parse_variable_def, entry);
+
+            assert(head_def->basic_type);
+            $3->basic_type = head_def->basic_type;
+            $3->modifiers = head_def->modifiers;
+            $3->modifiers_loc = head_def->modifiers_loc;
+
+            declare_var(ctx, $3);
+
+            $$ = $1;
+            list_add_tail($$, &$3->entry);
+        }
+
 variable_decl:
       any_identifier arrays colon_attribute
         {
@@ -5535,6 +5559,23 @@ variable_def:
             ctx->in_state_block = 0;
         }

+variable_def_typed:
+      var_modifiers type variable_def
+        {
+            unsigned int modifiers = $1;
+            struct hlsl_type *type;
+
+            if (!(type = apply_type_modifiers(ctx, $2, &modifiers, true, &@1)))
+                YYABORT;
+
+            check_invalid_in_out_modifiers(ctx, modifiers, &@1);
+
+            $$ = $3;
+            $$->basic_type = type;
+            $$->modifiers = modifiers;
+            $$->modifiers_loc = @1;
+        }
+
 arrays:
       %empty
         {
diff --git a/tests/hlsl-initializer-multi.shader_test b/tests/hlsl-initializer-multi.shader_test
index 9c45a5330..73dd9f2ff 100644
--- a/tests/hlsl-initializer-multi.shader_test
+++ b/tests/hlsl-initializer-multi.shader_test
@@ -1,4 +1,4 @@
-[pixel shader todo]
+[pixel shader]
 float4 main() : sv_target
 {
     float a = 2.0, b = a + 1.0, c = b;
@@ -7,7 +7,7 @@ float4 main() : sv_target
 }

[test]
-todo draw quad
+draw quad
 probe all rgba (2, 3, 3, 0)

[PATCH v3 5/5] vkd3d-shader/hlsl: Declare vars individually when parsing struct declarations.

[Francisco Casas]

From: Francisco Casas

A struct declaration with variables is now absorbed into the 'declaration'
rule, like any other variable declaration.

A struct declaration without variables is now reduced to the
'struct_declaration_without_vars' rule.

They both are reduced to a 'declaration_statement' in the end.
---
 libs/vkd3d-shader/hlsl.y                 | 75 +++++++++---------------
 tests/hlsl-initializer-multi.shader_test |  8 +--
 2 files changed, 31 insertions(+), 52 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 0a468110b..c3d085c83 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -4451,11 +4451,10 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type
 %type <list> shift_expr
 %type <list> statement
 %type <list> statement_list
-%type <list> struct_declaration
+%type <list> struct_declaration_without_vars
 %type <list> type_specs
 %type <list> unary_expr
 %type <list> variables_def
-%type <list> variables_def_optional
 %type <list> variables_def_typed

%token <name> VAR_IDENTIFIER
@@ -4588,47 +4587,19 @@ preproc_directive:
             }
         }

-struct_declaration:
-      var_modifiers struct_spec variables_def_optional ';'
+struct_declaration_without_vars:
+      var_modifiers struct_spec ';'
         {
-            struct parse_variable_def *v, *v_next;
-            struct hlsl_type *type;
-            unsigned int modifiers = $1;
+            if (!$2->name)
+                hlsl_error(ctx, &@2, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX,
+                    "Anonymous struct type must declare a variable.");

-            if (!$3)
-            {
-                if (!$2->name)
-                    hlsl_error(ctx, &@2, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX,
-                            "Anonymous struct type must declare a variable.");
-                if (modifiers)
-                    hlsl_error(ctx, &@1, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER,
-                            "Modifiers are not allowed on struct type declarations.");
-            }
+            if ($1)
+                hlsl_error(ctx, &@1, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER,
+                        "Modifiers are not allowed on struct type declarations.");

-            if (!(type = apply_type_modifiers(ctx, $2, &modifiers, true, &@1)))
+            if (!($$ = make_empty_list(ctx)))
                 YYABORT;
-
-            check_invalid_in_out_modifiers(ctx, modifiers, &@1);
-
-            if ($3)
-            {
-                LIST_FOR_EACH_ENTRY_SAFE(v, v_next, $3, struct parse_variable_def, entry)
-                {
-                    v->basic_type = type;
-                    v->modifiers = modifiers;
-                    v->modifiers_loc = @1;
-
-                    declare_var(ctx, v);
-                }
-
-                if (!($$ = initialize_vars(ctx, $3)))
-                    YYABORT;
-            }
-            else
-            {
-                if (!($$ = make_empty_list(ctx)))
-                    YYABORT;
-            }
         }

struct_spec:
@@ -5403,7 +5374,7 @@ type:

declaration_statement:
       declaration
-    | struct_declaration
+    | struct_declaration_without_vars
     | typedef
         {
             if (!($$ = make_empty_list(ctx)))
@@ -5471,13 +5442,6 @@ declaration:
                 YYABORT;
         }

-variables_def_optional:
-      %empty
-        {
-            $$ = NULL;
-        }
-    | variables_def
-
 variables_def:
       variable_def
         {
@@ -5560,7 +5524,22 @@ variable_def:
         }

variable_def_typed:
-      var_modifiers type variable_def
+      var_modifiers struct_spec variable_def
+        {
+            unsigned int modifiers = $1;
+            struct hlsl_type *type;
+
+            if (!(type = apply_type_modifiers(ctx, $2, &modifiers, true, &@1)))
+                YYABORT;
+
+            check_invalid_in_out_modifiers(ctx, modifiers, &@1);
+
+            $$ = $3;
+            $$->basic_type = type;
+            $$->modifiers = modifiers;
+            $$->modifiers_loc = @1;
+        }
+    | var_modifiers type variable_def
         {
             unsigned int modifiers = $1;
             struct hlsl_type *type;
diff --git a/tests/hlsl-initializer-multi.shader_test b/tests/hlsl-initializer-multi.shader_test
index 73dd9f2ff..8f8a31e20 100644
--- a/tests/hlsl-initializer-multi.shader_test
+++ b/tests/hlsl-initializer-multi.shader_test
@@ -20,7 +20,7 @@ float4 main() : sv_target
 }

-[pixel shader todo]
+[pixel shader]
 float4 main() : sv_target
 {
     struct apple {
@@ -32,11 +32,11 @@ float4 main() : sv_target
 }

[test]
-todo draw quad
+draw quad
 probe all rgba (7.2, 8.0, 7.2, 8.0)

-[pixel shader todo]
+[pixel shader]
 float4 main() : sv_target
 {
     struct apple {
@@ -48,5 +48,5 @@ float4 main() : sv_target
 }

[test]
-todo draw quad
+draw quad
 probe all rgba (5.2, 9.0, 5.2, 9.0)

[Francisco Casas (＠fcasas)]

On Thu Jun 29 17:14:17 2023 +0000, Zebediah Figura wrote:
> 3/5 has some memory-management parts which aren't exactly related to
> what the patch does. I'm not sure if it's worth splitting at this point, though.
> One problem with it though is that the invalid modifiers check is part
> of declare_var(), which means it'll be printed multiple times if there
> are multiple variables.
> Apart from that I think the series looks good, and simpler than I would
> have expected.

I moved the check outside declare_var() so that the error doesn't appear multiple times.

[PATCH v3 0/5] MR250: vkd3d-shader/hlsl: Declare vars individually when parsing declarations. - approved

[Giovanni Mascellani (＠giomasce)]

This merge request was approved by Giovanni Mascellani.