[PATCH vkd3d 1/8] vkd3d-shader/hlsl: Use a more sensible source location in add_constructor().

[Giovanni Mascellani]

Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
 libs/vkd3d-shader/hlsl.y | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 905dbfc5..e518430f 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -2049,7 +2049,7 @@ static struct list *add_constructor(struct hlsl_ctx *ctx, struct hlsl_type *type
             continue;
         }
 
-        initialize_var_components(ctx, params->instrs, var, &idx, arg, &loc);
+        initialize_var_components(ctx, params->instrs, var, &idx, arg, &arg->loc);
     }
 
     if (!(load = hlsl_new_var_load(ctx, var, loc)))
-- 
2.36.0

[PATCH vkd3d 2/8] vkd3d-shader/hlsl: Remove the location parameter from initialize_var_components().

[Giovanni Mascellani]

Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
---
 libs/vkd3d-shader/hlsl.y | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index e518430f..44e4964f 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -1536,8 +1536,7 @@ static bool add_increment(struct hlsl_ctx *ctx, struct list *instrs, bool decrem
 }
 
 static void initialize_var_components(struct hlsl_ctx *ctx, struct list *instrs,
-        struct hlsl_ir_var *dst, unsigned int *store_index, struct hlsl_ir_node *src,
-        const struct vkd3d_shader_location *loc)
+        struct hlsl_ir_var *dst, unsigned int *store_index, struct hlsl_ir_node *src)
 {
     unsigned int src_comp_count = hlsl_type_component_count(src->data_type);
     unsigned int k;
@@ -1554,21 +1553,21 @@ static void initialize_var_components(struct hlsl_ctx *ctx, struct list *instrs,
         dst_reg_offset = hlsl_compute_component_offset(ctx, dst->data_type, *store_index, &dst_comp_type);
         src_reg_offset = hlsl_compute_component_offset(ctx, src->data_type, k, &src_comp_type);
 
-        if (!(c = hlsl_new_uint_constant(ctx, src_reg_offset, loc)))
+        if (!(c = hlsl_new_uint_constant(ctx, src_reg_offset, &src->loc)))
             return;
         list_add_tail(instrs, &c->node.entry);
 
-        if (!(load = add_load(ctx, instrs, src, &c->node, src_comp_type, *loc)))
+        if (!(load = add_load(ctx, instrs, src, &c->node, src_comp_type, src->loc)))
             return;
 
-        if (!(conv = add_implicit_conversion(ctx, instrs, &load->node, dst_comp_type, loc)))
+        if (!(conv = add_implicit_conversion(ctx, instrs, &load->node, dst_comp_type, &src->loc)))
             return;
 
-        if (!(c = hlsl_new_uint_constant(ctx, dst_reg_offset, loc)))
+        if (!(c = hlsl_new_uint_constant(ctx, dst_reg_offset, &src->loc)))
             return;
         list_add_tail(instrs, &c->node.entry);
 
-        if (!(store = hlsl_new_store(ctx, dst, &c->node, conv, 0, *loc)))
+        if (!(store = hlsl_new_store(ctx, dst, &c->node, conv, 0, src->loc)))
             return;
         list_add_tail(instrs, &store->node.entry);
 
@@ -1716,7 +1715,7 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t
                 for (k = 0; k < v->initializer.args_count; ++k)
                 {
                     initialize_var_components(ctx, v->initializer.instrs, var,
-                            &store_index, v->initializer.args[k], &v->initializer.args[k]->loc);
+                            &store_index, v->initializer.args[k]);
                 }
             }
             else
@@ -2049,7 +2048,7 @@ static struct list *add_constructor(struct hlsl_ctx *ctx, struct hlsl_type *type
             continue;
         }
 
-        initialize_var_components(ctx, params->instrs, var, &idx, arg, &arg->loc);
+        initialize_var_components(ctx, params->instrs, var, &idx, arg);
     }
 
     if (!(load = hlsl_new_var_load(ctx, var, loc)))
-- 
2.36.0

[PATCH vkd3d 2/8] vkd3d-shader/hlsl: Remove the location parameter from initialize_var_components().

[Francisco Casas]

Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com>


May 3, 2022 6:14 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote:
> Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
> ---
> libs/vkd3d-shader/hlsl.y | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
> index e518430f..44e4964f 100644
> --- a/libs/vkd3d-shader/hlsl.y
> +++ b/libs/vkd3d-shader/hlsl.y
> @@ -1536,8 +1536,7 @@ static bool add_increment(struct hlsl_ctx *ctx, struct list *instrs, bool
> decrem
> }
>
> static void initialize_var_components(struct hlsl_ctx *ctx, struct list *instrs,
> - struct hlsl_ir_var *dst, unsigned int *store_index, struct hlsl_ir_node *src,
> - const struct vkd3d_shader_location *loc)
> + struct hlsl_ir_var *dst, unsigned int *store_index, struct hlsl_ir_node *src)
> {
> unsigned int src_comp_count = hlsl_type_component_count(src->data_type);
> unsigned int k;
> @@ -1554,21 +1553,21 @@ static void initialize_var_components(struct hlsl_ctx *ctx, struct list
> *instrs,
> dst_reg_offset = hlsl_compute_component_offset(ctx, dst->data_type, *store_index, &dst_comp_type);
> src_reg_offset = hlsl_compute_component_offset(ctx, src->data_type, k, &src_comp_type);
>
> - if (!(c = hlsl_new_uint_constant(ctx, src_reg_offset, loc)))
> + if (!(c = hlsl_new_uint_constant(ctx, src_reg_offset, &src->loc)))
> return;
> list_add_tail(instrs, &c->node.entry);
>
> - if (!(load = add_load(ctx, instrs, src, &c->node, src_comp_type, *loc)))
> + if (!(load = add_load(ctx, instrs, src, &c->node, src_comp_type, src->loc)))
> return;
>
> - if (!(conv = add_implicit_conversion(ctx, instrs, &load->node, dst_comp_type, loc)))
> + if (!(conv = add_implicit_conversion(ctx, instrs, &load->node, dst_comp_type, &src->loc)))
> return;
>
> - if (!(c = hlsl_new_uint_constant(ctx, dst_reg_offset, loc)))
> + if (!(c = hlsl_new_uint_constant(ctx, dst_reg_offset, &src->loc)))
> return;
> list_add_tail(instrs, &c->node.entry);
>
> - if (!(store = hlsl_new_store(ctx, dst, &c->node, conv, 0, *loc)))
> + if (!(store = hlsl_new_store(ctx, dst, &c->node, conv, 0, src->loc)))
> return;
> list_add_tail(instrs, &store->node.entry);
>
> @@ -1716,7 +1715,7 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type
> *basic_t
> for (k = 0; k < v->initializer.args_count; ++k)
> {
> initialize_var_components(ctx, v->initializer.instrs, var,
> - &store_index, v->initializer.args[k], &v->initializer.args[k]->loc);
> + &store_index, v->initializer.args[k]);
> }
> }
> else
> @@ -2049,7 +2048,7 @@ static struct list *add_constructor(struct hlsl_ctx *ctx, struct hlsl_type
> *type
> continue;
> }
>
> - initialize_var_components(ctx, params->instrs, var, &idx, arg, &arg->loc);
> + initialize_var_components(ctx, params->instrs, var, &idx, arg);
> }
>
> if (!(load = hlsl_new_var_load(ctx, var, loc)))
> -- 
> 2.36.0

