[PATCH vkd3d 1/5] vkd3d-shader: Don't use assignment instructions as sources.
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index ce4bd626..a9c9dd02 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1261,6 +1261,7 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in { struct hlsl_ir_assignment *assign; struct hlsl_type *lhs_type; + struct hlsl_ir_expr *copy; DWORD writemask = 0; lhs_type = lhs->data_type; @@ -1321,7 +1322,7 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in lhs = lhs_inner; } - init_node(&assign->node, HLSL_IR_ASSIGNMENT, lhs_type, lhs->loc); + init_node(&assign->node, HLSL_IR_ASSIGNMENT, NULL, lhs->loc); assign->writemask = writemask; assign->lhs.var = hlsl_ir_load(lhs)->src.var; hlsl_src_from_node(&assign->lhs.offset, hlsl_ir_load(lhs)->src.offset.node); @@ -1337,7 +1338,14 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in hlsl_src_from_node(&assign->rhs, rhs); list_add_tail(instrs, &assign->node.entry); - return &assign->node; + /* Don't use the instruction itself as a source, as this makes structure + * splitting easier. Instead copy it here. Since we retrieve sources from + * the last instruction in the list, we do need to copy. Use a cast + * instruction to the same type as a makeshift identity expression. */ + if (!(copy = hlsl_new_cast(rhs, rhs->data_type, &lhs->loc))) + return NULL; + list_add_tail(instrs, ©->node.entry); + return ©->node; } static void struct_var_initializer(struct hlsl_ctx *ctx, struct list *list, struct hlsl_ir_var *var, -- 2.30.1
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 2 -- libs/vkd3d-shader/hlsl.h | 2 -- libs/vkd3d-shader/hlsl.y | 28 ++++++++++++++++++++++------ 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index e78f05c3..2312b9d3 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -983,8 +983,6 @@ static const char *debug_expr_op(const struct hlsl_ir_expr *expr) "sat", - "pre++", - "pre--", "post++", "post--", diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 76734401..b64ce2d5 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -276,8 +276,6 @@ enum hlsl_ir_expr_op HLSL_IR_UNOP_SAT, - HLSL_IR_UNOP_PREINC, - HLSL_IR_UNOP_PREDEC, HLSL_IR_UNOP_POSTINC, HLSL_IR_UNOP_POSTDEC, diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index a9c9dd02..95a2ed69 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1348,6 +1348,22 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in return ©->node; } +static bool add_increment(struct hlsl_ctx *ctx, struct list *instrs, bool decrement, struct vkd3d_shader_location loc) +{ + struct hlsl_ir_node *lhs = node_from_list(instrs); + struct hlsl_ir_constant *one; + + if (lhs->data_type->modifiers & HLSL_MODIFIER_CONST) + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_MODIFIES_CONST, + "Argument to pre%screment operator is const.", decrement ? "de" : "in"); + + if (!(one = hlsl_new_uint_constant(ctx, 1, loc))) + return false; + list_add_tail(instrs, &one->node.entry); + + return !!add_assignment(ctx, instrs, lhs, decrement ? ASSIGN_OP_SUB : ASSIGN_OP_ADD, &one->node); +} + static void struct_var_initializer(struct hlsl_ctx *ctx, struct list *list, struct hlsl_ir_var *var, struct parse_initializer *initializer) { @@ -2785,21 +2801,21 @@ unary_expr: postfix_expr | OP_INC unary_expr { - if (node_from_list($2)->data_type->modifiers & HLSL_MODIFIER_CONST) + if (!add_increment(ctx, $2, false, @1)) { - hlsl_error(ctx, @1, VKD3D_SHADER_ERROR_HLSL_MODIFIES_CONST, "Statement modifies a const expression."); + hlsl_free_instr_list($2); YYABORT; } - $$ = append_unop($2, hlsl_new_unary_expr(HLSL_IR_UNOP_PREINC, node_from_list($2), @1)); + $$ = $2; } | OP_DEC unary_expr { - if (node_from_list($2)->data_type->modifiers & HLSL_MODIFIER_CONST) + if (!add_increment(ctx, $2, true, @1)) { - hlsl_error(ctx, @1, VKD3D_SHADER_ERROR_HLSL_MODIFIES_CONST, "Statement modifies a const expression."); + hlsl_free_instr_list($2); YYABORT; } - $$ = append_unop($2, hlsl_new_unary_expr(HLSL_IR_UNOP_PREDEC, node_from_list($2), @1)); + $$ = $2; } | unary_op unary_expr { -- 2.30.1
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 3 --- libs/vkd3d-shader/hlsl.h | 3 --- libs/vkd3d-shader/hlsl.y | 53 +++++++++++++++++++++++----------------- 3 files changed, 30 insertions(+), 29 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 2312b9d3..19c0e2e4 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -983,9 +983,6 @@ static const char *debug_expr_op(const struct hlsl_ir_expr *expr) "sat", - "post++", - "post--", - "+", "-", "*", diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index b64ce2d5..5a2f6af1 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -276,9 +276,6 @@ enum hlsl_ir_expr_op HLSL_IR_UNOP_SAT, - HLSL_IR_UNOP_POSTINC, - HLSL_IR_UNOP_POSTDEC, - HLSL_IR_BINOP_ADD, HLSL_IR_BINOP_SUB, HLSL_IR_BINOP_MUL, diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 95a2ed69..815aac9b 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1348,20 +1348,39 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in return ©->node; } -static bool add_increment(struct hlsl_ctx *ctx, struct list *instrs, bool decrement, struct vkd3d_shader_location loc) +static bool add_increment(struct hlsl_ctx *ctx, struct list *instrs, bool decrement, bool post, + struct vkd3d_shader_location loc) { struct hlsl_ir_node *lhs = node_from_list(instrs); struct hlsl_ir_constant *one; if (lhs->data_type->modifiers & HLSL_MODIFIER_CONST) hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_MODIFIES_CONST, - "Argument to pre%screment operator is const.", decrement ? "de" : "in"); + "Argument to %s%screment operator is const.", post ? "post" : "pre", decrement ? "de" : "in"); if (!(one = hlsl_new_uint_constant(ctx, 1, loc))) return false; list_add_tail(instrs, &one->node.entry); - return !!add_assignment(ctx, instrs, lhs, decrement ? ASSIGN_OP_SUB : ASSIGN_OP_ADD, &one->node); + if (!add_assignment(ctx, instrs, lhs, decrement ? ASSIGN_OP_SUB : ASSIGN_OP_ADD, &one->node)) + return false; + + if (post) + { + struct hlsl_ir_expr *copy; + + /* Use a cast to the same type as a makeshift identity expression. */ + if (!(copy = hlsl_new_cast(lhs, lhs->data_type, &lhs->loc))) + return false; + list_add_tail(instrs, ©->node.entry); + + /* Post increment/decrement expressions are considered const. */ + if (!(copy->node.data_type = hlsl_type_clone(ctx, copy->node.data_type, 0))) + return false; + copy->node.data_type->modifiers |= HLSL_MODIFIER_CONST; + } + + return true; } static void struct_var_initializer(struct hlsl_ctx *ctx, struct list *list, struct hlsl_ir_var *var, @@ -2624,33 +2643,21 @@ postfix_expr: primary_expr | postfix_expr OP_INC { - struct hlsl_ir_node *inc; - - if (node_from_list($1)->data_type->modifiers & HLSL_MODIFIER_CONST) + if (!add_increment(ctx, $1, false, true, @2)) { - hlsl_error(ctx, @2, VKD3D_SHADER_ERROR_HLSL_MODIFIES_CONST, "Statement modifies a const expression."); + hlsl_free_instr_list($1); YYABORT; } - inc = hlsl_new_unary_expr(HLSL_IR_UNOP_POSTINC, node_from_list($1), @2); - /* Post increment/decrement expressions are considered const */ - inc->data_type = hlsl_type_clone(ctx, inc->data_type, 0); - inc->data_type->modifiers |= HLSL_MODIFIER_CONST; - $$ = append_unop($1, inc); + $$ = $1; } | postfix_expr OP_DEC { - struct hlsl_ir_node *inc; - - if (node_from_list($1)->data_type->modifiers & HLSL_MODIFIER_CONST) + if (!add_increment(ctx, $1, true, true, @2)) { - hlsl_error(ctx, @2, VKD3D_SHADER_ERROR_HLSL_MODIFIES_CONST, "Statement modifies a const expression."); + hlsl_free_instr_list($1); YYABORT; } - inc = hlsl_new_unary_expr(HLSL_IR_UNOP_POSTDEC, node_from_list($1), @2); - /* Post increment/decrement expressions are considered const */ - inc->data_type = hlsl_type_clone(ctx, inc->data_type, 0); - inc->data_type->modifiers |= HLSL_MODIFIER_CONST; - $$ = append_unop($1, inc); + $$ = $1; } | postfix_expr '.' any_identifier { @@ -2801,7 +2808,7 @@ unary_expr: postfix_expr | OP_INC unary_expr { - if (!add_increment(ctx, $2, false, @1)) + if (!add_increment(ctx, $2, false, false, @1)) { hlsl_free_instr_list($2); YYABORT; @@ -2810,7 +2817,7 @@ unary_expr: } | OP_DEC unary_expr { - if (!add_increment(ctx, $2, true, @1)) + if (!add_increment(ctx, $2, true, false, @1)) { hlsl_free_instr_list($2); YYABORT; -- 2.30.1
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
On Wed, Mar 10, 2021 at 2:43 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 3 --- libs/vkd3d-shader/hlsl.h | 3 --- libs/vkd3d-shader/hlsl.y | 53 +++++++++++++++++++++++----------------- 3 files changed, 30 insertions(+), 29 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 95a2ed69..815aac9b 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1348,20 +1348,39 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in return ©->node; }
-static bool add_increment(struct hlsl_ctx *ctx, struct list *instrs, bool decrement, struct vkd3d_shader_location loc) +static bool add_increment(struct hlsl_ctx *ctx, struct list *instrs, bool decrement, bool post, + struct vkd3d_shader_location loc) { struct hlsl_ir_node *lhs = node_from_list(instrs); struct hlsl_ir_constant *one;
if (lhs->data_type->modifiers & HLSL_MODIFIER_CONST) hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_MODIFIES_CONST, - "Argument to pre%screment operator is const.", decrement ? "de" : "in"); + "Argument to %s%screment operator is const.", post ? "post" : "pre", decrement ? "de" : "in");
if (!(one = hlsl_new_uint_constant(ctx, 1, loc))) return false; list_add_tail(instrs, &one->node.entry);
- return !!add_assignment(ctx, instrs, lhs, decrement ? ASSIGN_OP_SUB : ASSIGN_OP_ADD, &one->node); + if (!add_assignment(ctx, instrs, lhs, decrement ? ASSIGN_OP_SUB : ASSIGN_OP_ADD, &one->node)) + return false; + + if (post) + { + struct hlsl_ir_expr *copy; + + /* Use a cast to the same type as a makeshift identity expression. */ + if (!(copy = hlsl_new_cast(lhs, lhs->data_type, &lhs->loc))) + return false;
Now that we have two places where we clone an expression result it might make sense to factor it out to a tiny helper. Mostly for clarity (obvious function name -> no need to add a comment every time) and in case in the future we decide to handle it differently.
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
From: Zebediah Figura <z.figura12(a)gmail.com> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 54 +++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 815aac9b..6fceafb6 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1259,12 +1259,22 @@ static bool invert_swizzle(unsigned int *swizzle, unsigned int *writemask, unsig static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *lhs, enum parse_assign_op assign_op, struct hlsl_ir_node *rhs) { + struct hlsl_type *lhs_type = lhs->data_type; struct hlsl_ir_assignment *assign; - struct hlsl_type *lhs_type; struct hlsl_ir_expr *copy; DWORD writemask = 0; - lhs_type = lhs->data_type; + if (assign_op != ASSIGN_OP_ASSIGN) + { + enum hlsl_ir_expr_op op = op_from_assignment(assign_op); + struct hlsl_ir_node *args[3] = {lhs, rhs}; + struct hlsl_ir_expr *expr; + + if (!(expr = add_expr(ctx, instrs, op, args, &rhs->loc))) + return NULL; + rhs = &expr->node; + } + if (lhs_type->type <= HLSL_CLASS_LAST_NUMERIC) { writemask = (1 << lhs_type->dimx) - 1; @@ -1278,8 +1288,6 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in while (lhs->type != HLSL_IR_LOAD) { - struct hlsl_ir_node *lhs_inner; - if (lhs->type == HLSL_IR_EXPR && hlsl_ir_expr(lhs)->op == HLSL_IR_UNOP_CAST) { FIXME("Cast on the lhs.\n"); @@ -1288,29 +1296,28 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in } else if (lhs->type == HLSL_IR_SWIZZLE) { - struct hlsl_ir_swizzle *swizzle = hlsl_ir_swizzle(lhs); - const struct hlsl_type *swizzle_type = swizzle->node.data_type; - unsigned int width; + struct hlsl_ir_swizzle *swizzle = hlsl_ir_swizzle(lhs), *new_swizzle; + unsigned int width, s = swizzle->swizzle; if (lhs->data_type->type == HLSL_CLASS_MATRIX) FIXME("Assignments with writemasks and matrices on lhs are not supported yet.\n"); - lhs_inner = swizzle->val.node; - hlsl_src_remove(&swizzle->val); - list_remove(&lhs->entry); - - list_add_after(&rhs->entry, &lhs->entry); - hlsl_src_from_node(&swizzle->val, rhs); - if (!invert_swizzle(&swizzle->swizzle, &writemask, &width)) + if (!invert_swizzle(&s, &writemask, &width)) { hlsl_error(ctx, lhs->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_WRITEMASK, "Invalid writemask."); vkd3d_free(assign); return NULL; } - assert(swizzle_type->type == HLSL_CLASS_VECTOR); - if (swizzle_type->dimx != width) - swizzle->node.data_type = ctx->builtin_types.vector[swizzle_type->base_type][width - 1]; - rhs = &swizzle->node; + + if (!(new_swizzle = hlsl_new_swizzle(ctx, s, width, rhs, &swizzle->node.loc))) + { + vkd3d_free(assign); + return NULL; + } + list_add_tail(instrs, &new_swizzle->node.entry); + + lhs = swizzle->val.node; + rhs = &new_swizzle->node; } else { @@ -1318,23 +1325,12 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in vkd3d_free(assign); return NULL; } - - lhs = lhs_inner; } init_node(&assign->node, HLSL_IR_ASSIGNMENT, NULL, lhs->loc); assign->writemask = writemask; assign->lhs.var = hlsl_ir_load(lhs)->src.var; hlsl_src_from_node(&assign->lhs.offset, hlsl_ir_load(lhs)->src.offset.node); - if (assign_op != ASSIGN_OP_ASSIGN) - { - enum hlsl_ir_expr_op op = op_from_assignment(assign_op); - struct hlsl_ir_node *expr; - - expr = hlsl_new_binary_expr(op, lhs, rhs); - list_add_after(&rhs->entry, &expr->entry); - rhs = expr; - } hlsl_src_from_node(&assign->rhs, rhs); list_add_tail(instrs, &assign->node.entry); -- 2.30.1
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com> --- Reviewing this part is always tricky but it seems to make sense to me.
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
From: Zebediah Figura <z.figura12(a)gmail.com> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- Makefile.am | 12 ++++++++++-- tests/writemask-assignop-0.shader_test | 15 +++++++++++++++ tests/writemask-assignop-1.shader_test | 16 ++++++++++++++++ tests/writemask-assignop-2.shader_test | 13 +++++++++++++ tests/writemask-assignop-3.shader_test | 11 +++++++++++ 5 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 tests/writemask-assignop-0.shader_test create mode 100644 tests/writemask-assignop-1.shader_test create mode 100644 tests/writemask-assignop-2.shader_test create mode 100644 tests/writemask-assignop-3.shader_test diff --git a/Makefile.am b/Makefile.am index a4121944..a79ad02f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -80,7 +80,11 @@ vkd3d_shader_tests = \ tests/swizzle-5.shader_test \ tests/swizzle-6.shader_test \ tests/swizzle-7.shader_test \ - tests/trigonometry.shader_test + tests/trigonometry.shader_test \ + tests/writemask-assignop-0.shader_test \ + tests/writemask-assignop-1.shader_test \ + tests/writemask-assignop-2.shader_test \ + tests/writemask-assignop-3.shader_test vkd3d_test_headers = \ tests/d3d12_crosstest.h \ @@ -267,7 +271,11 @@ XFAIL_TESTS = \ tests/swizzle-5.shader_test \ tests/swizzle-6.shader_test \ tests/swizzle-7.shader_test \ - tests/trigonometry.shader_test + tests/trigonometry.shader_test \ + tests/writemask-assignop-0.shader_test \ + tests/writemask-assignop-1.shader_test \ + tests/writemask-assignop-2.shader_test \ + tests/writemask-assignop-3.shader_test endif if BUILD_DEMOS diff --git a/tests/writemask-assignop-0.shader_test b/tests/writemask-assignop-0.shader_test new file mode 100644 index 00000000..374a38bb --- /dev/null +++ b/tests/writemask-assignop-0.shader_test @@ -0,0 +1,15 @@ +[pixel shader] +uniform float4 color; + +float4 main() : SV_target +{ + float4 ret = color; + ret.zw += float2(0.5, 0.3); + ret.yx -= float2(0.1, 0.5); + return ret; +} + +[test] +uniform 0 float4 0.0303 0.08 0.07 0.0202 +draw quad +probe all rgba (-0.4697, -0.02, 0.57, 0.3202) 2 diff --git a/tests/writemask-assignop-1.shader_test b/tests/writemask-assignop-1.shader_test new file mode 100644 index 00000000..61993257 --- /dev/null +++ b/tests/writemask-assignop-1.shader_test @@ -0,0 +1,16 @@ +[pixel shader] +uniform float4 color; + +float4 main() : SV_target +{ + float4 ret = color; + ret.zwx.yzx.xz += float2(0.4, 0.2); + ret.xzyw.wzx.xz.y.x -= 0.6; + ret.yxwz.yxwz *= -1; + return ret; +} + +[test] +uniform 0 float4 0.0303 0.08 0.07 0.0202 +draw quad +probe all rgba (0.5697, -0.08, -0.27, -0.4202) diff --git a/tests/writemask-assignop-2.shader_test b/tests/writemask-assignop-2.shader_test new file mode 100644 index 00000000..4e905e67 --- /dev/null +++ b/tests/writemask-assignop-2.shader_test @@ -0,0 +1,13 @@ +[pixel shader] +float4 main() : SV_target +{ + float4 ret = float4(2, 2, 2, 2); + ret.yxz.yx += float2(0.1, 0.2); + ret.w.x -= 0.3; + ret.wzyx.zyx.yx.x *= 0.4; + return ret; +} + +[test] +draw quad +probe all rgba (2.1, 2.2, 0.8, 1.7) diff --git a/tests/writemask-assignop-3.shader_test b/tests/writemask-assignop-3.shader_test new file mode 100644 index 00000000..90957cb0 --- /dev/null +++ b/tests/writemask-assignop-3.shader_test @@ -0,0 +1,11 @@ +[pixel shader] +float4 main() : SV_target +{ + float4 ret = float4(2, 2, 2, 2); + ret.xyzw.xyzw -= float4(0.1, 0.2, 0.3, 0.4); + return ret; +} + +[test] +draw quad +probe all rgba (1.9, 1.8, 1.7, 1.6) -- 2.30.1
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
participants (4)
-
Henri Verbeet -
Matteo Bruni -
Matteo Bruni -
Zebediah Figura