Skip to content
Snippets Groups Projects

vkd3d-shader/d3dbc: Normalise I/O register write masks when not disassembling.

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

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
  • Giovanni Mascellani approved this merge request

    approved this merge request

    • +    if (!sm1->disassembling)
      +        element->used_mask &= element->mask;

      Couldn't we just check for something like "program->normalisation_level >= VSIR_NORMALISED_SM4"?

           vkd3d_shader_parser_init(&sm1->p, program, message_context, compile_info->source_name);
           sm1->ptr = sm1->start;
      +    sm1->disassembling = compile_info->target_type == VKD3D_SHADER_TARGET_D3D_ASM;
      +
      +    if (sm1->disassembling)
      +        program->normalisation_level = VSIR_NORMALISED_SM1;

      Instead of modifying the normalisation level shortly after calling vsir_program_init(), it would make more sense to me to pass the appropriate level to vsir_program_init() in the first place.

      -    /* Here we'd likely want to validate that the usage mask is a subset of the
      -     * signature mask. Unfortunately the D3DBC parser sometimes violates this.
      -     * For example I've seen a shader like this:
      -     *   ps_3_0
      -     *   [...]
      -     *   dcl_texcoord0 v0
      -     *   [...]
      -     *   texld r2.xyzw, v0.xyzw, s1.xyzw
      -     *   [...]
      -     *
      -     * The dcl_textcoord0 instruction secretly has a .xy mask, which is used to
      -     * compute the signature mask, but the texld instruction apparently uses all
      -     * the components. Of course the last two components are ignored, but
      -     * formally they seem to be used. So we end up with a signature element with
      -     * mask .xy and usage mask .xyzw.
      -     *
      -     * The correct fix would probably be to make the D3DBC parser aware of which
      -     * components are really used for each instruction, but that would take some
      -     * time. */

      It may not hurt to retain that comment in some form; either here, in the parser when adjusting the mask, or both.

    • It may not hurt to retain that comment in some form; either here, in the parser when adjusting the mask, or both.

      Let me know whether the amended version sounds better.

      BTW, I think I'll eventually have to fix the register mask/swizzle too; actually, that's the part I really care about, because I need to ensure that each register resolves unambiguously to a single signature element, and since for later shader models more than a signature element might use the same register index the write mask has to be taken into consideration too.

    • Please register or sign in to reply
  • Henri Verbeet changed title from kd3d-shader/d3dbc: Normalise I/O register write masks when not disassembling. to vkd3d-shader/d3dbc: Normalise I/O register write masks when not disassembling.

    changed title from kd3d-shader/d3dbc: Normalise I/O register write masks when not disassembling. to vkd3d-shader/d3dbc: Normalise I/O register write masks when not disassembling.

  • added 1 commit

    • 5fcafe39 - vkd3d-shader/d3dbc: Normalise I/O register write masks when not disassembling.

    Compare with previous version

    • This is just an opinion, and I don't understand well the different vsir normalization levels yet, but I don't think it is good to start adding target_type checks in the sm1 parser.

      Have you considered leaving the parser to always retrieve SM1-normalized vsir and add these signature modifications in an independent ir.c pass that turns a SM1-normalized program into a SM4-normalized program? This pass to be skipped when disassembling.

      Edited by Francisco Casas
    • Have you considered leaving the parser to always retrieve SM1-normalized vsir and add these signature modifications in an independent ir.c pass that turns a SM1-normalized program into a SM4-normalized program? This pass to be skipped when disassembling.

      I considered that, and while on one hand I appreciate how completely decoupling the frontend from the backend is a cleaner decision, on the other hand that would require some more code. Not that much, but still more than the two-line change here. Also, to me disassembling is a somewhat more special target than all the others, since it needs us to preserve the exact program form, not just its semantics. So it's less of a sin to treat it differently even in the parser.

      On top of that, this decision can be reverted at any time without too much hassle. So I would bother with moving the change to a separate pass only once there is some evidence that it would make something easier. Currently we don't even apply passes when disassembling, do we?

    • Please register or sign in to reply
  • Francisco Casas approved this merge request

    approved this merge request

  • Henri Verbeet approved this merge request

    approved this merge request

  • Henri Verbeet added 17 commits

    added 17 commits

    • 5fcafe39...09095403 - 15 commits from branch wine:master
    • 64126a00 - vkd3d-shader/ir: Give more meaningful names to I/O normalisation levels.
    • dd0ed989 - vkd3d-shader/d3dbc: Normalise I/O register write masks when not disassembling.

    Compare with previous version

Please register or sign in to reply
Loading