[PATCH vkd3d 2/8] vkd3d-shader/hlsl: Remove the location parameter from initialize_var_components().

[Zebediah Figura (she/her)]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>

[PATCH vkd3d 2/8] vkd3d-shader/hlsl: Remove the location parameter from initialize_var_components().

[Henri Verbeet]

Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>

[PATCH vkd3d 3/8] vkd3d-shader/hlsl: Don't assert that the array element type has a nonzero register size.

[Giovanni Mascellani]

From: Zebediah Figura <zfigura(a)codeweavers.com>

It's legal to declare arrays of object types, which don't.
---
 libs/vkd3d-shader/hlsl.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
index 7239b183..95ce9445 100644
--- a/libs/vkd3d-shader/hlsl.c
+++ b/libs/vkd3d-shader/hlsl.c
@@ -165,7 +165,6 @@ static void hlsl_type_calculate_reg_size(struct hlsl_ctx *ctx, struct hlsl_type
         {
             unsigned int element_size = type->e.array.type->reg_size;
 
-            assert(element_size);
             if (is_sm4)
                 type->reg_size = (type->e.array.elements_count - 1) * align(element_size, 4) + element_size;
             else
-- 
2.36.0

[PATCH vkd3d 3/8] vkd3d-shader/hlsl: Don't assert that the array element type has a nonzero register size.

[Giovanni Mascellani]

Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>

Il 03/05/22 11:57, Giovanni Mascellani ha scritto:
> From: Zebediah Figura <zfigura(a)codeweavers.com>
>
> It's legal to declare arrays of object types, which don't.
> ---
>   libs/vkd3d-shader/hlsl.c | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
> index 7239b183..95ce9445 100644
> --- a/libs/vkd3d-shader/hlsl.c
> +++ b/libs/vkd3d-shader/hlsl.c
> @@ -165,7 +165,6 @@ static void hlsl_type_calculate_reg_size(struct hlsl_ctx *ctx, struct hlsl_type
>           {
>               unsigned int element_size = type->e.array.type->reg_size;
>
> -            assert(element_size);
>               if (is_sm4)
>                   type->reg_size = (type->e.array.elements_count - 1) * align(element_size, 4) + element_size;
>               else

[PATCH vkd3d 3/8] vkd3d-shader/hlsl: Don't assert that the array element type has a nonzero register size.

[Giovanni Mascellani]

Actually no, I don't think I should have cherry picked this commit, as 
in the master branch reg_size for objects is set to 1, so there is no 
point in removing the assert().

Giovanni.


Il 03/05/22 12:18, Giovanni Mascellani ha scritto:
> Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
>
> Il 03/05/22 11:57, Giovanni Mascellani ha scritto:
> > From: Zebediah Figura <zfigura(a)codeweavers.com>
> >
> > It's legal to declare arrays of object types, which don't.
> > ---
> >   libs/vkd3d-shader/hlsl.c | 1 -
> >   1 file changed, 1 deletion(-)
> >
> > diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
> > index 7239b183..95ce9445 100644
> > --- a/libs/vkd3d-shader/hlsl.c
> > +++ b/libs/vkd3d-shader/hlsl.c
> > @@ -165,7 +165,6 @@ static void hlsl_type_calculate_reg_size(struct 
> > hlsl_ctx *ctx, struct hlsl_type
> >           {
> >               unsigned int element_size = type->e.array.type->reg_size;
> > -            assert(element_size);
> >               if (is_sm4)
> >                   type->reg_size = (type->e.array.elements_count - 1) 
> > * align(element_size, 4) + element_size;
> >               else

[PATCH vkd3d 3/8] vkd3d-shader/hlsl: Don't assert that the array element type has a nonzero register size.

[Francisco Casas]

Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com>

I agree, it is also legal to declare empty structs, and we also will need to
represent implicit size arrays as size 0 arrays if they are incorrectly initialized.
(Unless we want to immediately abort when we find them).


May 3, 2022 6:14 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote:
> From: Zebediah Figura <zfigura(a)codeweavers.com>
>
> It's legal to declare arrays of object types, which don't.
> ---
> libs/vkd3d-shader/hlsl.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
> index 7239b183..95ce9445 100644
> --- a/libs/vkd3d-shader/hlsl.c
> +++ b/libs/vkd3d-shader/hlsl.c
> @@ -165,7 +165,6 @@ static void hlsl_type_calculate_reg_size(struct hlsl_ctx *ctx, struct hlsl_type
> {
> unsigned int element_size = type->e.array.type->reg_size;
>
> - assert(element_size);
> if (is_sm4)
> type->reg_size = (type->e.array.elements_count - 1) * align(element_size, 4) + element_size;
> else
> -- 
> 2.36.0

[PATCH vkd3d 4/8] tests: Add additional subtests to implicit array initialization.

[Giovanni Mascellani]

From: Francisco Casas <fcasas(a)codeweavers.com>

---
 ...lsl-initializer-implicit-array.shader_test | 181 ++++++++++++++++++
 1 file changed, 181 insertions(+)

diff --git a/tests/hlsl-initializer-implicit-array.shader_test b/tests/hlsl-initializer-implicit-array.shader_test
index d2b94da4..b2a08d4b 100644
--- a/tests/hlsl-initializer-implicit-array.shader_test
+++ b/tests/hlsl-initializer-implicit-array.shader_test
@@ -11,8 +11,189 @@ todo probe all rgba (50, 60, 70, 80)
 
 
 [pixel shader fail]
+// Incompatible number of arguments in implicit size array initializer
 float4 main() : SV_TARGET
 {
     float4 arr[] = {10, 20, 30, 40, 50, 60, 70};
     return arr[0];
 }
+
+
+[pixel shader fail]
+// Incompatible number of arguments in implicit size array initializer
+float4 main() : SV_TARGET
+{
+    float4 arr[] = {10, 20, 30, 40, 50, 60, 70, 80, 90};
+    return arr[0];
+}
+
+
+[pixel shader]
+float4 main() : sv_target
+{
+    float2 arr[][3] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12};
+    return float4(arr[1][0], arr[1][1]);
+}
+
+[test]
+todo draw quad
+todo probe all rgba (7.0, 8.0, 9.0, 10.0)
+
+
+[pixel shader fail]
+// Implicit size inner array
+float4 main() : sv_target
+{
+    float2 arr[3][] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12};
+    return float4(arr[1][0], arr[1][1]);
+}
+
+
+[pixel shader fail]
+// Implicit size array without initializer
+float4 main() : sv_target
+{
+    float4 arr[];
+    arr[0] = float4(1, 2, 3, 4);
+    arr[1] = float4(5, 6, 7, 8);
+    return arr[0];
+}
+
+
+[pixel shader fail]
+// Implicit size array as struct member
+struct foobar
+{
+    int a;
+    float4 arr[];
+};
+
+float4 main() : sv_target
+{
+    struct foobar s;
+
+    s.a = 20;
+    s.arr[0] = float4(1, 2, 3, 4);
+    s.arr[1] = float4(5, 6, 7, 8);
+
+    return arr[0];
+}
+
+
+[pixel shader fail]
+// Implicit size array as function argument
+float4 sum1st2nd(float4 arr[])
+{
+    return arr[0] + arr[1];
+}
+float4 main() : sv_target
+{
+    float4 arr[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12};
+    return sum1st2nd(arr);
+}
+
+
+[pixel shader fail]
+// Implicit size array as function return type
+// Note: explicit size arrays are not allowed either.
+float4[] merge(float4 a, float4 b)
+{
+    float4 ret[2];
+    ret[0] = a;
+    ret[1] = b;
+    return ret;
+}
+float4 main() : sv_target
+{
+    float4 a = float4(1, 2, 3, 4);
+    float4 b = float4(5, 6, 7, 8);
+    return merge(a, b)[0];
+}
+
+[pixel shader fail]
+// Implicit size array as a typedef
+float4 main() : sv_target
+{
+    float2 arr1[4] = {1, 2, 3, 4, 5, 6, 7, 8};
+    float4 arr2[2] = (float2 []) arr1;
+    return arr2[1];
+}
+
+[pixel shader fail]
+// Implicit array of elements of size 0
+struct emp
+{
+};
+
+float4 main() : sv_target
+{
+    struct emp arr[] = {1, 2, 3, 4};
+
+    return float4(1, 2, 3, 4);
+}
+
+[pixel shader fail]
+// Implicit array of elements of size 0, without initializer
+struct emp
+{
+};
+
+float4 main() : sv_target
+{
+    struct emp arr[];
+
+    return float4(1, 2, 3, 4);
+}
+
+
+[pixel shader fail]
+// Implicit array with an initializer of size 0
+struct emp
+{
+};
+
+float4 main() : sv_target
+{
+    float4 arr[] = (struct emp) 42;
+
+    return float4(1, 2, 3, 4);
+}
+
+
+[pixel shader fail]
+// Implicit array of elements of size 0 with initializer of size 0
+struct emp
+{
+};
+
+float4 main() : sv_target
+{
+    struct emp arr[] = (struct emp) 42;
+}
+
+
+[pixel shader]
+float4 main() : sv_target
+{
+    float4 arr1[2] = {1, 2, 3, 4, 5, 6, 7, 8};
+    float4 arr2[] = arr1;
+
+    return arr2[1];
+}
+
+[test]
+todo draw quad
+todo probe all rgba (5.0, 6.0, 7.0, 8.0)
+
+
+[pixel shader fail]
+// Implicit array of elements of size 0 with broadcast initialization
+struct emp {
+};
+
+float4 main() : sv_target
+{
+    struct emp p[] = 42;
+
+	return float4(1, 2, 3, 4);
+}
-- 
2.36.0

[PATCH vkd3d 4/8] tests: Add additional subtests to implicit array initialization.

[Giovanni Mascellani]

Hi,

Il 03/05/22 11:57, Giovanni Mascellani ha scritto:
> +[pixel shader fail]
> +// Implicit size array without initializer
> +float4 main() : sv_target
> +{
> +    float4 arr[];
> +    arr[0] = float4(1, 2, 3, 4);
> +    arr[1] = float4(5, 6, 7, 8);
> +    return arr[0];
> +}

While this is not wrong, I'd suggest to make tests meant to fail more 
minimal, so they are as specific as possible. In this case, for example, 
if our compile accepted "float4 arr[]" and then failed on actually 
accessing the array, the test wouldn't help us detect the mistake. 
Instead, I'd just write something like:
---
float4 arr[];
return 0.0;
---
so that it's clear that the line "float4 arr[]" itself must be 
considered an error, not anything else.

The general principle is: in a test meant to fail, try to avoid adding 
any language feature that is not the specific one meant to be 
demonstrated failing.

The same applies to a few other tests.
> +[pixel shader fail]
> +// Implicit size array as a typedef
> +float4 main() : sv_target
> +{
> +    float2 arr1[4] = {1, 2, 3, 4, 5, 6, 7, 8};
> +    float4 arr2[2] = (float2 []) arr1;
> +    return arr2[1];
> +}

There is no typedef here. Just a cast.

Thanks, Giovanni.

[PATCH vkd3d 4/8] tests: Add additional subtests to implicit array initialization.

[Zebediah Figura (she/her)]

As long as there's outstanding criticism, I'll add a couple of extra notes:

On 5/3/22 04:57, Giovanni Mascellani wrote:
> +[pixel shader fail]
> +// Implicit size array as function return type
> +// Note: explicit size arrays are not allowed either.
> +float4[] merge(float4 a, float4 b)
> +{
> +    float4 ret[2];
> +    ret[0] = a;
> +    ret[1] = b;
> +    return ret;
> +}
> +float4 main() : sv_target
> +{
> +    float4 a = float4(1, 2, 3, 4);
> +    float4 b = float4(5, 6, 7, 8);
> +    return merge(a, b)[0];
> +}

Out of scope of this patch perhaps, but that'd be another good thing to 
add explicit tests for. Frankly, if we had tests for explicitly sized 
arrays as function return types, I'd say that tests for implicitly sized 
arrays are superfluous.

As a side note, it's not necessarily obvious that that's the right place 
to put the brackets. In C, for example, the "right" place is after the 
close paren (it's still illegal, but it at least parses). It turns out 
that HLSL doesn't accept them anywhere.
> +
> +[pixel shader fail]
> +// Implicit size array as a typedef
> +float4 main() : sv_target
> +{
> +    float2 arr1[4] = {1, 2, 3, 4, 5, 6, 7, 8};
> +    float4 arr2[2] = (float2 []) arr1;
> +    return arr2[1];
> +}

As Giovanni pointed out, that's a cast, not a typedef. Adding one for a 
typedef would be welcome, though :-)
> +[pixel shader fail]
> +// Implicit array of elements of size 0 with broadcast initialization
> +struct emp {
> +};
> +
> +float4 main() : sv_target
> +{
> +    struct emp p[] = 42;
> +
> +	return float4(1, 2, 3, 4);
> +}

Indeed, although it turns out that the offending condition here is 
actually "initializing an array variable without braces and without an 
explicit cast", which is rather simpler. More interesting is that

     int a[4] = (int[])0;

is invalid.

[PATCH vkd3d 5/8] vkd3d-shader/hlsl: Support initialization of implicit size arrays.

[Giovanni Mascellani]

From: Francisco Casas <fcasas(a)codeweavers.com>

HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT (zero) is used as a temporal value
for elements_count for implicit size arrays. It should be replaced by
the correct value after parsing the initializer.

In case the implicit array is not initialized correctly, hlsl_error()
is called but the array size is kept at 0. So the rest of the code
must handle these cases.

Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com>

---
v2:
- Detection of incorrect use of implicit arrays was moved from the
  parser rules to the respective add_* functions.
---
 libs/vkd3d-shader/hlsl.c                      | 13 ++-
 libs/vkd3d-shader/hlsl.h                      |  3 +
 libs/vkd3d-shader/hlsl.y                      | 94 +++++++++++++++++++
 ...lsl-initializer-implicit-array.shader_test | 12 +--
 4 files changed, 114 insertions(+), 8 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
index 95ce9445..ed94e903 100644
--- a/libs/vkd3d-shader/hlsl.c
+++ b/libs/vkd3d-shader/hlsl.c
@@ -165,6 +165,8 @@ static void hlsl_type_calculate_reg_size(struct hlsl_ctx *ctx, struct hlsl_type
         {
             unsigned int element_size = type->e.array.type->reg_size;
 
+            if (type->e.array.elements_count == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT)
+                type->reg_size = 0;
             if (is_sm4)
                 type->reg_size = (type->e.array.elements_count - 1) * align(element_size, 4) + element_size;
             else
@@ -182,8 +184,10 @@ static void hlsl_type_calculate_reg_size(struct hlsl_ctx *ctx, struct hlsl_type
             LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry)
             {
                 unsigned int field_size = field->type->reg_size;
+                bool is_implicit_array = field->type->type == HLSL_CLASS_ARRAY &&
+                        field->type->e.array.elements_count == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT;
 
-                assert(field_size);
+                assert(is_implicit_array || field_size);
 
                 type->reg_size = hlsl_type_get_sm4_offset(field->type, type->reg_size);
                 field->reg_offset = type->reg_size;
@@ -1022,7 +1026,12 @@ struct vkd3d_string_buffer *hlsl_type_to_string(struct hlsl_ctx *ctx, const stru
             }
 
             for (t = type; t->type == HLSL_CLASS_ARRAY; t = t->e.array.type)
-                vkd3d_string_buffer_printf(string, "[%u]", t->e.array.elements_count);
+            {
+                if (t->e.array.elements_count == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT)
+                    vkd3d_string_buffer_printf(string, "[]");
+                else
+                    vkd3d_string_buffer_printf(string, "[%u]", t->e.array.elements_count);
+            }
             return string;
         }
 
diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h
index 28b2ff1b..1dce223a 100644
--- a/libs/vkd3d-shader/hlsl.h
+++ b/libs/vkd3d-shader/hlsl.h
@@ -139,6 +139,7 @@ struct hlsl_type
     size_t bytecode_offset;
 };
 
+
 struct hlsl_semantic
 {
     const char *name;
@@ -227,6 +228,8 @@ struct hlsl_src
 
 #define HLSL_MODIFIERS_MAJORITY_MASK (HLSL_MODIFIER_ROW_MAJOR | HLSL_MODIFIER_COLUMN_MAJOR)
 
+#define HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT 0
+
 struct hlsl_reg_reservation
 {
     char type;
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 44e4964f..eb96a4c2 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -789,7 +789,26 @@ static struct list *gen_struct_fields(struct hlsl_ctx *ctx,
 
         field->type = type;
         for (i = 0; i < v->arrays.count; ++i)
+        {
+            if (v->arrays.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT)
+            {
+                if (i < v->arrays.count - 1)
+                {
+                    hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                            "Inner array cannot be implicit.");
+                }
+                else if (type->type == HLSL_CLASS_OBJECT)
+                {
+                    hlsl_fixme(ctx, &v->loc, "Unbounded resource arrays.");
+                }
+                else
+                {
+                    hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                            "Implicit arrays not allowed in struct fieds.");
+                }
+            }
             field->type = hlsl_new_array_type(ctx, field->type, v->arrays.sizes[i]);
+        }
         vkd3d_free(v->arrays.sizes);
         field->loc = v->loc;
         field->name = v->name;
@@ -832,6 +851,11 @@ static bool add_typedef(struct hlsl_ctx *ctx, DWORD modifiers, struct hlsl_type
         ret = true;
         for (i = 0; i < v->arrays.count; ++i)
         {
+            if (v->arrays.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT)
+            {
+                hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Implicit arrays not allowed in typedefs.");
+            }
+
             if (!(type = hlsl_new_array_type(ctx, type, v->arrays.sizes[i])))
             {
                 free_parse_variable_def(v);
@@ -1604,8 +1628,57 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t
         unsigned int i;
 
         type = basic_type;
+
         for (i = 0; i < v->arrays.count; ++i)
+        {
+            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, "Inner array cannot 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 array of a size 0 type.");
+                    free_parse_initializer(&v->initializer);
+                    v->initializer.args_count = 0;
+                }
+                else if (size == 0)
+                {
+                    if (type->type == HLSL_CLASS_OBJECT)
+                    {
+                        hlsl_fixme(ctx, &v->loc, "Unbounded resource arrays.");
+                        free_parse_initializer(&v->initializer);
+                        v->initializer.args_count = 0;
+                    }
+                    else
+                    {
+                        hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Implicit arrays need to be initialized.");
+                        free_parse_initializer(&v->initializer);
+                        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 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;
+                }
+            }
             type = hlsl_new_array_type(ctx, type, v->arrays.sizes[i]);
+        }
         vkd3d_free(v->arrays.sizes);
 
         if (type->type != HLSL_CLASS_MATRIX)
@@ -3186,6 +3259,21 @@ arrays:
             $$.sizes = new_array;
             $$.sizes[$$.count++] = size;
         }
+    | '[' ']' arrays
+        {
+            uint32_t *new_array;
+
+            $$ = $3;
+
+            if (!(new_array = hlsl_realloc(ctx, $$.sizes, ($$.count + 1) * sizeof(*new_array))))
+            {
+                vkd3d_free($$.sizes);
+                YYABORT;
+            }
+
+            $$.sizes = new_array;
+            $$.sizes[$$.count++] = HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT;
+        }
 
 var_modifiers:
       %empty
@@ -3726,7 +3814,13 @@ unary_expr:
 
             dst_type = $3;
             for (i = 0; i < $4.count; ++i)
+            {
+                if ($4.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT)
+                {
+                    hlsl_error(ctx, &@3, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Implicit arrays not allowed in casts.");
+                }
                 dst_type = hlsl_new_array_type(ctx, dst_type, $4.sizes[i]);
+            }
 
             if (!compatible_data_types(src_type, dst_type))
             {
diff --git a/tests/hlsl-initializer-implicit-array.shader_test b/tests/hlsl-initializer-implicit-array.shader_test
index b2a08d4b..5ddc0c89 100644
--- a/tests/hlsl-initializer-implicit-array.shader_test
+++ b/tests/hlsl-initializer-implicit-array.shader_test
@@ -6,8 +6,8 @@ float4 main() : SV_TARGET
 }
 
 [test]
-todo draw quad
-todo probe all rgba (50, 60, 70, 80)
+draw quad
+probe all rgba (50, 60, 70, 80)
 
 
 [pixel shader fail]
@@ -36,8 +36,8 @@ float4 main() : sv_target
 }
 
 [test]
-todo draw quad
-todo probe all rgba (7.0, 8.0, 9.0, 10.0)
+draw quad
+probe all rgba (7.0, 8.0, 9.0, 10.0)
 
 
 [pixel shader fail]
@@ -182,8 +182,8 @@ float4 main() : sv_target
 }
 
 [test]
-todo draw quad
-todo probe all rgba (5.0, 6.0, 7.0, 8.0)
+draw quad
+probe all rgba (5.0, 6.0, 7.0, 8.0)
 
 
 [pixel shader fail]
-- 
2.36.0

[PATCH vkd3d 5/8] vkd3d-shader/hlsl: Support initialization of implicit size arrays.

[Giovanni Mascellani]

Hi,

Il 03/05/22 11:57, Giovanni Mascellani ha scritto:
> --- a/libs/vkd3d-shader/hlsl.c
> +++ b/libs/vkd3d-shader/hlsl.c
> @@ -165,6 +165,8 @@ static void hlsl_type_calculate_reg_size(struct hlsl_ctx *ctx, struct hlsl_type
>           {
>               unsigned int element_size = type->e.array.type->reg_size;
>
> +            if (type->e.array.elements_count == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT)
> +                type->reg_size = 0;
>               if (is_sm4)
>                   type->reg_size = (type->e.array.elements_count - 1) * align(element_size, 4) + element_size;
>               else

I guess "if (is_sm4)" should become an "else if"?
> diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h
> index 28b2ff1b..1dce223a 100644
> --- a/libs/vkd3d-shader/hlsl.h
> +++ b/libs/vkd3d-shader/hlsl.h
> @@ -139,6 +139,7 @@ struct hlsl_type
>       size_t bytecode_offset;
>   };
>
> +
>   struct hlsl_semantic
>   {
>       const char *name;

I guess this is just a mistake?
> diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
> index 44e4964f..eb96a4c2 100644
> --- a/libs/vkd3d-shader/hlsl.y
> +++ b/libs/vkd3d-shader/hlsl.y
> @@ -789,7 +789,26 @@ static struct list *gen_struct_fields(struct hlsl_ctx *ctx,
>
> field->type = type;
>           for (i = 0; i < v->arrays.count; ++i)
> +        {
> +            if (v->arrays.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT)
> +            {
> +                if (i < v->arrays.count - 1)
> +                {
> +                    hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
> +                            "Inner array cannot be implicit.");
> +                }
> +                else if (type->type == HLSL_CLASS_OBJECT)
> +                {
> +                    hlsl_fixme(ctx, &v->loc, "Unbounded resource arrays.");
> +                }
> +                else
> +                {
> +                    hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
> +                            "Implicit arrays not allowed in struct fieds.");
> +                }
> +            }
>               field->type = hlsl_new_array_type(ctx, field->type, v->arrays.sizes[i]);
> +        }

Minor typo: "fieds" instead of "fields".

It seems that unbounded texture arrays are not supported below shader 
model 5.1, so I wonder if the hlsl_fixme() should only be emitted when 
shader model >= 5.1. But maybe we can just ignore this point and leave 
it for when somebody will actually implement unbounded texture arrays.

Thanks, Giovanni.

[PATCH vkd3d 5/8] vkd3d-shader/hlsl: Support initialization of implicit size arrays.

[Francisco Casas]

Hello,

May 3, 2022 7:14 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote:

> Hi,
> 
> Il 03/05/22 11:57, Giovanni Mascellani ha scritto:
> 
>> --- a/libs/vkd3d-shader/hlsl.c
>> +++ b/libs/vkd3d-shader/hlsl.c
>> @@ -165,6 +165,8 @@ static void hlsl_type_calculate_reg_size(struct hlsl_ctx *ctx, struct hlsl_type
>> {
>> unsigned int element_size = type->e.array.type->reg_size;
>>> + if (type->e.array.elements_count == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT)
>> + type->reg_size = 0;
>> if (is_sm4)
>> type->reg_size = (type->e.array.elements_count - 1) * align(element_size, 4) + element_size;
>> else
> 
> I guess "if (is_sm4)" should become an "else if"?
> 

You are right!

>> diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h
>> index 28b2ff1b..1dce223a 100644
>> --- a/libs/vkd3d-shader/hlsl.h
>> +++ b/libs/vkd3d-shader/hlsl.h
>> @@ -139,6 +139,7 @@ struct hlsl_type
>> size_t bytecode_offset;
>> };
>>> +
>> struct hlsl_semantic
>> {
>> const char *name;
> 
> I guess this is just a mistake?
> 

Huh, yes, it is. 

>> diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
>> index 44e4964f..eb96a4c2 100644
>> --- a/libs/vkd3d-shader/hlsl.y
>> +++ b/libs/vkd3d-shader/hlsl.y
>> @@ -789,7 +789,26 @@ static struct list *gen_struct_fields(struct hlsl_ctx *ctx,
>>> field->type = type;
>> for (i = 0; i < v->arrays.count; ++i)
>> + {
>> + if (v->arrays.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT)
>> + {
>> + if (i < v->arrays.count - 1)
>> + {
>> + hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
>> + "Inner array cannot be implicit.");
>> + }
>> + else if (type->type == HLSL_CLASS_OBJECT)
>> + {
>> + hlsl_fixme(ctx, &v->loc, "Unbounded resource arrays.");
>> + }
>> + else
>> + {
>> + hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
>> + "Implicit arrays not allowed in struct fieds.");
>> + }
>> + }
>> field->type = hlsl_new_array_type(ctx, field->type, v->arrays.sizes[i]);
>> + }
> 
> Minor typo: "fieds" instead of "fields".
> 

Thanks.

> It seems that unbounded texture arrays are not supported below shader model 5.1, so I wonder if the
> hlsl_fixme() should only be emitted when shader model >= 5.1. But maybe we can just ignore this
> point and leave it for when somebody will actually implement unbounded texture arrays.
> 

For simplicity I will leave as it is for now. But we could introduce the check as another condition
in the 'else if'.

> Thanks, Giovanni.

[PATCH vkd3d 5/8] vkd3d-shader/hlsl: Support initialization of implicit size arrays.

[Zebediah Figura (she/her)]

On 5/3/22 04:57, Giovanni Mascellani wrote:
> diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
> index 95ce9445..ed94e903 100644
> --- a/libs/vkd3d-shader/hlsl.c
> +++ b/libs/vkd3d-shader/hlsl.c
> @@ -165,6 +165,8 @@ static void hlsl_type_calculate_reg_size(struct hlsl_ctx *ctx, struct hlsl_type
>           {
>               unsigned int element_size = type->e.array.type->reg_size;
>
> +            if (type->e.array.elements_count == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT)
> +                type->reg_size = 0;

This ends up breaking copy-prop, though, which is the main reason we 
needed to set reg_size for objects in the first place. Consider the 
following program, which is (surprisingly) legal in 5.1:

     Texture2D t1[];

     float4 main() : sv_target
     {
         Texture2D t2[] = t1;

         return t2[4].Load(0);
     }

On the other hand, this is legal in 5.0, but illegal in 5.1:

     Texture2D t1[5];
     Texture2D t3;

     float4 main() : sv_target
     {
         Texture2D t2[] = t1;

         t2[4] = t3;

         return t2[4].Load(0);
     }

Which suggests that, for the purposes of copy-prop, reg_size should be 1 
for texture arrays (regardless of element count) in 5.1.

(Perhaps there's an argument we shouldn't be abusing reg_size like that. 
I have never liked the logic, but I've also never found a better way to 
write it.)

It'd be fine to punt for now though, by skipping creation of resource 
array types (and variables).
> if (is_sm4)
>                   type->reg_size = (type->e.array.elements_count - 1) * align(element_size, 4) + element_size;
>               else

...
> diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
> index 44e4964f..eb96a4c2 100644
> --- a/libs/vkd3d-shader/hlsl.y
> +++ b/libs/vkd3d-shader/hlsl.y
> @@ -789,7 +789,26 @@ static struct list *gen_struct_fields(struct hlsl_ctx *ctx,
>
> field->type = type;
>           for (i = 0; i < v->arrays.count; ++i)
> +        {
> +            if (v->arrays.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT)
> +            {
> +                if (i < v->arrays.count - 1)
> +                {
> +                    hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
> +                            "Inner array cannot be implicit.");

Nitpick, I'd say "array /size/ cannot be implicit". Same below.

[PATCH vkd3d 6/8] vkd3d-shader/hlsl: Handle loops in copy propagation.

[Giovanni Mascellani]

Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
---
 libs/vkd3d-shader/hlsl_codegen.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index b967a2a3..78b22910 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -634,6 +634,21 @@ static bool copy_propagation_process_if(struct hlsl_ctx *ctx, struct hlsl_ir_if
     return progress;
 }
 
+static bool copy_propagation_process_loop(struct hlsl_ctx *ctx, struct hlsl_ir_loop *loop,
+        struct copy_propagation_state *state)
+{
+    struct copy_propagation_state inner_state;
+    bool progress = false;
+
+    copy_propagation_invalidate_from_block(ctx, state, &loop->body);
+
+    copy_propagation_state_init(ctx, &inner_state, state);
+    progress |= copy_propagation_transform_block(ctx, &loop->body, &inner_state);
+    copy_propagation_state_destroy(&inner_state);
+
+    return progress;
+}
+
 static bool copy_propagation_transform_block(struct hlsl_ctx *ctx, struct hlsl_block *block,
         struct copy_propagation_state *state)
 {
@@ -661,8 +676,8 @@ static bool copy_propagation_transform_block(struct hlsl_ctx *ctx, struct hlsl_b
                 break;
 
             case HLSL_IR_LOOP:
-                FIXME("Copy propagation doesn't support loops yet, leaving.\n");
-                return progress;
+                progress |= copy_propagation_process_loop(ctx, hlsl_ir_loop(instr), state);
+                break;
 
             default:
                 break;
-- 
2.36.0

[PATCH vkd3d 6/8] vkd3d-shader/hlsl: Handle loops in copy propagation.

[Zebediah Figura (she/her)]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>

[PATCH vkd3d 6/8] vkd3d-shader/hlsl: Handle loops in copy propagation.

[Henri Verbeet]

Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>

[PATCH vkd3d 6/8] vkd3d-shader/hlsl: Handle loops in copy propagation.

[Francisco Casas]

Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com>

May 3, 2022 6:14 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote:
> Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
> ---
> libs/vkd3d-shader/hlsl_codegen.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
> index b967a2a3..78b22910 100644
> --- a/libs/vkd3d-shader/hlsl_codegen.c
> +++ b/libs/vkd3d-shader/hlsl_codegen.c
> @@ -634,6 +634,21 @@ static bool copy_propagation_process_if(struct hlsl_ctx *ctx, struct
> hlsl_ir_if
> return progress;
> }
>
> +static bool copy_propagation_process_loop(struct hlsl_ctx *ctx, struct hlsl_ir_loop *loop,
> + struct copy_propagation_state *state)
> +{
> + struct copy_propagation_state inner_state;
> + bool progress = false;
> +
> + copy_propagation_invalidate_from_block(ctx, state, &loop->body);
> +
> + copy_propagation_state_init(ctx, &inner_state, state);
> + progress |= copy_propagation_transform_block(ctx, &loop->body, &inner_state);
> + copy_propagation_state_destroy(&inner_state);
> +
> + return progress;
> +}
> +
> static bool copy_propagation_transform_block(struct hlsl_ctx *ctx, struct hlsl_block *block,
> struct copy_propagation_state *state)
> {
> @@ -661,8 +676,8 @@ static bool copy_propagation_transform_block(struct hlsl_ctx *ctx, struct
> hlsl_b
> break;
>
> case HLSL_IR_LOOP:
> - FIXME("Copy propagation doesn't support loops yet, leaving.\n");
> - return progress;
> + progress |= copy_propagation_process_loop(ctx, hlsl_ir_loop(instr), state);
> + break;
>
> default:
> break;
> -- 
> 2.36.0

[PATCH vkd3d 7/8] vkd3d-shader/hlsl: Swap failure and success branches in copy_propagation_transform_load().

[Giovanni Mascellani]

Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
---
 libs/vkd3d-shader/hlsl_codegen.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index 78b22910..7c401e14 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -474,18 +474,20 @@ static bool copy_propagation_transform_load(struct hlsl_ctx *ctx,
             return false;
     }
 
-    if (!(new_node = copy_propagation_compute_replacement(ctx, state, &load->src, dimx, &swizzle)))
-        return false;
-
-    if (type->type != HLSL_CLASS_OBJECT)
+    if ((new_node = copy_propagation_compute_replacement(ctx, state, &load->src, dimx, &swizzle)))
     {
-        if (!(swizzle_node = hlsl_new_swizzle(ctx, swizzle, dimx, new_node, &node->loc)))
-            return false;
-        list_add_before(&node->entry, &swizzle_node->node.entry);
-        new_node = &swizzle_node->node;
+        if (type->type != HLSL_CLASS_OBJECT)
+        {
+            if (!(swizzle_node = hlsl_new_swizzle(ctx, swizzle, dimx, new_node, &node->loc)))
+                return false;
+            list_add_before(&node->entry, &swizzle_node->node.entry);
+            new_node = &swizzle_node->node;
+        }
+        hlsl_replace_node(node, new_node);
+        return true;
     }
-    hlsl_replace_node(node, new_node);
-    return true;
+
+    return false;
 }
 
 static bool copy_propagation_transform_object_load(struct hlsl_ctx *ctx,
-- 
2.36.0

[PATCH vkd3d 7/8] vkd3d-shader/hlsl: Swap failure and success branches in copy_propagation_transform_load().

[Zebediah Figura (she/her)]

On 5/3/22 04:57, Giovanni Mascellani wrote:
> Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
> ---
>   libs/vkd3d-shader/hlsl_codegen.c | 22 ++++++++++++----------
>   1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
> index 78b22910..7c401e14 100644
> --- a/libs/vkd3d-shader/hlsl_codegen.c
> +++ b/libs/vkd3d-shader/hlsl_codegen.c
> @@ -474,18 +474,20 @@ static bool copy_propagation_transform_load(struct hlsl_ctx *ctx,
>               return false;
>       }
>
> -    if (!(new_node = copy_propagation_compute_replacement(ctx, state, &load->src, dimx, &swizzle)))
> -        return false;
> -
> -    if (type->type != HLSL_CLASS_OBJECT)
> +    if ((new_node = copy_propagation_compute_replacement(ctx, state, &load->src, dimx, &swizzle)))
>       {
> -        if (!(swizzle_node = hlsl_new_swizzle(ctx, swizzle, dimx, new_node, &node->loc)))
> -            return false;
> -        list_add_before(&node->entry, &swizzle_node->node.entry);
> -        new_node = &swizzle_node->node;
> +        if (type->type != HLSL_CLASS_OBJECT)
> +        {
> +            if (!(swizzle_node = hlsl_new_swizzle(ctx, swizzle, dimx, new_node, &node->loc)))
> +                return false;
> +            list_add_before(&node->entry, &swizzle_node->node.entry);
> +            new_node = &swizzle_node->node;
> +        }
> +        hlsl_replace_node(node, new_node);
> +        return true;
>       }
> -    hlsl_replace_node(node, new_node);
> -    return true;
> +
> +    return false;
>   }
>
> static bool copy_propagation_transform_object_load(struct hlsl_ctx *ctx,

I'm not sure I see how this is an improvement?

[PATCH vkd3d 7/8] vkd3d-shader/hlsl: Swap failure and success branches in copy_propagation_transform_load().

[Francisco Casas]

Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com>

I understand that this is necessary for the next patch.
Albeit there may be an argument to squash them.



May 3, 2022 6:14 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote:
> Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
> ---
> libs/vkd3d-shader/hlsl_codegen.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
> index 78b22910..7c401e14 100644
> --- a/libs/vkd3d-shader/hlsl_codegen.c
> +++ b/libs/vkd3d-shader/hlsl_codegen.c
> @@ -474,18 +474,20 @@ static bool copy_propagation_transform_load(struct hlsl_ctx *ctx,
> return false;
> }
>
> - if (!(new_node = copy_propagation_compute_replacement(ctx, state, &load->src, dimx, &swizzle)))
> - return false;
> -
> - if (type->type != HLSL_CLASS_OBJECT)
> + if ((new_node = copy_propagation_compute_replacement(ctx, state, &load->src, dimx, &swizzle)))
> {
> - if (!(swizzle_node = hlsl_new_swizzle(ctx, swizzle, dimx, new_node, &node->loc)))
> - return false;
> - list_add_before(&node->entry, &swizzle_node->node.entry);
> - new_node = &swizzle_node->node;
> + if (type->type != HLSL_CLASS_OBJECT)
> + {
> + if (!(swizzle_node = hlsl_new_swizzle(ctx, swizzle, dimx, new_node, &node->loc)))
> + return false;
> + list_add_before(&node->entry, &swizzle_node->node.entry);
> + new_node = &swizzle_node->node;
> + }
> + hlsl_replace_node(node, new_node);
> + return true;
> }
> - hlsl_replace_node(node, new_node);
> - return true;
> +
> + return false;
> }
>
> static bool copy_propagation_transform_object_load(struct hlsl_ctx *ctx,
> -- 
> 2.36.0

[PATCH vkd3d 8/8] vkd3d-shader/hlsl: Propagate constant loads.

[Giovanni Mascellani]

Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
---
 libs/vkd3d-shader/hlsl_codegen.c | 60 ++++++++++++++++++++++++++++++++
 tests/sampler-offset.shader_test |  6 ++--
 2 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index 7c401e14..0942a86c 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -406,6 +406,59 @@ static void copy_propagation_set_value(struct copy_propagation_var_def *var_def,
     }
 }
 
+static struct hlsl_ir_node *copy_propagation_compute_constant(struct hlsl_ctx *ctx, struct copy_propagation_state *state,
+        const struct hlsl_deref *deref, unsigned int count, const struct vkd3d_shader_location *loc)
+{
+    const struct hlsl_ir_var *var = deref->var;
+    enum hlsl_base_type type = HLSL_TYPE_VOID;
+    struct hlsl_ir_constant *constant = NULL;
+    struct hlsl_ir_node *store_nodes[4];
+    unsigned store_components[4];
+    unsigned int i, offset;
+
+    if (!hlsl_offset_from_deref(ctx, deref, &offset))
+        return NULL;
+
+    if (var->data_type->type != HLSL_CLASS_OBJECT)
+        assert(offset + count <= var->data_type->reg_size);
+
+    for (i = 0; i < count; ++i)
+    {
+        struct copy_propagation_value *value = copy_propagation_get_value(state, var, offset + i);
+        struct hlsl_ir_node *store_node;
+        enum hlsl_base_type store_type;
+
+        if (!value)
+            return NULL;
+
+        store_node = value->node;
+        if (!store_node || store_node->type != HLSL_IR_CONSTANT)
+            return NULL;
+
+        store_type = store_node->data_type->base_type;
+
+        if (type == HLSL_TYPE_VOID)
+            type = store_type;
+        else if (type != store_type)
+            return NULL;
+
+        store_nodes[i] = store_node;
+        store_components[i] = value->component;
+    }
+
+    if (!(constant = hlsl_new_uint_constant(ctx, 0, loc)))
+        return NULL;
+    constant->node.data_type = hlsl_get_vector_type(ctx, type, count);
+
+    for (i = 0; i < count; ++i)
+        constant->value[i] = hlsl_ir_constant(store_nodes[i])->value[store_components[i]];
+
+    TRACE("Load from %s[%d-%d] reconstructed as constant value.\n",
+            var->name, offset, offset + count);
+
+    return &constant->node;
+}
+
 static struct hlsl_ir_node *copy_propagation_compute_replacement(struct hlsl_ctx *ctx,
         const struct copy_propagation_state *state, const struct hlsl_deref *deref,
         unsigned int count, unsigned int *swizzle)
@@ -474,6 +527,13 @@ static bool copy_propagation_transform_load(struct hlsl_ctx *ctx,
             return false;
     }
 
+    if ((new_node = copy_propagation_compute_constant(ctx, state, &load->src, dimx, &node->loc)))
+    {
+        list_add_before(&node->entry, &new_node->entry);
+        hlsl_replace_node(node, new_node);
+        return true;
+    }
+
     if ((new_node = copy_propagation_compute_replacement(ctx, state, &load->src, dimx, &swizzle)))
     {
         if (type->type != HLSL_CLASS_OBJECT)
diff --git a/tests/sampler-offset.shader_test b/tests/sampler-offset.shader_test
index f47d5bff..6f8357df 100644
--- a/tests/sampler-offset.shader_test
+++ b/tests/sampler-offset.shader_test
@@ -22,7 +22,7 @@ float4 main() : sv_target
 }
 
 [test]
-todo draw quad
+draw quad
 probe all rgba (0.1, 0.2, 0.5, 0.0)
 
 
@@ -36,7 +36,7 @@ float4 main() : sv_target
 }
 
 [test]
-todo draw quad
+draw quad
 probe all rgba (0.2, 0.2, 0.0, 0.4)
 
 
@@ -50,5 +50,5 @@ float4 main() : sv_target
 }
 
 [test]
-todo draw quad
+draw quad
 probe all rgba (0.0, 0.2, 0.0, 0.4)
-- 
2.36.0

[PATCH vkd3d 8/8] vkd3d-shader/hlsl: Propagate constant loads.

[Zebediah Figura (she/her)]

On 5/3/22 04:57, Giovanni Mascellani wrote:
> Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
> ---
>   libs/vkd3d-shader/hlsl_codegen.c | 60 ++++++++++++++++++++++++++++++++
>   tests/sampler-offset.shader_test |  6 ++--
>   2 files changed, 63 insertions(+), 3 deletions(-)

What transformation is this doing that isn't covered by other passes?

[PATCH vkd3d 8/8] vkd3d-shader/hlsl: Propagate constant loads.

[Francisco Casas]

Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com>

However, I wonder if we can achieve the same result lowering the loads
from a constant value, and that have a constant offset in hlsl_fold_constants().



May 3, 2022 6:14 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote:
> Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
> ---
> libs/vkd3d-shader/hlsl_codegen.c | 60 ++++++++++++++++++++++++++++++++
> tests/sampler-offset.shader_test | 6 ++--
> 2 files changed, 63 insertions(+), 3 deletions(-)
>
> diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
> index 7c401e14..0942a86c 100644
> --- a/libs/vkd3d-shader/hlsl_codegen.c
> +++ b/libs/vkd3d-shader/hlsl_codegen.c
> @@ -406,6 +406,59 @@ static void copy_propagation_set_value(struct copy_propagation_var_def
> *var_def,
> }
> }
>
> +static struct hlsl_ir_node *copy_propagation_compute_constant(struct hlsl_ctx *ctx, struct
> copy_propagation_state *state,
> + const struct hlsl_deref *deref, unsigned int count, const struct vkd3d_shader_location *loc)
> +{
> + const struct hlsl_ir_var *var = deref->var;
> + enum hlsl_base_type type = HLSL_TYPE_VOID;
> + struct hlsl_ir_constant *constant = NULL;
> + struct hlsl_ir_node *store_nodes[4];
> + unsigned store_components[4];
> + unsigned int i, offset;
> +
> + if (!hlsl_offset_from_deref(ctx, deref, &offset))
> + return NULL;
> +
> + if (var->data_type->type != HLSL_CLASS_OBJECT)
> + assert(offset + count <= var->data_type->reg_size);
> +
> + for (i = 0; i < count; ++i)
> + {
> + struct copy_propagation_value *value = copy_propagation_get_value(state, var, offset + i);
> + struct hlsl_ir_node *store_node;
> + enum hlsl_base_type store_type;
> +
> + if (!value)
> + return NULL;
> +
> + store_node = value->node;
> + if (!store_node || store_node->type != HLSL_IR_CONSTANT)
> + return NULL;
> +
> + store_type = store_node->data_type->base_type;
> +
> + if (type == HLSL_TYPE_VOID)
> + type = store_type;
> + else if (type != store_type)
> + return NULL;
> +
> + store_nodes[i] = store_node;
> + store_components[i] = value->component;
> + }
> +
> + if (!(constant = hlsl_new_uint_constant(ctx, 0, loc)))
> + return NULL;
> + constant->node.data_type = hlsl_get_vector_type(ctx, type, count);
> +
> + for (i = 0; i < count; ++i)
> + constant->value[i] = hlsl_ir_constant(store_nodes[i])->value[store_components[i]];
> +
> + TRACE("Load from %s[%d-%d] reconstructed as constant value.\n",
> + var->name, offset, offset + count);
> +
> + return &constant->node;
> +}
> +
> static struct hlsl_ir_node *copy_propagation_compute_replacement(struct hlsl_ctx *ctx,
> const struct copy_propagation_state *state, const struct hlsl_deref *deref,
> unsigned int count, unsigned int *swizzle)
> @@ -474,6 +527,13 @@ static bool copy_propagation_transform_load(struct hlsl_ctx *ctx,
> return false;
> }
>
> + if ((new_node = copy_propagation_compute_constant(ctx, state, &load->src, dimx, &node->loc)))
> + {
> + list_add_before(&node->entry, &new_node->entry);
> + hlsl_replace_node(node, new_node);
> + return true;
> + }
> +
> if ((new_node = copy_propagation_compute_replacement(ctx, state, &load->src, dimx, &swizzle)))
> {
> if (type->type != HLSL_CLASS_OBJECT)
> diff --git a/tests/sampler-offset.shader_test b/tests/sampler-offset.shader_test
> index f47d5bff..6f8357df 100644
> --- a/tests/sampler-offset.shader_test
> +++ b/tests/sampler-offset.shader_test
> @@ -22,7 +22,7 @@ float4 main() : sv_target
> }
>
> [test]
> -todo draw quad
> +draw quad
> probe all rgba (0.1, 0.2, 0.5, 0.0)
>
> @@ -36,7 +36,7 @@ float4 main() : sv_target
> }
>
> [test]
> -todo draw quad
> +draw quad
> probe all rgba (0.2, 0.2, 0.0, 0.4)
>
> @@ -50,5 +50,5 @@ float4 main() : sv_target
> }
>
> [test]
> -todo draw quad
> +draw quad
> probe all rgba (0.0, 0.2, 0.0, 0.4)
> -- 
> 2.36.0

[Francisco Casas]

Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com>


May 3, 2022 6:14 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote:
> Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
> ---
> libs/vkd3d-shader/hlsl.y | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
> index 905dbfc5..e518430f 100644
> --- a/libs/vkd3d-shader/hlsl.y
> +++ b/libs/vkd3d-shader/hlsl.y
> @@ -2049,7 +2049,7 @@ static struct list *add_constructor(struct hlsl_ctx *ctx, struct hlsl_type
> *type
> continue;
> }
>
> - initialize_var_components(ctx, params->instrs, var, &idx, arg, &loc);
> + initialize_var_components(ctx, params->instrs, var, &idx, arg, &arg->loc);
> }
>
> if (!(load = hlsl_new_var_load(ctx, var, loc)))
> -- 
> 2.36.0

[Zebediah Figura (she/her)]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>

[Henri Verbeet]

Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>