Skip to content
Snippets Groups Projects

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

Merged Francisco Casas requested to merge fcasas/vkd3d:sm1_logic_part3 into master
5 unresolved threads

Mainly the implementation of SM1 comparison operators, defined in terms of CMP and SLT instructions, and the capacity to transform SLT->CMP for pixel shaders and CMP->SLT for vertex shaders.

Edited by Francisco Casas

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
1348 1348 return hlsl_new_expr(ctx, op, operands, arg1->data_type, &arg1->loc);
1349 1349 }
1350 1350
1351 struct hlsl_ir_node *hlsl_new_ternary_expr(struct hlsl_ctx *ctx, enum hlsl_ir_expr_op op,
  • Giovanni Mascellani
    Giovanni Mascellani @giomasce started a thread on an outdated change in commit e13f4e11
  • 3057 struct hlsl_constant_value zero_value;
    3058
    3059 memset(operands, 0, sizeof(operands));
    3060 operands[0] = arg2_cast;
    3061 if (!(neg = hlsl_new_expr(ctx, HLSL_OP1_NEG, operands, float_type, &instr->loc)))
    3062 return false;
    3063 hlsl_block_add_instr(block, neg);
    3064
    3065 memset(operands, 0, sizeof(operands));
    3066 operands[0] = arg1_cast;
    3067 operands[1] = neg;
    3068 if (!(sub = hlsl_new_expr(ctx, HLSL_OP2_ADD, operands, float_type, &instr->loc)))
    3069 return false;
    3070 hlsl_block_add_instr(block, sub);
    3071
    3072 /* Use MUL as a precarious ABS. */
    • Why aren't you using ABS itself?

    • Author Developer

      While according to the documentation the "abs" instruction exists for all shader profiles starting from ps_2_0 and vs_2_0, the native compiler doesn't use it directly.

      • For both ps_2_0 and vs_2_0 it uses this mul trick, because we only care if the absolute value is not 0.
      • I checked now and for ps_3_0 and vs_3_0 it adds the "_abs" modifier to the src registers of the comparison instruction.

      So it may make sense to actually add the path to use ABS if the profile->major_version is larger than 2.0, similarly to what lower_ternary() does. I don't know if this is necessary, the only case I can think of where this differ is for very large values where the multiplication falls outside the representable range of as a float, but even in those cases (which I think map to "inf"?) the sign should also be correct.

      I will check what happens on Windows on these cases.

    • Francisco Casas changed this line in version 2 of the diff

      changed this line in version 2 of the diff

    • Author Developer

      I checked. It doesn't make a difference for large numbers (whose multiplication results in inf), but it does make a difference for smaller numbers whole multiplication results in zero. So I added the condition on the profile->major_version paths.

      I added a pertinent tests.

    • Please register or sign in to reply
    • vs_2_0 uses "slt dst, -mul, mul" instead of "slt dst, 0, mul". Does this ever make a difference? Should we do it anyway just to avoid an otherwise unnecessary constant?

    • Author Developer

      If I am not mistaken, it should not make a difference. But since we don't have neg as a modifier, only as an instruction, in HLSL IR, we have to add the NEG as an additional instruction.

      I added the change though, in case there is some difference I am not considering.

    • Please register or sign in to reply
  • Francisco Casas added 5 commits

    added 5 commits

    • 77935062 - vkd3d-shader/hlsl: Implement SM1 comparison operators.
    • 55dd6b45 - tests: Test equality between tiny and between large numbers on ps_2_0.
    • 4b188e1f - vkd3d-shader/hlsl: Lower SLT instructions for pixel shaders.
    • dc7d15d0 - vkd3d-shader/hlsl: Use hlsl_fixme() on missing SM1 matrix writemask lowering.
    • f4f78656 - vkd3d-shader/hlsl: Lower CMP instructions for vertex shaders.

    Compare with previous version

  • Elizabeth Figura approved this merge request

    approved this merge request

  • Giovanni Mascellani
  • Giovanni Mascellani
  • Giovanni Mascellani
    Giovanni Mascellani @giomasce started a thread on commit 77935062
  • 3076
    3077 if (!(slt = hlsl_new_binary_expr(ctx, HLSL_OP2_SLT, zero, mul)))
    3078 return false;
    3079 hlsl_block_add_instr(block, slt);
    3080
    3081 negate = (expr->op == HLSL_OP2_EQUAL);
    3082 break;
    3083 }
    3084 case HLSL_OP2_GEQUAL:
    3085 case HLSL_OP2_LESS:
    3086 {
    3087 if (!(slt = hlsl_new_binary_expr(ctx, HLSL_OP2_SLT, arg1_cast, arg2_cast)))
    3088 return false;
    3089 hlsl_block_add_instr(block, slt);
    3090
    3091 negate = (expr->op == HLSL_OP2_GEQUAL);
  • Giovanni Mascellani approved this merge request

    approved this merge request

  • Francisco Casas added 5 commits

    added 5 commits

    • ef82cf11 - vkd3d-shader/hlsl: Implement SM1 comparison operators.
    • 5ec5fae3 - tests: Test equality between tiny and between large numbers on ps_2_0.
    • e93ddb6e - vkd3d-shader/hlsl: Lower SLT instructions for pixel shaders.
    • 1099abf6 - vkd3d-shader/hlsl: Use hlsl_fixme() on missing SM1 matrix writemask lowering.
    • 2a9bcbb9 - vkd3d-shader/hlsl: Lower CMP instructions for vertex shaders.

    Compare with previous version

  • Henri Verbeet approved this merge request

    approved this merge request

  • Alexandre Julliard approved this merge request

    approved this merge request

  • Please register or sign in to reply
    Loading