[PATCH v15 0/7] MR450: vkd3d-shader/ir: Flatten structured control flow instructions.
-- v15: vkd3d-shader/ir: Store code block names in struct vkd3d_shader_desc. vkd3d-shader/ir: Flatten SWITCH/CASE/DEFAULT/ENDSWITCH control flow instructions. vkd3d-shader/ir: Flatten LOOP/BREAK/CONTINUE/ENDLOOP control flow instructions. vkd3d-shader/ir: Flatten IF/ELSE/ENDIF control flow instructions. vkd3d-shader/spirv: Handle RETP in spirv_compiler_handle_instruction(). vkd3d-shader/spirv: Handle DISCARD and TEXKILL in spirv_compiler_handle_instruction(). vkd3d-shader/spirv: Emit descriptor offset loads in the function entry block. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/450
From: Conor McCarthy <cmccarthy(a)codeweavers.com> Ensures they are loaded only once per function independent of the control flow graph. --- libs/vkd3d-shader/spirv.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 980c26bc0..9832f8a38 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -2427,6 +2427,7 @@ static bool is_in_fork_or_join_phase(const struct spirv_compiler *compiler) } static void spirv_compiler_emit_initial_declarations(struct spirv_compiler *compiler); +static size_t spirv_compiler_get_current_function_location(struct spirv_compiler *compiler); static const char *spirv_compiler_get_entry_point_name(const struct spirv_compiler *compiler) { @@ -3500,11 +3501,13 @@ static uint32_t spirv_compiler_get_descriptor_index(struct spirv_compiler *compi index_ids[0] = compiler->descriptor_offsets_member_id; index_ids[1] = spirv_compiler_get_constant_uint(compiler, push_constant_index); ptr_type_id = vkd3d_spirv_get_op_type_pointer(builder, SpvStorageClassPushConstant, type_id); + vkd3d_spirv_begin_function_stream_insertion(builder, + spirv_compiler_get_current_function_location(compiler)); ptr_id = vkd3d_spirv_build_op_in_bounds_access_chain(builder, ptr_type_id, compiler->push_constants_var_id, index_ids, 2); offset_id = vkd3d_spirv_build_op_load(builder, type_id, ptr_id, SpvMemoryAccessMaskNone); - if (!compiler->control_flow_depth) - compiler->descriptor_offset_ids[push_constant_index] = offset_id; + vkd3d_spirv_end_function_stream_insertion(builder); + compiler->descriptor_offset_ids[push_constant_index] = offset_id; } index_id = vkd3d_spirv_build_op_iadd(builder, type_id, index_id, offset_id); } @@ -6624,6 +6627,9 @@ static void spirv_compiler_emit_hull_shader_main(struct spirv_compiler *compiler struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; uint32_t void_id; + /* If a patch constant function used descriptor indexing the offsets must be reloaded. */ + memset(compiler->descriptor_offset_ids, 0, compiler->offset_info.descriptor_table_count + * sizeof(*compiler->descriptor_offset_ids)); vkd3d_spirv_builder_begin_main_function(builder); void_id = vkd3d_spirv_get_op_type_void(builder); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/450
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/spirv.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 9832f8a38..178128eb7 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -7928,11 +7928,6 @@ static int spirv_compiler_emit_control_flow_instruction(struct spirv_compiler *c spirv_compiler_emit_retc(compiler, instruction); break; - case VKD3DSIH_DISCARD: - case VKD3DSIH_TEXKILL: - spirv_compiler_emit_kill(compiler, instruction); - break; - default: ERR("Unexpected instruction %#x.\n", instruction->handler_idx); break; @@ -9691,7 +9686,6 @@ static int spirv_compiler_handle_instruction(struct spirv_compiler *compiler, case VKD3DSIH_CONTINUE: case VKD3DSIH_CONTINUEP: case VKD3DSIH_DEFAULT: - case VKD3DSIH_DISCARD: case VKD3DSIH_ELSE: case VKD3DSIH_ENDIF: case VKD3DSIH_ENDLOOP: @@ -9701,9 +9695,12 @@ static int spirv_compiler_handle_instruction(struct spirv_compiler *compiler, case VKD3DSIH_RET: case VKD3DSIH_RETP: case VKD3DSIH_SWITCH: - case VKD3DSIH_TEXKILL: ret = spirv_compiler_emit_control_flow_instruction(compiler, instruction); break; + case VKD3DSIH_DISCARD: + case VKD3DSIH_TEXKILL: + spirv_compiler_emit_kill(compiler, instruction); + break; case VKD3DSIH_DSX: case VKD3DSIH_DSX_COARSE: case VKD3DSIH_DSX_FINE: -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/450
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/spirv.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 178128eb7..170c76c70 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -7924,10 +7924,6 @@ static int spirv_compiler_emit_control_flow_instruction(struct spirv_compiler *c compiler->main_block_open = false; break; - case VKD3DSIH_RETP: - spirv_compiler_emit_retc(compiler, instruction); - break; - default: ERR("Unexpected instruction %#x.\n", instruction->handler_idx); break; @@ -9693,10 +9689,12 @@ static int spirv_compiler_handle_instruction(struct spirv_compiler *compiler, case VKD3DSIH_IF: case VKD3DSIH_LOOP: case VKD3DSIH_RET: - case VKD3DSIH_RETP: case VKD3DSIH_SWITCH: ret = spirv_compiler_emit_control_flow_instruction(compiler, instruction); break; + case VKD3DSIH_RETP: + spirv_compiler_emit_retc(compiler, instruction); + break; case VKD3DSIH_DISCARD: case VKD3DSIH_TEXKILL: spirv_compiler_emit_kill(compiler, instruction); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/450
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/d3d_asm.c | 24 +- libs/vkd3d-shader/ir.c | 328 +++++++++++++++++++++++ libs/vkd3d-shader/spirv.c | 180 +++++++------ libs/vkd3d-shader/vkd3d_shader_private.h | 2 + 4 files changed, 442 insertions(+), 92 deletions(-) diff --git a/libs/vkd3d-shader/d3d_asm.c b/libs/vkd3d-shader/d3d_asm.c index 86ddd1ffd..c47d94b23 100644 --- a/libs/vkd3d-shader/d3d_asm.c +++ b/libs/vkd3d-shader/d3d_asm.c @@ -44,6 +44,7 @@ static const char * const shader_opcode_names[] = [VKD3DSIH_BEM ] = "bem", [VKD3DSIH_BFI ] = "bfi", [VKD3DSIH_BFREV ] = "bfrev", + [VKD3DSIH_BRANCH ] = "branch", [VKD3DSIH_BREAK ] = "break", [VKD3DSIH_BREAKC ] = "breakc", [VKD3DSIH_BREAKP ] = "breakp", @@ -841,6 +842,13 @@ static void shader_print_bool_literal(struct vkd3d_d3d_asm_compiler *compiler, compiler->colours.literal, b ? "true" : "false", compiler->colours.reset, suffix); } +static void shader_print_label_id(struct vkd3d_d3d_asm_compiler *compiler, + const char *prefix, unsigned int label_id) +{ + vkd3d_string_buffer_printf(&compiler->buffer, "%s%sl%u%s", prefix, + compiler->colours.reg, label_id, compiler->colours.reset); +} + static void shader_print_subscript(struct vkd3d_d3d_asm_compiler *compiler, unsigned int offset, const struct vkd3d_shader_src_param *rel_addr) { @@ -873,6 +881,14 @@ static void shader_dump_register(struct vkd3d_d3d_asm_compiler *compiler, const static const char * const rastout_reg_names[] = {"oPos", "oFog", "oPts"}; static const char * const misctype_reg_names[] = {"vPos", "vFace"}; + if (reg->type == VKD3DSPR_LABEL) + { + shader_print_label_id(compiler, "", reg->idx[0].offset); + if (reg->idx_count > 1) + shader_print_label_id(compiler, ", ", reg->idx[1].offset); + return; + } + shader_addline(buffer, "%s", compiler->colours.reg); switch (reg->type) { @@ -938,10 +954,6 @@ static void shader_dump_register(struct vkd3d_d3d_asm_compiler *compiler, const shader_addline(buffer, "b"); break; - case VKD3DSPR_LABEL: - shader_addline(buffer, "l"); - break; - case VKD3DSPR_LOOP: shader_addline(buffer, "aL"); break; @@ -1563,6 +1575,10 @@ static void shader_dump_instruction_flags(struct vkd3d_d3d_asm_compiler *compile switch (ins->handler_idx) { + case VKD3DSIH_BRANCH: + if (ins->src_count < 3) + break; + /* fall through */ case VKD3DSIH_BREAKP: case VKD3DSIH_CONTINUEP: case VKD3DSIH_DISCARD: diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index 1a5696197..4bd15a4b3 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -1427,6 +1427,331 @@ static enum vkd3d_result normalise_combined_samplers(struct vkd3d_shader_parser return VKD3D_OK; } +struct cf_flattener_if_info +{ + struct vkd3d_shader_src_param *targets; + uint32_t merge_block_id; + unsigned int else_block_id; +}; + +struct cf_flattener_info +{ + union + { + struct cf_flattener_if_info if_; + } u; + + enum + { + VKD3D_BLOCK_IF, + } current_block; + bool inside_block; +}; + +struct cf_flattener +{ + struct vkd3d_shader_parser *parser; + + struct vkd3d_shader_location location; + bool allocation_failed; + + struct vkd3d_shader_instruction *instructions; + size_t instruction_capacity; + size_t instruction_count; + + unsigned int block_id; + + unsigned int control_flow_depth; + struct cf_flattener_info *control_flow_info; + size_t control_flow_info_size; +}; + +static struct vkd3d_shader_instruction *cf_flattener_require_space(struct cf_flattener *flattener, size_t count) +{ + if (!vkd3d_array_reserve((void **)&flattener->instructions, &flattener->instruction_capacity, + flattener->instruction_count + count, sizeof(*flattener->instructions))) + { + ERR("Failed to allocate instructions.\n"); + flattener->allocation_failed = true; + return NULL; + } + return &flattener->instructions[flattener->instruction_count]; +} + +static bool cf_flattener_copy_instruction(struct cf_flattener *flattener, + const struct vkd3d_shader_instruction *instruction) +{ + struct vkd3d_shader_instruction *dst_ins; + + if (instruction->handler_idx == VKD3DSIH_NOP) + return true; + + if (!(dst_ins = cf_flattener_require_space(flattener, 1))) + return false; + + *dst_ins = *instruction; + ++flattener->instruction_count; + return true; +} + +static unsigned int cf_flattener_alloc_block_id(struct cf_flattener *flattener) +{ + return ++flattener->block_id; +} + +static struct vkd3d_shader_src_param *instruction_src_params_alloc(struct vkd3d_shader_instruction *ins, + unsigned int count, struct cf_flattener *flattener) +{ + struct vkd3d_shader_src_param *params = shader_parser_get_src_params(flattener->parser, count); + if (!params) + { + flattener->allocation_failed = true; + return NULL; + } + ins->src = params; + ins->src_count = count; + return params; +} + +static void src_param_init_label(struct vkd3d_shader_src_param *param, unsigned int label_id0, unsigned int label_id1) +{ + param->swizzle = 0; + param->modifiers = VKD3DSPSM_NONE; + vsir_register_init(¶m->reg, VKD3DSPR_LABEL, VKD3D_DATA_UINT, 1 + !!label_id1); + param->reg.dimension = VSIR_DIMENSION_NONE; + param->reg.idx[0].offset = label_id0; + param->reg.idx[1].offset = label_id1; +} + +static void cf_flattener_emit_label(struct cf_flattener *flattener, unsigned int label_id) +{ + struct vkd3d_shader_src_param *src_param; + struct vkd3d_shader_instruction *ins; + + if (!(ins = cf_flattener_require_space(flattener, 1))) + return; + vsir_instruction_init(ins, &flattener->location, VKD3DSIH_LABEL); + + if (!(src_param = instruction_src_params_alloc(ins, 1, flattener))) + return; + src_param_init_label(src_param, label_id, 0); + + ++flattener->instruction_count; +} + +/* For conditional branches, this returns the true+false target branch parameter. */ +static struct vkd3d_shader_src_param *cf_flattener_emit_branch(struct cf_flattener *flattener, + unsigned int merge_block_id, const struct vkd3d_shader_src_param *condition, + unsigned int true_id, unsigned int false_id, + unsigned int flags) +{ + struct vkd3d_shader_src_param *src_params, *target_branch_param; + struct vkd3d_shader_instruction *ins; + + if (!(ins = cf_flattener_require_space(flattener, 1))) + return NULL; + vsir_instruction_init(ins, &flattener->location, VKD3DSIH_BRANCH); + + if (condition) + { + if (!(src_params = instruction_src_params_alloc(ins, 3, flattener))) + return NULL; + src_params[0] = *condition; + src_param_init_label(&src_params[1], true_id, false_id); + src_param_init_label(&src_params[2], merge_block_id, 0); + target_branch_param = &src_params[1]; + } + else + { + if (!(src_params = instruction_src_params_alloc(ins, 1 + !!merge_block_id, flattener))) + return NULL; + src_param_init_label(&src_params[0], true_id, 0); + if (merge_block_id) + src_param_init_label(&src_params[1], merge_block_id, 0); + target_branch_param = NULL; + } + + ins->flags = flags; + ++flattener->instruction_count; + + return target_branch_param; +} + +static void cf_flattener_emit_unconditional_branch(struct cf_flattener *flattener, unsigned int target_block_id) +{ + cf_flattener_emit_branch(flattener, 0, NULL, target_block_id, 0, 0); +} + +static struct cf_flattener_info *cf_flattener_push_control_flow_level(struct cf_flattener *flattener) +{ + if (!vkd3d_array_reserve((void **)&flattener->control_flow_info, &flattener->control_flow_info_size, + flattener->control_flow_depth + 1, sizeof(*flattener->control_flow_info))) + { + ERR("Failed to allocate control flow info structure.\n"); + flattener->allocation_failed = true; + return NULL; + } + + return &flattener->control_flow_info[flattener->control_flow_depth++]; +} + +static void cf_flattener_pop_control_flow_level(struct cf_flattener *flattener) +{ + struct cf_flattener_info *cf_info; + + cf_info = &flattener->control_flow_info[--flattener->control_flow_depth]; + memset(cf_info, 0, sizeof(*cf_info)); +} + +static enum vkd3d_result cf_flattener_iterate_instruction_array(struct cf_flattener *flattener) +{ + struct vkd3d_shader_parser *parser = flattener->parser; + struct vkd3d_shader_instruction_array *instructions; + struct vkd3d_shader_instruction *dst_ins; + unsigned int depth = 0; + bool main_block_open; + size_t i; + + instructions = &parser->instructions; + main_block_open = parser->shader_version.type != VKD3D_SHADER_TYPE_HULL; + + if (!cf_flattener_require_space(flattener, instructions->count)) + return VKD3D_ERROR_OUT_OF_MEMORY; + + for (i = 0; i < instructions->count; ++i) + { + const struct vkd3d_shader_instruction *instruction = &instructions->elements[i]; + const struct vkd3d_shader_src_param *src = instruction->src; + unsigned int merge_block_id, true_block_id; + struct cf_flattener_info *cf_info; + + flattener->location = instruction->location; + + cf_info = flattener->control_flow_depth + ? &flattener->control_flow_info[flattener->control_flow_depth - 1] : NULL; + + switch (instruction->handler_idx) + { + case VKD3DSIH_LABEL: + vkd3d_shader_parser_error(parser, VKD3D_SHADER_ERROR_VSIR_NOT_IMPLEMENTED, + "Aborting due to not yet implemented feature: Label instruction."); + return VKD3D_ERROR_NOT_IMPLEMENTED; + + case VKD3DSIH_IF: + if (!(cf_info = cf_flattener_push_control_flow_level(flattener))) + return VKD3D_ERROR_OUT_OF_MEMORY; + + true_block_id = cf_flattener_alloc_block_id(flattener); + merge_block_id = cf_flattener_alloc_block_id(flattener); + cf_info->u.if_.targets = cf_flattener_emit_branch(flattener, merge_block_id, + src, true_block_id, merge_block_id, instruction->flags); + if (!cf_info->u.if_.targets) + return VKD3D_ERROR_OUT_OF_MEMORY; + + cf_flattener_emit_label(flattener, true_block_id); + + cf_info->u.if_.merge_block_id = merge_block_id; + cf_info->u.if_.else_block_id = 0; + cf_info->inside_block = true; + cf_info->current_block = VKD3D_BLOCK_IF; + break; + + case VKD3DSIH_ELSE: + if (cf_info->inside_block) + cf_flattener_emit_unconditional_branch(flattener, cf_info->u.if_.merge_block_id); + + cf_info->u.if_.else_block_id = cf_flattener_alloc_block_id(flattener); + cf_info->u.if_.targets->reg.idx[1].offset = cf_info->u.if_.else_block_id; + + cf_flattener_emit_label(flattener, cf_info->u.if_.else_block_id); + + cf_info->inside_block = true; + break; + + case VKD3DSIH_ENDIF: + if (cf_info->inside_block) + cf_flattener_emit_unconditional_branch(flattener, cf_info->u.if_.merge_block_id); + + cf_flattener_emit_label(flattener, cf_info->u.if_.merge_block_id); + + cf_flattener_pop_control_flow_level(flattener); + break; + + case VKD3DSIH_LOOP: + case VKD3DSIH_SWITCH: + ++depth; + if (!cf_flattener_copy_instruction(flattener, instruction)) + return VKD3D_ERROR_OUT_OF_MEMORY; + break; + + case VKD3DSIH_ENDLOOP: + case VKD3DSIH_ENDSWITCH: + --depth; + if (cf_info) + cf_info->inside_block = true; /* Dead code removal ensures this is correct. */ + if (!cf_flattener_copy_instruction(flattener, instruction)) + return VKD3D_ERROR_OUT_OF_MEMORY; + break; + + case VKD3DSIH_RET: + if (!cf_flattener_copy_instruction(flattener, instruction)) + return VKD3D_ERROR_OUT_OF_MEMORY; + + if (cf_info) + cf_info->inside_block = false; + else if (!depth) + main_block_open = false; + break; + + case VKD3DSIH_BREAK: + case VKD3DSIH_CONTINUE: + if (cf_info) + cf_info->inside_block = false; + /* fall through */ + default: + if (!cf_flattener_copy_instruction(flattener, instruction)) + return VKD3D_ERROR_OUT_OF_MEMORY; + break; + } + } + + if (main_block_open) + { + if (!(dst_ins = cf_flattener_require_space(flattener, 1))) + return VKD3D_ERROR_OUT_OF_MEMORY; + vsir_instruction_init(dst_ins, &flattener->location, VKD3DSIH_RET); + ++flattener->instruction_count; + } + + return flattener->allocation_failed ? VKD3D_ERROR_OUT_OF_MEMORY : VKD3D_OK; +} + +static enum vkd3d_result flatten_control_flow_constructs(struct vkd3d_shader_parser *parser) +{ + struct cf_flattener flattener = {0}; + enum vkd3d_result result; + + flattener.parser = parser; + result = cf_flattener_iterate_instruction_array(&flattener); + + if (result >= 0) + { + vkd3d_free(parser->instructions.elements); + parser->instructions.elements = flattener.instructions; + parser->instructions.capacity = flattener.instruction_capacity; + parser->instructions.count = flattener.instruction_count; + parser->shader_desc.block_count = flattener.block_id; + } + else + { + vkd3d_free(flattener.instructions); + } + + vkd3d_free(flattener.control_flow_info); + + return result; +} + enum vkd3d_result vkd3d_shader_normalise(struct vkd3d_shader_parser *parser, const struct vkd3d_shader_compile_info *compile_info) { @@ -1455,6 +1780,9 @@ enum vkd3d_result vkd3d_shader_normalise(struct vkd3d_shader_parser *parser, if (result >= 0) remove_dead_code(parser); + if (result >= 0) + result = flatten_control_flow_constructs(parser); + if (result >= 0) result = normalise_combined_samplers(parser); diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 170c76c70..f924f670f 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -855,20 +855,6 @@ static void vkd3d_spirv_end_function_stream_insertion(struct vkd3d_spirv_builder builder->insertion_location = ~(size_t)0; } -struct vkd3d_spirv_op_branch_conditional -{ - uint32_t opcode; - uint32_t condition_id; - uint32_t true_label; - uint32_t false_label; -}; - -static struct vkd3d_spirv_op_branch_conditional *vkd3d_spirv_as_op_branch_conditional( - struct vkd3d_spirv_stream *stream, size_t location) -{ - return (struct vkd3d_spirv_op_branch_conditional *)&stream->words[location]; -} - static void vkd3d_spirv_build_op_capability(struct vkd3d_spirv_stream *stream, SpvCapability cap) { @@ -2254,14 +2240,6 @@ static const char *debug_vkd3d_symbol(const struct vkd3d_symbol *symbol) } } -struct vkd3d_if_cf_info -{ - size_t stream_location; - unsigned int id; - uint32_t merge_block_id; - uint32_t else_block_id; -}; - struct vkd3d_loop_cf_info { uint32_t header_block_id; @@ -2285,14 +2263,12 @@ struct vkd3d_control_flow_info { union { - struct vkd3d_if_cf_info if_; struct vkd3d_loop_cf_info loop; struct vkd3d_switch_cf_info switch_; } u; enum { - VKD3D_BLOCK_IF, VKD3D_BLOCK_LOOP, VKD3D_BLOCK_SWITCH, } current_block; @@ -2352,7 +2328,6 @@ struct spirv_compiler enum vkd3d_shader_type shader_type; - unsigned int branch_id; unsigned int loop_id; unsigned int switch_id; unsigned int control_flow_depth; @@ -2367,7 +2342,6 @@ struct spirv_compiler struct vkd3d_push_constant_buffer_binding *push_constants; const struct vkd3d_shader_spirv_target_info *spirv_target_info; - bool main_block_open; bool after_declarations_section; struct shader_signature input_signature; struct shader_signature output_signature; @@ -2409,6 +2383,9 @@ struct spirv_compiler unsigned int ssa_register_count; uint64_t config_flags; + + uint32_t *block_label_ids; + unsigned int block_count; }; static bool is_in_default_phase(const struct spirv_compiler *compiler) @@ -2458,6 +2435,7 @@ static void spirv_compiler_destroy(struct spirv_compiler *compiler) shader_signature_cleanup(&compiler->patch_constant_signature); vkd3d_free(compiler->ssa_register_info); + vkd3d_free(compiler->block_label_ids); vkd3d_free(compiler); } @@ -2793,6 +2771,14 @@ static struct vkd3d_string_buffer *vkd3d_shader_register_range_string(struct spi return buffer; } +static uint32_t spirv_compiler_get_label_id(struct spirv_compiler *compiler, unsigned int block_id) +{ + --block_id; + if (!compiler->block_label_ids[block_id]) + compiler->block_label_ids[block_id] = vkd3d_spirv_alloc_id(&compiler->spirv_builder); + return compiler->block_label_ids[block_id]; +} + static struct vkd3d_shader_descriptor_binding spirv_compiler_get_descriptor_binding( struct spirv_compiler *compiler, const struct vkd3d_shader_register *reg, const struct vkd3d_shader_register_range *range, enum vkd3d_shader_resource_type resource_type, @@ -5510,7 +5496,6 @@ static void spirv_compiler_emit_initial_declarations(struct spirv_compiler *comp if (compiler->shader_type != VKD3D_SHADER_TYPE_HULL) { vkd3d_spirv_builder_begin_main_function(builder); - compiler->main_block_open = true; } } @@ -7618,72 +7603,27 @@ static struct vkd3d_control_flow_info *spirv_compiler_find_innermost_breakable_c return NULL; } +static void spirv_compiler_emit_label(struct spirv_compiler *compiler, + const struct vkd3d_shader_instruction *instruction); + static int spirv_compiler_emit_control_flow_instruction(struct spirv_compiler *compiler, const struct vkd3d_shader_instruction *instruction) { uint32_t loop_header_block_id, loop_body_block_id, continue_block_id; struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; const struct vkd3d_shader_src_param *src = instruction->src; - uint32_t merge_block_id, val_id, condition_id, true_label; struct vkd3d_control_flow_info *cf_info; + uint32_t merge_block_id, val_id; cf_info = compiler->control_flow_depth ? &compiler->control_flow_info[compiler->control_flow_depth - 1] : NULL; switch (instruction->handler_idx) { - case VKD3DSIH_IF: - if (!(cf_info = spirv_compiler_push_control_flow_level(compiler))) - return VKD3D_ERROR_OUT_OF_MEMORY; - - val_id = spirv_compiler_emit_load_src(compiler, src, VKD3DSP_WRITEMASK_0); - condition_id = spirv_compiler_emit_int_to_bool(compiler, instruction->flags, 1, val_id); - - true_label = vkd3d_spirv_alloc_id(builder); - merge_block_id = vkd3d_spirv_alloc_id(builder); - vkd3d_spirv_build_op_selection_merge(builder, merge_block_id, SpvSelectionControlMaskNone); - cf_info->u.if_.stream_location = vkd3d_spirv_stream_current_location(&builder->function_stream); - vkd3d_spirv_build_op_branch_conditional(builder, condition_id, true_label, merge_block_id); - - vkd3d_spirv_build_op_label(builder, true_label); - - cf_info->u.if_.id = compiler->branch_id; - cf_info->u.if_.merge_block_id = merge_block_id; - cf_info->u.if_.else_block_id = 0; - cf_info->inside_block = true; - cf_info->current_block = VKD3D_BLOCK_IF; - - vkd3d_spirv_build_op_name(builder, merge_block_id, "branch%u_merge", compiler->branch_id); - vkd3d_spirv_build_op_name(builder, true_label, "branch%u_true", compiler->branch_id); - ++compiler->branch_id; - break; - - case VKD3DSIH_ELSE: - assert(compiler->control_flow_depth); - assert(cf_info->current_block == VKD3D_BLOCK_IF); - - if (cf_info->inside_block) - vkd3d_spirv_build_op_branch(builder, cf_info->u.if_.merge_block_id); - - cf_info->u.if_.else_block_id = vkd3d_spirv_alloc_id(builder); - vkd3d_spirv_as_op_branch_conditional(&builder->function_stream, - cf_info->u.if_.stream_location)->false_label = cf_info->u.if_.else_block_id; - vkd3d_spirv_build_op_name(builder, - cf_info->u.if_.else_block_id, "branch%u_false", cf_info->u.if_.id); - vkd3d_spirv_build_op_label(builder, cf_info->u.if_.else_block_id); - cf_info->inside_block = true; - break; - - case VKD3DSIH_ENDIF: - assert(compiler->control_flow_depth); - assert(cf_info->current_block == VKD3D_BLOCK_IF); - - if (cf_info->inside_block) - vkd3d_spirv_build_op_branch(builder, cf_info->u.if_.merge_block_id); - - vkd3d_spirv_build_op_label(builder, cf_info->u.if_.merge_block_id); - - spirv_compiler_pop_control_flow_level(compiler); + case VKD3DSIH_LABEL: + if (cf_info) + cf_info->inside_block = true; + spirv_compiler_emit_label(compiler, instruction); break; case VKD3DSIH_LOOP: @@ -7920,8 +7860,6 @@ static int spirv_compiler_emit_control_flow_instruction(struct spirv_compiler *c if (cf_info) cf_info->inside_block = false; - else - compiler->main_block_open = false; break; default: @@ -7932,6 +7870,71 @@ static int spirv_compiler_emit_control_flow_instruction(struct spirv_compiler *c return VKD3D_OK; } +static bool spirv_compiler_init_blocks(struct spirv_compiler *compiler, unsigned int block_count) +{ + compiler->block_count = block_count; + + if (!(compiler->block_label_ids = vkd3d_calloc(block_count, sizeof(*compiler->block_label_ids)))) + return false; + + return true; +} + +static void spirv_compiler_emit_label(struct spirv_compiler *compiler, + const struct vkd3d_shader_instruction *instruction) +{ + struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; + const struct vkd3d_shader_src_param *src = instruction->src; + unsigned int block_id = src->reg.idx[0].offset; + uint32_t label_id; + + label_id = spirv_compiler_get_label_id(compiler, block_id); + vkd3d_spirv_build_op_label(builder, label_id); +} + +static void spirv_compiler_emit_merge(struct spirv_compiler *compiler, + uint32_t merge_block_id) +{ + struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; + + if (!merge_block_id) + return; + + merge_block_id = spirv_compiler_get_label_id(compiler, merge_block_id); + vkd3d_spirv_build_op_selection_merge(builder, merge_block_id, SpvSelectionControlMaskNone); +} + +static void spirv_compiler_emit_branch(struct spirv_compiler *compiler, + const struct vkd3d_shader_instruction *instruction) +{ + struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; + const struct vkd3d_shader_src_param *src = instruction->src; + uint32_t condition_id; + + if (instruction->src_count <= 2) + { + if (instruction->src_count == 2) + spirv_compiler_emit_merge(compiler, src[1].reg.idx[0].offset); + vkd3d_spirv_build_op_branch(builder, spirv_compiler_get_label_id(compiler, src[0].reg.idx[0].offset)); + return; + } + + if (!vkd3d_swizzle_is_scalar(src->swizzle)) + { + WARN("Unexpected src swizzle %#x.\n", src->swizzle); + spirv_compiler_warning(compiler, VKD3D_SHADER_WARNING_SPV_INVALID_SWIZZLE, + "The swizzle for a branch condition value is not scalar."); + } + + condition_id = spirv_compiler_emit_load_src(compiler, &src[0], VKD3DSP_WRITEMASK_0); + condition_id = spirv_compiler_emit_int_to_bool(compiler, instruction->flags, 1, condition_id); + /* Emit the merge immediately before the branch instruction. */ + spirv_compiler_emit_merge(compiler, src[2].reg.idx[0].offset); + vkd3d_spirv_build_op_branch_conditional(builder, condition_id, + spirv_compiler_get_label_id(compiler, src[1].reg.idx[0].offset), + spirv_compiler_get_label_id(compiler, src[1].reg.idx[1].offset)); +} + static void spirv_compiler_emit_deriv_instruction(struct spirv_compiler *compiler, const struct vkd3d_shader_instruction *instruction) { @@ -9682,11 +9685,9 @@ static int spirv_compiler_handle_instruction(struct spirv_compiler *compiler, case VKD3DSIH_CONTINUE: case VKD3DSIH_CONTINUEP: case VKD3DSIH_DEFAULT: - case VKD3DSIH_ELSE: - case VKD3DSIH_ENDIF: case VKD3DSIH_ENDLOOP: case VKD3DSIH_ENDSWITCH: - case VKD3DSIH_IF: + case VKD3DSIH_LABEL: case VKD3DSIH_LOOP: case VKD3DSIH_RET: case VKD3DSIH_SWITCH: @@ -9699,6 +9700,9 @@ static int spirv_compiler_handle_instruction(struct spirv_compiler *compiler, case VKD3DSIH_TEXKILL: spirv_compiler_emit_kill(compiler, instruction); break; + case VKD3DSIH_BRANCH: + spirv_compiler_emit_branch(compiler, instruction); + break; case VKD3DSIH_DSX: case VKD3DSIH_DSX_COARSE: case VKD3DSIH_DSX_FINE: @@ -9919,6 +9923,9 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler, if ((result = vkd3d_shader_normalise(parser, compile_info)) < 0) return result; + if (parser->shader_desc.block_count && !spirv_compiler_init_blocks(compiler, parser->shader_desc.block_count)) + return VKD3D_ERROR_OUT_OF_MEMORY; + instructions = parser->instructions; memset(&parser->instructions, 0, sizeof(parser->instructions)); @@ -9949,9 +9956,6 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler, if (result < 0) return result; - if (compiler->main_block_open) - vkd3d_spirv_build_op_return(builder); - if (!is_in_default_phase(compiler)) spirv_compiler_leave_shader_phase(compiler); else diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index bdb7163e3..c122fc59a 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -237,6 +237,7 @@ enum vkd3d_shader_opcode VKD3DSIH_BEM, VKD3DSIH_BFI, VKD3DSIH_BFREV, + VKD3DSIH_BRANCH, VKD3DSIH_BREAK, VKD3DSIH_BREAKC, VKD3DSIH_BREAKP, @@ -1007,6 +1008,7 @@ struct vkd3d_shader_desc uint32_t temp_count; unsigned int ssa_count; + unsigned int block_count; struct { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/450
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/ir.c | 189 +++++++++++++++++++++++++++++++++----- libs/vkd3d-shader/spirv.c | 147 +++-------------------------- 2 files changed, 182 insertions(+), 154 deletions(-) diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index 4bd15a4b3..4ecb67e88 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -1434,16 +1434,26 @@ struct cf_flattener_if_info unsigned int else_block_id; }; +struct cf_flattener_loop_info +{ + unsigned int header_block_id; + unsigned int continue_block_id; + uint32_t merge_block_id; +}; + struct cf_flattener_info { union { struct cf_flattener_if_info if_; + struct cf_flattener_loop_info loop; } u; enum { VKD3D_BLOCK_IF, + VKD3D_BLOCK_LOOP, + VKD3D_BLOCK_SWITCH, } current_block; bool inside_block; }; @@ -1541,8 +1551,8 @@ static void cf_flattener_emit_label(struct cf_flattener *flattener, unsigned int /* For conditional branches, this returns the true+false target branch parameter. */ static struct vkd3d_shader_src_param *cf_flattener_emit_branch(struct cf_flattener *flattener, - unsigned int merge_block_id, const struct vkd3d_shader_src_param *condition, - unsigned int true_id, unsigned int false_id, + unsigned int merge_block_id, unsigned int continue_block_id, + const struct vkd3d_shader_src_param *condition, unsigned int true_id, unsigned int false_id, unsigned int flags) { struct vkd3d_shader_src_param *src_params, *target_branch_param; @@ -1558,16 +1568,16 @@ static struct vkd3d_shader_src_param *cf_flattener_emit_branch(struct cf_flatten return NULL; src_params[0] = *condition; src_param_init_label(&src_params[1], true_id, false_id); - src_param_init_label(&src_params[2], merge_block_id, 0); + src_param_init_label(&src_params[2], merge_block_id, continue_block_id); target_branch_param = &src_params[1]; } else { - if (!(src_params = instruction_src_params_alloc(ins, 1 + !!merge_block_id, flattener))) + if (!(src_params = instruction_src_params_alloc(ins, 1 + (merge_block_id || continue_block_id), flattener))) return NULL; src_param_init_label(&src_params[0], true_id, 0); - if (merge_block_id) - src_param_init_label(&src_params[1], merge_block_id, 0); + if (merge_block_id || continue_block_id) + src_param_init_label(&src_params[1], merge_block_id, continue_block_id); target_branch_param = NULL; } @@ -1577,9 +1587,19 @@ static struct vkd3d_shader_src_param *cf_flattener_emit_branch(struct cf_flatten return target_branch_param; } +static void cf_flattener_emit_conditional_branch_and_merge(struct cf_flattener *flattener, + const struct vkd3d_shader_src_param *condition, unsigned int true_id, unsigned int flags) +{ + unsigned int merge_block_id; + + merge_block_id = cf_flattener_alloc_block_id(flattener); + cf_flattener_emit_branch(flattener, merge_block_id, 0, condition, true_id, merge_block_id, flags); + cf_flattener_emit_label(flattener, merge_block_id); +} + static void cf_flattener_emit_unconditional_branch(struct cf_flattener *flattener, unsigned int target_block_id) { - cf_flattener_emit_branch(flattener, 0, NULL, target_block_id, 0, 0); + cf_flattener_emit_branch(flattener, 0, 0, NULL, target_block_id, 0, 0); } static struct cf_flattener_info *cf_flattener_push_control_flow_level(struct cf_flattener *flattener) @@ -1603,12 +1623,38 @@ static void cf_flattener_pop_control_flow_level(struct cf_flattener *flattener) memset(cf_info, 0, sizeof(*cf_info)); } +static struct cf_flattener_info *cf_flattener_find_innermost_loop(struct cf_flattener *flattener) +{ + int depth; + + for (depth = flattener->control_flow_depth - 1; depth >= 0; --depth) + { + if (flattener->control_flow_info[depth].current_block == VKD3D_BLOCK_LOOP) + return &flattener->control_flow_info[depth]; + } + + return NULL; +} + +static struct cf_flattener_info *cf_flattener_find_innermost_breakable_cf_construct(struct cf_flattener *flattener) +{ + int depth; + + for (depth = flattener->control_flow_depth - 1; depth >= 0; --depth) + { + if (flattener->control_flow_info[depth].current_block == VKD3D_BLOCK_LOOP + || flattener->control_flow_info[depth].current_block == VKD3D_BLOCK_SWITCH) + return &flattener->control_flow_info[depth]; + } + + return NULL; +} + static enum vkd3d_result cf_flattener_iterate_instruction_array(struct cf_flattener *flattener) { struct vkd3d_shader_parser *parser = flattener->parser; struct vkd3d_shader_instruction_array *instructions; struct vkd3d_shader_instruction *dst_ins; - unsigned int depth = 0; bool main_block_open; size_t i; @@ -1620,9 +1666,9 @@ static enum vkd3d_result cf_flattener_iterate_instruction_array(struct cf_flatte for (i = 0; i < instructions->count; ++i) { + unsigned int loop_header_block_id, loop_body_block_id, continue_block_id, merge_block_id, true_block_id; const struct vkd3d_shader_instruction *instruction = &instructions->elements[i]; const struct vkd3d_shader_src_param *src = instruction->src; - unsigned int merge_block_id, true_block_id; struct cf_flattener_info *cf_info; flattener->location = instruction->location; @@ -1643,7 +1689,7 @@ static enum vkd3d_result cf_flattener_iterate_instruction_array(struct cf_flatte true_block_id = cf_flattener_alloc_block_id(flattener); merge_block_id = cf_flattener_alloc_block_id(flattener); - cf_info->u.if_.targets = cf_flattener_emit_branch(flattener, merge_block_id, + cf_info->u.if_.targets = cf_flattener_emit_branch(flattener, merge_block_id, 0, src, true_block_id, merge_block_id, instruction->flags); if (!cf_info->u.if_.targets) return VKD3D_ERROR_OUT_OF_MEMORY; @@ -1678,36 +1724,137 @@ static enum vkd3d_result cf_flattener_iterate_instruction_array(struct cf_flatte break; case VKD3DSIH_LOOP: + if (!(cf_info = cf_flattener_push_control_flow_level(flattener))) + return VKD3D_ERROR_OUT_OF_MEMORY; + + loop_header_block_id = cf_flattener_alloc_block_id(flattener); + loop_body_block_id = cf_flattener_alloc_block_id(flattener); + continue_block_id = cf_flattener_alloc_block_id(flattener); + merge_block_id = cf_flattener_alloc_block_id(flattener); + + cf_flattener_emit_unconditional_branch(flattener, loop_header_block_id); + cf_flattener_emit_label(flattener, loop_header_block_id); + cf_flattener_emit_branch(flattener, merge_block_id, continue_block_id, + NULL, loop_body_block_id, 0, 0); + + cf_flattener_emit_label(flattener, loop_body_block_id); + + cf_info->u.loop.header_block_id = loop_header_block_id; + cf_info->u.loop.continue_block_id = continue_block_id; + cf_info->u.loop.merge_block_id = merge_block_id; + cf_info->current_block = VKD3D_BLOCK_LOOP; + cf_info->inside_block = true; + break; + + case VKD3DSIH_ENDLOOP: + if (cf_info->inside_block) + cf_flattener_emit_unconditional_branch(flattener, cf_info->u.loop.continue_block_id); + + cf_flattener_emit_label(flattener, cf_info->u.loop.continue_block_id); + cf_flattener_emit_unconditional_branch(flattener, cf_info->u.loop.header_block_id); + cf_flattener_emit_label(flattener, cf_info->u.loop.merge_block_id); + + cf_flattener_pop_control_flow_level(flattener); + break; + case VKD3DSIH_SWITCH: - ++depth; + if (!(cf_info = cf_flattener_push_control_flow_level(flattener))) + return VKD3D_ERROR_OUT_OF_MEMORY; + if (!cf_flattener_copy_instruction(flattener, instruction)) return VKD3D_ERROR_OUT_OF_MEMORY; + + cf_info->current_block = VKD3D_BLOCK_SWITCH; + break; - case VKD3DSIH_ENDLOOP: case VKD3DSIH_ENDSWITCH: - --depth; - if (cf_info) - cf_info->inside_block = true; /* Dead code removal ensures this is correct. */ + cf_flattener_pop_control_flow_level(flattener); + if (!cf_flattener_copy_instruction(flattener, instruction)) return VKD3D_ERROR_OUT_OF_MEMORY; break; + case VKD3DSIH_BREAK: + { + struct cf_flattener_info *breakable_cf_info; + + if (!(breakable_cf_info = cf_flattener_find_innermost_breakable_cf_construct(flattener))) + { + FIXME("Unhandled break instruction.\n"); + return VKD3D_ERROR_INVALID_SHADER; + } + + if (breakable_cf_info->current_block == VKD3D_BLOCK_LOOP) + { + cf_flattener_emit_unconditional_branch(flattener, breakable_cf_info->u.loop.merge_block_id); + } + else if (breakable_cf_info->current_block == VKD3D_BLOCK_SWITCH) + { + if (!cf_flattener_copy_instruction(flattener, instruction)) + return VKD3D_ERROR_OUT_OF_MEMORY; + } + + cf_info->inside_block = false; + break; + } + + case VKD3DSIH_BREAKP: + { + struct cf_flattener_info *loop_cf_info; + + if (!(loop_cf_info = cf_flattener_find_innermost_loop(flattener))) + { + ERR("Invalid 'breakc' instruction outside loop.\n"); + return VKD3D_ERROR_INVALID_SHADER; + } + + cf_flattener_emit_conditional_branch_and_merge(flattener, + src, loop_cf_info->u.loop.merge_block_id, instruction->flags); + break; + } + + case VKD3DSIH_CONTINUE: + { + struct cf_flattener_info *loop_cf_info; + + if (!(loop_cf_info = cf_flattener_find_innermost_loop(flattener))) + { + ERR("Invalid 'continue' instruction outside loop.\n"); + return VKD3D_ERROR_INVALID_SHADER; + } + + cf_flattener_emit_unconditional_branch(flattener, loop_cf_info->u.loop.continue_block_id); + + cf_info->inside_block = false; + break; + } + + case VKD3DSIH_CONTINUEP: + { + struct cf_flattener_info *loop_cf_info; + + if (!(loop_cf_info = cf_flattener_find_innermost_loop(flattener))) + { + ERR("Invalid 'continuec' instruction outside loop.\n"); + return VKD3D_ERROR_INVALID_SHADER; + } + + cf_flattener_emit_conditional_branch_and_merge(flattener, + src, loop_cf_info->u.loop.continue_block_id, instruction->flags); + break; + } + case VKD3DSIH_RET: if (!cf_flattener_copy_instruction(flattener, instruction)) return VKD3D_ERROR_OUT_OF_MEMORY; if (cf_info) cf_info->inside_block = false; - else if (!depth) + else main_block_open = false; break; - case VKD3DSIH_BREAK: - case VKD3DSIH_CONTINUE: - if (cf_info) - cf_info->inside_block = false; - /* fall through */ default: if (!cf_flattener_copy_instruction(flattener, instruction)) return VKD3D_ERROR_OUT_OF_MEMORY; diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index f924f670f..490cdea05 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -2240,13 +2240,6 @@ static const char *debug_vkd3d_symbol(const struct vkd3d_symbol *symbol) } } -struct vkd3d_loop_cf_info -{ - uint32_t header_block_id; - uint32_t continue_block_id; - uint32_t merge_block_id; -}; - struct vkd3d_switch_cf_info { size_t stream_location; @@ -2263,13 +2256,11 @@ struct vkd3d_control_flow_info { union { - struct vkd3d_loop_cf_info loop; struct vkd3d_switch_cf_info switch_; } u; enum { - VKD3D_BLOCK_LOOP, VKD3D_BLOCK_SWITCH, } current_block; bool inside_block; @@ -2328,7 +2319,6 @@ struct spirv_compiler enum vkd3d_shader_type shader_type; - unsigned int loop_id; unsigned int switch_id; unsigned int control_flow_depth; struct vkd3d_control_flow_info *control_flow_info; @@ -7574,20 +7564,6 @@ static void spirv_compiler_pop_control_flow_level(struct spirv_compiler *compile memset(cf_info, 0, sizeof(*cf_info)); } -static struct vkd3d_control_flow_info *spirv_compiler_find_innermost_loop( - struct spirv_compiler *compiler) -{ - int depth; - - for (depth = compiler->control_flow_depth - 1; depth >= 0; --depth) - { - if (compiler->control_flow_info[depth].current_block == VKD3D_BLOCK_LOOP) - return &compiler->control_flow_info[depth]; - } - - return NULL; -} - static struct vkd3d_control_flow_info *spirv_compiler_find_innermost_breakable_cf_construct( struct spirv_compiler *compiler) { @@ -7595,8 +7571,7 @@ static struct vkd3d_control_flow_info *spirv_compiler_find_innermost_breakable_c for (depth = compiler->control_flow_depth - 1; depth >= 0; --depth) { - if (compiler->control_flow_info[depth].current_block == VKD3D_BLOCK_LOOP - || compiler->control_flow_info[depth].current_block == VKD3D_BLOCK_SWITCH) + if (compiler->control_flow_info[depth].current_block == VKD3D_BLOCK_SWITCH) return &compiler->control_flow_info[depth]; } @@ -7609,7 +7584,6 @@ static void spirv_compiler_emit_label(struct spirv_compiler *compiler, static int spirv_compiler_emit_control_flow_instruction(struct spirv_compiler *compiler, const struct vkd3d_shader_instruction *instruction) { - uint32_t loop_header_block_id, loop_body_block_id, continue_block_id; struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; const struct vkd3d_shader_src_param *src = instruction->src; struct vkd3d_control_flow_info *cf_info; @@ -7626,51 +7600,6 @@ static int spirv_compiler_emit_control_flow_instruction(struct spirv_compiler *c spirv_compiler_emit_label(compiler, instruction); break; - case VKD3DSIH_LOOP: - if (!(cf_info = spirv_compiler_push_control_flow_level(compiler))) - return VKD3D_ERROR_OUT_OF_MEMORY; - - loop_header_block_id = vkd3d_spirv_alloc_id(builder); - loop_body_block_id = vkd3d_spirv_alloc_id(builder); - continue_block_id = vkd3d_spirv_alloc_id(builder); - merge_block_id = vkd3d_spirv_alloc_id(builder); - - vkd3d_spirv_build_op_branch(builder, loop_header_block_id); - vkd3d_spirv_build_op_label(builder, loop_header_block_id); - vkd3d_spirv_build_op_loop_merge(builder, merge_block_id, continue_block_id, SpvLoopControlMaskNone); - vkd3d_spirv_build_op_branch(builder, loop_body_block_id); - - vkd3d_spirv_build_op_label(builder, loop_body_block_id); - - cf_info->u.loop.header_block_id = loop_header_block_id; - cf_info->u.loop.continue_block_id = continue_block_id; - cf_info->u.loop.merge_block_id = merge_block_id; - cf_info->current_block = VKD3D_BLOCK_LOOP; - cf_info->inside_block = true; - - vkd3d_spirv_build_op_name(builder, loop_header_block_id, "loop%u_header", compiler->loop_id); - vkd3d_spirv_build_op_name(builder, loop_body_block_id, "loop%u_body", compiler->loop_id); - vkd3d_spirv_build_op_name(builder, continue_block_id, "loop%u_continue", compiler->loop_id); - vkd3d_spirv_build_op_name(builder, merge_block_id, "loop%u_merge", compiler->loop_id); - ++compiler->loop_id; - break; - - case VKD3DSIH_ENDLOOP: - assert(compiler->control_flow_depth); - assert(cf_info->current_block == VKD3D_BLOCK_LOOP); - - /* The loop block may have already been ended by an unconditional - * break instruction right before the end of the loop. */ - if (cf_info->inside_block) - vkd3d_spirv_build_op_branch(builder, cf_info->u.loop.continue_block_id); - - vkd3d_spirv_build_op_label(builder, cf_info->u.loop.continue_block_id); - vkd3d_spirv_build_op_branch(builder, cf_info->u.loop.header_block_id); - vkd3d_spirv_build_op_label(builder, cf_info->u.loop.merge_block_id); - - spirv_compiler_pop_control_flow_level(compiler); - break; - case VKD3DSIH_SWITCH: if (!(cf_info = spirv_compiler_push_control_flow_level(compiler))) return VKD3D_ERROR_OUT_OF_MEMORY; @@ -7787,11 +7716,7 @@ static int spirv_compiler_emit_control_flow_instruction(struct spirv_compiler *c return VKD3D_ERROR_INVALID_SHADER; } - if (breakable_cf_info->current_block == VKD3D_BLOCK_LOOP) - { - vkd3d_spirv_build_op_branch(builder, breakable_cf_info->u.loop.merge_block_id); - } - else if (breakable_cf_info->current_block == VKD3D_BLOCK_SWITCH) + if (breakable_cf_info->current_block == VKD3D_BLOCK_SWITCH) { /* The current case block may have already been ended by an * unconditional continue instruction. */ @@ -7803,58 +7728,6 @@ static int spirv_compiler_emit_control_flow_instruction(struct spirv_compiler *c break; } - case VKD3DSIH_BREAKP: - { - struct vkd3d_control_flow_info *loop_cf_info; - - assert(compiler->control_flow_depth); - - if (!(loop_cf_info = spirv_compiler_find_innermost_loop(compiler))) - { - ERR("Invalid 'breakc' instruction outside loop.\n"); - return VKD3D_ERROR_INVALID_SHADER; - } - - merge_block_id = spirv_compiler_emit_conditional_branch(compiler, - instruction, loop_cf_info->u.loop.merge_block_id); - vkd3d_spirv_build_op_label(builder, merge_block_id); - break; - } - - case VKD3DSIH_CONTINUE: - { - struct vkd3d_control_flow_info *loop_cf_info; - - assert(compiler->control_flow_depth); - - if (!(loop_cf_info = spirv_compiler_find_innermost_loop(compiler))) - { - ERR("Invalid 'continue' instruction outside loop.\n"); - return VKD3D_ERROR_INVALID_SHADER; - } - - vkd3d_spirv_build_op_branch(builder, loop_cf_info->u.loop.continue_block_id); - - cf_info->inside_block = false; - break; - } - - case VKD3DSIH_CONTINUEP: - { - struct vkd3d_control_flow_info *loop_cf_info; - - if (!(loop_cf_info = spirv_compiler_find_innermost_loop(compiler))) - { - ERR("Invalid 'continuec' instruction outside loop.\n"); - return VKD3D_ERROR_INVALID_SHADER; - } - - merge_block_id = spirv_compiler_emit_conditional_branch(compiler, - instruction, loop_cf_info->u.loop.continue_block_id); - vkd3d_spirv_build_op_label(builder, merge_block_id); - break; - } - case VKD3DSIH_RET: spirv_compiler_emit_return(compiler, instruction); @@ -7893,7 +7766,7 @@ static void spirv_compiler_emit_label(struct spirv_compiler *compiler, } static void spirv_compiler_emit_merge(struct spirv_compiler *compiler, - uint32_t merge_block_id) + uint32_t merge_block_id, uint32_t continue_block_id) { struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; @@ -7901,7 +7774,15 @@ static void spirv_compiler_emit_merge(struct spirv_compiler *compiler, return; merge_block_id = spirv_compiler_get_label_id(compiler, merge_block_id); - vkd3d_spirv_build_op_selection_merge(builder, merge_block_id, SpvSelectionControlMaskNone); + if (!continue_block_id) + { + vkd3d_spirv_build_op_selection_merge(builder, merge_block_id, SpvSelectionControlMaskNone); + } + else + { + continue_block_id = spirv_compiler_get_label_id(compiler, continue_block_id); + vkd3d_spirv_build_op_loop_merge(builder, merge_block_id, continue_block_id, SpvLoopControlMaskNone); + } } static void spirv_compiler_emit_branch(struct spirv_compiler *compiler, @@ -7914,7 +7795,7 @@ static void spirv_compiler_emit_branch(struct spirv_compiler *compiler, if (instruction->src_count <= 2) { if (instruction->src_count == 2) - spirv_compiler_emit_merge(compiler, src[1].reg.idx[0].offset); + spirv_compiler_emit_merge(compiler, src[1].reg.idx[0].offset, src[1].reg.idx[1].offset); vkd3d_spirv_build_op_branch(builder, spirv_compiler_get_label_id(compiler, src[0].reg.idx[0].offset)); return; } @@ -7929,7 +7810,7 @@ static void spirv_compiler_emit_branch(struct spirv_compiler *compiler, condition_id = spirv_compiler_emit_load_src(compiler, &src[0], VKD3DSP_WRITEMASK_0); condition_id = spirv_compiler_emit_int_to_bool(compiler, instruction->flags, 1, condition_id); /* Emit the merge immediately before the branch instruction. */ - spirv_compiler_emit_merge(compiler, src[2].reg.idx[0].offset); + spirv_compiler_emit_merge(compiler, src[2].reg.idx[0].offset, src[2].reg.idx[1].offset); vkd3d_spirv_build_op_branch_conditional(builder, condition_id, spirv_compiler_get_label_id(compiler, src[1].reg.idx[0].offset), spirv_compiler_get_label_id(compiler, src[1].reg.idx[1].offset)); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/450
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/d3d_asm.c | 4 + libs/vkd3d-shader/ir.c | 109 ++++++++- libs/vkd3d-shader/spirv.c | 291 ++++------------------- libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 4 files changed, 153 insertions(+), 252 deletions(-) diff --git a/libs/vkd3d-shader/d3d_asm.c b/libs/vkd3d-shader/d3d_asm.c index c47d94b23..5e0456aeb 100644 --- a/libs/vkd3d-shader/d3d_asm.c +++ b/libs/vkd3d-shader/d3d_asm.c @@ -279,6 +279,7 @@ static const char * const shader_opcode_names[] = [VKD3DSIH_SUB ] = "sub", [VKD3DSIH_SWAPC ] = "swapc", [VKD3DSIH_SWITCH ] = "switch", + [VKD3DSIH_SWITCH_MONOLITHIC ] = "switch", [VKD3DSIH_SYNC ] = "sync", [VKD3DSIH_TEX ] = "texld", [VKD3DSIH_TEXBEM ] = "texbem", @@ -883,6 +884,9 @@ static void shader_dump_register(struct vkd3d_d3d_asm_compiler *compiler, const if (reg->type == VKD3DSPR_LABEL) { + /* Scalar dimension indicates a case label. */ + if (reg->dimension == VSIR_DIMENSION_SCALAR) + shader_print_int_literal(compiler, "", reg->u.immconst_uint[0], ", "); shader_print_label_id(compiler, "", reg->idx[0].offset); if (reg->idx_count > 1) shader_print_label_id(compiler, ", ", reg->idx[1].offset); diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index 4ecb67e88..61c455229 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -1441,12 +1441,30 @@ struct cf_flattener_loop_info uint32_t merge_block_id; }; +struct cf_flattener_switch_case +{ + unsigned int value; + unsigned int block_id; +}; + +struct cf_flattener_switch_info +{ + size_t ins_location; + const struct vkd3d_shader_src_param *condition; + unsigned int merge_block_id; + unsigned int default_block_id; + struct cf_flattener_switch_case *cases; + size_t cases_size; + unsigned int cases_count; +}; + struct cf_flattener_info { union { struct cf_flattener_if_info if_; struct cf_flattener_loop_info loop; + struct cf_flattener_switch_info switch_; } u; enum @@ -1761,18 +1779,100 @@ static enum vkd3d_result cf_flattener_iterate_instruction_array(struct cf_flatte if (!(cf_info = cf_flattener_push_control_flow_level(flattener))) return VKD3D_ERROR_OUT_OF_MEMORY; - if (!cf_flattener_copy_instruction(flattener, instruction)) - return VKD3D_ERROR_OUT_OF_MEMORY; + merge_block_id = cf_flattener_alloc_block_id(flattener); + + cf_info->u.switch_.ins_location = flattener->instruction_count; + cf_info->u.switch_.condition = src; + if (!(dst_ins = cf_flattener_require_space(flattener, 1))) + return VKD3D_ERROR_OUT_OF_MEMORY; + vsir_instruction_init(dst_ins, &instruction->location, VKD3DSIH_SWITCH_MONOLITHIC); + ++flattener->instruction_count; + + cf_info->u.switch_.merge_block_id = merge_block_id; + cf_info->u.switch_.cases = NULL; + cf_info->u.switch_.cases_size = 0; + cf_info->u.switch_.cases_count = 0; + cf_info->u.switch_.default_block_id = 0; + cf_info->inside_block = false; cf_info->current_block = VKD3D_BLOCK_SWITCH; + if (!vkd3d_array_reserve((void **)&cf_info->u.switch_.cases, &cf_info->u.switch_.cases_size, + 10, sizeof(*cf_info->u.switch_.cases))) + return VKD3D_ERROR_OUT_OF_MEMORY; + break; case VKD3DSIH_ENDSWITCH: + { + struct vkd3d_shader_src_param *src_params; + unsigned int j; + + if (!cf_info->u.switch_.default_block_id) + cf_info->u.switch_.default_block_id = cf_info->u.switch_.merge_block_id; + + cf_flattener_emit_label(flattener, cf_info->u.switch_.merge_block_id); + + /* The SWITCH instruction is completed when the endswitch + * instruction is processed because we do not know the number + * of case statements or the default block id in advance.*/ + dst_ins = &flattener->instructions[cf_info->u.switch_.ins_location]; + if (!(src_params = instruction_src_params_alloc(dst_ins, cf_info->u.switch_.cases_count + 2, flattener))) + { + vkd3d_free(cf_info->u.switch_.cases); + return VKD3D_ERROR_OUT_OF_MEMORY; + } + src_params[0] = *cf_info->u.switch_.condition; + src_param_init_label(&src_params[1], cf_info->u.switch_.default_block_id, + cf_info->u.switch_.merge_block_id); + for (j = 0; j < cf_info->u.switch_.cases_count; ++j) + { + src_param_init_label(&src_params[j + 2], cf_info->u.switch_.cases[j].block_id, 0); + src_params[j + 2].reg.dimension = VSIR_DIMENSION_SCALAR; + src_params[j + 2].reg.u.immconst_uint[0] = cf_info->u.switch_.cases[j].value; + } + vkd3d_free(cf_info->u.switch_.cases); + cf_flattener_pop_control_flow_level(flattener); + break; + } - if (!cf_flattener_copy_instruction(flattener, instruction)) + case VKD3DSIH_CASE: + { + unsigned int label_id, value; + + if (src->swizzle != VKD3D_SHADER_SWIZZLE(X, X, X, X)) + { + WARN("Unexpected src swizzle %#x.\n", src->swizzle); + vkd3d_shader_parser_error(parser, VKD3D_SHADER_ERROR_VSIR_INVALID_SWIZZLE, + "The swizzle for a switch case value is not scalar X."); + } + value = *src->reg.u.immconst_uint; + + if (!vkd3d_array_reserve((void **)&cf_info->u.switch_.cases, &cf_info->u.switch_.cases_size, + cf_info->u.switch_.cases_count + 1, sizeof(*cf_info->u.switch_.cases))) return VKD3D_ERROR_OUT_OF_MEMORY; + + label_id = cf_flattener_alloc_block_id(flattener); + if (cf_info->inside_block) /* fall-through */ + cf_flattener_emit_unconditional_branch(flattener, label_id); + + cf_info->u.switch_.cases[cf_info->u.switch_.cases_count].value = value; + cf_info->u.switch_.cases[cf_info->u.switch_.cases_count].block_id = label_id; + ++cf_info->u.switch_.cases_count; + + cf_flattener_emit_label(flattener, label_id); + cf_info->inside_block = true; + break; + } + + case VKD3DSIH_DEFAULT: + cf_info->u.switch_.default_block_id = cf_flattener_alloc_block_id(flattener); + if (cf_info->inside_block) /* fall-through */ + cf_flattener_emit_unconditional_branch(flattener, cf_info->u.switch_.default_block_id); + + cf_flattener_emit_label(flattener, cf_info->u.switch_.default_block_id); + cf_info->inside_block = true; break; case VKD3DSIH_BREAK: @@ -1791,8 +1891,7 @@ static enum vkd3d_result cf_flattener_iterate_instruction_array(struct cf_flatte } else if (breakable_cf_info->current_block == VKD3D_BLOCK_SWITCH) { - if (!cf_flattener_copy_instruction(flattener, instruction)) - return VKD3D_ERROR_OUT_OF_MEMORY; + cf_flattener_emit_unconditional_branch(flattener, breakable_cf_info->u.switch_.merge_block_id); } cf_info->inside_block = false; diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 490cdea05..9b9551e0b 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -352,6 +352,12 @@ static bool vkd3d_spirv_stream_append(struct vkd3d_spirv_stream *dst_stream, return true; } +static void vkd3d_spirv_stream_add_word_count(struct vkd3d_spirv_stream *stream, size_t location, + unsigned int word_count) +{ + stream->words[location] += word_count << SpvWordCountShift; +} + struct vkd3d_spirv_builder { uint64_t capability_mask; @@ -2240,32 +2246,6 @@ static const char *debug_vkd3d_symbol(const struct vkd3d_symbol *symbol) } } -struct vkd3d_switch_cf_info -{ - size_t stream_location; - unsigned int id; - uint32_t selector_id; - uint32_t merge_block_id; - uint32_t default_block_id; - uint32_t *case_blocks; - size_t case_blocks_size; - unsigned int case_block_count; -}; - -struct vkd3d_control_flow_info -{ - union - { - struct vkd3d_switch_cf_info switch_; - } u; - - enum - { - VKD3D_BLOCK_SWITCH, - } current_block; - bool inside_block; -}; - struct vkd3d_push_constant_buffer_binding { struct vkd3d_shader_register reg; @@ -2319,11 +2299,6 @@ struct spirv_compiler enum vkd3d_shader_type shader_type; - unsigned int switch_id; - unsigned int control_flow_depth; - struct vkd3d_control_flow_info *control_flow_info; - size_t control_flow_info_size; - struct vkd3d_shader_interface_info shader_interface; struct vkd3d_shader_descriptor_offset_info offset_info; uint32_t descriptor_offsets_member_id; @@ -2405,8 +2380,6 @@ static const char *spirv_compiler_get_entry_point_name(const struct spirv_compil static void spirv_compiler_destroy(struct spirv_compiler *compiler) { - vkd3d_free(compiler->control_flow_info); - vkd3d_free(compiler->output_info); vkd3d_free(compiler->push_constants); @@ -7541,208 +7514,6 @@ static void spirv_compiler_emit_kill(struct spirv_compiler *compiler, vkd3d_spirv_build_op_label(builder, merge_block_id); } -static struct vkd3d_control_flow_info *spirv_compiler_push_control_flow_level( - struct spirv_compiler *compiler) -{ - if (!vkd3d_array_reserve((void **)&compiler->control_flow_info, &compiler->control_flow_info_size, - compiler->control_flow_depth + 1, sizeof(*compiler->control_flow_info))) - { - ERR("Failed to allocate control flow info structure.\n"); - return NULL; - } - - return &compiler->control_flow_info[compiler->control_flow_depth++]; -} - -static void spirv_compiler_pop_control_flow_level(struct spirv_compiler *compiler) -{ - struct vkd3d_control_flow_info *cf_info; - - assert(compiler->control_flow_depth); - - cf_info = &compiler->control_flow_info[--compiler->control_flow_depth]; - memset(cf_info, 0, sizeof(*cf_info)); -} - -static struct vkd3d_control_flow_info *spirv_compiler_find_innermost_breakable_cf_construct( - struct spirv_compiler *compiler) -{ - int depth; - - for (depth = compiler->control_flow_depth - 1; depth >= 0; --depth) - { - if (compiler->control_flow_info[depth].current_block == VKD3D_BLOCK_SWITCH) - return &compiler->control_flow_info[depth]; - } - - return NULL; -} - -static void spirv_compiler_emit_label(struct spirv_compiler *compiler, - const struct vkd3d_shader_instruction *instruction); - -static int spirv_compiler_emit_control_flow_instruction(struct spirv_compiler *compiler, - const struct vkd3d_shader_instruction *instruction) -{ - struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; - const struct vkd3d_shader_src_param *src = instruction->src; - struct vkd3d_control_flow_info *cf_info; - uint32_t merge_block_id, val_id; - - cf_info = compiler->control_flow_depth - ? &compiler->control_flow_info[compiler->control_flow_depth - 1] : NULL; - - switch (instruction->handler_idx) - { - case VKD3DSIH_LABEL: - if (cf_info) - cf_info->inside_block = true; - spirv_compiler_emit_label(compiler, instruction); - break; - - case VKD3DSIH_SWITCH: - if (!(cf_info = spirv_compiler_push_control_flow_level(compiler))) - return VKD3D_ERROR_OUT_OF_MEMORY; - - merge_block_id = vkd3d_spirv_alloc_id(builder); - - assert(src->reg.data_type == VKD3D_DATA_INT); - val_id = spirv_compiler_emit_load_src(compiler, src, VKD3DSP_WRITEMASK_0); - - vkd3d_spirv_build_op_selection_merge(builder, merge_block_id, SpvSelectionControlMaskNone); - - cf_info->u.switch_.id = compiler->switch_id; - cf_info->u.switch_.merge_block_id = merge_block_id; - cf_info->u.switch_.stream_location = vkd3d_spirv_stream_current_location(&builder->function_stream); - cf_info->u.switch_.selector_id = val_id; - cf_info->u.switch_.case_blocks = NULL; - cf_info->u.switch_.case_blocks_size = 0; - cf_info->u.switch_.case_block_count = 0; - cf_info->u.switch_.default_block_id = 0; - cf_info->inside_block = false; - cf_info->current_block = VKD3D_BLOCK_SWITCH; - - vkd3d_spirv_build_op_name(builder, merge_block_id, "switch%u_merge", compiler->switch_id); - - ++compiler->switch_id; - - if (!vkd3d_array_reserve((void **)&cf_info->u.switch_.case_blocks, &cf_info->u.switch_.case_blocks_size, - 10, sizeof(*cf_info->u.switch_.case_blocks))) - return VKD3D_ERROR_OUT_OF_MEMORY; - - break; - - case VKD3DSIH_ENDSWITCH: - assert(compiler->control_flow_depth); - assert(cf_info->current_block == VKD3D_BLOCK_SWITCH); - assert(!cf_info->inside_block); - - if (!cf_info->u.switch_.default_block_id) - cf_info->u.switch_.default_block_id = cf_info->u.switch_.merge_block_id; - - vkd3d_spirv_build_op_label(builder, cf_info->u.switch_.merge_block_id); - - /* The OpSwitch instruction is inserted when the endswitch - * instruction is processed because we do not know the number - * of case statements in advance.*/ - vkd3d_spirv_begin_function_stream_insertion(builder, cf_info->u.switch_.stream_location); - vkd3d_spirv_build_op_switch(builder, cf_info->u.switch_.selector_id, - cf_info->u.switch_.default_block_id, cf_info->u.switch_.case_blocks, - cf_info->u.switch_.case_block_count); - vkd3d_spirv_end_function_stream_insertion(builder); - - vkd3d_free(cf_info->u.switch_.case_blocks); - spirv_compiler_pop_control_flow_level(compiler); - break; - - case VKD3DSIH_CASE: - { - uint32_t label_id, value; - - assert(compiler->control_flow_depth); - assert(cf_info->current_block == VKD3D_BLOCK_SWITCH); - - if (src->swizzle != VKD3D_SHADER_SWIZZLE(X, X, X, X)) - { - WARN("Unexpected src swizzle %#x.\n", src->swizzle); - spirv_compiler_warning(compiler, VKD3D_SHADER_WARNING_SPV_INVALID_SWIZZLE, - "The swizzle for a switch case value is not scalar."); - } - assert(src->reg.type == VKD3DSPR_IMMCONST); - value = *src->reg.u.immconst_uint; - - if (!vkd3d_array_reserve((void **)&cf_info->u.switch_.case_blocks, &cf_info->u.switch_.case_blocks_size, - 2 * (cf_info->u.switch_.case_block_count + 1), sizeof(*cf_info->u.switch_.case_blocks))) - return VKD3D_ERROR_OUT_OF_MEMORY; - - label_id = vkd3d_spirv_alloc_id(builder); - if (cf_info->inside_block) /* fall-through */ - vkd3d_spirv_build_op_branch(builder, label_id); - - cf_info->u.switch_.case_blocks[2 * cf_info->u.switch_.case_block_count + 0] = value; - cf_info->u.switch_.case_blocks[2 * cf_info->u.switch_.case_block_count + 1] = label_id; - ++cf_info->u.switch_.case_block_count; - - vkd3d_spirv_build_op_label(builder, label_id); - cf_info->inside_block = true; - vkd3d_spirv_build_op_name(builder, label_id, "switch%u_case%u", cf_info->u.switch_.id, value); - break; - } - - case VKD3DSIH_DEFAULT: - assert(compiler->control_flow_depth); - assert(cf_info->current_block == VKD3D_BLOCK_SWITCH); - assert(!cf_info->u.switch_.default_block_id); - - cf_info->u.switch_.default_block_id = vkd3d_spirv_alloc_id(builder); - if (cf_info->inside_block) /* fall-through */ - vkd3d_spirv_build_op_branch(builder, cf_info->u.switch_.default_block_id); - - vkd3d_spirv_build_op_label(builder, cf_info->u.switch_.default_block_id); - vkd3d_spirv_build_op_name(builder, cf_info->u.switch_.default_block_id, - "switch%u_default", cf_info->u.switch_.id); - cf_info->inside_block = true; - break; - - case VKD3DSIH_BREAK: - { - struct vkd3d_control_flow_info *breakable_cf_info; - - assert(compiler->control_flow_depth); - - if (!(breakable_cf_info = spirv_compiler_find_innermost_breakable_cf_construct(compiler))) - { - FIXME("Unhandled break instruction.\n"); - return VKD3D_ERROR_INVALID_SHADER; - } - - if (breakable_cf_info->current_block == VKD3D_BLOCK_SWITCH) - { - /* The current case block may have already been ended by an - * unconditional continue instruction. */ - if (breakable_cf_info->inside_block) - vkd3d_spirv_build_op_branch(builder, breakable_cf_info->u.switch_.merge_block_id); - } - - cf_info->inside_block = false; - break; - } - - case VKD3DSIH_RET: - spirv_compiler_emit_return(compiler, instruction); - - if (cf_info) - cf_info->inside_block = false; - break; - - default: - ERR("Unexpected instruction %#x.\n", instruction->handler_idx); - break; - } - - return VKD3D_OK; -} - static bool spirv_compiler_init_blocks(struct spirv_compiler *compiler, unsigned int block_count) { compiler->block_count = block_count; @@ -7816,6 +7587,37 @@ static void spirv_compiler_emit_branch(struct spirv_compiler *compiler, spirv_compiler_get_label_id(compiler, src[1].reg.idx[1].offset)); } +static void spirv_compiler_emit_switch(struct spirv_compiler *compiler, + const struct vkd3d_shader_instruction *instruction) +{ + struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; + struct vkd3d_spirv_stream *stream = &builder->function_stream; + const struct vkd3d_shader_src_param *src = instruction->src; + uint32_t val_id; + size_t location; + unsigned int i; + + if (!vkd3d_swizzle_is_scalar(src[0].swizzle)) + { + WARN("Unexpected src swizzle %#x.\n", src[0].swizzle); + spirv_compiler_warning(compiler, VKD3D_SHADER_WARNING_SPV_INVALID_SWIZZLE, + "The swizzle for a switch value is not scalar."); + } + + val_id = spirv_compiler_emit_load_src(compiler, &src[0], VKD3DSP_WRITEMASK_0); + /* Emit the merge immediately before the switch instruction. */ + spirv_compiler_emit_merge(compiler, src[1].reg.idx[1].offset, 0); + location = vkd3d_spirv_stream_current_location(stream); + vkd3d_spirv_build_op_switch(builder, val_id, + spirv_compiler_get_label_id(compiler, src[1].reg.idx[0].offset), NULL, 0); + for (i = 2; i < instruction->src_count; ++i) + { + vkd3d_spirv_build_word(stream, src[i].reg.u.immconst_uint[0]); + vkd3d_spirv_build_word(stream, spirv_compiler_get_label_id(compiler, src[i].reg.idx[0].offset)); + } + vkd3d_spirv_stream_add_word_count(stream, location, (instruction->src_count - 2) * 2u); +} + static void spirv_compiler_emit_deriv_instruction(struct spirv_compiler *compiler, const struct vkd3d_shader_instruction *instruction) { @@ -9560,19 +9362,8 @@ static int spirv_compiler_handle_instruction(struct spirv_compiler *compiler, case VKD3DSIH_F32TOF16: spirv_compiler_emit_f32tof16(compiler, instruction); break; - case VKD3DSIH_BREAK: - case VKD3DSIH_BREAKP: - case VKD3DSIH_CASE: - case VKD3DSIH_CONTINUE: - case VKD3DSIH_CONTINUEP: - case VKD3DSIH_DEFAULT: - case VKD3DSIH_ENDLOOP: - case VKD3DSIH_ENDSWITCH: - case VKD3DSIH_LABEL: - case VKD3DSIH_LOOP: case VKD3DSIH_RET: - case VKD3DSIH_SWITCH: - ret = spirv_compiler_emit_control_flow_instruction(compiler, instruction); + spirv_compiler_emit_return(compiler, instruction); break; case VKD3DSIH_RETP: spirv_compiler_emit_retc(compiler, instruction); @@ -9581,9 +9372,15 @@ static int spirv_compiler_handle_instruction(struct spirv_compiler *compiler, case VKD3DSIH_TEXKILL: spirv_compiler_emit_kill(compiler, instruction); break; + case VKD3DSIH_LABEL: + spirv_compiler_emit_label(compiler, instruction); + break; case VKD3DSIH_BRANCH: spirv_compiler_emit_branch(compiler, instruction); break; + case VKD3DSIH_SWITCH_MONOLITHIC: + spirv_compiler_emit_switch(compiler, instruction); + break; case VKD3DSIH_DSX: case VKD3DSIH_DSX_COARSE: case VKD3DSIH_DSX_FINE: diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index c122fc59a..e3cb0ec0e 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -472,6 +472,7 @@ enum vkd3d_shader_opcode VKD3DSIH_SUB, VKD3DSIH_SWAPC, VKD3DSIH_SWITCH, + VKD3DSIH_SWITCH_MONOLITHIC, VKD3DSIH_SYNC, VKD3DSIH_TEX, VKD3DSIH_TEXBEM, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/450
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxbc.c | 5 ++ libs/vkd3d-shader/ir.c | 58 ++++++++++++++++++++++++ libs/vkd3d-shader/spirv.c | 8 ++++ libs/vkd3d-shader/vkd3d_shader_private.h | 3 ++ 4 files changed, 74 insertions(+) diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 63deaaad2..22e29387f 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -551,9 +551,14 @@ static int shdr_handler(const struct vkd3d_shader_dxbc_section_desc *section, void free_shader_desc(struct vkd3d_shader_desc *desc) { + size_t i; + shader_signature_cleanup(&desc->input_signature); shader_signature_cleanup(&desc->output_signature); shader_signature_cleanup(&desc->patch_constant_signature); + for (i = 0; i < desc->block_name_count; ++i) + vkd3d_free((void *)desc->block_names[i]); + vkd3d_free(desc->block_names); } int shader_extract_from_dxbc(const struct vkd3d_shader_code *dxbc, diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index 61c455229..45cc4aff7 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -1430,6 +1430,7 @@ static enum vkd3d_result normalise_combined_samplers(struct vkd3d_shader_parser struct cf_flattener_if_info { struct vkd3d_shader_src_param *targets; + unsigned int id; uint32_t merge_block_id; unsigned int else_block_id; }; @@ -1451,6 +1452,7 @@ struct cf_flattener_switch_info { size_t ins_location; const struct vkd3d_shader_src_param *condition; + unsigned int id; unsigned int merge_block_id; unsigned int default_block_id; struct cf_flattener_switch_case *cases; @@ -1488,6 +1490,13 @@ struct cf_flattener size_t instruction_count; unsigned int block_id; + const char **block_names; + size_t block_name_capacity; + size_t block_name_count; + + unsigned int branch_id; + unsigned int loop_id; + unsigned int switch_id; unsigned int control_flow_depth; struct cf_flattener_info *control_flow_info; @@ -1668,6 +1677,31 @@ static struct cf_flattener_info *cf_flattener_find_innermost_breakable_cf_constr return NULL; } +static void VKD3D_PRINTF_FUNC(3, 4) cf_flattener_create_block_name(struct cf_flattener *flattener, + unsigned int block_id, const char *fmt, ...) +{ + struct vkd3d_string_buffer buffer; + size_t block_name_count; + va_list args; + + --block_id; + + block_name_count = max(flattener->block_name_count, block_id + 1); + if (!vkd3d_array_reserve((void **)&flattener->block_names, &flattener->block_name_capacity, + block_name_count, sizeof(*flattener->block_names))) + return; + memset(&flattener->block_names[flattener->block_name_count], 0, + (block_name_count - flattener->block_name_count) * sizeof(*flattener->block_names)); + flattener->block_name_count = block_name_count; + + vkd3d_string_buffer_init(&buffer); + va_start(args, fmt); + vkd3d_string_buffer_vprintf(&buffer, fmt, args); + va_end(args); + + flattener->block_names[block_id] = buffer.buffer; +} + static enum vkd3d_result cf_flattener_iterate_instruction_array(struct cf_flattener *flattener) { struct vkd3d_shader_parser *parser = flattener->parser; @@ -1714,10 +1748,15 @@ static enum vkd3d_result cf_flattener_iterate_instruction_array(struct cf_flatte cf_flattener_emit_label(flattener, true_block_id); + cf_info->u.if_.id = flattener->branch_id; cf_info->u.if_.merge_block_id = merge_block_id; cf_info->u.if_.else_block_id = 0; cf_info->inside_block = true; cf_info->current_block = VKD3D_BLOCK_IF; + + cf_flattener_create_block_name(flattener, merge_block_id, "branch%u_merge", flattener->branch_id); + cf_flattener_create_block_name(flattener, true_block_id, "branch%u_true", flattener->branch_id); + ++flattener->branch_id; break; case VKD3DSIH_ELSE: @@ -1727,6 +1766,8 @@ static enum vkd3d_result cf_flattener_iterate_instruction_array(struct cf_flatte cf_info->u.if_.else_block_id = cf_flattener_alloc_block_id(flattener); cf_info->u.if_.targets->reg.idx[1].offset = cf_info->u.if_.else_block_id; + cf_flattener_create_block_name(flattener, + cf_info->u.if_.else_block_id, "branch%u_false", cf_info->u.if_.id); cf_flattener_emit_label(flattener, cf_info->u.if_.else_block_id); cf_info->inside_block = true; @@ -1762,6 +1803,12 @@ static enum vkd3d_result cf_flattener_iterate_instruction_array(struct cf_flatte cf_info->u.loop.merge_block_id = merge_block_id; cf_info->current_block = VKD3D_BLOCK_LOOP; cf_info->inside_block = true; + + cf_flattener_create_block_name(flattener, loop_header_block_id, "loop%u_header", flattener->loop_id); + cf_flattener_create_block_name(flattener, loop_body_block_id, "loop%u_body", flattener->loop_id); + cf_flattener_create_block_name(flattener, continue_block_id, "loop%u_continue", flattener->loop_id); + cf_flattener_create_block_name(flattener, merge_block_id, "loop%u_merge", flattener->loop_id); + ++flattener->loop_id; break; case VKD3DSIH_ENDLOOP: @@ -1789,6 +1836,7 @@ static enum vkd3d_result cf_flattener_iterate_instruction_array(struct cf_flatte vsir_instruction_init(dst_ins, &instruction->location, VKD3DSIH_SWITCH_MONOLITHIC); ++flattener->instruction_count; + cf_info->u.switch_.id = flattener->switch_id; cf_info->u.switch_.merge_block_id = merge_block_id; cf_info->u.switch_.cases = NULL; cf_info->u.switch_.cases_size = 0; @@ -1797,6 +1845,9 @@ static enum vkd3d_result cf_flattener_iterate_instruction_array(struct cf_flatte cf_info->inside_block = false; cf_info->current_block = VKD3D_BLOCK_SWITCH; + cf_flattener_create_block_name(flattener, merge_block_id, "switch%u_merge", flattener->switch_id); + ++flattener->switch_id; + if (!vkd3d_array_reserve((void **)&cf_info->u.switch_.cases, &cf_info->u.switch_.cases_size, 10, sizeof(*cf_info->u.switch_.cases))) return VKD3D_ERROR_OUT_OF_MEMORY; @@ -1862,6 +1913,7 @@ static enum vkd3d_result cf_flattener_iterate_instruction_array(struct cf_flatte ++cf_info->u.switch_.cases_count; cf_flattener_emit_label(flattener, label_id); + cf_flattener_create_block_name(flattener, label_id, "switch%u_case%u", cf_info->u.switch_.id, value); cf_info->inside_block = true; break; } @@ -1872,6 +1924,9 @@ static enum vkd3d_result cf_flattener_iterate_instruction_array(struct cf_flatte cf_flattener_emit_unconditional_branch(flattener, cf_info->u.switch_.default_block_id); cf_flattener_emit_label(flattener, cf_info->u.switch_.default_block_id); + + cf_flattener_create_block_name(flattener, cf_info->u.switch_.default_block_id, + "switch%u_default", cf_info->u.switch_.id); cf_info->inside_block = true; break; @@ -1994,6 +2049,9 @@ static enum vkd3d_result flatten_control_flow_constructs(struct vkd3d_shader_par } vkd3d_free(flattener.control_flow_info); + /* Simpler to always free these in free_shader_desc(). */ + parser->shader_desc.block_names = flattener.block_names; + parser->shader_desc.block_name_count = flattener.block_name_count; return result; } diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 9b9551e0b..20d0926aa 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -2351,6 +2351,8 @@ struct spirv_compiler uint32_t *block_label_ids; unsigned int block_count; + const char **block_names; + size_t block_name_count; }; static bool is_in_default_phase(const struct spirv_compiler *compiler) @@ -7534,6 +7536,10 @@ static void spirv_compiler_emit_label(struct spirv_compiler *compiler, label_id = spirv_compiler_get_label_id(compiler, block_id); vkd3d_spirv_build_op_label(builder, label_id); + + --block_id; + if (block_id < compiler->block_name_count && compiler->block_names[block_id]) + vkd3d_spirv_build_op_name(builder, label_id, compiler->block_names[block_id]); } static void spirv_compiler_emit_merge(struct spirv_compiler *compiler, @@ -9614,6 +9620,8 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler, memset(&shader_desc->output_signature, 0, sizeof(shader_desc->output_signature)); memset(&shader_desc->patch_constant_signature, 0, sizeof(shader_desc->patch_constant_signature)); compiler->use_vocp = parser->shader_desc.use_vocp; + compiler->block_names = parser->shader_desc.block_names; + compiler->block_name_count = parser->shader_desc.block_name_count; compiler->input_control_point_count = shader_desc->input_control_point_count; compiler->output_control_point_count = shader_desc->output_control_point_count; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index e3cb0ec0e..fd16584a9 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -1017,6 +1017,9 @@ struct vkd3d_shader_desc } flat_constant_count[3]; bool use_vocp; + + const char **block_names; + size_t block_name_count; }; struct vkd3d_shader_register_semantic -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/450
Well, we've definitely had Wine bugs filed for assertions in the HLSL compiler triggered by trying to compile a particular shader.
What would have been the alternative? Maybe everything magically works anyway by luck, maybe you just get a bad output, maybe the bug resurfaces in a totally different place and you have to spend a day of work to understand what it is, instead of having an assertion message that immediately tells you which invariant has been violated. And still, if you're willing to bet on the first two alternatives, you can use `-DNDEBUG` and have it. But you can't do the other way around. So I think it's better to leave the opportunity on the table. As you say, it's true that a user using their distribution's packages doesn't directly control `-DNDEBUG`, but first they can recompile packages, if they deem it useful or necessary; and second the user is already trusting the distribution with a lot of decisions on how their operating system is supposed to work: this is just another one, and each distribution can have its own philosophy, and each user can choose which distribution better matches their own philosophy. Again, by leaving assertions in the code you're leaving more freedom on the table, not less. If you think that vkd3d should by default use `-DNDEBUG`, that's a sensible point (but I'll propose to have assertions enabled in the CI). That's what Conor is proposing, basically.
I don't think that's a decision a library can reliably make.
If the library's internal state is inconsistent, it basically cannot reliably make any decision any more. So it's not a bad idea to make this condition apparent.
In theory, sure. In practice, I think we have a number of cases where assert() is essentially used as a substitute for input validation. It also tends to make review a bit harder; instead of verifying the condition is handled appropriately, the reviewer needs to verify it can never happen.
Yeah, `assert()` should not be used for input validation, I totally agree (and while it might seem that I at some point I wanted to use it for input validation in the context of the VSIR validator, let me point out that I didn't mean the VSIR validator to check user input, I was convinced that the TPF parser would already do input validation, the principle still stands). Also, the reviewer probably has to check the asserted condition anyway. The assertion only makes it clearer what condition has to be checked and helps the reviewer ensuring that in practice the condition is not violated during test runs, so I think it is helpful. To elaborate, suppose that I have this code: ```c assert(buffer.size >= 3); do_something(buffer.buffer[0]); do_something(buffer.buffer[1]); do_something(buffer.buffer[2]); ``` If we remove the `assert()`, the reviewer still has to ensure that the buffer is big enough. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/450#note_56079
On Fri Dec 15 13:50:25 2023 +0000, Giovanni Mascellani wrote:
Well, we've definitely had Wine bugs filed for assertions in the HLSL compiler triggered by trying to compile a particular shader. What would have been the alternative? Maybe everything magically works anyway by luck, maybe you just get a bad output, maybe the bug resurfaces in a totally different place and you have to spend a day of work to understand what it is, instead of having an assertion message that immediately tells you which invariant has been violated. And still, if you're willing to bet on the first two alternatives, you can use `-DNDEBUG` and have it. But you can't do the other way around. So I think it's better to leave the opportunity on the table. As you say, it's true that a user using their distribution's packages doesn't directly control `-DNDEBUG`, but first they can recompile packages, if they deem it useful or necessary; and second the user is already trusting the distribution with a lot of decisions on how their operating system is supposed to work: this is just another one, and each distribution can have its own philosophy, and each user can choose which distribution better matches their own philosophy. Again, by leaving assertions in the code you're leaving more freedom on the table, not less. If you think that vkd3d should by default use `-DNDEBUG`, that's a sensible point (but I'll propose to have assertions enabled in the CI). That's what Conor is proposing, basically. I don't think that's a decision a library can reliably make. If the library's internal state is inconsistent, it basically cannot reliably make any decision any more. So it's not a bad idea to make this condition apparent. In theory, sure. In practice, I think we have a number of cases where assert() is essentially used as a substitute for input validation. It also tends to make review a bit harder; instead of verifying the condition is handled appropriately, the reviewer needs to verify it can never happen. Yeah, `assert()` should not be used for input validation, I totally agree (and while it might seem that I at some point I wanted to use it for input validation in the context of the VSIR validator, let me point out that I didn't mean the VSIR validator to check user input, I was convinced that the TPF parser would already do input validation, the principle still stands). Also, the reviewer probably has to check the asserted condition anyway. The assertion only makes it clearer what condition has to be checked and helps the reviewer ensuring that in practice the condition is not violated during test runs, so I think it is helpful. To elaborate, suppose that I have this code:
assert(buffer.size >= 3); do_something(buffer.buffer[0]); do_something(buffer.buffer[1]); do_something(buffer.buffer[2]);If we remove the `assert()`, the reviewer still has to ensure that the buffer is big enough. Having assertions or assertion-like checks which are excluded from releases, but included in our test builds, definitely would improve the odds of catching errors in new code, and regressions. Probably a very small subset, but that's not nothing.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/450#note_56084
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Giovanni Mascellani (＠giomasce)