[PATCH v2 0/3] MR574: vkd3d-shader/ir: Refactor VSIR passes.
-- v2: vkd3d-shader/ir: Remove DCL_TEMPS instructions. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/574
From: Conor McCarthy <cmccarthy(a)codeweavers.com> We started with only one or two of these but it has become excessive. Patch originally written by Conor McCarthy and updated by Giovanni Mascellani. --- libs/vkd3d-shader/ir.c | 47 +++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index d6978171b..d4cd0fa14 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -2236,38 +2236,43 @@ enum vkd3d_result vkd3d_shader_normalise(struct vkd3d_shader_parser *parser, if (parser->shader_desc.is_dxil) return result; - if (parser->shader_version.type != VKD3D_SHADER_TYPE_PIXEL - && (result = remap_output_signature(parser, compile_info)) < 0) - return result; + if (parser->shader_version.type != VKD3D_SHADER_TYPE_PIXEL) + { + if ((result = remap_output_signature(parser, compile_info)) < 0) + return result; + } - if (parser->shader_version.type == VKD3D_SHADER_TYPE_HULL - && (result = instruction_array_flatten_hull_shader_phases(instructions)) >= 0) + if (parser->shader_version.type == VKD3D_SHADER_TYPE_HULL) { - result = instruction_array_normalise_hull_shader_control_point_io(instructions, - &parser->shader_desc.input_signature); + if ((result = instruction_array_flatten_hull_shader_phases(instructions)) < 0) + return result; + + if ((result = instruction_array_normalise_hull_shader_control_point_io(instructions, + &parser->shader_desc.input_signature)) < 0) + return result; } - if (result >= 0) - result = shader_normalise_io_registers(parser); - if (result >= 0) - result = instruction_array_normalise_flat_constants(parser); + if ((result = shader_normalise_io_registers(parser)) < 0) + return result; - if (result >= 0) - remove_dead_code(parser); + if ((result = instruction_array_normalise_flat_constants(parser)) < 0) + return result; - if (result >= 0) - result = flatten_control_flow_constructs(parser); + remove_dead_code(parser); - if (result >= 0) - result = normalise_combined_samplers(parser); + if ((result = flatten_control_flow_constructs(parser)) < 0) + return result; - if (result >= 0 && TRACE_ON()) + if ((result = normalise_combined_samplers(parser)) < 0) + return result; + + if (TRACE_ON()) vkd3d_shader_trace(instructions, &parser->shader_version); - if (result >= 0 && !parser->failed) - result = vsir_validate(parser); + if (!parser->failed && (result = vsir_validate(parser)) < 0) + return result; - if (result >= 0 && parser->failed) + if (parser->failed) result = VKD3D_ERROR_INVALID_SHADER; return result; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/574
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/ir.c | 48 +++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index d4cd0fa14..57783de68 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -2233,38 +2233,38 @@ enum vkd3d_result vkd3d_shader_normalise(struct vkd3d_shader_parser *parser, struct vkd3d_shader_instruction_array *instructions = &parser->instructions; enum vkd3d_result result = VKD3D_OK; - if (parser->shader_desc.is_dxil) - return result; - - if (parser->shader_version.type != VKD3D_SHADER_TYPE_PIXEL) + if (!parser->shader_desc.is_dxil) { - if ((result = remap_output_signature(parser, compile_info)) < 0) - return result; - } + if (parser->shader_version.type != VKD3D_SHADER_TYPE_PIXEL) + { + if ((result = remap_output_signature(parser, compile_info)) < 0) + return result; + } - if (parser->shader_version.type == VKD3D_SHADER_TYPE_HULL) - { - if ((result = instruction_array_flatten_hull_shader_phases(instructions)) < 0) - return result; + if (parser->shader_version.type == VKD3D_SHADER_TYPE_HULL) + { + if ((result = instruction_array_flatten_hull_shader_phases(instructions)) < 0) + return result; - if ((result = instruction_array_normalise_hull_shader_control_point_io(instructions, - &parser->shader_desc.input_signature)) < 0) - return result; - } + if ((result = instruction_array_normalise_hull_shader_control_point_io(instructions, + &parser->shader_desc.input_signature)) < 0) + return result; + } - if ((result = shader_normalise_io_registers(parser)) < 0) - return result; + if ((result = shader_normalise_io_registers(parser)) < 0) + return result; - if ((result = instruction_array_normalise_flat_constants(parser)) < 0) - return result; + if ((result = instruction_array_normalise_flat_constants(parser)) < 0) + return result; - remove_dead_code(parser); + remove_dead_code(parser); - if ((result = flatten_control_flow_constructs(parser)) < 0) - return result; + if ((result = flatten_control_flow_constructs(parser)) < 0) + return result; - if ((result = normalise_combined_samplers(parser)) < 0) - return result; + if ((result = normalise_combined_samplers(parser)) < 0) + return result; + } if (TRACE_ON()) vkd3d_shader_trace(instructions, &parser->shader_version); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/574
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> We have to do work to keep it updated across passes and we never read it. --- libs/vkd3d-shader/ir.c | 57 +++++++++++++-------------------------- libs/vkd3d-shader/spirv.c | 1 - 2 files changed, 18 insertions(+), 40 deletions(-) diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index 57783de68..fb45e49ef 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -36,6 +36,19 @@ static void vkd3d_shader_instruction_make_nop(struct vkd3d_shader_instruction *i vsir_instruction_init(ins, &location, VKD3DSIH_NOP); } +static void remove_dcl_temps(struct vkd3d_shader_parser *parser) +{ + unsigned int i; + + for (i = 0; i < parser->instructions.count; ++i) + { + struct vkd3d_shader_instruction *ins = &parser->instructions.elements[i]; + + if (ins->handler_idx == VKD3DSIH_DCL_TEMPS) + vkd3d_shader_instruction_make_nop(ins); + } +} + static void shader_register_eliminate_phase_addressing(struct vkd3d_shader_register *reg, unsigned int instance_id) { @@ -143,9 +156,6 @@ struct hull_flattener { struct vkd3d_shader_instruction_array instructions; - unsigned int max_temp_count; - unsigned int temp_dcl_idx; - unsigned int instance_count; unsigned int phase_body_idx; enum vkd3d_shader_opcode phase; @@ -203,21 +213,6 @@ static void flattener_eliminate_phase_related_dcls(struct hull_flattener *normal vkd3d_shader_instruction_make_nop(ins); return; } - else if (ins->handler_idx == VKD3DSIH_DCL_TEMPS && normaliser->phase != VKD3DSIH_INVALID) - { - /* Leave only the first temp declaration and set it to the max count later. */ - if (!normaliser->max_temp_count) - { - normaliser->max_temp_count = ins->declaration.count; - normaliser->temp_dcl_idx = index; - } - else - { - normaliser->max_temp_count = max(normaliser->max_temp_count, ins->declaration.count); - vkd3d_shader_instruction_make_nop(ins); - } - return; - } if (normaliser->phase == VKD3DSIH_INVALID || shader_instruction_is_dcl(ins)) return; @@ -371,9 +366,6 @@ static enum vkd3d_result instruction_array_flatten_hull_shader_phases(struct vkd if (flattener.phase != VKD3DSIH_INVALID) { - if (flattener.temp_dcl_idx) - instructions->elements[flattener.temp_dcl_idx].declaration.count = flattener.max_temp_count; - if (!shader_instruction_array_reserve(&flattener.instructions, flattener.instructions.count + 1)) return VKD3D_ERROR_OUT_OF_MEMORY; vsir_instruction_init(&instructions->elements[instructions->count++], &flattener.last_ret_location, VKD3DSIH_RET); @@ -570,9 +562,6 @@ struct io_normaliser struct shader_signature *output_signature; struct shader_signature *patch_constant_signature; - unsigned int max_temp_count; - unsigned int temp_dcl_idx; - unsigned int instance_count; unsigned int phase_body_idx; enum vkd3d_shader_opcode phase; @@ -2233,6 +2222,8 @@ enum vkd3d_result vkd3d_shader_normalise(struct vkd3d_shader_parser *parser, struct vkd3d_shader_instruction_array *instructions = &parser->instructions; enum vkd3d_result result = VKD3D_OK; + remove_dcl_temps(parser); + if (!parser->shader_desc.is_dxil) { if (parser->shader_version.type != VKD3D_SHADER_TYPE_PIXEL) @@ -2284,7 +2275,6 @@ struct validation_context size_t instruction_idx; bool invalid_instruction_idx; bool dcl_temps_found; - unsigned int temp_count; enum vkd3d_shader_opcode phase; struct validation_context_temp_data @@ -2339,11 +2329,7 @@ static void vsir_validate_src_param(struct validation_context *ctx, static void vsir_validate_register(struct validation_context *ctx, const struct vkd3d_shader_register *reg) { - unsigned int i, temp_count = ctx->temp_count; - - /* SM1-3 shaders do not include a DCL_TEMPS instruction. */ - if (ctx->parser->shader_version.major <= 3) - temp_count = ctx->parser->shader_desc.temp_count; + unsigned int i; if (reg->type >= VKD3DSPR_COUNT) validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_REGISTER_TYPE, "Invalid register type %#x.", @@ -2388,18 +2374,13 @@ static void vsir_validate_register(struct validation_context *ctx, if (reg->idx[0].rel_addr) validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_INDEX, "Non-NULL relative address for a TEMP register."); - if (reg->idx[0].offset >= temp_count) + if (reg->idx[0].offset >= ctx->parser->shader_desc.temp_count) { validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_INDEX, "TEMP register index %u exceeds the maximum count %u.", - reg->idx[0].offset, temp_count); + reg->idx[0].offset, ctx->parser->shader_desc.temp_count); break; } - /* parser->shader_desc.temp_count might be smaller then - * temp_count if the parser made a mistake; we still don't - * want to overflow the array. */ - if (reg->idx[0].offset >= ctx->parser->shader_desc.temp_count) - break; data = &ctx->temps[reg->idx[0].offset]; if (reg->dimension == VSIR_DIMENSION_NONE) @@ -2641,7 +2622,6 @@ static void vsir_validate_instruction(struct validation_context *ctx) instruction->handler_idx); ctx->phase = instruction->handler_idx; ctx->dcl_temps_found = false; - ctx->temp_count = 0; return; default: @@ -2664,7 +2644,6 @@ static void vsir_validate_instruction(struct validation_context *ctx) validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_DCL_TEMPS, "Invalid DCL_TEMPS count %u, expected at most %u.", instruction->declaration.count, ctx->parser->shader_desc.temp_count); ctx->dcl_temps_found = true; - ctx->temp_count = instruction->declaration.count; break; case VKD3DSIH_IF: diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 5798f262c..9b6cf6133 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -9577,7 +9577,6 @@ static int spirv_compiler_handle_instruction(struct spirv_compiler *compiler, case VKD3DSIH_DCL_RESOURCE_RAW: case VKD3DSIH_DCL_RESOURCE_STRUCTURED: case VKD3DSIH_DCL_SAMPLER: - case VKD3DSIH_DCL_TEMPS: case VKD3DSIH_DCL_UAV_RAW: case VKD3DSIH_DCL_UAV_STRUCTURED: case VKD3DSIH_DCL_UAV_TYPED: -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/574
On Mon Jan 22 21:14:17 2024 +0000, Giovanni Mascellani wrote:
changed this line in [version 2 of the diff](/wine/vkd3d/-/merge_requests/574/diffs?diff_id=94949&start_sha=5636617b19d2d798a952dc17568e3d3ddb3e4e84#4fae87cf1e8d06a5c7871f244e3dd75ef2dac299_2639_2642) On second thought, I noticed that it was easy to keep the duplicate DCL_TEMPS check, so I added it back. The general point is still valid, but in this specific case there is indeed no reason to kill correct code.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/574#note_58486
This merge request was approved by Francisco Casas. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/574
participants (4)
-
Conor McCarthy -
Francisco Casas (@fcasas) -
Giovanni Mascellani -
Giovanni Mascellani (@giomasce)