[PATCH v5 0/7] MR568: vkd3d-shader: Miscellaneous changes in view of the CFG structurizer.
-- v5: vkd3d-shader/ir: Remove DCL_TEMPS instructions. vkd3d-shader/ir: Run tracing and validation for DXIL code too. vkd3d-shader/ir: Refactor return code checking in vkd3d_shader_normalise(). vkd3d-shader/spirv: Normalise the shader before allocating registers. vkd3d-shader: Make relative address sources mutable too.
From: Giovanni Mascellani gmascellani@codeweavers.com
--- tests/hlsl/for.shader_test | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/tests/hlsl/for.shader_test b/tests/hlsl/for.shader_test index 9407c81bc..7abd20672 100644 --- a/tests/hlsl/for.shader_test +++ b/tests/hlsl/for.shader_test @@ -28,6 +28,40 @@ probe ( 0, 0, 159, 480) rgba (10.0, 35.0, 0.0, 0.0) probe (161, 0, 479, 480) rgba (10.0, 38.0, 0.0, 0.0) probe (481, 0, 640, 480) rgba ( 5.0, 10.0, 0.0, 0.0)
+[require] +shader model >= 4.0 + +% Identical to the previous, except we prevent DXC from unrolling the +% loop so we can test non-trivial control flow +[pixel shader] +uniform uint iter; + +float4 main(float tex : texcoord) : sv_target +{ + int i; + float x = 0.0; + for (i = 0; i < iter; i++) + { + x += i; + if (tex > 0.5 && i == 5) + break; + if (tex > -0.5 && i >= 7) + continue; + x -= 1; + } + return float4(i, x, 0.0, 0.0); +} + +[test] +uniform 0 uint4 10 0 0 0 +todo(sm>=6) draw quad +probe ( 0, 0, 159, 480) rgba (10.0, 35.0, 0.0, 0.0) +probe (161, 0, 479, 480) rgba (10.0, 38.0, 0.0, 0.0) +probe (481, 0, 640, 480) rgba ( 5.0, 10.0, 0.0, 0.0) + +[require] +% Reset requirements + [pixel shader] float4 main(float tex : texcoord) : sv_target {
From: Giovanni Mascellani gmascellani@codeweavers.com
--- tests/hlsl/switch.shader_test | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
diff --git a/tests/hlsl/switch.shader_test b/tests/hlsl/switch.shader_test index b7edadccf..01624f97c 100644 --- a/tests/hlsl/switch.shader_test +++ b/tests/hlsl/switch.shader_test @@ -28,6 +28,41 @@ uniform 0 uint4 0 0 0 0 draw quad probe all rgba (3.0, 3.0, 3.0, 3.0)
+% just a default case +[pixel shader] +uint4 v; + +float4 main() : sv_target +{ + switch (v.x) + { + default: + return 5.0; + } +} + +[test] +uniform 0 uint4 3 0 0 0 +draw quad +probe all rgba (5.0, 5.0, 5.0, 5.0) +uniform 0 uint4 1 0 0 0 +draw quad +probe all rgba (5.0, 5.0, 5.0, 5.0) +uniform 0 uint4 0 0 0 0 +draw quad +probe all rgba (5.0, 5.0, 5.0, 5.0) + +% completely empty +[pixel shader fail] +uint4 v; + +float4 main() : sv_target +{ + switch (v.x) + { + } +} + % falling through is only supported for empty case statements [pixel shader] uint4 v;
From: Giovanni Mascellani gmascellani@codeweavers.com
They were forgotten in 78d95889bd49c6028f0c26aaec5589786381da4a.
Fixes: 78d95889bd49c6028f0c26aaec5589786381da4a --- libs/vkd3d-shader/d3dbc.c | 4 ++-- libs/vkd3d-shader/vkd3d_shader_private.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index aa0dd8f4b..dcd510355 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -444,7 +444,7 @@ static uint32_t swizzle_from_sm1(uint32_t swizzle) shader_sm1_get_swizzle_component(swizzle, 3)); }
-static void shader_sm1_parse_src_param(uint32_t param, const struct vkd3d_shader_src_param *rel_addr, +static void shader_sm1_parse_src_param(uint32_t param, struct vkd3d_shader_src_param *rel_addr, struct vkd3d_shader_src_param *src) { enum vkd3d_shader_register_type reg_type = ((param & VKD3D_SM1_REGISTER_TYPE_MASK) >> VKD3D_SM1_REGISTER_TYPE_SHIFT) @@ -465,7 +465,7 @@ static void shader_sm1_parse_src_param(uint32_t param, const struct vkd3d_shader src->modifiers = (param & VKD3D_SM1_SRC_MODIFIER_MASK) >> VKD3D_SM1_SRC_MODIFIER_SHIFT; }
-static void shader_sm1_parse_dst_param(uint32_t param, const struct vkd3d_shader_src_param *rel_addr, +static void shader_sm1_parse_dst_param(uint32_t param, struct vkd3d_shader_src_param *rel_addr, struct vkd3d_shader_dst_param *dst) { enum vkd3d_shader_register_type reg_type = ((param & VKD3D_SM1_REGISTER_TYPE_MASK) >> VKD3D_SM1_REGISTER_TYPE_SHIFT) diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 8c5955d3d..4c5f961aa 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -826,7 +826,7 @@ struct vkd3d_shader_indexable_temp
struct vkd3d_shader_register_index { - const struct vkd3d_shader_src_param *rel_addr; + struct vkd3d_shader_src_param *rel_addr; unsigned int offset; /* address is known to fall within the object (for optimisation) */ bool is_in_bounds;
From: Giovanni Mascellani gmascellani@codeweavers.com
--- libs/vkd3d-shader/spirv.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 5798f262c..184144ccc 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -9675,6 +9675,9 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler, enum vkd3d_result result = VKD3D_OK; unsigned int i;
+ if ((result = vkd3d_shader_normalise(parser, compile_info)) < 0) + return result; + if (parser->shader_desc.temp_count) spirv_compiler_emit_temps(compiler, parser->shader_desc.temp_count); if (parser->shader_desc.ssa_count) @@ -9685,9 +9688,6 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler, compiler->location.column = 0; compiler->location.line = 1;
- 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;
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 | 62 ++++++++++++--------------------------- libs/vkd3d-shader/spirv.c | 1 - 2 files changed, 18 insertions(+), 45 deletions(-)
diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index 57783de68..150eae7fc 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) @@ -2283,8 +2274,6 @@ struct validation_context struct vkd3d_shader_parser *parser; 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 +2328,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 +2373,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) @@ -2640,8 +2620,6 @@ static void vsir_validate_instruction(struct validation_context *ctx) validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_INSTRUCTION_NESTING, "Phase instruction %#x must appear to top level.", instruction->handler_idx); ctx->phase = instruction->handler_idx; - ctx->dcl_temps_found = false; - ctx->temp_count = 0; return;
default: @@ -2658,13 +2636,9 @@ static void vsir_validate_instruction(struct validation_context *ctx) case VKD3DSIH_DCL_TEMPS: vsir_validate_dst_count(ctx, instruction, 0); vsir_validate_src_count(ctx, instruction, 0); - if (ctx->dcl_temps_found) - validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_DUPLICATE_DCL_TEMPS, "Duplicate DCL_TEMPS instruction."); if (instruction->declaration.count > ctx->parser->shader_desc.temp_count) 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 184144ccc..e097857f8 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:
I added a few other changes that should help reducing conflict between !564 and !514.
This merge request was approved by Giovanni Mascellani.
-
Conor McCarthy -
Giovanni Mascellani
-
Giovanni Mascellani (＠giomasce)