Skip to content
Snippets Groups Projects

vkd3d-shader/hlsl: Fixes for SM1 writemasks and swizzles.

Merged Francisco Casas requested to merge fcasas/vkd3d:register_alloc_3 into master

Because of the change introduced in f21693b2, SM1 scalars and vectors were not longer getting the correct writemask when they are allocated, so this is fixed.

Also, the mapping of sm1 src register swizzles is moved outside write_sm1_instruction() since there are some instructions that don't do this, remarkably dp2add. This is fixed.

Before the last patch we are writing the operation as:

dp2add r0.x, r1.x, r0.x, r2.x

and now it is:

dp2add r0.x, r1.xyxx, r0.xyxx, r2.x

dp2add now has its own function, write_sm1_dp2add(), since it seems to be the only instruction with this structure.

Ideally we would be using the default swizzles for the first two src arguments:

dp2add r0.x, r1, r0, r2.x

since, according to native's documentation, these are supported for all sm < 4.

But using default swizzles whenever is possible -- along with following the conversion of repeating the last component of the swizzle when fewer than 4 components are to be specified -- has a higher scope. Probably would involve modifying hlsl_swizzle_from_writemask() and hlsl_map_swizzle().

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
  • Francisco Casas changed title from vkd3d-shader/hlsl: Set writemasks correctly for SM1 scalar and vector types. to vkd3d-shader/hlsl: Fixes for SM1 writemasks and swizzles.

    changed title from vkd3d-shader/hlsl: Set writemasks correctly for SM1 scalar and vector types. to vkd3d-shader/hlsl: Fixes for SM1 writemasks and swizzles.

  • Francisco Casas changed the description

    changed the description

    • Patch 1/3 does two things at once [introduces a helper and changes allocate_register()], which made it somewhat harder to review. Anyway, the concept seems fine, but "dimx" and "component_count" aren't great names. Honestly I'd use "component_count" and "reg_size" respectively.

      Patch 2/3 is fine, although I'd prefer something closer to the sm4 architecture.

      But this change -- along with following the conversion of repeating the last component of the swizzle when fewer than 4 components are to be specified -- would require more global changes, probably in hlsl_swizzle_from_writemask() and hlsl_map_swizzle().

      Yeah, and while I wouldn't object to it on the grounds of making it a bit easier to compare output by eye, I don't think it's necessary.

    • Author Developer

      Patch 1/3 does two things at once [introduces a helper and changes allocate_register()], which made it somewhat harder to review.

      I split that patch in two, albeit the intermediate state is a little weird since I have to pass the reg_size twice in the callers in order to preserve the current behavior. The patch that solves the problem is still the one that introduces the helper.

      Otherwise I would have had to add ifs around all allocate_register() calls during the intermediate state.

      Anyway, the concept seems fine, but "dimx" and "component_count" aren't great names. Honestly I'd use "component_count" and "reg_size" respectively.

      I agree. It is worth noting that this only makes sense if we rename the old "component_count" as "reg_size" and "dimx" is a new "component_count". So, it also made sense to do the rename "component_count" -> "reg_size" in the other functions related to register allocation to avoid confusion. I prepended a patch for that purpose.

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

    added 5 commits

    • bf2b2d34 - vkd3d-shader/hlsl: Rename 'component_count' arguments to 'reg_size'.
    • 9a5c23e6 - vkd3d-shader/hlsl: Expect component count in allocate_register().
    • 0b647022 - vkd3d-shader/hlsl: Set writemasks correctly for SM1 scalar and vector types.
    • 3c2c04cb - vkd3d-shader/hlsl: Map SM1 src swizzles outside write_sm1_instruction().
    • 2fbc6347 - vkd3d-shader/hlsl: Fix SM1 dp2add swizzles.

    Compare with previous version

  • Elizabeth Figura approved this merge request

    approved this merge request

  • Giovanni Mascellani approved this merge request

    approved this merge request

  • Unfortunately this will need a rebase after !79 (merged).

  • Francisco Casas added 15 commits

    added 15 commits

    • 2fbc6347...ab601d65 - 10 commits from branch wine:master
    • 5dc257b7 - vkd3d-shader/hlsl: Rename 'component_count' arguments to 'reg_size'.
    • 9e177271 - vkd3d-shader/hlsl: Expect component count in allocate_register().
    • 9a9ff7d8 - vkd3d-shader/hlsl: Set writemasks correctly for SM1 scalar and vector types.
    • ae67a08a - vkd3d-shader/hlsl: Map SM1 src swizzles outside write_sm1_instruction().
    • 4521fb80 - vkd3d-shader/hlsl: Fix SM1 dp2add swizzles.

    Compare with previous version

  • Henri Verbeet approved this merge request

    approved this merge request

  • added 5 commits

    • 0ecac994 - vkd3d-shader/hlsl: Rename 'component_count' arguments to 'reg_size'.
    • 6b4a4c4c - vkd3d-shader/hlsl: Expect component count in allocate_register().
    • b84f560b - vkd3d-shader/hlsl: Set writemasks correctly for SM1 scalar and vector types.
    • d0172992 - vkd3d-shader/hlsl: Map SM1 src swizzles outside write_sm1_instruction().
    • 902ddee5 - vkd3d-shader/hlsl: Fix SM1 dp2add swizzles.

    Compare with previous version

  • Alexandre Julliard approved this merge request

    approved this merge request

Please register or sign in to reply
Loading