[PATCH v21 0/8] MR361: vkd3d-shader/hlsl: Initial support for 'switch' statement.
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- v21: vkd3d-shader/tpf: Write out 'switch' statements. vkd3d-shader/hlsl: Add a pass to normalize switch cases blocks. vkd3d-shader/hlsl: Add a pass to remove unreachable code. vkd3d-shader/hlsl: Add copy propagation logic for switches. vkd3d-shader/hlsl: Validate break/continue context. vkd3d-shader/hlsl: Check for duplicate case statements. vkd3d-shader/hlsl: Add initial support for parsing 'switch' statements. tests: Add some tests for the 'switch' statements. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- Makefile.am | 1 + tests/hlsl/switch.shader_test | 441 ++++++++++++++++++++++++++++++++++ 2 files changed, 442 insertions(+) create mode 100644 tests/hlsl/switch.shader_test diff --git a/Makefile.am b/Makefile.am index 8364aaa37..1afa4bd95 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 000000000..27fde175f --- /dev/null +++ b/tests/hlsl/switch.shader_test @@ -0,0 +1,441 @@ +[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 fail(sm>=6)] +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; +} + +% multiple default cases +[pixel shader fail] +uint4 v; + +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(sm<6)] +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; +} + +[pixel shader fail] +uint4 v; + +float4 main() : sv_target +{ + switch (v.x) + { + case 0: + return 3.0; + case 1: + return 4.0; + case 2: + } + return 0.0; +} + +[pixel shader fail] +uint4 v; + +float4 main() : sv_target +{ + switch (v.x) + { + case 0: + return 3.0; + case 1: + return 4.0; + default: + } + return 0.0; +} + +% 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) + +% default case placement +[pixel shader todo] +uint4 v; + +float4 main() : sv_target +{ + float4 c = {1.0f, 2.0f, 3.0f, 4.0f}; + + switch (v.x) + { + case 2: + c += 1.0f; + break; + case 1: + c -= 1.0f; + break; + case 3: + default: + case 0: + c += 3.0f; + break; + } + + return c; +} + +[test] +uniform 0 uint4 0 0 0 0 +todo draw quad +todo probe all rgba (4.0, 5.0, 6.0, 7.0) +uniform 0 uint4 2 0 0 0 +todo draw quad +todo probe all rgba (2.0, 3.0, 4.0, 5.0) +uniform 0 uint4 3 0 0 0 +todo draw quad +todo probe all rgba (4.0, 5.0, 6.0, 7.0) + +[pixel shader todo] +uint4 v; + +float4 main() : sv_target +{ + float4 c = {1.0f, 2.0f, 3.0f, 4.0f}; + + switch (v.x) + { + case 2: + c += 1.0f; + break; + case 1: + c -= 1.0f; + break; + case 3: + default: + break; + case 0: + c += 3.0f; + break; + } + + return c; +} + +[test] +uniform 0 uint4 3 0 0 0 +todo draw quad +todo probe all rgba (1.0, 2.0, 3.0, 4.0) +uniform 0 uint4 0 0 0 0 +todo draw quad +todo probe all rgba (4.0, 5.0, 6.0, 7.0) +uniform 0 uint4 5 0 0 0 +todo draw quad +todo probe all rgba (1.0, 2.0, 3.0, 4.0) + +% 'continue' is not supported in switches +[pixel shader fail(sm<6)] +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) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 141 +++++++++++++++++++++++++++++++ libs/vkd3d-shader/hlsl.h | 32 +++++++ libs/vkd3d-shader/hlsl.l | 4 +- libs/vkd3d-shader/hlsl.y | 115 ++++++++++++++++++++++++- libs/vkd3d-shader/hlsl_codegen.c | 79 +++++++++++++++++ tests/hlsl/switch.shader_test | 6 +- 6 files changed, 370 insertions(+), 7 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index b42e30888..5591973c0 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,60 @@ static struct hlsl_ir_node *clone_index(struct hlsl_ctx *ctx, struct clone_instr return dst; } +void hlsl_free_ir_switch_case(struct hlsl_ir_switch_case *c) +{ + hlsl_block_cleanup(&c->body); + list_remove(&c->entry); + vkd3d_free(c); +} + +void hlsl_cleanup_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_free_ir_switch_case(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) + { + if (!(clone_block(ctx, &body, &c->body, map))) + { + hlsl_cleanup_ir_switch_cases(&cases); + return NULL; + } + + d = hlsl_new_switch_case(ctx, NULL, &body, &c->loc); + hlsl_block_cleanup(&body); + if (!d) + { + hlsl_cleanup_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); + hlsl_cleanup_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 +1938,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 +2359,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 +2784,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 +2865,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 +3029,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); + hlsl_cleanup_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 +3097,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 9f1a3fe21..d9b5e0ebf 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, @@ -945,6 +962,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); @@ -1118,6 +1141,9 @@ bool hlsl_copy_deref(struct hlsl_ctx *ctx, struct hlsl_deref *deref, const struc void hlsl_cleanup_deref(struct hlsl_deref *deref); void hlsl_cleanup_semantic(struct hlsl_semantic *semantic); +void hlsl_cleanup_ir_switch_cases(struct list *cases); +void hlsl_free_ir_switch_case(struct hlsl_ir_switch_case *c); + void hlsl_replace_node(struct hlsl_ir_node *old, struct hlsl_ir_node *new); void hlsl_free_attribute(struct hlsl_attribute *attr); @@ -1211,6 +1237,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); @@ -1265,6 +1295,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 90abd64a3..d457c6622 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 ba738473f..dcaca929f 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -162,6 +162,12 @@ static void destroy_block(struct hlsl_block *block) vkd3d_free(block); } +static void destroy_switch_cases(struct list *cases) +{ + hlsl_cleanup_ir_switch_cases(cases); + vkd3d_free(cases); +} + static bool hlsl_types_are_componentwise_compatible(struct hlsl_ctx *ctx, struct hlsl_type *src, struct hlsl_type *dst) { @@ -1154,7 +1160,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 +1186,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 +4691,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 +4803,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 +4847,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 +5367,11 @@ loop_scope_start: ctx->cur_scope->loop = true; } +switch_scope_start: + %empty + { + } + var_identifier: VAR_IDENTIFIER | NEW_IDENTIFIER @@ -5674,7 +5691,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 +5965,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 +6190,7 @@ statement: | jump_statement | selection_statement | loop_statement + | switch_statement jump_statement: KW_BREAK ';' @@ -6321,6 +6339,97 @@ 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))) + { + destroy_switch_cases($8); + destroy_block($5); + YYABORT; + } + + s = hlsl_new_switch(ctx, selector, $8, &@3); + + destroy_switch_cases($8); + + if (!s) + { + destroy_block($5); + YYABORT; + } + + $$ = $5; + hlsl_block_add_instr($$, s); + } + +switch_case: + KW_CASE expr ':' statement_list + { + struct hlsl_ir_switch_case *c; + + c = hlsl_new_switch_case(ctx, $2, $4, &@2); + + destroy_block($2); + destroy_block($4); + + if (!c) + YYABORT; + $$ = &c->entry; + } + | KW_CASE expr ':' + { + struct hlsl_ir_switch_case *c; + + c = hlsl_new_switch_case(ctx, $2, NULL, &@2); + + destroy_block($2); + + if (!c) + YYABORT; + $$ = &c->entry; + } + | KW_DEFAULT ':' statement_list + { + struct hlsl_ir_switch_case *c; + + c = hlsl_new_switch_case(ctx, NULL, $3, &@1); + + destroy_block($3); + + if (!c) + 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 + { + struct hlsl_ir_switch_case *c = LIST_ENTRY($1, struct hlsl_ir_switch_case, entry); + if (!($$ = make_empty_list(ctx))) + { + hlsl_free_ir_switch_case(c); + 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 7d17ca8ce..c7cce8238 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 27fde175f..56838bb6c 100644 --- a/tests/hlsl/switch.shader_test +++ b/tests/hlsl/switch.shader_test @@ -202,7 +202,7 @@ float4 main() : sv_target return c; } -[pixel shader fail] +[pixel shader fail todo] uint4 v; float4 main() : sv_target @@ -218,7 +218,7 @@ float4 main() : sv_target return 0.0; } -[pixel shader fail] +[pixel shader fail todo] uint4 v; float4 main() : sv_target @@ -378,7 +378,7 @@ todo draw quad todo probe all rgba (1.0, 2.0, 3.0, 4.0) % 'continue' is not supported in switches -[pixel shader fail(sm<6)] +[pixel shader todo fail(sm<6)] uint4 v; float4 main() : sv_target -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)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 5591973c0..dc4226358 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; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)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 d9b5e0ebf..d66242d23 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 dcaca929f..75341be82 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -4640,12 +4640,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; } } @@ -5370,6 +5381,8 @@ loop_scope_start: switch_scope_start: %empty { + hlsl_push_scope(ctx); + ctx->cur_scope->_switch = true; } var_identifier: @@ -6197,12 +6210,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))) @@ -6214,9 +6225,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."); @@ -6364,6 +6374,8 @@ switch_statement: $$ = $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 56838bb6c..0ad45071d 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(sm<6)] +[pixel shader todo fail(sm<6)] uint4 v; float4 main() : sv_target @@ -378,7 +378,7 @@ todo draw quad todo probe all rgba (1.0, 2.0, 3.0, 4.0) % 'continue' is not supported in switches -[pixel shader todo fail(sm<6)] +[pixel shader fail(sm<6)] uint4 v; float4 main() : sv_target -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)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 c7cce8238..ef622afff 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; } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)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 ef622afff..fef871e3f 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); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 100 +++++++++++++++++++++++++++++++ tests/hlsl/switch.shader_test | 6 +- 2 files changed, 103 insertions(+), 3 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index fef871e3f..89aa7e27a 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2167,6 +2167,105 @@ static bool remove_trivial_conditional_branches(struct hlsl_ctx *ctx, struct hls return true; } +static bool normalize_switch_cases(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) +{ + struct hlsl_ir_switch_case *c, *def = NULL; + 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) + { + terminal_break = false; + + if (list_empty(&c->body.instrs)) + continue; + + 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, + "Switch cases blocks must be terminated with 'break' or 'return'."); + return true; + } + + LIST_FOR_EACH_ENTRY(c, &s->cases, struct hlsl_ir_switch_case, entry) + { + if (c->is_default) + { + def = c; + + /* Remove preceding empty cases. */ + while (list_prev(&s->cases, &def->entry)) + { + c = LIST_ENTRY(list_prev(&s->cases, &def->entry), struct hlsl_ir_switch_case, entry); + if (!list_empty(&c->body.instrs)) + break; + hlsl_free_ir_switch_case(c); + } + + if (list_empty(&def->body.instrs)) + { + /* Remove following empty cases. */ + while (list_next(&s->cases, &def->entry)) + { + c = LIST_ENTRY(list_next(&s->cases, &def->entry), struct hlsl_ir_switch_case, entry); + if (!list_empty(&c->body.instrs)) + break; + hlsl_free_ir_switch_case(c); + } + + /* Merge with the next case. */ + if (list_next(&s->cases, &def->entry)) + { + c = LIST_ENTRY(list_next(&s->cases, &def->entry), struct hlsl_ir_switch_case, entry); + c->is_default = true; + hlsl_free_ir_switch_case(def); + def = c; + } + } + + break; + } + } + + if (def) + { + list_remove(&def->entry); + } + else + { + struct hlsl_ir_node *jump; + + if (!(def = hlsl_new_switch_case(ctx, NULL, NULL, &s->node.loc))) + return true; + if (!(jump = hlsl_new_jump(ctx, HLSL_IR_JUMP_BREAK, NULL, &s->node.loc))) + { + hlsl_free_ir_switch_case(def); + return true; + } + hlsl_block_add_instr(&def->body, jump); + } + list_add_tail(&s->cases, &def->entry); + + 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 +4726,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, normalize_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 0ad45071d..27fde175f 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 todo fail(sm<6)] +[pixel shader fail(sm<6)] uint4 v; float4 main() : sv_target @@ -202,7 +202,7 @@ float4 main() : sv_target return c; } -[pixel shader fail todo] +[pixel shader fail] uint4 v; float4 main() : sv_target @@ -218,7 +218,7 @@ float4 main() : sv_target return 0.0; } -[pixel shader fail todo] +[pixel shader fail] uint4 v; float4 main() : sv_target -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/tpf.c | 44 ++++++++++++++ tests/hlsl/switch.shader_test | 106 +++++++++++++++++----------------- 2 files changed, 97 insertions(+), 53 deletions(-) diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index bf4c03c05..5a8c73f7d 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -5464,6 +5464,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; @@ -5551,6 +5591,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 27fde175f..a880b52e9 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) +todo(sm>=6) 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) +todo(sm>=6) 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) +todo(sm>=6) 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) +todo(sm>=6) 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) +todo(sm>=6) 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) +todo(sm>=6) 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) +todo(sm>=6) 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) +todo(sm>=6) draw quad +probe all rgba (1.0, 2.0, 3.0, 4.0) % floats are accepted -[pixel shader todo fail(sm>=6)] +[pixel shader fail(sm>=6)] 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; @@ -235,7 +235,7 @@ float4 main() : sv_target } % more complicated breaks -[pixel shader todo] +[pixel shader] uint4 v; float4 main() : sv_target @@ -262,17 +262,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) +todo(sm>=6) 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) +todo(sm>=6) 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) +todo(sm>=6) 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 @@ -300,11 +300,11 @@ 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) +todo(sm>=6) draw quad +probe all rgba (5.0, 6.0, 7.0, 8.0) % default case placement -[pixel shader todo] +[pixel shader] uint4 v; float4 main() : sv_target @@ -331,16 +331,16 @@ float4 main() : sv_target [test] uniform 0 uint4 0 0 0 0 -todo draw quad -todo probe all rgba (4.0, 5.0, 6.0, 7.0) +todo(sm>=6) draw quad +probe all rgba (4.0, 5.0, 6.0, 7.0) uniform 0 uint4 2 0 0 0 -todo draw quad -todo probe all rgba (2.0, 3.0, 4.0, 5.0) +todo(sm>=6) draw quad +probe all rgba (2.0, 3.0, 4.0, 5.0) uniform 0 uint4 3 0 0 0 -todo draw quad -todo probe all rgba (4.0, 5.0, 6.0, 7.0) +todo(sm>=6) draw quad +probe all rgba (4.0, 5.0, 6.0, 7.0) -[pixel shader todo] +[pixel shader] uint4 v; float4 main() : sv_target @@ -368,14 +368,14 @@ float4 main() : sv_target [test] uniform 0 uint4 3 0 0 0 -todo draw quad -todo probe all rgba (1.0, 2.0, 3.0, 4.0) +todo(sm>=6) draw quad +probe all rgba (1.0, 2.0, 3.0, 4.0) uniform 0 uint4 0 0 0 0 -todo draw quad -todo probe all rgba (4.0, 5.0, 6.0, 7.0) +todo(sm>=6) draw quad +probe all rgba (4.0, 5.0, 6.0, 7.0) uniform 0 uint4 5 0 0 0 -todo draw quad -todo probe all rgba (1.0, 2.0, 3.0, 4.0) +todo(sm>=6) draw quad +probe all rgba (1.0, 2.0, 3.0, 4.0) % 'continue' is not supported in switches [pixel shader fail(sm<6)] @@ -403,7 +403,7 @@ float4 main() : sv_target return c; } -[pixel shader todo] +[pixel shader] uint4 v; float4 main() : sv_target @@ -434,8 +434,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) +todo(sm>=6) 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) +todo(sm>=6) draw quad +probe all rgba (7.0, 8.0, 9.0, 10.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361
The test failure turned out to be from dxc compiler runs. I updated todo's to accommodate for that. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361#note_49197
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361
Zebediah Figura (@zfigura) commented about tests/hlsl/switch.shader_test:
+ } + + 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 fail(sm>=6)] +uint4 v; You can make this a little bit stronger by making the selector also float, that proves that they're always converted to uint regardless.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361#note_49493
Zebediah Figura (@zfigura) commented about tests/hlsl/switch.shader_test:
+[require] +shader model >= 4.0 Wow, so switch just doesn't work with sm1 at all. I thought they might just be lowered like if/else is, but no.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361#note_49494
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl.c:
+ 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; + } + I... guess, but this is kind of contrary to the way we usually do these things, and having hlsl.c call hlsl.y is kind of a layering violation. I'd rather just see us interpret the value in hlsl.y and pass it to hlsl_new_switch_case(). That will also make clone_switch() less awkward.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361#note_49495
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl.y:
+ hlsl_block_add_instr($$, s); + } + +switch_case: + KW_CASE expr ':' statement_list + { + struct hlsl_ir_switch_case *c; + + c = hlsl_new_switch_case(ctx, $2, $4, &@2); + + destroy_block($2); + destroy_block($4); + + if (!c) + YYABORT; + $$ = &c->entry; Why doesn't this rule just return a 'struct hlsl_ir_switch_case *'?
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361#note_49496
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl_codegen.c:
} + 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. */
This isn't true, though, because "break" only breaks out of the switch. We'll need to handle these like loops. We don't have any tests for this case, either. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361#note_49497
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl.c:
{ + 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; + } In general we want error reporting to live in the frontend if possible.
INVALID_SYNTAX is also... not what I would use here; I think this may warrant a new error message. Something nice to have here would be to print the case value itself. Also nice would be to print the location of the previous definition in a hlsl_note(). -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361#note_49498
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl.c:
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; + }
This will report an error twice for each duplicate case. I guess in some sense that's fine, but it's not how we report most redefinition errors; we use an hlsl_error() for the redefinition and a hlsl_note() for the previous location. Along the lines of "report errors when parsing", we may want to structure this so that we check for a redefinition when adding a new case, like we do for e.g. struct fields. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361#note_49499
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl.y:
| 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.");
It's a bit unfortunate that that's our error message for cases where we have a continue in a switch in a loop. Maybe we could put the hlsl_error() inside is_continue_allowed() instead? [and then rename it to something like check_continue()]. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361#note_49500
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl_codegen.c:
+ 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, + "Switch cases blocks must be terminated with 'break' or 'return'."); + return true; + } Why is this error not per-case? I guess you're trying to report it only once for the whole switch, but why?
I also notice we don't have tests for ending a case with 'discard' instead of return. That's indeed forbidden but we should explicitly test it. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361#note_49502
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl.h:
struct hlsl_scope *upper; /* The scope was created for the loop statement. */ bool loop; + /* The scope was created for the switch statement. */ + bool _switch;
Minor refactoring thought, but we could make this an enum, and pass that to hlsl_push_scope(). Not really important though. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/361#note_49501
participants (4)
-
Giovanni Mascellani (＠giomasce) -
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)