[PATCH v6 0/6] 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.~~ -- v6: vkd3d-shader/ir: Validate swizzles for 64 bit types. vkd3d-shader/dxil: Generate valid swizzles for 64 bit types. 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. 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
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> Differently from write masks, swizzles are considered to be applied to the raw 4x32 bit register, rather than to its typed interpretation. Even for scalar registers, we still want to generate swizzles that are valid for a 64 bit type (i.e., .xyxy, .xyzw, .zwxy or .zwzw) so that the backend can use them oblivious of whether the register is scalar or not. --- libs/vkd3d-shader/dxil.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 186460540..7c77695ed 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -2144,20 +2144,26 @@ static void dst_param_init_ssa_scalar(struct vkd3d_shader_dst_param *param, cons static inline void src_param_init(struct vkd3d_shader_src_param *param) { - param->swizzle = VKD3D_SHADER_SWIZZLE(X, X, X, X); + if (data_type_is_64_bit(param->reg.data_type)) + param->swizzle = VKD3D_SHADER_SWIZZLE(X, Y, X, Y); + else + param->swizzle = VKD3D_SHADER_SWIZZLE(X, X, X, X); param->modifiers = VKD3DSPSM_NONE; } static void src_param_init_scalar(struct vkd3d_shader_src_param *param, unsigned int component_idx) { - param->swizzle = vkd3d_shader_create_swizzle(component_idx, component_idx, component_idx, component_idx); + if (data_type_is_64_bit(param->reg.data_type)) + param->swizzle = vkd3d_shader_create_swizzle(2 * component_idx, 2 * component_idx + 1, 2 * component_idx, 2 * component_idx + 1); + else + param->swizzle = vkd3d_shader_create_swizzle(component_idx, component_idx, component_idx, component_idx); param->modifiers = VKD3DSPSM_NONE; } static void src_param_init_from_value(struct vkd3d_shader_src_param *param, const struct sm6_value *src) { - src_param_init(param); param->reg = src->u.reg; + src_param_init(param); } static void src_param_init_vector_from_reg(struct vkd3d_shader_src_param *param, @@ -4268,7 +4274,10 @@ static void sm6_parser_emit_extractval(struct sm6_parser *sm6, const struct dxil src_param = instruction_src_params_alloc(ins, 1, sm6); src_param_init_from_value(src_param, src); - src_param->swizzle = vkd3d_shader_create_swizzle(elem_idx, elem_idx, elem_idx, elem_idx); + if (data_type_is_64_bit(src_param->reg.data_type)) + src_param->swizzle = vkd3d_shader_create_swizzle(2 * elem_idx, 2 * elem_idx + 1, 2 * elem_idx, 2 * elem_idx + 1); + else + src_param->swizzle = vkd3d_shader_create_swizzle(elem_idx, elem_idx, elem_idx, elem_idx); instruction_dst_param_init_ssa_scalar(ins, sm6); } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/559
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/ir.c | 51 +++++++++++++++++++++++++++++++++--------- 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index 0cea6e590..d9774e10a 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -2544,9 +2544,19 @@ static void vsir_validate_dst_param(struct validation_context *ctx, { vsir_validate_register(ctx, &dst->reg); - if (dst->write_mask & ~VKD3DSP_WRITEMASK_ALL) - validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_WRITE_MASK, "Destination has invalid write mask %#x.", - dst->write_mask); + + if (!data_type_is_64_bit(dst->reg.data_type)) + { + if (dst->write_mask & ~VKD3DSP_WRITEMASK_ALL) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_WRITE_MASK, + "Destination has invalid write mask %#x.", dst->write_mask); + } + else + { + if (dst->write_mask & ~(VKD3DSP_WRITEMASK_0 | VKD3DSP_WRITEMASK_1)) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_WRITE_MASK, + "64 bit destination has invalid write mask %#x.", dst->write_mask); + } switch (dst->reg.dimension) { @@ -2597,7 +2607,10 @@ static void vsir_validate_dst_param(struct validation_context *ctx, if (data->write_mask == 0) { - data->write_mask = dst->write_mask; + if (data_type_is_64_bit(dst->reg.data_type)) + data->write_mask = vsir_write_mask_32_from_64(dst->write_mask); + else + data->write_mask = dst->write_mask; data->first_assigned = ctx->instruction_idx; } else @@ -2629,13 +2642,31 @@ static void vsir_validate_src_param(struct validation_context *ctx, { vsir_validate_register(ctx, &src->reg); - if (src->swizzle & ~0x03030303u) - validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_SWIZZLE, "Source has invalid swizzle %#x.", - src->swizzle); + if (!data_type_is_64_bit(src->reg.data_type)) + { + if (src->swizzle & ~0x03030303u) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_SWIZZLE, + "Source has invalid swizzle %#x.", src->swizzle); + + if (src->reg.dimension != VSIR_DIMENSION_VEC4 && src->swizzle != VKD3D_SHADER_SWIZZLE(X, X, X, X)) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_SWIZZLE, + "Source of dimension %u has invalid swizzle %#x.", + src->reg.dimension, src->swizzle); + } + else + { + if (src->swizzle != VKD3D_SHADER_SWIZZLE(X, Y, X, Y) + && src->swizzle != VKD3D_SHADER_SWIZZLE(X, Y, Z, W) + && src->swizzle != VKD3D_SHADER_SWIZZLE(Z, W, X, Y) + && src->swizzle != VKD3D_SHADER_SWIZZLE(Z, W, Z, W)) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_SWIZZLE, + "64 bit source has invalid swizzle %#x.", src->swizzle); - if (src->reg.dimension != VSIR_DIMENSION_VEC4 && src->swizzle != 0) - validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_SWIZZLE, "Source of dimension %u has invalid swizzle %#x.", - src->reg.dimension, src->swizzle); + if (src->reg.dimension != VSIR_DIMENSION_VEC4 && src->swizzle != VKD3D_SHADER_SWIZZLE(X, Y, X, Y)) + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_SWIZZLE, + "64 bit source of dimension %u has invalid swizzle %#x.", + src->reg.dimension, src->swizzle); + } if (src->modifiers >= VKD3DSPSM_COUNT) validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_MODIFIERS, "Source has invalid modifiers %#x.", -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/559
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/559
participants (2)
-
Giovanni Mascellani -
Giovanni Mascellani (＠giomasce)