Skip to content
Snippets Groups Projects

vkd3d-shader/hlsl: Improve SM1 support for non-float operations, part 4.

Merged Francisco Casas requested to merge fcasas/vkd3d:sm1_logic_part4 into master
4 unresolved threads

SM1 support for LOGIC_NOT, LOGIC_OR, and LOGIC_AND.

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
449 450
450 451 check_condition_type(ctx, condition);
451 452
452 if (!(not = hlsl_new_unary_expr(ctx, HLSL_OP1_LOGIC_NOT, condition, &condition->loc)))
453 bool_type = hlsl_get_numeric_type(ctx, condition->data_type->class, HLSL_TYPE_BOOL,
454 condition->data_type->dimx, condition->data_type->dimy);
  • Giovanni Mascellani
    Giovanni Mascellani @giomasce started a thread on an outdated change in commit c2f91bcb
  • 2333 2333 }
    2334 2334 break;
    2335 2335
    2336 case HLSL_OP2_LOGIC_AND:
    2337 write_sm1_binary_op(ctx, buffer, D3DSIO_MUL, &instr->reg, &arg1->reg, &arg2->reg);
    • Is the commit message for Zeb's commit intended?

    • Yes, it explains the rationale for the next commit. I am not sure if that is the best format to add a comment in other person's commit though.

    • Hmm, it wasn't obvious to me that that was a comment by you on a commit by somebody else, it looked to me more like broken formatting or copy and paste.

      In this case I think I'd just add the comment without specifying its author, it's a pretty factual sentence and doesn't benefit that much from knowing who said that. However, no big deal whatever you prefer.

    • Please register or sign in to reply
  • Giovanni Mascellani approved this merge request

    approved this merge request

  • added 5 commits

    • 8dddcf4e - vkd3d-shader/hlsl: Cast to bool before applying LOGIC_NOT.
    • 0ccd6a52 - vkd3d-shader/d3dbc: Implement HLSL_OP2_LOGIC_OR for SM1.
    • 63d06493 - vkd3d-shader/d3dbc: Implement HLSL_OP2_LOGIC_AND for SM1.
    • ddb04872 - vkd3d-shader/spirv: Implement VKD3DSIH_ABS.
    • e096a1d5 - vkd3d-shader/spirv: Throw compiler error on unmapped VKD3DSPR_MISCTYPE register.

    Compare with previous version

    • Wait, how does vPos get into the symbol table in the first place? That doesn't seem right.

    • vPos doesn't get into context->symbol_table.

      We enter the

          if (!(entry = rb_get(&compiler->symbol_table, &reg_symbol)))
          {
              FIXME("Unrecognized register (%s).\n", debug_vkd3d_symbol(&reg_symbol));
              memset(register_info, 0, sizeof(*register_info));
              return false;
          }

      path.

      But because this is a FIXME and not a compiler error, compilation continues using a zeroed register_info which, I think, results in a misscompilation.


      In the fwidth.shader_test case in particular, we have a

          mov r0, vPos

      and when spirv_compiler_emit_mov() is called, we go through this path:

          if (vkd3d_swizzle_is_equal(dst_reg_info.write_mask, src->swizzle, src_reg_info.write_mask))
          {
              dst_id = spirv_compiler_get_register_id(compiler, &dst->reg);
              src_id = spirv_compiler_get_register_id(compiler, &src->reg);
      
              vkd3d_spirv_build_op_copy_memory(builder, dst_id, src_id, SpvMemoryAccessMaskNone);
              return;
          }

      and spirv_compiler_get_register_id() just ends up emitting a SpvStorageClassPrivate variable for the src register, instead of using the register in the input signature.


      So patch 7/7 makes it throw a compiler error to avoid the misscompilation.

      I guess a proper solution would be to map the registers whose type is VKD3DSPR_MISCTYPE to the right symbol in the input|output signature. In this case "VPOS", which gets assigned to type VKD3DSPR_INPUT and index 0. But I felt that that is out of the scope of this MR and maybe something for someone with more experience in spirv.c to handle.

      (please tell me if I am missing something in my analysis!).

    • vPos doesn't get into context->symbol_table.

      I meant, it doesn't get into the symbol table as vPos per-se, but it does get into the symbol table as a register with reg->type=VKD3DSPR_INPUT and reg->idx=0.

    • But because this is a FIXME and not a compiler error, compilation continues using a zeroed register_info which, I think, results in a misscompilation.

      Shouldn't we turn that from a FIXME to a compiler error, then? The problem isn't going to be specific to MISCTYPE.

    • Okay, I wasn't sure at first if that was the case.

    • Please register or sign in to reply
  • added 1 commit

    • 8285fe70 - vkd3d-shader/spirv: Throw compiler error on unrecognized register.

    Compare with previous version

  • Elizabeth Figura approved this merge request

    approved this merge request

  • Francisco Casas added 31 commits

    added 31 commits

    • 8285fe70...164608a0 - 24 commits from branch wine:master
    • 1100001a - tests: Add tests for LOGIC_NOT on uniforms.
    • 770be46f - vkd3d-shader/hlsl: Support LOGIC_NOT for SM1.
    • fb907210 - vkd3d-shader/hlsl: Cast to bool before applying LOGIC_NOT.
    • 02862bdd - vkd3d-shader/d3dbc: Implement HLSL_OP2_LOGIC_OR for SM1.
    • 2bcedbd1 - vkd3d-shader/d3dbc: Implement HLSL_OP2_LOGIC_AND for SM1.
    • c33da9eb - vkd3d-shader/spirv: Implement VKD3DSIH_ABS.
    • 62517535 - vkd3d-shader/spirv: Throw compiler error on unrecognized register.

    Compare with previous version

  • Francisco Casas added 11 commits

    added 11 commits

    • 62517535...166dc24b - 4 commits from branch wine:master
    • a11621b4 - tests: Add tests for LOGIC_NOT on uniforms.
    • 207cf77c - vkd3d-shader/hlsl: Support LOGIC_NOT for SM1.
    • 93925298 - vkd3d-shader/hlsl: Cast to bool before applying LOGIC_NOT.
    • e034b84b - vkd3d-shader/d3dbc: Implement HLSL_OP2_LOGIC_OR for SM1.
    • 36c5e721 - vkd3d-shader/d3dbc: Implement HLSL_OP2_LOGIC_AND for SM1.
    • c05ce224 - vkd3d-shader/spirv: Implement VKD3DSIH_ABS.
    • de6952f7 - vkd3d-shader/spirv: Throw compiler error on unrecognized register.

    Compare with previous version

  • Henri Verbeet approved this merge request

    approved this merge request

  • added 30 commits

    • de6952f7...68b31b73 - 23 commits from branch wine:master
    • 90e8a541 - tests: Add tests for LOGIC_NOT on uniforms.
    • ee5fc7e9 - vkd3d-shader/hlsl: Support LOGIC_NOT for SM1.
    • a838f97e - vkd3d-shader/hlsl: Cast to bool before applying LOGIC_NOT.
    • 777bf772 - vkd3d-shader/d3dbc: Implement HLSL_OP2_LOGIC_OR for SM1.
    • 9a222d2b - vkd3d-shader/d3dbc: Implement HLSL_OP2_LOGIC_AND for SM1.
    • 172cb758 - vkd3d-shader/spirv: Implement VKD3DSIH_ABS.
    • 11e72658 - vkd3d-shader/spirv: Throw compiler error on unrecognized register.

    Compare with previous version

  • Alexandre Julliard approved this merge request

    approved this merge request

  • Please register or sign in to reply
    Loading