[PATCH 1/5] d3dcompiler: Fix up the swizzle data type when unwrapping it from the assignment LHS.
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
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
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
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
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
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...
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.
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
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
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
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com> --- Thank you for improving a bunch of messages while at it, much appreciated.
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com> --- Resending in case the previous email doesn't go through...
participants (4)
-
Matteo Bruni -
Matteo Bruni -
Zebediah Figura -
Zebediah Figura