[PATCH v7 0/4] MR559: vkd3d-shader/ir: Validate block-based CF, part 2.
~~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.~~ -- v7: https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/559
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/ir.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index 14288a38b..4727ee2d6 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,34 @@ static void vsir_validate_instruction(struct validation_context *ctx) ctx->cf_type = CF_TYPE_STRUCTURED; } + if (ctx->cf_type == CF_TYPE_BLOCKS && !(instruction->handler_idx >= VKD3DSIH_DCL + && instruction->handler_idx <= VKD3DSIH_DCL_VERTICES_OUT)) + { + 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 +3031,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]; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/559
From: Giovanni Mascellani <gmascellani(a)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 4727ee2d6..10985d675 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; } } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/559
From: Giovanni Mascellani <gmascellani(a)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 10985d675..b8d1e26ca 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; } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/559
From: Giovanni Mascellani <gmascellani(a)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 b8d1e26ca..0cea6e590 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -3026,6 +3026,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; } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/559
I removed the 64 bit part, which requires some more work. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/559#note_58161
```diff + if (ctx->cf_type == CF_TYPE_BLOCKS && !(instruction->handler_idx >= VKD3DSIH_DCL + && instruction->handler_idx <= VKD3DSIH_DCL_VERTICES_OUT)) ```
Should that use vsir_instruction_is_dcl() instead? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/559#note_58446
participants (3)
-
Giovanni Mascellani -
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)