[PATCH 1/5] d3dcompiler: Fix up the swizzle data type when unwrapping it from the assignment LHS.

[Zebediah Figura]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
 dlls/d3dcompiler_43/utils.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/dlls/d3dcompiler_43/utils.c b/dlls/d3dcompiler_43/utils.c
index 60bb5ff8145..7093e9eb2c0 100644
--- a/dlls/d3dcompiler_43/utils.c
+++ b/dlls/d3dcompiler_43/utils.c
@@ -1403,29 +1403,29 @@ static enum hlsl_ir_expr_op op_from_assignment(enum parse_assign_op op)
     return ops[op];
 }
 
-static unsigned int invert_swizzle(unsigned int *swizzle, unsigned int writemask)
+static BOOL invert_swizzle(unsigned int *swizzle, unsigned int *writemask, unsigned int *ret_width)
 {
-    unsigned int i, j, bit = 0, inverted = 0, components, new_writemask = 0, new_swizzle = 0;
+    unsigned int i, j, bit = 0, inverted = 0, width, new_writemask = 0, new_swizzle = 0;
 
     /* Apply the writemask to the swizzle to get a new writemask and swizzle. */
     for (i = 0; i < 4; ++i)
     {
-        if (writemask & (1 << i))
+        if (*writemask & (1 << i))
         {
             unsigned int s = (*swizzle >> (i * 2)) & 3;
             new_swizzle |= s << (bit++ * 2);
             if (new_writemask & (1 << s))
-                return 0;
+                return FALSE;
             new_writemask |= 1 << s;
         }
     }
-    components = bit;
+    width = bit;
 
     /* Invert the swizzle. */
     bit = 0;
     for (i = 0; i < 4; ++i)
     {
-        for (j = 0; j < components; ++j)
+        for (j = 0; j < width; ++j)
         {
             unsigned int s = (new_swizzle >> (j * 2)) & 3;
             if (s == i)
@@ -1434,7 +1434,9 @@ static unsigned int invert_swizzle(unsigned int *swizzle, unsigned int writemask
     }
 
     *swizzle = inverted;
-    return new_writemask;
+    *writemask = new_writemask;
+    *ret_width = width;
+    return TRUE;
 }
 
 struct hlsl_ir_node *make_assignment(struct hlsl_ir_node *lhs, enum parse_assign_op assign_op,
@@ -1463,6 +1465,8 @@ struct hlsl_ir_node *make_assignment(struct hlsl_ir_node *lhs, enum parse_assign
         else if (lhs->type == HLSL_IR_SWIZZLE)
         {
             struct hlsl_ir_swizzle *swizzle = swizzle_from_node(lhs);
+            const struct hlsl_type *swizzle_type = swizzle->node.data_type;
+            unsigned int width;
 
             if (lhs->data_type->type == HLSL_CLASS_MATRIX)
                 FIXME("Assignments with writemasks and matrices on lhs are not supported yet.\n");
@@ -1472,12 +1476,23 @@ struct hlsl_ir_node *make_assignment(struct hlsl_ir_node *lhs, enum parse_assign
 
             list_add_after(&rhs->entry, &lhs->entry);
             swizzle->val = rhs;
-            if (!(writemask = invert_swizzle(&swizzle->swizzle, writemask)))
+            if (!invert_swizzle(&swizzle->swizzle, &writemask, &width))
             {
                 hlsl_report_message(lhs->loc, HLSL_LEVEL_ERROR, "invalid writemask");
                 d3dcompiler_free(assign);
                 return NULL;
             }
+            assert(swizzle_type->type == HLSL_CLASS_VECTOR);
+            if (swizzle_type->dimx != width)
+            {
+                struct hlsl_type *type;
+                if (!(type = new_hlsl_type(NULL, HLSL_CLASS_VECTOR, swizzle_type->base_type, width, 1)))
+                {
+                    d3dcompiler_free(assign);
+                    return NULL;
+                }
+                swizzle->node.data_type = type;
+            }
             rhs = &swizzle->node;
         }
         else
-- 
2.26.2

[PATCH 2/5] d3dcompiler: Remove a redundant check for writemasks on non-numeric types.

[Zebediah Figura]

This can only happen if a swizzle is emitted on a non-numeric type, but this is
checked when emitting swizzles.

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
 dlls/d3dcompiler_43/utils.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/dlls/d3dcompiler_43/utils.c b/dlls/d3dcompiler_43/utils.c
index 7093e9eb2c0..767d2dee500 100644
--- a/dlls/d3dcompiler_43/utils.c
+++ b/dlls/d3dcompiler_43/utils.c
@@ -1514,13 +1514,6 @@ struct hlsl_ir_node *make_assignment(struct hlsl_ir_node *lhs, enum parse_assign
         DWORD bitmask;
         enum hlsl_type_class type_class;
 
-        if (lhs->data_type->type > HLSL_CLASS_LAST_NUMERIC)
-        {
-            hlsl_report_message(lhs->loc, HLSL_LEVEL_ERROR,
-                    "writemask on a non scalar/vector/matrix type");
-            d3dcompiler_free(assign);
-            return NULL;
-        }
         bitmask = writemask & ((1 << lhs->data_type->dimx) - 1);
         while (bitmask)
         {
-- 
2.26.2

[PATCH 2/5] d3dcompiler: Remove a redundant check for writemasks on non-numeric types.

[Matteo Bruni]

Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>

[PATCH 3/5] d3dcompiler: Coerce the RHS before unwrapping swizzles.

[Zebediah Figura]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
 dlls/d3dcompiler_43/utils.c | 43 +++++++++++--------------------------
 1 file changed, 12 insertions(+), 31 deletions(-)

diff --git a/dlls/d3dcompiler_43/utils.c b/dlls/d3dcompiler_43/utils.c
index 767d2dee500..ec0da2d4034 100644
--- a/dlls/d3dcompiler_43/utils.c
+++ b/dlls/d3dcompiler_43/utils.c
@@ -1443,8 +1443,17 @@ struct hlsl_ir_node *make_assignment(struct hlsl_ir_node *lhs, enum parse_assign
         struct hlsl_ir_node *rhs)
 {
     struct hlsl_ir_assignment *assign = d3dcompiler_alloc(sizeof(*assign));
-    DWORD writemask = (1 << lhs->data_type->dimx) - 1;
-    struct hlsl_type *type;
+    struct hlsl_type *lhs_type;
+    DWORD writemask = 0;
+
+    lhs_type = lhs->data_type;
+    if (lhs_type->type <= HLSL_CLASS_LAST_NUMERIC)
+    {
+        writemask = (1 << lhs_type->dimx) - 1;
+
+        if (!(rhs = implicit_conversion(rhs, lhs_type, &rhs->loc)))
+            return NULL;
+    }
 
     if (!assign)
     {
@@ -1505,36 +1514,8 @@ struct hlsl_ir_node *make_assignment(struct hlsl_ir_node *lhs, enum parse_assign
         lhs = lhs_inner;
     }
 
-    TRACE("Creating proper assignment expression.\n");
-    if (writemask == BWRITERSP_WRITEMASK_ALL)
-        type = lhs->data_type;
-    else
-    {
-        unsigned int dimx = 0;
-        DWORD bitmask;
-        enum hlsl_type_class type_class;
-
-        bitmask = writemask & ((1 << lhs->data_type->dimx) - 1);
-        while (bitmask)
-        {
-            if (bitmask & 1)
-                dimx++;
-            bitmask >>= 1;
-        }
-        if (lhs->data_type->type == HLSL_CLASS_MATRIX)
-            FIXME("Assignments with writemasks and matrices on lhs are not supported yet.\n");
-        if (dimx == 1)
-            type_class = HLSL_CLASS_SCALAR;
-        else
-            type_class = lhs->data_type->type;
-        type = new_hlsl_type(NULL, type_class, lhs->data_type->base_type, dimx, 1);
-    }
-
-    init_node(&assign->node, HLSL_IR_ASSIGNMENT, type, lhs->loc);
+    init_node(&assign->node, HLSL_IR_ASSIGNMENT, lhs_type, lhs->loc);
     assign->writemask = writemask;
-
-    rhs = implicit_conversion(rhs, type, &rhs->loc);
-
     assign->lhs = load_from_node(lhs)->src;
     if (assign_op != ASSIGN_OP_ASSIGN)
     {
-- 
2.26.2

[PATCH 3/5] d3dcompiler: Coerce the RHS before unwrapping swizzles.

[Matteo Bruni]

Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>

[PATCH 3/5] d3dcompiler: Coerce the RHS before unwrapping swizzles.

[Matteo Bruni]

On Thu, Jun 4, 2020 at 4:12 AM Zebediah Figura <z.figura12(a)gmail.com> wrote:
> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
> ---
>  dlls/d3dcompiler_43/utils.c | 43 +++++++++++--------------------------
>  1 file changed, 12 insertions(+), 31 deletions(-)

This patch is a clear improvement and the dropped FIXME() didn't
really make sense given that, if you have a swizzle on the lhs, that
means you have a vector.

Still, you can have matrix swizzles and those aren't handled correctly
yet. One nasty example (which could be a test):

float4 main() : COLOR
{
    float4x4 ret = float4x4(0.0, 0.0, 0.0, 0.0,
            0.0, 0.0, 0.0, 0.0,
            0.0, 0.0, 0.0, 0.0,
            0.0, 0.0, 0.0, 0.0);
    ret._11_22_33 = float3(0.1, 0.2, 0.3);
    return ret[0];
}

Nothing you need to care about right now, clearly (and probably not
something that you'd handle in this area of the compiler either), just
putting it out there for another day...

[PATCH 3/5] d3dcompiler: Coerce the RHS before unwrapping swizzles.

[Zebediah Figura]

On 6/5/20 4:41 AM, Matteo Bruni wrote:
> On Thu, Jun 4, 2020 at 4:12 AM Zebediah Figura <z.figura12(a)gmail.com> wrote:
> > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
> > ---
> >  dlls/d3dcompiler_43/utils.c | 43 +++++++++++--------------------------
> >  1 file changed, 12 insertions(+), 31 deletions(-)
>
> This patch is a clear improvement and the dropped FIXME() didn't
> really make sense given that, if you have a swizzle on the lhs, that
> means you have a vector.
>
> Still, you can have matrix swizzles and those aren't handled correctly
> yet. One nasty example (which could be a test):
>
> float4 main() : COLOR
> {
>     float4x4 ret = float4x4(0.0, 0.0, 0.0, 0.0,
>             0.0, 0.0, 0.0, 0.0,
>             0.0, 0.0, 0.0, 0.0,
>             0.0, 0.0, 0.0, 0.0);
>     ret._11_22_33 = float3(0.1, 0.2, 0.3);
>     return ret[0];
> }
>
> Nothing you need to care about right now, clearly (and probably not
> something that you'd handle in this area of the compiler either), just
> putting it out there for another day...

Ech, you're right, and actually, the FIXME was sort of valid, although
the complex logic around writemasks probably threw it off: after we've
unwrapped all the swizzles from the lhs and converted them to
writemasks, what remains is either a vector or a matrix.

Not that I've forgotten we don't handle matrix swizzles, but I did
forget to retain that FIXME in the patch. I'll send a follow-up.

[PATCH 4/5] d3dcompiler: Fix matrix dimension declaration for matrix<...> syntax.

[Zebediah Figura]

Addendum to 4952bcfa4cf.

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
 dlls/d3dcompiler_43/hlsl.y            | 2 +-
 dlls/d3dcompiler_43/tests/hlsl_d3d9.c | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y
index 1797996ddbd..820352b0466 100644
--- a/dlls/d3dcompiler_43/hlsl.y
+++ b/dlls/d3dcompiler_43/hlsl.y
@@ -1866,7 +1866,7 @@ type:                     base_type
                                     YYABORT;
                                 }
 
-                                $$ = new_hlsl_type(NULL, HLSL_CLASS_MATRIX, $3->base_type, $5, $7);
+                                $$ = new_hlsl_type(NULL, HLSL_CLASS_MATRIX, $3->base_type, $7, $5);
                             }
 
 base_type:
diff --git a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c
index c8cf86d05c4..059f39e4c3e 100644
--- a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c
+++ b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c
@@ -897,9 +897,10 @@ static void test_constant_table(void)
         "uniform float2 g[5];\n"
         "uniform matrix_t i;\n"
         "uniform struct matrix_record j;\n"
+        "uniform matrix<float,3,1> k;\n"
         "float4 main(uniform float4 h) : COLOR\n"
         "{\n"
-        "    return a + b + c._31 + d._31 + f.d._22 + g[e].x + h + i._33 + j.a._33;\n"
+        "    return a + b + c._31 + d._31 + f.d._22 + g[e].x + h + i._33 + j.a._33 + k._31;\n"
         "}";
 
     D3DXCONSTANTTABLE_DESC table_desc;
@@ -923,6 +924,7 @@ static void test_constant_table(void)
         {"g", D3DXRS_FLOAT4, 0, 5, D3DXPC_VECTOR, D3DXPT_FLOAT, 1, 2, 5, 0, 40},
         {"i", D3DXRS_FLOAT4, 0, 3, D3DXPC_MATRIX_ROWS, D3DXPT_FLOAT, 3, 3, 1, 0, 36},
         {"j", D3DXRS_FLOAT4, 0, 3, D3DXPC_STRUCT, D3DXPT_VOID, 1, 9, 1, 1, 36},
+        {"k", D3DXRS_FLOAT4, 0, 3, D3DXPC_MATRIX_ROWS, D3DXPT_FLOAT, 3, 1, 1, 0, 12},
     };
 
     static const D3DXCONSTANT_DESC expect_fields_f[] =
-- 
2.26.2

[PATCH 4/5] d3dcompiler: Fix matrix dimension declaration for matrix<...> syntax.

[Matteo Bruni]

Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>

[PATCH 5/5] d3dcompiler: Use hlsl_report_message() where possible.

[Zebediah Figura]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
 dlls/d3dcompiler_43/hlsl.y | 111 ++++++++++++++++++-------------------
 1 file changed, 54 insertions(+), 57 deletions(-)

diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y
index 820352b0466..436b731fd16 100644
--- a/dlls/d3dcompiler_43/hlsl.y
+++ b/dlls/d3dcompiler_43/hlsl.y
@@ -1826,48 +1826,52 @@ input_mod:                KW_IN
                                 $$ = HLSL_STORAGE_IN | HLSL_STORAGE_OUT;
                             }
 
-type:                     base_type
-                            {
-                                $$ = $1;
-                            }
-                        | KW_VECTOR '<' base_type ',' C_INTEGER '>'
-                            {
-                                if ($3->type != HLSL_CLASS_SCALAR)
-                                {
-                                    hlsl_message("Line %u: vectors of non-scalar types are not allowed.\n",
-                                            hlsl_ctx.line_no);
-                                    set_parse_status(&hlsl_ctx.status, PARSE_ERR);
-                                    YYABORT;
-                                }
-                                if ($5 < 1 || $5 > 4)
-                                {
-                                    hlsl_message("Line %u: vector size must be between 1 and 4.\n",
-                                            hlsl_ctx.line_no);
-                                    set_parse_status(&hlsl_ctx.status, PARSE_ERR);
-                                    YYABORT;
-                                }
+type:
 
-                                $$ = new_hlsl_type(NULL, HLSL_CLASS_VECTOR, $3->base_type, $5, 1);
-                            }
-                        | KW_MATRIX '<' base_type ',' C_INTEGER ',' C_INTEGER '>'
-                            {
-                                if ($3->type != HLSL_CLASS_SCALAR)
-                                {
-                                    hlsl_message("Line %u: matrices of non-scalar types are not allowed.\n",
-                                            hlsl_ctx.line_no);
-                                    set_parse_status(&hlsl_ctx.status, PARSE_ERR);
-                                    YYABORT;
-                                }
-                                if ($5 < 1 || $5 > 4 || $7 < 1 || $7 > 4)
-                                {
-                                    hlsl_message("Line %u: matrix dimensions must be between 1 and 4.\n",
-                                            hlsl_ctx.line_no);
-                                    set_parse_status(&hlsl_ctx.status, PARSE_ERR);
-                                    YYABORT;
-                                }
+      base_type
+        {
+            $$ = $1;
+        }
+    | KW_VECTOR '<' base_type ',' C_INTEGER '>'
+        {
+            if ($3->type != HLSL_CLASS_SCALAR)
+            {
+                hlsl_report_message(get_location(&@3), HLSL_LEVEL_ERROR,
+                        "vectors of non-scalar types are not allowed\n");
+                YYABORT;
+            }
+            if ($5 < 1 || $5 > 4)
+            {
+                hlsl_report_message(get_location(&@5), HLSL_LEVEL_ERROR,
+                        "vector size must be between 1 and 4\n");
+                YYABORT;
+            }
 
-                                $$ = new_hlsl_type(NULL, HLSL_CLASS_MATRIX, $3->base_type, $7, $5);
-                            }
+            $$ = new_hlsl_type(NULL, HLSL_CLASS_VECTOR, $3->base_type, $5, 1);
+        }
+    | KW_MATRIX '<' base_type ',' C_INTEGER ',' C_INTEGER '>'
+        {
+            if ($3->type != HLSL_CLASS_SCALAR)
+            {
+                hlsl_report_message(get_location(&@3), HLSL_LEVEL_ERROR,
+                        "matrices of non-scalar types are not allowed\n");
+                YYABORT;
+            }
+            if ($5 < 1 || $5 > 4)
+            {
+                hlsl_report_message(get_location(&@5), HLSL_LEVEL_ERROR,
+                        "matrix row count must be between 1 and 4\n");
+                YYABORT;
+            }
+            if ($7 < 1 || $7 > 4)
+            {
+                hlsl_report_message(get_location(&@7), HLSL_LEVEL_ERROR,
+                        "matrix column count must be between 1 and 4\n");
+                YYABORT;
+            }
+
+            $$ = new_hlsl_type(NULL, HLSL_CLASS_MATRIX, $3->base_type, $7, $5);
+        }
 
 base_type:
 
@@ -1904,11 +1908,7 @@ base_type:
         {
             $$ = get_type(hlsl_ctx.cur_scope, $2, TRUE);
             if ($$->type != HLSL_CLASS_STRUCT)
-            {
-                hlsl_message("Line %u: redefining %s as a structure.\n",
-                        hlsl_ctx.line_no, $2);
-                set_parse_status(&hlsl_ctx.status, PARSE_ERR);
-            }
+                hlsl_report_message(get_location(&@1), HLSL_LEVEL_ERROR, "'%s' redefined as a structure\n", $2);
             d3dcompiler_free($2);
         }
 
@@ -2291,9 +2291,8 @@ primary_expr:             C_FLOAT
 
                                 if (!(var = get_variable(hlsl_ctx.cur_scope, $1)))
                                 {
-                                    hlsl_message("Line %d: variable '%s' not declared\n",
-                                            hlsl_ctx.line_no, $1);
-                                    set_parse_status(&hlsl_ctx.status, PARSE_ERR);
+                                    hlsl_report_message(get_location(&@1), HLSL_LEVEL_ERROR,
+                                            "variable '%s' is not declared\n", $1);
                                     YYABORT;
                                 }
                                 if ((load = new_var_load(var, get_location(&@1))))
@@ -2425,23 +2424,21 @@ postfix_expr:             primary_expr
                                 TRACE("%s constructor.\n", debug_hlsl_type($2));
                                 if ($1)
                                 {
-                                    hlsl_message("Line %u: unexpected modifier in a constructor.\n",
-                                            hlsl_ctx.line_no);
-                                    set_parse_status(&hlsl_ctx.status, PARSE_ERR);
+                                    hlsl_report_message(get_location(&@1), HLSL_LEVEL_ERROR,
+                                            "unexpected modifier on a constructor\n");
                                     YYABORT;
                                 }
                                 if ($2->type > HLSL_CLASS_LAST_NUMERIC)
                                 {
-                                    hlsl_message("Line %u: constructors are allowed only for numeric data types.\n",
-                                            hlsl_ctx.line_no);
-                                    set_parse_status(&hlsl_ctx.status, PARSE_ERR);
+                                    hlsl_report_message(get_location(&@2), HLSL_LEVEL_ERROR,
+                                            "constructors may only be used with numeric data types\n");
                                     YYABORT;
                                 }
                                 if ($2->dimx * $2->dimy != initializer_size(&$4))
                                 {
-                                    hlsl_message("Line %u: wrong number of components in constructor.\n",
-                                            hlsl_ctx.line_no);
-                                    set_parse_status(&hlsl_ctx.status, PARSE_ERR);
+                                    hlsl_report_message(get_location(&@4), HLSL_LEVEL_ERROR,
+                                            "expected %u components in constructor, but got %u\n",
+                                            $2->dimx * $2->dimy, initializer_size(&$4));
                                     YYABORT;
                                 }
                                 assert($4.args_count <= ARRAY_SIZE(constructor->args));
-- 
2.26.2

[PATCH 5/5] d3dcompiler: Use hlsl_report_message() where possible.

[Matteo Bruni]

Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
---
Thank you for improving a bunch of messages while at it, much
appreciated.

[Matteo Bruni]

Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>

[Matteo Bruni]

Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
---
Resending in case the previous email doesn't go through...