[PATCH 1/5] d3dcompiler/tests: Add some more tests for return statements.
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dcompiler_43/tests/hlsl_d3d9.c | 68 ++++++++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c index 9bf3784c1da..062ef0a84a9 100644 --- a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c +++ b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c @@ -604,6 +604,56 @@ static void test_comma(void) release_test_context(&test_context); } +static void test_return(void) +{ + struct test_context test_context; + ID3D10Blob *ps_code = NULL; + struct vec4 v; + + static const char *void_source = + "void main(float x : TEXCOORD0, out float4 ret : COLOR)\n" + "{\n" + " ret = float4(0.1, 0.2, 0.3, 0.4);\n" + " return;\n" + " ret = float4(0.5, 0.6, 0.7, 0.8);\n" + "}"; + + static const char *implicit_conversion_source = + "float4 main(float x : TEXCOORD0) : COLOR\n" + "{\n" + " return float2x2(0.4, 0.3, 0.2, 0.1);\n" + "}"; + + if (!init_test_context(&test_context)) + return; + + todo_wine ps_code = compile_shader(void_source, "ps_2_0"); + if (ps_code) + { + draw_quad(test_context.device, ps_code); + + v = get_color_vec4(test_context.device, 0, 0); + ok(compare_vec4(&v, 0.1f, 0.2f, 0.3f, 0.4f, 0), + "Got unexpected value {%.8e, %.8e, %.8e, %.8e}.\n", v.x, v.y, v.z, v.w); + + ID3D10Blob_Release(ps_code); + } + + todo_wine ps_code = compile_shader(implicit_conversion_source, "ps_2_0"); + if (ps_code) + { + draw_quad(test_context.device, ps_code); + + v = get_color_vec4(test_context.device, 0, 0); + ok(compare_vec4(&v, 0.4f, 0.3f, 0.2f, 0.1f, 0), + "Got unexpected value {%.8e, %.8e, %.8e, %.8e}.\n", v.x, v.y, v.z, v.w); + + ID3D10Blob_Release(ps_code); + } + + release_test_context(&test_context); +} + static void test_fail(void) { static const char *tests[] = @@ -650,6 +700,21 @@ static void test_fail(void) " struct {} x = {};\n" " return y;\n" "}", + + "float4 test(float2 pos : TEXCOORD0) : SV_TARGET\n" + "{\n" + " return;\n" + "}", + + "void test(float2 pos : TEXCOORD0)\n" + "{\n" + " return pos;\n" + "}", + + "float4 test(float2 pos : TEXCOORD0) : SV_TARGET\n" + "{\n" + " return pos;\n" + "}", }; static const char *targets[] = {"ps_2_0", "ps_3_0", "ps_4_0"}; @@ -665,7 +730,7 @@ static void test_fail(void) compiled = errors = NULL; hr = ppD3DCompile(tests[i], strlen(tests[i]), NULL, NULL, NULL, "test", targets[j], 0, 0, &compiled, &errors); todo_wine ok(hr == E_FAIL, "Test %u, target %s, got unexpected hr %#x.\n", i, targets[j], hr); - todo_wine_if (i == 1) ok(!!errors, "Test %u, target %s, expected non-NULL error blob.\n", i, targets[j]); + todo_wine_if (i == 1 || i >= 8) ok(!!errors, "Test %u, target %s, expected non-NULL error blob.\n", i, targets[j]); ok(!compiled, "Test %u, target %s, expected no compiled shader blob.\n", i, targets[j]); if (errors) ID3D10Blob_Release(errors); @@ -710,5 +775,6 @@ START_TEST(hlsl_d3d9) test_float_vectors(); test_trig(); test_comma(); + test_return(); test_fail(); } -- 2.25.1
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dcompiler_43/hlsl.y | 55 +++++++++++++++++++-------- dlls/d3dcompiler_43/tests/hlsl_d3d9.c | 2 +- 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y index b6478bf3fed..0f9676df16e 100644 --- a/dlls/d3dcompiler_43/hlsl.y +++ b/dlls/d3dcompiler_43/hlsl.y @@ -481,6 +481,28 @@ static struct hlsl_ir_swizzle *get_swizzle(struct hlsl_ir_node *value, const cha return NULL; } +static struct hlsl_ir_jump *new_return(struct hlsl_ir_node *value, struct source_location loc) +{ + struct hlsl_ir_jump *jump = d3dcompiler_alloc(sizeof(*jump)); + if (!jump) + { + ERR("Out of memory\n"); + return NULL; + } + jump->node.type = HLSL_IR_JUMP; + jump->node.loc = loc; + jump->type = HLSL_IR_JUMP_RETURN; + jump->node.data_type = value ? value->data_type + : new_hlsl_type(d3dcompiler_strdup("void"), HLSL_CLASS_OBJECT, HLSL_TYPE_VOID, 1, 1); + jump->return_value = value; + + FIXME("Check for valued return on void function.\n"); + FIXME("Implicit conversion to the return type if needed, " + "error out if conversion not possible.\n"); + + return jump; +} + static void struct_var_initializer(struct list *list, struct hlsl_ir_var *var, struct parse_initializer *initializer) { @@ -1769,28 +1791,31 @@ statement: declaration_statement | selection_statement | loop_statement - /* FIXME: add rule for return with no value */ jump_statement: KW_RETURN expr ';' { - struct hlsl_ir_jump *jump = d3dcompiler_alloc(sizeof(*jump)); - if (!jump) - { - ERR("Out of memory\n"); - YYABORT; - } - jump->node.type = HLSL_IR_JUMP; - set_location(&jump->node.loc, &@1); - jump->type = HLSL_IR_JUMP_RETURN; - jump->node.data_type = node_from_list($2)->data_type; - jump->return_value = node_from_list($2); + struct source_location loc; + struct hlsl_ir_jump *jump; - FIXME("Check for valued return on void function.\n"); - FIXME("Implicit conversion to the return type if needed, " - "error out if conversion not possible.\n"); + set_location(&loc, &@1); + if (!(jump = new_return(node_from_list($2), loc))) + YYABORT; $$ = $2; list_add_tail($$, &jump->node.entry); } + | KW_RETURN ';' + { + struct source_location loc; + struct hlsl_ir_jump *jump; + + set_location(&loc, &@1); + if (!(jump = new_return(NULL, loc))) + YYABORT; + + $$ = d3dcompiler_alloc(sizeof(*$$)); + list_init($$); + list_add_tail($$, &jump->node.entry); + } selection_statement: KW_IF '(' expr ')' if_body { diff --git a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c index 062ef0a84a9..f1ce5dc5bca 100644 --- a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c +++ b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c @@ -730,7 +730,7 @@ static void test_fail(void) compiled = errors = NULL; hr = ppD3DCompile(tests[i], strlen(tests[i]), NULL, NULL, NULL, "test", targets[j], 0, 0, &compiled, &errors); todo_wine ok(hr == E_FAIL, "Test %u, target %s, got unexpected hr %#x.\n", i, targets[j], hr); - todo_wine_if (i == 1 || i >= 8) ok(!!errors, "Test %u, target %s, expected non-NULL error blob.\n", i, targets[j]); + todo_wine_if (i == 1 || i >= 7) ok(!!errors, "Test %u, target %s, expected non-NULL error blob.\n", i, targets[j]); ok(!compiled, "Test %u, target %s, expected no compiled shader blob.\n", i, targets[j]); if (errors) ID3D10Blob_Release(errors); -- 2.25.1
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
On Mon, Mar 2, 2020 at 5:56 AM Zebediah Figura <z.figura12(a)gmail.com> wrote:
diff --git a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c index 062ef0a84a9..f1ce5dc5bca 100644 --- a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c +++ b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c @@ -730,7 +730,7 @@ static void test_fail(void) compiled = errors = NULL; hr = ppD3DCompile(tests[i], strlen(tests[i]), NULL, NULL, NULL, "test", targets[j], 0, 0, &compiled, &errors); todo_wine ok(hr == E_FAIL, "Test %u, target %s, got unexpected hr %#x.\n", i, targets[j], hr); - todo_wine_if (i == 1 || i >= 8) ok(!!errors, "Test %u, target %s, expected non-NULL error blob.\n", i, targets[j]); + todo_wine_if (i == 1 || i >= 7) ok(!!errors, "Test %u, target %s, expected non-NULL error blob.\n", i, targets[j]);
Idea for another patch: maybe add a comment here in tests[] every 5 shaders or so to make it easier figuring out which test is which.
On 3/3/20 7:10 AM, Matteo Bruni wrote:
On Mon, Mar 2, 2020 at 5:56 AM Zebediah Figura <z.figura12(a)gmail.com> wrote:
diff --git a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c index 062ef0a84a9..f1ce5dc5bca 100644 --- a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c +++ b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c @@ -730,7 +730,7 @@ static void test_fail(void) compiled = errors = NULL; hr = ppD3DCompile(tests[i], strlen(tests[i]), NULL, NULL, NULL, "test", targets[j], 0, 0, &compiled, &errors); todo_wine ok(hr == E_FAIL, "Test %u, target %s, got unexpected hr %#x.\n", i, targets[j], hr); - todo_wine_if (i == 1 || i >= 8) ok(!!errors, "Test %u, target %s, expected non-NULL error blob.\n", i, targets[j]); + todo_wine_if (i == 1 || i >= 7) ok(!!errors, "Test %u, target %s, expected non-NULL error blob.\n", i, targets[j]);
Idea for another patch: maybe add a comment here in tests[] every 5 shaders or so to make it easier figuring out which test is which.
Good idea, thanks.
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dcompiler_43/utils.c | 60 +++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/dlls/d3dcompiler_43/utils.c b/dlls/d3dcompiler_43/utils.c index 5e497495e60..65f5ead333d 100644 --- a/dlls/d3dcompiler_43/utils.c +++ b/dlls/d3dcompiler_43/utils.c @@ -1275,16 +1275,31 @@ static struct hlsl_type *expr_common_type(struct hlsl_type *t1, struct hlsl_type return new_hlsl_type(NULL, type, base, dimx, dimy); } -static struct hlsl_ir_node *implicit_conversion(struct hlsl_ir_node *node, struct hlsl_type *type, +static struct hlsl_ir_node *implicit_conversion(struct hlsl_ir_node *node, struct hlsl_type *dst_type, struct source_location *loc) { + struct hlsl_type *src_type = node->data_type; struct hlsl_ir_expr *cast; - if (compare_hlsl_types(node->data_type, type)) + if (compare_hlsl_types(src_type, dst_type)) return node; - TRACE("Implicit conversion of expression to %s\n", debug_hlsl_type(type)); - if ((cast = new_cast(node, type, loc))) - list_add_after(&node->entry, &cast->node.entry); + + if (!implicit_compatible_data_types(src_type, dst_type)) + { + hlsl_report_message(loc->file, loc->line, loc->col, HLSL_LEVEL_ERROR, + "can't implicitly convert %s to %s", debug_hlsl_type(src_type), debug_hlsl_type(dst_type)); + return NULL; + } + + if (dst_type->dimx * dst_type->dimy < src_type->dimx * src_type->dimy) + hlsl_report_message(loc->file, loc->line, loc->col, HLSL_LEVEL_WARNING, + "implicit truncation of vector type"); + + TRACE("Implicit conversion from %s to %s.\n", debug_hlsl_type(src_type), debug_hlsl_type(dst_type)); + + if (!(cast = new_cast(node, dst_type, loc))) + return NULL; + list_add_after(&node->entry, &cast->node.entry); return &cast->node; } @@ -1316,6 +1331,8 @@ struct hlsl_ir_expr *new_expr(enum hlsl_ir_expr_op op, struct hlsl_ir_node **ope } for (i = 0; i <= 2; ++i) { + struct hlsl_ir_expr *cast; + if (!operands[i]) break; if (compare_hlsl_types(operands[i]->data_type, type)) @@ -1328,13 +1345,14 @@ struct hlsl_ir_expr *new_expr(enum hlsl_ir_expr_op op, struct hlsl_ir_node **ope operands[i]->loc.line, operands[i]->loc.col, HLSL_LEVEL_WARNING, "implicit truncation of vector/matrix type"); } - operands[i] = implicit_conversion(operands[i], type, &operands[i]->loc); - if (!operands[i]) + + if (!(cast = new_cast(operands[i], type, &operands[i]->loc))) { - ERR("Impossible to convert expression operand %u to %s\n", i + 1, debug_hlsl_type(type)); d3dcompiler_free(expr); return NULL; } + list_add_after(&operands[i]->entry, &cast->node.entry); + operands[i] = &cast->node; } expr->node.data_type = type; expr->op = op; @@ -1463,31 +1481,7 @@ struct hlsl_ir_node *make_assignment(struct hlsl_ir_node *left, enum parse_assig lhs = left; /* FIXME: check for invalid writemasks on the lhs. */ - if (!compare_hlsl_types(type, rhs->data_type)) - { - struct hlsl_ir_node *converted_rhs; - - if (!implicit_compatible_data_types(rhs->data_type, type)) - { - hlsl_report_message(rhs->loc.file, rhs->loc.line, rhs->loc.col, HLSL_LEVEL_ERROR, - "can't implicitly convert %s to %s", - debug_hlsl_type(rhs->data_type), debug_hlsl_type(type)); - d3dcompiler_free(assign); - return NULL; - } - if (lhs->data_type->dimx * lhs->data_type->dimy < rhs->data_type->dimx * rhs->data_type->dimy) - hlsl_report_message(rhs->loc.file, rhs->loc.line, rhs->loc.col, HLSL_LEVEL_WARNING, - "implicit truncation of vector type"); - - converted_rhs = implicit_conversion(rhs, type, &rhs->loc); - if (!converted_rhs) - { - ERR("Couldn't implicitly convert expression to %s.\n", debug_hlsl_type(type)); - d3dcompiler_free(assign); - return NULL; - } - rhs = converted_rhs; - } + rhs = implicit_conversion(rhs, type, &rhs->loc); assign->lhs = lhs; if (assign_op != ASSIGN_OP_ASSIGN) -- 2.25.1
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
On Mon, Mar 2, 2020 at 5:56 AM Zebediah Figura <z.figura12(a)gmail.com> wrote:
@@ -1328,13 +1345,14 @@ struct hlsl_ir_expr *new_expr(enum hlsl_ir_expr_op op, struct hlsl_ir_node **ope operands[i]->loc.line, operands[i]->loc.col, HLSL_LEVEL_WARNING, "implicit truncation of vector/matrix type"); } - operands[i] = implicit_conversion(operands[i], type, &operands[i]->loc); - if (!operands[i]) + + if (!(cast = new_cast(operands[i], type, &operands[i]->loc))) { - ERR("Impossible to convert expression operand %u to %s\n", i + 1, debug_hlsl_type(type)); d3dcompiler_free(expr); return NULL; }
Again, not something that changed with this patch, it just made me think. This implicit conversion is unchecked: is it correct to do so?
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dcompiler_43/d3dcompiler_private.h | 3 +++ dlls/d3dcompiler_43/hlsl.y | 24 ++++++++++++++++------- dlls/d3dcompiler_43/tests/hlsl_d3d9.c | 2 +- dlls/d3dcompiler_43/utils.c | 2 +- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/dlls/d3dcompiler_43/d3dcompiler_private.h b/dlls/d3dcompiler_43/d3dcompiler_private.h index 22cc41db56a..ab712c8d963 100644 --- a/dlls/d3dcompiler_43/d3dcompiler_private.h +++ b/dlls/d3dcompiler_43/d3dcompiler_private.h @@ -1046,6 +1046,7 @@ struct hlsl_parse_ctx struct list types; struct wine_rb_tree functions; + const struct hlsl_ir_function_decl *cur_function; enum hlsl_matrix_majority matrix_majority; }; @@ -1138,6 +1139,8 @@ struct hlsl_ir_expr *new_expr(enum hlsl_ir_expr_op op, struct hlsl_ir_node **ope struct source_location *loc) DECLSPEC_HIDDEN; struct hlsl_ir_expr *new_cast(struct hlsl_ir_node *node, struct hlsl_type *type, struct source_location *loc) DECLSPEC_HIDDEN; +struct hlsl_ir_node *implicit_conversion(struct hlsl_ir_node *node, struct hlsl_type *type, + struct source_location *loc) DECLSPEC_HIDDEN; struct hlsl_ir_deref *new_var_deref(struct hlsl_ir_var *var) DECLSPEC_HIDDEN; struct hlsl_ir_deref *new_record_deref(struct hlsl_ir_node *record, struct hlsl_struct_field *field) DECLSPEC_HIDDEN; struct hlsl_ir_node *make_assignment(struct hlsl_ir_node *left, enum parse_assign_op assign_op, diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y index 0f9676df16e..34adab2855b 100644 --- a/dlls/d3dcompiler_43/hlsl.y +++ b/dlls/d3dcompiler_43/hlsl.y @@ -492,13 +492,22 @@ static struct hlsl_ir_jump *new_return(struct hlsl_ir_node *value, struct source jump->node.type = HLSL_IR_JUMP; jump->node.loc = loc; jump->type = HLSL_IR_JUMP_RETURN; - jump->node.data_type = value ? value->data_type - : new_hlsl_type(d3dcompiler_strdup("void"), HLSL_CLASS_OBJECT, HLSL_TYPE_VOID, 1, 1); - jump->return_value = value; - - FIXME("Check for valued return on void function.\n"); - FIXME("Implicit conversion to the return type if needed, " - "error out if conversion not possible.\n"); + jump->node.data_type = hlsl_ctx.cur_function->return_type; + if (value) + { + if (!(jump->return_value = implicit_conversion(value, jump->node.data_type, &loc))) + { + d3dcompiler_free(jump); + return NULL; + } + } + else if (jump->node.data_type->base_type != HLSL_TYPE_VOID) + { + hlsl_report_message(loc.file, loc.line, loc.col, HLSL_LEVEL_ERROR, + "non-void function must return a value"); + d3dcompiler_free(jump); + return NULL; + } return jump; } @@ -1336,6 +1345,7 @@ func_prototype: var_modifiers type var_identifier '(' parameters ')' c $$.name = $3; $$.decl->semantic = $7.semantic; set_location(&$$.decl->loc, &@3); + hlsl_ctx.cur_function = $$.decl; } compound_statement: '{' '}' diff --git a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c index f1ce5dc5bca..c673e2be327 100644 --- a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c +++ b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c @@ -730,7 +730,7 @@ static void test_fail(void) compiled = errors = NULL; hr = ppD3DCompile(tests[i], strlen(tests[i]), NULL, NULL, NULL, "test", targets[j], 0, 0, &compiled, &errors); todo_wine ok(hr == E_FAIL, "Test %u, target %s, got unexpected hr %#x.\n", i, targets[j], hr); - todo_wine_if (i == 1 || i >= 7) ok(!!errors, "Test %u, target %s, expected non-NULL error blob.\n", i, targets[j]); + todo_wine_if (i == 1) ok(!!errors, "Test %u, target %s, expected non-NULL error blob.\n", i, targets[j]); ok(!compiled, "Test %u, target %s, expected no compiled shader blob.\n", i, targets[j]); if (errors) ID3D10Blob_Release(errors); diff --git a/dlls/d3dcompiler_43/utils.c b/dlls/d3dcompiler_43/utils.c index 65f5ead333d..28ec47423ab 100644 --- a/dlls/d3dcompiler_43/utils.c +++ b/dlls/d3dcompiler_43/utils.c @@ -1275,7 +1275,7 @@ static struct hlsl_type *expr_common_type(struct hlsl_type *t1, struct hlsl_type return new_hlsl_type(NULL, type, base, dimx, dimy); } -static struct hlsl_ir_node *implicit_conversion(struct hlsl_ir_node *node, struct hlsl_type *dst_type, +struct hlsl_ir_node *implicit_conversion(struct hlsl_ir_node *node, struct hlsl_type *dst_type, struct source_location *loc) { struct hlsl_type *src_type = node->data_type; -- 2.25.1
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
Essentially just because it doesn't make sense to do so; a return instruction is not an expression usable as a source to other instructions. Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- I'm not particularly attached to this patch. dlls/d3dcompiler_43/hlsl.y | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y index 34adab2855b..3d04677f2ec 100644 --- a/dlls/d3dcompiler_43/hlsl.y +++ b/dlls/d3dcompiler_43/hlsl.y @@ -483,6 +483,7 @@ static struct hlsl_ir_swizzle *get_swizzle(struct hlsl_ir_node *value, const cha static struct hlsl_ir_jump *new_return(struct hlsl_ir_node *value, struct source_location loc) { + struct hlsl_type *return_type = hlsl_ctx.cur_function->return_type; struct hlsl_ir_jump *jump = d3dcompiler_alloc(sizeof(*jump)); if (!jump) { @@ -492,16 +493,15 @@ static struct hlsl_ir_jump *new_return(struct hlsl_ir_node *value, struct source jump->node.type = HLSL_IR_JUMP; jump->node.loc = loc; jump->type = HLSL_IR_JUMP_RETURN; - jump->node.data_type = hlsl_ctx.cur_function->return_type; if (value) { - if (!(jump->return_value = implicit_conversion(value, jump->node.data_type, &loc))) + if (!(jump->return_value = implicit_conversion(value, return_type, &loc))) { d3dcompiler_free(jump); return NULL; } } - else if (jump->node.data_type->base_type != HLSL_TYPE_VOID) + else if (return_type->base_type != HLSL_TYPE_VOID) { hlsl_report_message(loc.file, loc.line, loc.col, HLSL_LEVEL_ERROR, "non-void function must return a value"); -- 2.25.1
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
On Mon, Mar 2, 2020 at 5:56 AM Zebediah Figura <z.figura12(a)gmail.com> wrote:
Essentially just because it doesn't make sense to do so; a return instruction is not an expression usable as a source to other instructions.
Seems good. It might be that at the time I wrote this it happened to come in handy when inlining function calls but, most likely, it wasn't a intentional design decision.
participants (4)
-
Matteo Bruni -
Matteo Bruni -
Zebediah Figura -
Zebediah Figura