~~Other validation stuff I plan to send as soon as !450 and !550 are in.~~
~~I guess the CI is going to timeout because of too many commits, so I'll preemptively stop it.~~
-- v8: vkd3d-shader/ir: Validate PHI instructions. vkd3d-shader/ir: Do not allow IMMCONST and IMMCONST64 as destination registers. vkd3d-shader/ir: Refactor register-type-specific code in parameter validation. vkd3d-shader/ir: Check that all instructions appear in a block.
From: Giovanni Mascellani gmascellani@codeweavers.com
--- libs/vkd3d-shader/ir.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index 14288a38b..e7b063b38 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -2299,6 +2299,7 @@ struct validation_context CF_TYPE_STRUCTURED, CF_TYPE_BLOCKS, } cf_type; + bool inside_block;
struct validation_context_temp_data { @@ -2765,6 +2766,33 @@ static void vsir_validate_instruction(struct validation_context *ctx) ctx->cf_type = CF_TYPE_STRUCTURED; }
+ if (ctx->cf_type == CF_TYPE_BLOCKS && !vsir_instruction_is_dcl(instruction)) + { + switch (instruction->handler_idx) + { + case VKD3DSIH_LABEL: + if (ctx->inside_block) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_CONTROL_FLOW, "Invalid LABEL instruction inside a block."); + ctx->inside_block = true; + break; + + case VKD3DSIH_RET: + case VKD3DSIH_BRANCH: + case VKD3DSIH_SWITCH_MONOLITHIC: + if (!ctx->inside_block) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_CONTROL_FLOW, "Invalid instruction %#x outside any block.", + instruction->handler_idx); + ctx->inside_block = false; + break; + + default: + if (!ctx->inside_block) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_CONTROL_FLOW, "Invalid instruction %#x outside any block.", + instruction->handler_idx); + break; + } + } + switch (instruction->handler_idx) { case VKD3DSIH_DCL_TEMPS: @@ -3002,6 +3030,9 @@ enum vkd3d_result vsir_validate(struct vkd3d_shader_parser *parser) if (ctx.depth != 0) validator_error(&ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_CONTROL_FLOW, "%zu nested blocks were not closed.", ctx.depth);
+ if (ctx.inside_block) + validator_error(&ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_CONTROL_FLOW, "Last block was not closed."); + for (i = 0; i < parser->shader_desc.ssa_count; ++i) { struct validation_context_ssa_data *data = &ctx.ssas[i];
From: Giovanni Mascellani gmascellani@codeweavers.com
To better accommodate code for other register types. --- libs/vkd3d-shader/ir.c | 52 +++++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 18 deletions(-)
diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index e7b063b38..f866198f0 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -2588,21 +2588,29 @@ static void vsir_validate_dst_param(struct validation_context *ctx, dst->shift); }
- if (dst->reg.type == VKD3DSPR_SSA && dst->reg.idx[0].offset < ctx->parser->shader_desc.ssa_count) + switch (dst->reg.type) { - struct validation_context_ssa_data *data = &ctx->ssas[dst->reg.idx[0].offset]; + case VKD3DSPR_SSA: + if (dst->reg.idx[0].offset < ctx->parser->shader_desc.ssa_count) + { + struct validation_context_ssa_data *data = &ctx->ssas[dst->reg.idx[0].offset];
- if (data->write_mask == 0) - { - data->write_mask = dst->write_mask; - data->first_assigned = ctx->instruction_idx; - } - else - { - validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_SSA_USAGE, - "SSA register is already assigned at instruction %zu.", - data->first_assigned); - } + if (data->write_mask == 0) + { + data->write_mask = dst->write_mask; + data->first_assigned = ctx->instruction_idx; + } + else + { + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_SSA_USAGE, + "SSA register is already assigned at instruction %zu.", + data->first_assigned); + } + } + break; + + default: + break; } }
@@ -2623,13 +2631,21 @@ static void vsir_validate_src_param(struct validation_context *ctx, validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_MODIFIERS, "Source has invalid modifiers %#x.", src->modifiers);
- if (src->reg.type == VKD3DSPR_SSA && src->reg.idx[0].offset < ctx->parser->shader_desc.ssa_count) + switch (src->reg.type) { - struct validation_context_ssa_data *data = &ctx->ssas[src->reg.idx[0].offset]; - unsigned int i; + case VKD3DSPR_SSA: + if (src->reg.idx[0].offset < ctx->parser->shader_desc.ssa_count) + { + struct validation_context_ssa_data *data = &ctx->ssas[src->reg.idx[0].offset]; + unsigned int i;
- for (i = 0; i < VKD3D_VEC4_SIZE; ++i) - data->read_mask |= (1u << vsir_swizzle_get_component(src->swizzle, i)); + for (i = 0; i < VKD3D_VEC4_SIZE; ++i) + data->read_mask |= (1u << vsir_swizzle_get_component(src->swizzle, i)); + } + break; + + default: + break; } }
From: Giovanni Mascellani gmascellani@codeweavers.com
--- libs/vkd3d-shader/ir.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index f866198f0..24071cb2e 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -2609,6 +2609,16 @@ static void vsir_validate_dst_param(struct validation_context *ctx, } break;
+ case VKD3DSPR_IMMCONST: + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_REGISTER_TYPE, + "Invalid IMMCONST register used as destination parameter."); + break; + + case VKD3DSPR_IMMCONST64: + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_REGISTER_TYPE, + "Invalid IMMCONST64 register used as destination parameter."); + break; + default: break; }
From: Giovanni Mascellani gmascellani@codeweavers.com
--- libs/vkd3d-shader/ir.c | 56 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index 24071cb2e..b3c3338a0 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -3025,6 +3025,62 @@ static void vsir_validate_instruction(struct validation_context *ctx) break; }
+ case VKD3DSIH_PHI: + { + unsigned int incoming_count; + + vsir_validate_cf_type(ctx, instruction, CF_TYPE_BLOCKS); + vsir_validate_dst_count(ctx, instruction, 1); + vsir_validate_src_min_count(ctx, instruction, 2); + if (instruction->src_count % 2 != 0) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_SOURCE_COUNT, + "Invalid source count %u for a PHI instruction, it must be an even number.", + instruction->src_count); + incoming_count = instruction->src_count / 2; + + if (!register_is_ssa(&instruction->dst[0].reg)) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_REGISTER_TYPE, + "Invalid destination of type %#x in PHI instruction, expected SSA.", + instruction->dst[0].reg.type); + + if (instruction->dst[0].reg.dimension != VSIR_DIMENSION_SCALAR) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_DIMENSION, + "Invalid destination dimension %#x in PHI instruction, expected scalar.", + instruction->dst[0].reg.dimension); + + if (instruction->dst[0].modifiers != VKD3DSPDM_NONE) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_MODIFIERS, + "Invalid modifiers %#x for the destination of a PHI instruction, expected none.", + instruction->dst[0].modifiers); + + if (instruction->dst[0].shift != 0) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_SHIFT, + "Invalid shift %#x for the destination of a PHI instruction, expected none.", + instruction->dst[0].shift); + + for (i = 0; i < incoming_count; ++i) + { + unsigned int value_idx = 2 * i; + unsigned int label_idx = 2 * i + 1; + + if (!register_is_constant(&instruction->src[value_idx].reg) && !register_is_ssa(&instruction->src[value_idx].reg)) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_REGISTER_TYPE, + "Invalid value register for incoming %zu of type %#x in PHI instruction, " + "expected SSA, IMMCONST or IMMCONST64.", i, instruction->src[value_idx].reg.type); + + if (instruction->src[value_idx].reg.dimension != VSIR_DIMENSION_SCALAR) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_DIMENSION, + "Invalid value dimension %#x for incoming %zu in PHI instruction, expected scalar.", + instruction->src[value_idx].reg.dimension, i); + + if (!vsir_register_is_label(&instruction->src[label_idx].reg)) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_REGISTER_TYPE, + "Invalid label register for case %zu of type %#x in PHI instruction, " + "expected LABEL.", i, instruction->src[value_idx].reg.type); + } + break; + } + default: break; }