-- v2: vkd3d-shader/ir: Remove DCL_TEMPS instructions.
From: Conor McCarthy cmccarthy@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;
From: Giovanni Mascellani gmascellani@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);
From: Giovanni Mascellani gmascellani@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:
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.
This merge request was approved by Francisco Casas.
-
Conor McCarthy -
Francisco Casas (＠fcasas)
-
Giovanni Mascellani
-
Giovanni Mascellani (＠giomasce)