[PATCH v7 0/4] MR559: vkd3d-shader/ir: Validate block-based CF, part 2.

[Giovanni Mascellani (＠giomasce)]

~~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

[PATCH v7 1/4] vkd3d-shader/ir: Check that all instructions appear in a block.

[Giovanni Mascellani]

From: Giovanni Mascellani

---
 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];

[PATCH v7 2/4] vkd3d-shader/ir: Refactor register-type-specific code in parameter validation.

[Giovanni Mascellani]

From: Giovanni Mascellani

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;
     }
 }

[PATCH v7 3/4] vkd3d-shader/ir: Do not allow IMMCONST and IMMCONST64 as destination registers.

[Giovanni Mascellani]

From: Giovanni Mascellani

---
 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;
     }

[PATCH v7 4/4] vkd3d-shader/ir: Validate PHI instructions.

[Giovanni Mascellani]

From: Giovanni Mascellani

---
 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;
     }

[Giovanni Mascellani (＠giomasce)]

I removed the 64 bit part, which requires some more work.

[Henri Verbeet (＠hverbeet)]

> +    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?