Skip to content
Snippets Groups Projects

vkd3d-shader/ir: Check that blocks are correctly nested.

Merged Giovanni Mascellani requested to merge giomasce/vkd3d:amiata into master
1 unresolved thread

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
    • @@ -1489,6 +1489,7 @@ static void VKD3D_PRINTF_FUNC(3, 4) validator_error(struct validation_context *c
           va_end(args);
       
           vkd3d_shader_parser_error(ctx->parser, error, "instruction %zu: %s", ctx->instruction_idx + 1, buf.buffer);
      +    FIXME("VSIR validation error: instruction %zu: %s\n", ctx->instruction_idx + 1, buf.buffer);
       
           vkd3d_string_buffer_cleanup(&buf);
       }

      ERR seems appropriate. There's perhaps also an argument to be made for dumping the messages buffer to ERR on failure in vsir_validate() instead of duplicating the message here; I don't feel strongly about it though.

      Hull shaders have a different temps count for each phase, and the
      parser only reports the count for the patch constant phase.
      In order to properly check for temps count on hull shaders, we first
      need to decode its phases.

      Right, though in principle DCL_TEMPS should at least never have a count greater than "shader_desc.temp_count" either.

      @@ -209,6 +209,7 @@ enum vkd3d_shader_error
           VKD3D_SHADER_ERROR_VSIR_DUPLICATE_DCL_TEMPS         = 9013,
           VKD3D_SHADER_ERROR_VSIR_INVALID_DCL_TEMPS           = 9014,
           VKD3D_SHADER_ERROR_VSIR_INVALID_INDEX               = 9015,
      +    VKD3D_SHADER_ERROR_VSIR_INVALID_INSTRUCTION_NESTING  = 9016,

      Stray space.

    • ERR seems appropriate. There's perhaps also an argument to be made for dumping the messages buffer to ERR on failure in vsir_validate() instead of duplicating the message here; I don't feel strongly about it though.

      So far I converted to ERR(). The problem with dumping the error buffer is that if the parser is created with logging disabled (which, for example, always happens when vkd3d wants to convert a binary shader to SPIR-V) the buffer will always be empty. Since that behavior makes sense I wouldn't want to change it, but at the same time it can be useful for developers to see validation errors.

      Right, though in principle DCL_TEMPS should at least never have a count greater than "shader_desc.temp_count" either.

      I'll soon parse HS phases, so I will able to validate this properly. So, temp_count is intended to be the maximum of all the DCL_TEMPS, right?

      I also fixed a bug with underflowing depth when blocks are not matched properly.

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

    • b3d2db2a - vkd3d-shader/ir: Emit an ERR() on validation errors.
    • 3e92cbfd - vkd3d-shader/ir: Do not enfore DCL_TEMPS count for hull shaders.
    • 7855f481 - vkd3d-shader/ir: Check that IF blocks are correctly nested.
    • 780a618e - vkd3d-shader/ir: Check that LOOP blocks are correctly nested.
    • cce347bf - vkd3d-shader/ir: Check that REP blocks are correctly nested.
    • c925d12d - vkd3d-shader/ir: Check that SWITCH blocks are correctly nested.

    Compare with previous version

  • Giovanni Mascellani approved this merge request

    approved this merge request

  • ERR seems appropriate. There's perhaps also an argument to be made for dumping the messages buffer to ERR on failure in vsir_validate() instead of duplicating the message here; I don't feel strongly about it though.

    So far I converted to ERR(). The problem with dumping the error buffer is that if the parser is created with logging disabled (which, for example, always happens when vkd3d wants to convert a binary shader to SPIR-V) the buffer will always be empty. Since that behavior makes sense I wouldn't want to change it, but at the same time it can be useful for developers to see validation errors.

    We might need to create a secondary output buffer with an independent log level for debug output. I think the problem is solvable, but no need to worry about it for now.

    Right, though in principle DCL_TEMPS should at least never have a count greater than "shader_desc.temp_count" either.

    I'll soon parse HS phases, so I will able to validate this properly. So, temp_count is intended to be the maximum of all the DCL_TEMPS, right?

    Effectively, yes. More formally it's the number of unique temporaries used by the shader.

  • Henri Verbeet approved this merge request

    approved this merge request

  • Alexandre Julliard approved this merge request

    approved this merge request

  • Alexandre Julliard added 34 commits

    added 34 commits

    • c925d12d...0ef25ad1 - 28 commits from branch wine:master
    • ca3f594a - vkd3d-shader/ir: Emit an ERR() on validation errors.
    • 0a7e200f - vkd3d-shader/ir: Do not enfore DCL_TEMPS count for hull shaders.
    • 2f7d52db - vkd3d-shader/ir: Check that IF blocks are correctly nested.
    • 92c36615 - vkd3d-shader/ir: Check that LOOP blocks are correctly nested.
    • 93632fb4 - vkd3d-shader/ir: Check that REP blocks are correctly nested.
    • 7d49f963 - vkd3d-shader/ir: Check that SWITCH blocks are correctly nested.

    Compare with previous version

Please register or sign in to reply
Loading