Skip to content
Snippets Groups Projects

ir: Validate TEMP registers.

Merged Giovanni Mascellani requested to merge giomasce/vkd3d:amiata into master
2 unresolved threads

Merge request reports

Merge request pipeline #17523 skipped

Merge request pipeline skipped for af72466d

Merged by Alexandre JulliardAlexandre Julliard 1 year ago (Nov 2, 2023 9:49pm UTC)

Merge details

  • Changes merged into with af72466d.
  • Did not delete the source branch.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • added 9 commits

    • 7d851e9f - vkd3d-shader: Deduplicate profile version comparison functions.
    • 1381a6fb - vkd3d-shader/dxil: Use vsir_register_init() to initialize registers.
    • eabd759a - vkd3d-shader/ir: Validate the register precision.
    • a9095430 - vkd3d-shader/ir: Validate the register data type.
    • 20159f86 - vkd3d-shader/ir: Validate the register dimension.
    • cf99b80c - vkd3d-shader/ir: Validate the register index count.
    • 1393f214 - vkd3d-shader/ir: Validate unused indices in a register.
    • 8e55ab6c - vkd3d-shader/ir: Validate the DCL_TEMPS instruction.
    • d27f860c - vkd3d-shader/ir: Validate the index of a TEMP register.

    Compare with previous version

    • +    for (i = reg->idx_count; i < ARRAY_SIZE(reg->idx); ++i)
      +    {
      +        if (reg->idx[i].offset != ~0u || reg->idx[i].rel_addr != NULL)
      +            validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_INDEX, "Unused index %u has offset %#x and non-NULL relative address.",
      +                i, reg->idx[i].offset);
      +    }

      I don't know about that one. There are probably still a few places left that depend on this property, but we should get rid of them and just use "idx_count".

      +static void vsir_validate_dst_count(struct validation_context *ctx,
      +        const struct vkd3d_shader_instruction *instruction, unsigned int count)
      +{
      +    if (instruction->dst_count != 0)
      +        validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_DEST_COUNT,
      +                "Invalid destination count %u for an instruction of type %#x.", instruction->dst_count, instruction->handler_idx);
      +}
      +
      +static void vsir_validate_src_count(struct validation_context *ctx,
      +        const struct vkd3d_shader_instruction *instruction, unsigned int count)
      +{
      +    if (instruction->src_count != 0)
      +        validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_SOURCE_COUNT,
      +                "Invalid source count %u for an instruction of type %#x.", instruction->src_count, instruction->handler_idx);
      +}

      "count" is not used here.

      +            /* There is not DCL_TEMPS in SM1-3. */
      +            if (vkd3d_shader_ver_ge(&ctx->parser->shader_version, 4, 0) && reg->idx_count >= 1 && reg->idx[0].offset >= ctx->temp_count)
      +                validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_INDEX, "TEMP register index %u exceeds the declared count %u.",
      +                reg->idx[0].offset, ctx->temp_count);
      +            break;

      We do have "ctx->parser->shader_desc.temp_count" though, and that's what we should use here.

               case VKD3DSIH_DCL_TEMPS:
                   vsir_validate_dst_count(ctx, instruction, 0);
                   vsir_validate_src_count(ctx, instruction, 0);
      +            /* TODO Check that each phase in a hull shader has a at
      +             * most one occurrence. */
      +            if (ctx->dcl_temps_found && ctx->parser->shader_version.type != VKD3D_SHADER_TYPE_HULL)
      +                validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_DUPLICATE_DCL_TEMPS, "Duplicate DCL_TEMPS instruction.");
      +            ctx->dcl_temps_found = true;
      +            ctx->temp_count = instruction->declaration.count;
                   break;

      And we should probably just validate that DCL_TEMPS is consistent with "ctx->parser->shader_desc.temp_count" here.

    • I don't know about that one. There are probably still a few places left that depend on this property, but we should get rid of them and just use "idx_count".

      Would it be ok for you to accept this today, and for the next MR I'll get rid of all those places and remove the check?

    • Please register or sign in to reply
  • added 2 commits

    • 4528aba1 - vkd3d-shader/ir: Validate the DCL_TEMPS instruction.
    • 102f6e68 - vkd3d-shader/ir: Validate the index of a TEMP register.

    Compare with previous version

    • I don't know about that one. There are probably still a few places left that depend on this property, but we should get rid of them and just use "idx_count".

      Would it be ok for you to accept this today, and for the next MR I'll get rid of all those places and remove the check?

      Sure. It's perhaps a bit unfortunate that we allocate an error code for it though.

      +            /* There is not DCL_TEMPS in SM1-3. */
      +            if (reg->idx_count >= 1 && 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 declared count %u.",
      +                        reg->idx[0].offset, ctx->parser->shader_desc.temp_count);
      +            break;

      The comment is perhaps a bit superfluous now, but if we're going to keep it, I think that should say "no DCL_TEMPS".

      +            /* TODO Check that each phase in a hull shader has a at
      +             * most one occurrence. */

      "has at most"

    • Sure. It's perhaps a bit unfortunate that we allocate an error code for it though.

      Right, let's just drop it. ACK to the rest.

    • Please register or sign in to reply
  • added 2 commits

    • cae9e92b - vkd3d-shader/ir: Validate the DCL_TEMPS instruction.
    • df14a75c - vkd3d-shader/ir: Validate the index of a TEMP register.

    Compare with previous version

  • added 1 commit

    • 66793227 - vkd3d-shader/ir: Validate the index of a TEMP register.

    Compare with previous version

  • added 46 commits

    • 66793227...ecdc3f39 - 38 commits from branch wine:master
    • 70f240d1 - vkd3d-shader: Deduplicate profile version comparison functions.
    • eb314ecc - vkd3d-shader/dxil: Use vsir_register_init() to initialize registers.
    • 32ecdf60 - vkd3d-shader/ir: Validate the register precision.
    • 87ae17f9 - vkd3d-shader/ir: Validate the register data type.
    • 957319a9 - vkd3d-shader/ir: Validate the register dimension.
    • 7e7c7c90 - vkd3d-shader/ir: Validate the register index count.
    • 6f5eab02 - vkd3d-shader/ir: Validate the DCL_TEMPS instruction.
    • 11fc1c05 - vkd3d-shader/ir: Validate the index of a TEMP register.

    Compare with previous version

  • Henri Verbeet approved this merge request

    approved this merge request

  • Alexandre Julliard added 10 commits

    added 10 commits

    • 11fc1c05...dd96fe50 - 2 commits from branch wine:master
    • 2ba8c577 - vkd3d-shader: Deduplicate profile version comparison functions.
    • f3c1a15a - vkd3d-shader/dxil: Use vsir_register_init() to initialize registers.
    • 72d0f765 - vkd3d-shader/ir: Validate the register precision.
    • f3a20be3 - vkd3d-shader/ir: Validate the register data type.
    • 26e4191d - vkd3d-shader/ir: Validate the register dimension.
    • 79fa5fd8 - vkd3d-shader/ir: Validate the register index count.
    • 4140b874 - vkd3d-shader/ir: Validate the DCL_TEMPS instruction.
    • af72466d - vkd3d-shader/ir: Validate the index of a TEMP register.

    Compare with previous version

  • Alexandre Julliard approved this merge request

    approved this merge request

Please register or sign in to reply
Loading