Signed-off-by: Nikolay Sivov nsivov@codeweavers.com
-- v13: vkd3d-shader/tpf: Write out 'switch' statements.
From: Nikolay Sivov nsivov@codeweavers.com
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com --- Makefile.am | 1 + tests/hlsl/switch.shader_test | 335 ++++++++++++++++++++++++++++++++++ 2 files changed, 336 insertions(+) create mode 100644 tests/hlsl/switch.shader_test
diff --git a/Makefile.am b/Makefile.am index 8364aaa3..1afa4bd9 100644 --- a/Makefile.am +++ b/Makefile.am @@ -165,6 +165,7 @@ vkd3d_shader_tests = \ tests/hlsl/struct-array.shader_test \ tests/hlsl/struct-assignment.shader_test \ tests/hlsl/struct-semantics.shader_test \ + tests/hlsl/switch.shader_test \ tests/hlsl/swizzle-constant-prop.shader_test \ tests/hlsl/swizzle-matrix.shader_test \ tests/hlsl/swizzles.shader_test \ diff --git a/tests/hlsl/switch.shader_test b/tests/hlsl/switch.shader_test new file mode 100644 index 00000000..02e499c9 --- /dev/null +++ b/tests/hlsl/switch.shader_test @@ -0,0 +1,335 @@ +[require] +shader model >= 4.0 + +[pixel shader todo] +uint4 v; + +float4 main() : sv_target +{ + switch (v.x) + { + case 0: + return 3.0; + case 1: + return 4.0; + default: + return 5.0; + } +} + +[test] +uniform 0 uint4 3 0 0 0 +todo draw quad +todo probe all rgba (5.0, 5.0, 5.0, 5.0) +uniform 0 uint4 1 0 0 0 +todo draw quad +todo probe all rgba (4.0, 4.0, 4.0, 4.0) +uniform 0 uint4 0 0 0 0 +todo draw quad +todo probe all rgba (3.0, 3.0, 3.0, 3.0) + +% falling through is only supported for empty case statements +[pixel shader todo] +uint4 v; + +float4 main() : sv_target +{ + float4 c = {1.0, 2.0, 3.0, 4.0}; + + switch (v.x) + { + case 0: + case 1: + c.x += 0.1f; + break; + } + + return c; +} + +[test] +uniform 0 uint4 2 0 0 0 +todo draw quad +todo probe all rgba (1.0, 2.0, 3.0, 4.0) +uniform 0 uint4 1 0 0 0 +todo draw quad +todo probe all rgba (1.1, 2.0, 3.0, 4.0) +uniform 0 uint4 0 0 0 0 +todo draw quad +todo probe all rgba (1.1, 2.0, 3.0, 4.0) + +% case value evaluation +[pixel shader todo] +uint4 v; + +float4 main() : sv_target +{ + float4 c = {1.0, 2.0, 3.0, 4.0}; + + switch (v.x) + { + case 1+1: + c += 0.1f; + break; + case 0: + c += 0.2f; + break; + } + + return c; +} + +[test] +uniform 0 uint4 2 0 0 0 +todo draw quad +todo probe all rgba (1.1, 2.1, 3.1, 4.1) +uniform 0 uint4 1 0 0 0 +todo draw quad +todo probe all rgba (1.0, 2.0, 3.0, 4.0) + +% floats are accepted +[pixel shader todo] +uint4 v; + +float4 main() : sv_target +{ + float4 c = {1.0, 2.0, 3.0, 4.0}; + + switch (v.x) + { + case 2.1f: + c += 0.1f; + break; + case 0.9f: + c += 0.2f; + break; + } + + return c; +} + +[test] +uniform 0 uint4 2 0 0 0 +todo draw quad +todo probe all rgba (1.1, 2.1, 3.1, 4.1) +uniform 0 uint4 1 0 0 0 +todo draw quad +todo probe all rgba (1.0, 2.0, 3.0, 4.0) + +[pixel shader fail] +uint4 v; + +float4 main() : sv_target +{ + float4 c = {1.0, 2.0, 3.0, 4.0}; + uint a = 1; + + switch (v.x) + { + case 1+a: + c += 0.1f; + break; + case 0: + c += 0.2f; + break; + } + + return c; +} + +% duplicate cases +[pixel shader fail] +uint4 v; + +float4 main() : sv_target +{ + float4 c = {1.0, 2.0, 3.0, 4.0}; + + switch (v.x) + { + case 2: + c += 0.1f; + break; + case 1+1: + c += 0.2f; + break; + } + + return c; +} + +[pixel shader fail] +uint4 v; + +% multiple default cases +float4 main() : sv_target +{ + float4 c = {1.0, 2.0, 3.0, 4.0}; + + switch (v.x) + { + default: + case 2: + c += 0.1f; + break; + case 1: + c += 0.2f; + break; + default: + break; + } + + return c; +} + +% unterminated cases +[pixel shader fail] +uint4 v; + +float4 main() : sv_target +{ + float4 c = {1.0, 2.0, 3.0, 4.0}; + + switch (v.x) + { + case 0: + c += 0.1f; + case 1: + c += 0.2f; + break; + } + + return c; +} + +% more complicated breaks +[pixel shader todo] +uint4 v; + +float4 main() : sv_target +{ + float4 c = {1.0, 2.0, 3.0, 4.0}; + + switch (v.x) + { + case 2: + c += 0.1f; + if (true) break; + c = 9.0f; + case 1: + if (false) break; + c += 0.2f; + break; + default: + case 0: + break; + } + + return c; +} + +[test] +uniform 0 uint4 2 0 0 0 +todo draw quad +todo probe all rgba (1.1, 2.1, 3.1, 4.1) +uniform 0 uint4 1 0 0 0 +todo draw quad +todo probe all rgba (1.2, 2.2, 3.2, 4.2) +uniform 0 uint4 0 0 0 0 +todo draw quad +todo probe all rgba (1.0, 2.0, 3.0, 4.0) + +% switch breaks within a loop +[pixel shader todo] +uint4 v; + +float4 main() : sv_target +{ + float4 c = {1.0f, 2.0f, 3.0f, 4.0f}; + + for (int i = 0; i < 4; ++i) + { + switch (v.x) + { + case 2: + c += 1.0f; + break; + case 1: + c -= 1.0f; + break; + default: + case 0: + break; + } + } + + return c; +} + +[test] +uniform 0 uint4 2 0 0 0 +todo draw quad +todo probe all rgba (5.0, 6.0, 7.0, 8.0) + +% 'continue' is not supported in switches +[pixel shader fail] +uint4 v; + +float4 main() : sv_target +{ + float4 c = {1.0, 2.0, 3.0, 4.0}; + uint i, j; + + for (i = 0; i < v.z; i++) + { + switch (v.x) + { + case 0: + c += 0.1f; + continue; + break; + case 1: + c += 0.2f; + break; + } + } + + return c; +} + +[pixel shader todo] +uint4 v; + +float4 main() : sv_target +{ + float4 c = {1.0, 2.0, 3.0, 4.0}; + uint i, j; + + for (i = 0; i < v.z; i++) + { + switch (v.x) + { + case 0: + for (j = 0; j < v.z; j++) + { + c += 1.0f; + if (v.w) + continue; + } + break; + case 1: + c += 2.0f; + break; + } + } + + return c; +} + +[test] +uniform 0 uint4 0 0 3 1 +todo draw quad +todo probe all rgba (10.0, 11.0, 12.0, 13.0) +uniform 0 uint4 1 0 3 1 +todo draw quad +todo probe all rgba (7.0, 8.0, 9.0, 10.0)
From: Nikolay Sivov nsivov@codeweavers.com
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com --- libs/vkd3d-shader/hlsl.c | 131 +++++++++++++++++++++++++++++++ libs/vkd3d-shader/hlsl.h | 29 +++++++ libs/vkd3d-shader/hlsl.l | 4 +- libs/vkd3d-shader/hlsl.y | 85 +++++++++++++++++++- libs/vkd3d-shader/hlsl_codegen.c | 79 +++++++++++++++++++ tests/hlsl/switch.shader_test | 2 +- 6 files changed, 325 insertions(+), 5 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index b42e3088..2283f962 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1343,6 +1343,47 @@ struct hlsl_ir_node *hlsl_new_if(struct hlsl_ctx *ctx, struct hlsl_ir_node *cond return &iff->node; }
+struct hlsl_ir_switch_case *hlsl_new_switch_case(struct hlsl_ctx *ctx, struct hlsl_block *value, + struct hlsl_block *body, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_switch_case *c; + + if (!(c = hlsl_alloc(ctx, sizeof(*c)))) + return NULL; + + if (value) + { + c->value = hlsl_evaluate_static_expression_as_uint(ctx, value, loc); + } + else + { + c->is_default = true; + } + + hlsl_block_init(&c->body); + if (body) + hlsl_block_add_block(&c->body, body); + c->loc = *loc; + + return c; +} + +struct hlsl_ir_node *hlsl_new_switch(struct hlsl_ctx *ctx, struct hlsl_ir_node *selector, + struct list *cases, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_switch *s; + + if (!(s = hlsl_alloc(ctx, sizeof(*s)))) + return NULL; + init_node(&s->node, HLSL_IR_SWITCH, NULL, loc); + hlsl_src_from_node(&s->selector, selector); + list_init(&s->cases); + if (cases) + list_move_head(&s->cases, cases); + + return &s->node; +} + struct hlsl_ir_load *hlsl_new_load_index(struct hlsl_ctx *ctx, const struct hlsl_deref *deref, struct hlsl_ir_node *idx, const struct vkd3d_shader_location *loc) { @@ -1805,6 +1846,50 @@ static struct hlsl_ir_node *clone_index(struct hlsl_ctx *ctx, struct clone_instr return dst; }
+static void free_ir_switch_cases(struct list *cases) +{ + struct hlsl_ir_switch_case *c, *next; + + LIST_FOR_EACH_ENTRY_SAFE(c, next, cases, struct hlsl_ir_switch_case, entry) + { + hlsl_block_cleanup(&c->body); + list_remove(&c->entry); + vkd3d_free(c); + } +} + +static struct hlsl_ir_node *clone_switch(struct hlsl_ctx *ctx, + struct clone_instr_map *map, struct hlsl_ir_switch *s) +{ + struct hlsl_ir_switch_case *c, *d; + struct hlsl_ir_node *ret; + struct hlsl_block body; + struct list cases; + + list_init(&cases); + + LIST_FOR_EACH_ENTRY(c, &s->cases, struct hlsl_ir_switch_case, entry) + { + clone_block(ctx, &body, &c->body, map); + + if (!(d = hlsl_new_switch_case(ctx, NULL, &body, &c->loc))) + { + free_ir_switch_cases(&cases); + return NULL; + } + + d->value = c->value; + d->is_default = c->is_default; + list_add_tail(&cases, &d->entry); + } + + ret = hlsl_new_switch(ctx, map_instr(map, s->selector.node), &cases, &s->node.loc); + if (!ret) + free_ir_switch_cases(&cases); + + return ret; +} + static struct hlsl_ir_node *clone_instr(struct hlsl_ctx *ctx, struct clone_instr_map *map, const struct hlsl_ir_node *instr) { @@ -1843,6 +1928,9 @@ static struct hlsl_ir_node *clone_instr(struct hlsl_ctx *ctx, case HLSL_IR_STORE: return clone_store(ctx, map, hlsl_ir_store(instr));
+ case HLSL_IR_SWITCH: + return clone_switch(ctx, map, hlsl_ir_switch(instr)); + case HLSL_IR_SWIZZLE: return clone_swizzle(ctx, map, hlsl_ir_swizzle(instr)); } @@ -2261,6 +2349,7 @@ const char *hlsl_node_type_to_string(enum hlsl_ir_node_type type) [HLSL_IR_RESOURCE_LOAD ] = "HLSL_IR_RESOURCE_LOAD", [HLSL_IR_RESOURCE_STORE] = "HLSL_IR_RESOURCE_STORE", [HLSL_IR_STORE ] = "HLSL_IR_STORE", + [HLSL_IR_SWITCH ] = "HLSL_IR_SWITCH", [HLSL_IR_SWIZZLE ] = "HLSL_IR_SWIZZLE", };
@@ -2685,6 +2774,32 @@ static void dump_ir_index(struct vkd3d_string_buffer *buffer, const struct hlsl_ vkd3d_string_buffer_printf(buffer, "]"); }
+static void dump_ir_switch(struct hlsl_ctx *ctx, struct vkd3d_string_buffer *buffer, const struct hlsl_ir_switch *s) +{ + struct hlsl_ir_switch_case *c; + + vkd3d_string_buffer_printf(buffer, "switch ("); + dump_src(buffer, &s->selector); + vkd3d_string_buffer_printf(buffer, ") {\n"); + + LIST_FOR_EACH_ENTRY(c, &s->cases, struct hlsl_ir_switch_case, entry) + { + if (c->is_default) + { + vkd3d_string_buffer_printf(buffer, " %10s default: {\n", ""); + } + else + { + vkd3d_string_buffer_printf(buffer, " %10s case %u : {\n", "", c->value); + } + + dump_block(ctx, buffer, &c->body); + vkd3d_string_buffer_printf(buffer, " %10s }\n", ""); + } + + vkd3d_string_buffer_printf(buffer, " %10s }", ""); +} + static void dump_instr(struct hlsl_ctx *ctx, struct vkd3d_string_buffer *buffer, const struct hlsl_ir_node *instr) { if (instr->index) @@ -2740,6 +2855,10 @@ static void dump_instr(struct hlsl_ctx *ctx, struct vkd3d_string_buffer *buffer, dump_ir_store(buffer, hlsl_ir_store(instr)); break;
+ case HLSL_IR_SWITCH: + dump_ir_switch(ctx, buffer, hlsl_ir_switch(instr)); + break; + case HLSL_IR_SWIZZLE: dump_ir_swizzle(buffer, hlsl_ir_swizzle(instr)); break; @@ -2900,6 +3019,14 @@ static void free_ir_swizzle(struct hlsl_ir_swizzle *swizzle) vkd3d_free(swizzle); }
+static void free_ir_switch(struct hlsl_ir_switch *s) +{ + hlsl_src_remove(&s->selector); + free_ir_switch_cases(&s->cases); + + vkd3d_free(s); +} + static void free_ir_index(struct hlsl_ir_index *index) { hlsl_src_remove(&index->val); @@ -2960,6 +3087,10 @@ void hlsl_free_instr(struct hlsl_ir_node *node) case HLSL_IR_SWIZZLE: free_ir_swizzle(hlsl_ir_swizzle(node)); break; + + case HLSL_IR_SWITCH: + free_ir_switch(hlsl_ir_switch(node)); + break; } }
diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 59c543c7..e1364176 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -281,6 +281,7 @@ enum hlsl_ir_node_type HLSL_IR_RESOURCE_STORE, HLSL_IR_STORE, HLSL_IR_SWIZZLE, + HLSL_IR_SWITCH, };
/* Common data for every type of IR instruction node. */ @@ -499,6 +500,22 @@ struct hlsl_ir_loop unsigned int next_index; /* liveness index of the end of the loop */ };
+struct hlsl_ir_switch_case +{ + unsigned int value; + bool is_default; + struct hlsl_block body; + struct list entry; + struct vkd3d_shader_location loc; +}; + +struct hlsl_ir_switch +{ + struct hlsl_ir_node node; + struct hlsl_src selector; + struct list cases; +}; + enum hlsl_ir_expr_op { HLSL_OP0_VOID, @@ -952,6 +969,12 @@ static inline struct hlsl_ir_index *hlsl_ir_index(const struct hlsl_ir_node *nod return CONTAINING_RECORD(node, struct hlsl_ir_index, node); }
+static inline struct hlsl_ir_switch *hlsl_ir_switch(const struct hlsl_ir_node *node) +{ + assert(node->type == HLSL_IR_SWITCH); + return CONTAINING_RECORD(node, struct hlsl_ir_switch, node); +} + static inline void hlsl_block_init(struct hlsl_block *block) { list_init(&block->instrs); @@ -1218,6 +1241,10 @@ struct hlsl_ir_node *hlsl_new_unary_expr(struct hlsl_ctx *ctx, enum hlsl_ir_expr struct hlsl_ir_var *hlsl_new_var(struct hlsl_ctx *ctx, const char *name, struct hlsl_type *type, const struct vkd3d_shader_location *loc, const struct hlsl_semantic *semantic, unsigned int modifiers, const struct hlsl_reg_reservation *reg_reservation); +struct hlsl_ir_switch_case *hlsl_new_switch_case(struct hlsl_ctx *ctx, struct hlsl_block *value, + struct hlsl_block *body, const struct vkd3d_shader_location *loc); +struct hlsl_ir_node *hlsl_new_switch(struct hlsl_ctx *ctx, struct hlsl_ir_node *selector, + struct list *cases, const struct vkd3d_shader_location *loc);
void hlsl_error(struct hlsl_ctx *ctx, const struct vkd3d_shader_location *loc, enum vkd3d_shader_error error, const char *fmt, ...) VKD3D_PRINTF_FUNC(4, 5); @@ -1272,6 +1299,8 @@ bool hlsl_fold_constant_exprs(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, bool hlsl_fold_constant_swizzles(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context); bool hlsl_transform_ir(struct hlsl_ctx *ctx, bool (*func)(struct hlsl_ctx *ctx, struct hlsl_ir_node *, void *), struct hlsl_block *block, void *context); +unsigned int hlsl_evaluate_static_expression_as_uint(struct hlsl_ctx *ctx, struct hlsl_block *block, + const struct vkd3d_shader_location *loc);
bool hlsl_sm1_register_from_semantic(struct hlsl_ctx *ctx, const struct hlsl_semantic *semantic, bool output, D3DSHADER_PARAM_REGISTER_TYPE *type, unsigned int *reg); diff --git a/libs/vkd3d-shader/hlsl.l b/libs/vkd3d-shader/hlsl.l index 90abd64a..d457c662 100644 --- a/libs/vkd3d-shader/hlsl.l +++ b/libs/vkd3d-shader/hlsl.l @@ -46,7 +46,7 @@ static void update_location(struct hlsl_ctx *ctx, YYLTYPE *loc);
%x pp pp_line pp_pragma pp_ignore
-RESERVED1 auto|case|catch|char|class|const_cast|default|delete|dynamic_cast|enum +RESERVED1 auto|catch|char|class|const_cast|delete|dynamic_cast|enum RESERVED2 explicit|friend|goto|long|mutable|new|operator|private|protected|public RESERVED3 reinterpret_cast|short|signed|sizeof|static_cast|template|this|throw|try RESERVED4 typename|union|unsigned|using|virtual @@ -73,6 +73,7 @@ ANY (.) BlendState {return KW_BLENDSTATE; } break {return KW_BREAK; } Buffer {return KW_BUFFER; } +case {return KW_CASE; } cbuffer {return KW_CBUFFER; } centroid {return KW_CENTROID; } compile {return KW_COMPILE; } @@ -80,6 +81,7 @@ const {return KW_CONST; } continue {return KW_CONTINUE; } DepthStencilState {return KW_DEPTHSTENCILSTATE; } DepthStencilView {return KW_DEPTHSTENCILVIEW; } +default {return KW_DEFAULT; } discard {return KW_DISCARD; } do {return KW_DO; } double {return KW_DOUBLE; } diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index ba738473..fa1421e4 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1154,7 +1154,7 @@ static struct hlsl_block *make_block(struct hlsl_ctx *ctx, struct hlsl_ir_node * return block; }
-static unsigned int evaluate_static_expression_as_uint(struct hlsl_ctx *ctx, struct hlsl_block *block, +unsigned int hlsl_evaluate_static_expression_as_uint(struct hlsl_ctx *ctx, struct hlsl_block *block, const struct vkd3d_shader_location *loc) { struct hlsl_ir_constant *constant; @@ -1180,6 +1180,7 @@ static unsigned int evaluate_static_expression_as_uint(struct hlsl_ctx *ctx, str case HLSL_IR_RESOURCE_LOAD: case HLSL_IR_RESOURCE_STORE: case HLSL_IR_STORE: + case HLSL_IR_SWITCH: hlsl_error(ctx, &node->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX, "Expected literal expression."); } @@ -4684,12 +4685,14 @@ static struct hlsl_scope *get_loop_scope(struct hlsl_scope *scope) %token KW_BLENDSTATE %token KW_BREAK %token KW_BUFFER +%token KW_CASE %token KW_CBUFFER %token KW_CENTROID %token KW_COLUMN_MAJOR %token KW_COMPILE %token KW_CONST %token KW_CONTINUE +%token KW_DEFAULT %token KW_DEPTHSTENCILSTATE %token KW_DEPTHSTENCILVIEW %token KW_DISCARD @@ -4794,6 +4797,8 @@ static struct hlsl_scope *get_loop_scope(struct hlsl_scope *scope) %type <list> type_specs %type <list> variables_def %type <list> variables_def_typed +%type <list> switch_case +%type <list> switch_cases
%token <name> VAR_IDENTIFIER %token <name> NEW_IDENTIFIER @@ -4836,6 +4841,7 @@ static struct hlsl_scope *get_loop_scope(struct hlsl_scope *scope) %type <block> statement %type <block> statement_list %type <block> struct_declaration_without_vars +%type <block> switch_statement %type <block> unary_expr
%type <boolval> boolean @@ -5355,6 +5361,11 @@ loop_scope_start: ctx->cur_scope->loop = true; }
+switch_scope_start: + %empty + { + } + var_identifier: VAR_IDENTIFIER | NEW_IDENTIFIER @@ -5674,7 +5685,7 @@ type_no_void: hlsl_block_init(&block); hlsl_block_add_block(&block, $5);
- sample_count = evaluate_static_expression_as_uint(ctx, &block, &@5); + sample_count = hlsl_evaluate_static_expression_as_uint(ctx, &block, &@5);
hlsl_block_cleanup(&block);
@@ -5948,7 +5959,7 @@ arrays: uint32_t *new_array; unsigned int size;
- size = evaluate_static_expression_as_uint(ctx, $2, &@2); + size = hlsl_evaluate_static_expression_as_uint(ctx, $2, &@2);
destroy_block($2);
@@ -6173,6 +6184,7 @@ statement: | jump_statement | selection_statement | loop_statement + | switch_statement
jump_statement: KW_BREAK ';' @@ -6321,6 +6333,73 @@ loop_statement: hlsl_pop_scope(ctx); }
+switch_statement: + attribute_list_optional switch_scope_start KW_SWITCH '(' expr ')' '{' switch_cases '}' + { + struct hlsl_ir_node *selector = node_from_block($5); + struct hlsl_ir_node *s; + + if (!(selector = add_implicit_conversion(ctx, $5, selector, hlsl_get_scalar_type(ctx, HLSL_TYPE_UINT), &@5))) + YYABORT; + + if (!(s = hlsl_new_switch(ctx, selector, $8, &@3))) + YYABORT; + vkd3d_free($8); + $$ = $5; + hlsl_block_add_instr($$, s); + } + +switch_case: + KW_CASE expr ':' statement_list + { + struct hlsl_ir_switch_case *c; + + if (!(c = hlsl_new_switch_case(ctx, $2, $4, &@2))) + YYABORT; + $$ = &c->entry; + + destroy_block($2); + } + | KW_CASE expr ':' + { + struct hlsl_ir_switch_case *c; + + if (!(c = hlsl_new_switch_case(ctx, $2, NULL, &@2))) + YYABORT; + $$ = &c->entry; + + destroy_block($2); + } + | KW_DEFAULT ':' statement_list + { + struct hlsl_ir_switch_case *c; + + if (!(c = hlsl_new_switch_case(ctx, NULL, $3, &@1))) + YYABORT; + $$ = &c->entry; + } + | KW_DEFAULT ':' + { + struct hlsl_ir_switch_case *c; + + if (!(c = hlsl_new_switch_case(ctx, NULL, NULL, &@1))) + YYABORT; + $$ = &c->entry; + } + +switch_cases: + switch_case + { + if (!($$ = make_empty_list(ctx))) + YYABORT; + list_add_head($$, $1); + } + | switch_cases switch_case + { + $$ = $1; + list_add_tail($$, $2); + } + expr_optional: %empty { diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 5c816e89..672dff76 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -575,7 +575,19 @@ bool hlsl_transform_ir(struct hlsl_ctx *ctx, bool (*func)(struct hlsl_ctx *ctx, progress |= hlsl_transform_ir(ctx, func, &iff->else_block, context); } else if (instr->type == HLSL_IR_LOOP) + { progress |= hlsl_transform_ir(ctx, func, &hlsl_ir_loop(instr)->body, context); + } + else if (instr->type == HLSL_IR_SWITCH) + { + struct hlsl_ir_switch *s = hlsl_ir_switch(instr); + struct hlsl_ir_switch_case *c; + + LIST_FOR_EACH_ENTRY(c, &s->cases, struct hlsl_ir_switch_case, entry) + { + progress |= hlsl_transform_ir(ctx, func, &c->body, context); + } + }
progress |= func(ctx, instr, context); } @@ -835,6 +847,28 @@ static bool lower_return(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *fun } } } + else if (instr->type == HLSL_IR_SWITCH) + { + struct hlsl_ir_switch *s = hlsl_ir_switch(instr); + struct hlsl_ir_switch_case *c; + + LIST_FOR_EACH_ENTRY(c, &s->cases, struct hlsl_ir_switch_case, entry) + { + has_early_return |= lower_return(ctx, func, &c->body, true); + } + + if (has_early_return) + { + /* If we're in a loop, we don't need to do anything here. We + * turned the return into a break, and that will already skip + * anything that comes after this "if" block. */ + if (!in_loop) + { + cf_instr = instr; + break; + } + } + } }
if (return_instr) @@ -2929,6 +2963,7 @@ static bool dce(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) case HLSL_IR_JUMP: case HLSL_IR_LOOP: case HLSL_IR_RESOURCE_STORE: + case HLSL_IR_SWITCH: break; }
@@ -2956,6 +2991,16 @@ static unsigned int index_instructions(struct hlsl_block *block, unsigned int in index = index_instructions(&hlsl_ir_loop(instr)->body, index); hlsl_ir_loop(instr)->next_index = index; } + else if (instr->type == HLSL_IR_SWITCH) + { + struct hlsl_ir_switch *s = hlsl_ir_switch(instr); + struct hlsl_ir_switch_case *c; + + LIST_FOR_EACH_ENTRY(c, &s->cases, struct hlsl_ir_switch_case, entry) + { + index = index_instructions(&c->body, index); + } + } }
return index; @@ -3173,6 +3218,16 @@ static void compute_liveness_recurse(struct hlsl_block *block, unsigned int loop jump->condition.node->last_read = last_read; break; } + case HLSL_IR_SWITCH: + { + struct hlsl_ir_switch *s = hlsl_ir_switch(instr); + struct hlsl_ir_switch_case *c; + + LIST_FOR_EACH_ENTRY(c, &s->cases, struct hlsl_ir_switch_case, entry) + compute_liveness_recurse(&c->body, loop_first, loop_last); + s->selector.node->last_read = last_read; + break; + } case HLSL_IR_CONSTANT: break; } @@ -3524,6 +3579,18 @@ static void allocate_temp_registers_recurse(struct hlsl_ctx *ctx, break; }
+ case HLSL_IR_SWITCH: + { + struct hlsl_ir_switch *s = hlsl_ir_switch(instr); + struct hlsl_ir_switch_case *c; + + LIST_FOR_EACH_ENTRY(c, &s->cases, struct hlsl_ir_switch_case, entry) + { + allocate_temp_registers_recurse(ctx, &c->body, allocator); + } + break; + } + default: break; } @@ -3633,6 +3700,18 @@ static void allocate_const_registers_recurse(struct hlsl_ctx *ctx, break; }
+ case HLSL_IR_SWITCH: + { + struct hlsl_ir_switch *s = hlsl_ir_switch(instr); + struct hlsl_ir_switch_case *c; + + LIST_FOR_EACH_ENTRY(c, &s->cases, struct hlsl_ir_switch_case, entry) + { + allocate_const_registers_recurse(ctx, &c->body, allocator); + } + break; + } + default: break; } diff --git a/tests/hlsl/switch.shader_test b/tests/hlsl/switch.shader_test index 02e499c9..c7eb57c5 100644 --- a/tests/hlsl/switch.shader_test +++ b/tests/hlsl/switch.shader_test @@ -272,7 +272,7 @@ todo draw quad todo probe all rgba (5.0, 6.0, 7.0, 8.0)
% 'continue' is not supported in switches -[pixel shader fail] +[pixel shader fail todo] uint4 v;
float4 main() : sv_target
From: Nikolay Sivov nsivov@codeweavers.com
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com --- libs/vkd3d-shader/hlsl.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 2283f962..b5696b2e 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1368,10 +1368,41 @@ struct hlsl_ir_switch_case *hlsl_new_switch_case(struct hlsl_ctx *ctx, struct hl return c; }
+static bool hlsl_switch_has_case(const struct hlsl_ir_switch_case *check, struct list *cases) +{ + struct hlsl_ir_switch_case *c; + + if (check->is_default) + return false; + + LIST_FOR_EACH_ENTRY(c, cases, struct hlsl_ir_switch_case, entry) + { + if (c->is_default) continue; + if (c->value == check->value && c != check) + return true; + } + + return false; +} + struct hlsl_ir_node *hlsl_new_switch(struct hlsl_ctx *ctx, struct hlsl_ir_node *selector, struct list *cases, const struct vkd3d_shader_location *loc) { + struct hlsl_ir_switch_case *c; struct hlsl_ir_switch *s; + bool has_default = false; + + /* Check for duplicated cases and multiple default statements. */ + LIST_FOR_EACH_ENTRY(c, cases, struct hlsl_ir_switch_case, entry) + { + if (has_default && c->is_default) + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX, "Found multiple 'default' statements."); + + if (hlsl_switch_has_case(c, cases)) + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX, "Found duplicate 'case' statement."); + + has_default |= c->is_default; + }
if (!(s = hlsl_alloc(ctx, sizeof(*s)))) return NULL;
From: Nikolay Sivov nsivov@codeweavers.com
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com --- libs/vkd3d-shader/hlsl.h | 2 ++ libs/vkd3d-shader/hlsl.y | 30 +++++++++++++++++++++--------- tests/hlsl/switch.shader_test | 4 ++-- 3 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index e1364176..5ef45244 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -727,6 +727,8 @@ struct hlsl_scope struct hlsl_scope *upper; /* The scope was created for the loop statement. */ bool loop; + /* The scope was created for the switch statement. */ + bool _switch; };
struct hlsl_profile_info diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index fa1421e4..9e455217 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -4634,12 +4634,23 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type } }
-static struct hlsl_scope *get_loop_scope(struct hlsl_scope *scope) +static bool is_continue_allowed(const struct hlsl_scope *scope) { + if (scope->_switch) + return false; + if (scope->loop) - return scope; + return true; + + return scope->upper ? is_continue_allowed(scope->upper) : false; +} + +static bool is_break_allowed(const struct hlsl_scope *scope) +{ + if (scope->loop || scope->_switch) + return true;
- return scope->upper ? get_loop_scope(scope->upper) : NULL; + return scope->upper ? is_break_allowed(scope->upper) : false; }
} @@ -5364,6 +5375,8 @@ loop_scope_start: switch_scope_start: %empty { + hlsl_push_scope(ctx); + ctx->cur_scope->_switch = true; }
var_identifier: @@ -6191,12 +6204,10 @@ jump_statement: { struct hlsl_ir_node *jump;
- /* TODO: allow 'break' in the 'switch' statements. */ - - if (!get_loop_scope(ctx->cur_scope)) + if (!is_break_allowed(ctx->cur_scope)) { hlsl_error(ctx, &@1, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX, - "The 'break' statement must be used inside of a loop."); + "The 'break' statement must be used inside of a loop or a switch."); }
if (!($$ = make_empty_block(ctx))) @@ -6208,9 +6219,8 @@ jump_statement: | KW_CONTINUE ';' { struct hlsl_ir_node *jump; - struct hlsl_scope *scope;
- if (!(scope = get_loop_scope(ctx->cur_scope))) + if (!is_continue_allowed(ctx->cur_scope)) { hlsl_error(ctx, &@1, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX, "The 'continue' statement must be used inside of a loop."); @@ -6347,6 +6357,8 @@ switch_statement: vkd3d_free($8); $$ = $5; hlsl_block_add_instr($$, s); + + hlsl_pop_scope(ctx); }
switch_case: diff --git a/tests/hlsl/switch.shader_test b/tests/hlsl/switch.shader_test index c7eb57c5..999158c1 100644 --- a/tests/hlsl/switch.shader_test +++ b/tests/hlsl/switch.shader_test @@ -183,7 +183,7 @@ float4 main() : sv_target }
% unterminated cases -[pixel shader fail] +[pixel shader fail todo] uint4 v;
float4 main() : sv_target @@ -272,7 +272,7 @@ todo draw quad todo probe all rgba (5.0, 6.0, 7.0, 8.0)
% 'continue' is not supported in switches -[pixel shader fail todo] +[pixel shader fail] uint4 v;
float4 main() : sv_target
From: Nikolay Sivov nsivov@codeweavers.com
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com --- libs/vkd3d-shader/hlsl_codegen.c | 39 ++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 672dff76..70737190 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -1673,6 +1673,19 @@ static void copy_propagation_invalidate_from_block(struct hlsl_ctx *ctx, struct break; }
+ case HLSL_IR_SWITCH: + { + struct hlsl_ir_switch *s = hlsl_ir_switch(instr); + struct hlsl_ir_switch_case *c; + + LIST_FOR_EACH_ENTRY(c, &s->cases, struct hlsl_ir_switch_case, entry) + { + copy_propagation_invalidate_from_block(ctx, state, &c->body); + } + + break; + } + default: break; } @@ -1721,6 +1734,28 @@ static bool copy_propagation_process_loop(struct hlsl_ctx *ctx, struct hlsl_ir_l return progress; }
+static bool copy_propagation_process_switch(struct hlsl_ctx *ctx, struct hlsl_ir_switch *s, + struct copy_propagation_state *state) +{ + struct copy_propagation_state inner_state; + struct hlsl_ir_switch_case *c; + bool progress = false; + + LIST_FOR_EACH_ENTRY(c, &s->cases, struct hlsl_ir_switch_case, entry) + { + copy_propagation_state_init(ctx, &inner_state, state); + progress |= copy_propagation_transform_block(ctx, &c->body, &inner_state); + copy_propagation_state_destroy(&inner_state); + } + + LIST_FOR_EACH_ENTRY(c, &s->cases, struct hlsl_ir_switch_case, entry) + { + copy_propagation_invalidate_from_block(ctx, state, &c->body); + } + + return progress; +} + static bool copy_propagation_transform_block(struct hlsl_ctx *ctx, struct hlsl_block *block, struct copy_propagation_state *state) { @@ -1759,6 +1794,10 @@ static bool copy_propagation_transform_block(struct hlsl_ctx *ctx, struct hlsl_b progress |= copy_propagation_process_loop(ctx, hlsl_ir_loop(instr), state); break;
+ case HLSL_IR_SWITCH: + progress |= copy_propagation_process_switch(ctx, hlsl_ir_switch(instr), state); + break; + default: break; }
From: Nikolay Sivov nsivov@codeweavers.com
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com --- libs/vkd3d-shader/hlsl_codegen.c | 57 ++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 70737190..5398352b 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -4453,6 +4453,62 @@ static bool type_has_object_components(struct hlsl_type *type) return false; }
+static void remove_unreachable_code(struct hlsl_ctx *ctx, struct hlsl_block *body) +{ + struct hlsl_ir_node *instr, *next; + struct hlsl_block block; + struct list *start; + + LIST_FOR_EACH_ENTRY_SAFE(instr, next, &body->instrs, struct hlsl_ir_node, entry) + { + if (instr->type == HLSL_IR_IF) + { + struct hlsl_ir_if *iff = hlsl_ir_if(instr); + + remove_unreachable_code(ctx, &iff->then_block); + remove_unreachable_code(ctx, &iff->else_block); + } + else if (instr->type == HLSL_IR_LOOP) + { + struct hlsl_ir_loop *loop = hlsl_ir_loop(instr); + + remove_unreachable_code(ctx, &loop->body); + } + else if (instr->type == HLSL_IR_SWITCH) + { + struct hlsl_ir_switch *s = hlsl_ir_switch(instr); + struct hlsl_ir_switch_case *c; + + LIST_FOR_EACH_ENTRY(c, &s->cases, struct hlsl_ir_switch_case, entry) + { + remove_unreachable_code(ctx, &c->body); + } + } + } + + /* Remove instructions past unconditional jumps. */ + LIST_FOR_EACH_ENTRY(instr, &body->instrs, struct hlsl_ir_node, entry) + { + struct hlsl_ir_jump *jump; + + if (instr->type != HLSL_IR_JUMP) + continue; + + jump = hlsl_ir_jump(instr); + if (jump->type != HLSL_IR_JUMP_BREAK && jump->type != HLSL_IR_JUMP_CONTINUE) + continue; + + if (!(start = list_next(&body->instrs, &instr->entry))) + break; + + hlsl_block_init(&block); + list_move_slice_tail(&block.instrs, start, list_tail(&body->instrs)); + hlsl_block_cleanup(&block); + + break; + } +} + int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_func, enum vkd3d_shader_target_type target_type, struct vkd3d_shader_code *out) { @@ -4570,6 +4626,7 @@ int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry progress |= hlsl_transform_ir(ctx, remove_trivial_conditional_branches, body, NULL); } while (progress); + remove_unreachable_code(ctx, body);
lower_ir(ctx, lower_nonconstant_vector_derefs, body); lower_ir(ctx, lower_casts_to_bool, body);
From: Nikolay Sivov nsivov@codeweavers.com
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com --- libs/vkd3d-shader/hlsl_codegen.c | 39 ++++++++++++++++++++++++++++++++ tests/hlsl/switch.shader_test | 2 +- 2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 5398352b..c09c9222 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2167,6 +2167,44 @@ static bool remove_trivial_conditional_branches(struct hlsl_ctx *ctx, struct hls return true; }
+static bool validate_switch_cases(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) +{ + struct hlsl_ir_switch_case *c; + bool terminal_break = false; + struct hlsl_ir_node *node; + struct hlsl_ir_jump *jump; + struct hlsl_ir_switch *s; + + if (instr->type != HLSL_IR_SWITCH) + return false; + s = hlsl_ir_switch(instr); + + LIST_FOR_EACH_ENTRY(c, &s->cases, struct hlsl_ir_switch_case, entry) + { + if (list_empty(&c->body.instrs)) + continue; + + terminal_break = false; + node = LIST_ENTRY(list_tail(&c->body.instrs), struct hlsl_ir_node, entry); + if (node->type == HLSL_IR_JUMP) + { + jump = hlsl_ir_jump(node); + terminal_break = jump->type == HLSL_IR_JUMP_BREAK; + } + + if (!terminal_break) + break; + } + + if (!terminal_break) + { + hlsl_error(ctx, &instr->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX, + "Fallthrough is only allowed for empty switch cases."); + } + + return true; +} + static bool lower_nonconstant_vector_derefs(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, struct hlsl_block *block) { struct hlsl_ir_node *idx; @@ -4627,6 +4665,7 @@ int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry } while (progress); remove_unreachable_code(ctx, body); + hlsl_transform_ir(ctx, validate_switch_cases, body, NULL);
lower_ir(ctx, lower_nonconstant_vector_derefs, body); lower_ir(ctx, lower_casts_to_bool, body); diff --git a/tests/hlsl/switch.shader_test b/tests/hlsl/switch.shader_test index 999158c1..02e499c9 100644 --- a/tests/hlsl/switch.shader_test +++ b/tests/hlsl/switch.shader_test @@ -183,7 +183,7 @@ float4 main() : sv_target }
% unterminated cases -[pixel shader fail todo] +[pixel shader fail] uint4 v;
float4 main() : sv_target
From: Nikolay Sivov nsivov@codeweavers.com
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com --- libs/vkd3d-shader/tpf.c | 44 ++++++++++++++++++++ tests/hlsl/switch.shader_test | 78 +++++++++++++++++------------------ 2 files changed, 83 insertions(+), 39 deletions(-)
diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 596a0c49..c8f5e8cf 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -5463,6 +5463,46 @@ static void write_sm4_store(const struct tpf_writer *tpf, const struct hlsl_ir_s write_sm4_instruction(tpf, &instr); }
+static void write_sm4_switch(const struct tpf_writer *tpf, const struct hlsl_ir_switch *s) +{ + const struct hlsl_ir_node *selector = s->selector.node; + struct hlsl_ir_switch_case *c; + struct sm4_instruction instr; + + memset(&instr, 0, sizeof(instr)); + instr.opcode = VKD3D_SM4_OP_SWITCH; + + sm4_src_from_node(tpf, &instr.srcs[0], selector, VKD3DSP_WRITEMASK_ALL); + instr.src_count = 1; + + write_sm4_instruction(tpf, &instr); + + LIST_FOR_EACH_ENTRY(c, &s->cases, struct hlsl_ir_switch_case, entry) + { + memset(&instr, 0, sizeof(instr)); + if (c->is_default) + { + instr.opcode = VKD3D_SM4_OP_DEFAULT; + } + else + { + struct hlsl_constant_value value = { .u[0].u = c->value }; + + instr.opcode = VKD3D_SM4_OP_CASE; + sm4_src_from_constant_value(&instr.srcs[0], &value, 1, VKD3DSP_WRITEMASK_ALL); + instr.src_count = 1; + } + + write_sm4_instruction(tpf, &instr); + write_sm4_block(tpf, &c->body); + } + + memset(&instr, 0, sizeof(instr)); + instr.opcode = VKD3D_SM4_OP_ENDSWITCH; + + write_sm4_instruction(tpf, &instr); +} + static void write_sm4_swizzle(const struct tpf_writer *tpf, const struct hlsl_ir_swizzle *swizzle) { unsigned int hlsl_swizzle; @@ -5550,6 +5590,10 @@ static void write_sm4_block(const struct tpf_writer *tpf, const struct hlsl_bloc write_sm4_store(tpf, hlsl_ir_store(instr)); break;
+ case HLSL_IR_SWITCH: + write_sm4_switch(tpf, hlsl_ir_switch(instr)); + break; + case HLSL_IR_SWIZZLE: write_sm4_swizzle(tpf, hlsl_ir_swizzle(instr)); break; diff --git a/tests/hlsl/switch.shader_test b/tests/hlsl/switch.shader_test index 02e499c9..8e286fe9 100644 --- a/tests/hlsl/switch.shader_test +++ b/tests/hlsl/switch.shader_test @@ -1,7 +1,7 @@ [require] shader model >= 4.0
-[pixel shader todo] +[pixel shader] uint4 v;
float4 main() : sv_target @@ -19,17 +19,17 @@ float4 main() : sv_target
[test] uniform 0 uint4 3 0 0 0 -todo draw quad -todo probe all rgba (5.0, 5.0, 5.0, 5.0) +draw quad +probe all rgba (5.0, 5.0, 5.0, 5.0) uniform 0 uint4 1 0 0 0 -todo draw quad -todo probe all rgba (4.0, 4.0, 4.0, 4.0) +draw quad +probe all rgba (4.0, 4.0, 4.0, 4.0) uniform 0 uint4 0 0 0 0 -todo draw quad -todo probe all rgba (3.0, 3.0, 3.0, 3.0) +draw quad +probe all rgba (3.0, 3.0, 3.0, 3.0)
% falling through is only supported for empty case statements -[pixel shader todo] +[pixel shader] uint4 v;
float4 main() : sv_target @@ -49,17 +49,17 @@ float4 main() : sv_target
[test] uniform 0 uint4 2 0 0 0 -todo draw quad -todo probe all rgba (1.0, 2.0, 3.0, 4.0) +draw quad +probe all rgba (1.0, 2.0, 3.0, 4.0) uniform 0 uint4 1 0 0 0 -todo draw quad -todo probe all rgba (1.1, 2.0, 3.0, 4.0) +draw quad +probe all rgba (1.1, 2.0, 3.0, 4.0) uniform 0 uint4 0 0 0 0 -todo draw quad -todo probe all rgba (1.1, 2.0, 3.0, 4.0) +draw quad +probe all rgba (1.1, 2.0, 3.0, 4.0)
% case value evaluation -[pixel shader todo] +[pixel shader] uint4 v;
float4 main() : sv_target @@ -81,14 +81,14 @@ float4 main() : sv_target
[test] uniform 0 uint4 2 0 0 0 -todo draw quad -todo probe all rgba (1.1, 2.1, 3.1, 4.1) +draw quad +probe all rgba (1.1, 2.1, 3.1, 4.1) uniform 0 uint4 1 0 0 0 -todo draw quad -todo probe all rgba (1.0, 2.0, 3.0, 4.0) +draw quad +probe all rgba (1.0, 2.0, 3.0, 4.0)
% floats are accepted -[pixel shader todo] +[pixel shader] uint4 v;
float4 main() : sv_target @@ -110,11 +110,11 @@ float4 main() : sv_target
[test] uniform 0 uint4 2 0 0 0 -todo draw quad -todo probe all rgba (1.1, 2.1, 3.1, 4.1) +draw quad +probe all rgba (1.1, 2.1, 3.1, 4.1) uniform 0 uint4 1 0 0 0 -todo draw quad -todo probe all rgba (1.0, 2.0, 3.0, 4.0) +draw quad +probe all rgba (1.0, 2.0, 3.0, 4.0)
[pixel shader fail] uint4 v; @@ -203,7 +203,7 @@ float4 main() : sv_target }
% more complicated breaks -[pixel shader todo] +[pixel shader] uint4 v;
float4 main() : sv_target @@ -230,17 +230,17 @@ float4 main() : sv_target
[test] uniform 0 uint4 2 0 0 0 -todo draw quad -todo probe all rgba (1.1, 2.1, 3.1, 4.1) +draw quad +probe all rgba (1.1, 2.1, 3.1, 4.1) uniform 0 uint4 1 0 0 0 -todo draw quad -todo probe all rgba (1.2, 2.2, 3.2, 4.2) +draw quad +probe all rgba (1.2, 2.2, 3.2, 4.2) uniform 0 uint4 0 0 0 0 -todo draw quad -todo probe all rgba (1.0, 2.0, 3.0, 4.0) +draw quad +probe all rgba (1.0, 2.0, 3.0, 4.0)
% switch breaks within a loop -[pixel shader todo] +[pixel shader] uint4 v;
float4 main() : sv_target @@ -268,8 +268,8 @@ float4 main() : sv_target
[test] uniform 0 uint4 2 0 0 0 -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)
% 'continue' is not supported in switches [pixel shader fail] @@ -297,7 +297,7 @@ float4 main() : sv_target return c; }
-[pixel shader todo] +[pixel shader] uint4 v;
float4 main() : sv_target @@ -328,8 +328,8 @@ float4 main() : sv_target
[test] uniform 0 uint4 0 0 3 1 -todo draw quad -todo probe all rgba (10.0, 11.0, 12.0, 13.0) +draw quad +probe all rgba (10.0, 11.0, 12.0, 13.0) uniform 0 uint4 1 0 3 1 -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)
On Thu Oct 12 14:24:13 2023 +0000, Nikolay Sivov wrote:
changed this line in [version 10 of the diff](/wine/vkd3d/-/merge_requests/361/diffs?diff_id=75725&start_sha=3888a7ddc3ef9d1975f82b540dbf89c1ed08b87a#50ceca3ac6853864b9418acfdf7ada74eb7e333e_1019_1019)
Actually this turns out to be unnecessary. I removed it instead.
On Thu Oct 12 14:43:45 2023 +0000, Giovanni Mascellani wrote:
Thanks for splitting, it's much better to review it now.
I think I addressed everything, except YYABORT part, which is not specific to this MR.
On Thu Oct 12 11:40:12 2023 +0000, Nikolay Sivov wrote:
I thought FOR_EACH_ENTRY_SAFE() is not safe enough if I'm going to remove 'instr', and potentially 'next' and past it.
I don't think that's a problem: the pointers will become dangling, but that's not a problem as long as you don't dereference them (which you do not, given that you immediately break out of the loop). If you like it better, you can also break out of the loop before slicing and slice (and deallocate) after the loop.
Well, you can also leave it like it is, it's not broken. Just a bit slower than it could be.
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.c:
struct clone_instr_map *map, struct hlsl_ir_switch *s)+{
- struct hlsl_ir_switch_case *c, *d;
- struct hlsl_ir_node *ret;
- struct hlsl_block body;
- struct list cases;
- list_init(&cases);
- LIST_FOR_EACH_ENTRY(c, &s->cases, struct hlsl_ir_switch_case, entry)
- {
clone_block(ctx, &body, &c->body, map);if (!(d = hlsl_new_switch_case(ctx, NULL, &body, &c->loc))){free_ir_switch_cases(&cases);
Here you have to cleanup `body`. If `hlsl_new_switch_case()` succeeds, than `body` is guaranteed to be empty, and you can just leave it go out of scope. But if `hlsl_new_switch_case()` fails because of memory allocation, this doesn't happen and you have to care about it in the caller. Alternatively, you can modify `hlsl_new_switch_case()` so that `body` is always cleaned up there. In other words, it must be clear whether `hlsl_new_switch_case()` takes ownership of `body` or not.
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
YYABORT;vkd3d_free($8);$$ = $5;hlsl_block_add_instr($$, s);}+switch_case:
KW_CASE expr ':' statement_list{struct hlsl_ir_switch_case *c;if (!(c = hlsl_new_switch_case(ctx, $2, $4, &@2)))YYABORT;$$ = &c->entry;destroy_block($2);
I think you have to destroy `$4` too. Function `hlsl_new_switch_case()` leaves it empty (though not always, see above), but the block itself is allocated on the heap here, so you have to free it. `destroy_block()` cares about that. This also applies to the other `case` and `default` rules.
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.c:
- hlsl_block_init(&c->body);
- if (body)
hlsl_block_add_block(&c->body, body);- c->loc = *loc;
- return c;
+}
+struct hlsl_ir_node *hlsl_new_switch(struct hlsl_ctx *ctx, struct hlsl_ir_node *selector,
struct list *cases, const struct vkd3d_shader_location *loc)+{
- struct hlsl_ir_switch *s;
- if (!(s = hlsl_alloc(ctx, sizeof(*s))))
return NULL;
This has a similar problem to `hlsl_new_switch_case()`: if you return here the content of `cases` is leaked.
On Thu Oct 12 14:43:45 2023 +0000, Nikolay Sivov wrote:
I think I addressed everything, except YYABORT part, which is not specific to this MR.
I think it is, at least for the grammar rules you're adding in this MR.
On Thu Oct 12 11:17:18 2023 +0000, Giovanni Mascellani wrote:
I think I should already have learned that by now, but what are the obligations we have when we `YYABORT` to avoid leaking resources? I guess we should at least cleanup the input variables (`$1` etc), I don't know about `$$`. Maybe @zfigura can help?
I realized that in this MR `YYABORT` always appears before assigning `$$`, so clearly you don't have to cleanup `$$` at that point. Still, you have to ensure that any exit path, including `YYABORT`, cleans the right hand side symbols.
-
Giovanni Mascellani (＠giomasce) -
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)