Skip to content
Snippets Groups Projects

vkd3d-shader/hlsl: Implement Interlocked*() intrinsics for UAV.

Merged Shaun Ren requested to merge shaunren/vkd3d:hlsl-atomics/for-review into master
All threads resolved!

Merge request reports

Merge request pipeline #36695 skipped

Merge request pipeline skipped for 802d7317

Approval is optional

Merged by Henri VerbeetHenri Verbeet 2 months ago (Jan 20, 2025 3:18pm UTC)

Merge details

  • Changes merged into master with 802d7317.
  • Deleted the source branch.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Shaun Ren changed title from vkd3d-shader/hlsl: Implement Interlocked() intrinsics.* to vkd3d-shader/hlsl: Implement Interlocked() intrinsics for UAV.*

    changed title from vkd3d-shader/hlsl: Implement Interlocked() intrinsics.* to vkd3d-shader/hlsl: Implement Interlocked() intrinsics for UAV.*

    • Resolved by Shaun Ren

      Did you consider using HLSL_IR_RESOURCE_STORE instead, and adding a resource_store_type like resource_load_type?

      I know that this is valid for TGSM as well, which doesn't look like a resource, but... maybe it should. I don't think there's any reason not to. There's not that much code that deals with HLSL_IR_RESOURCE_STORE that we'd need to make aware of store type or destination storage type (not that there's much code that deals with it in the first place).

      Though on the other hand, the part where it modifies the memory and returns a value is unique... hmm.

      From 5/13:

      +    if (lhs_type->class != HLSL_CLASS_SCALAR || (lhs_type->e.numeric.type != HLSL_TYPE_UINT
      +            && lhs_type->e.numeric.type != HLSL_TYPE_INT))

      We don't have tests for this. Specifically I'd like to see a test for a vec1 here.

      +    val_type = hlsl_get_scalar_type(ctx, HLSL_TYPE_UINT);
      +    if (!(val = add_implicit_conversion(ctx, params->instrs, params->args[1], val_type, loc)))
      +        return false;

      This looks wrong (and in a way that matters even for operations other than max/min). You'd expect a cast to the type of the LHS. While tests bear it out, I think it deserves a comment to say "yes, this is correct".

      +        if (!hlsl_index_is_resource_access(hlsl_ir_index(lhs)))
      +        {
      +            hlsl_fixme(ctx, &lhs->loc, "Non-direct structured resource interlocked targets.");
      +            return false;
      +        }

      Are these ever valid?

      Also: I believe this commit is also missing handling in validate_dereferences().

      Also: Interlocked*() intrinsics return void, at least according to Microsoft. That means we should be constructing a void expr like in add_getdimensions_method_call(). Assuming that's correct [and a test here wouldn't hurt.]

      From 7/13:

      -    if (params->args_count != 2 && params->args_count != 3)
      +    if (op != HLSL_INTERLOCKED_CMP_EXCH && params->args_count != 2 && params->args_count != 3)

      This doesn't actually validate the number of parameters for cmpxchg though. Similarly for InterlockedCompareStore() in the next commit.

  • Shaun Ren added 12 commits

    added 12 commits

    • 07cec5fc...aaa81e47 - 2 earlier commits
    • 9b77c7a1 - tests/hlsl: Add tests for valid dst types of atomic ops.
    • 626398c1 - vkd3d-shader/hlsl: Implement InterlockedAdd() intrinsic.
    • 2b5dd65b - vkd3d-shader/hlsl: Implement InterlockedAnd() intrinsic.
    • 94547cff - vkd3d-shader/hlsl: Implement InterlockedCompareExchange() intrinsic.
    • c5f9df38 - vkd3d-shader/hlsl: Implement InterlockedCompareStore() intrinsic.
    • 01e55c99 - vkd3d-shader/hlsl: Implement InterlockedExchange() intrinsic.
    • 78b9afcd - vkd3d-shader/hlsl: Implement InterlockedMax() intrinsic.
    • 2891c10d - vkd3d-shader/hlsl: Implement InterlockedMin() intrinsic.
    • e0203e0f - vkd3d-shader/hlsl: Implement InterlockedOr() intrinsic.
    • d7bb60dd - vkd3d-shader/hlsl: Implement InterlockedXor() intrinsic.

    Compare with previous version

  • Shaun Ren added 5 commits

    added 5 commits

    • f22dc9cb - vkd3d-shader/hlsl: Implement InterlockedExchange() intrinsic.
    • ec77af20 - vkd3d-shader/hlsl: Implement InterlockedMax() intrinsic.
    • a0de4a3c - vkd3d-shader/hlsl: Implement InterlockedMin() intrinsic.
    • 93985859 - vkd3d-shader/hlsl: Implement InterlockedOr() intrinsic.
    • 3e4b2aad - vkd3d-shader/hlsl: Implement InterlockedXor() intrinsic.

    Compare with previous version

  • Shaun Ren added 12 commits

    added 12 commits

    • 3e4b2aad...1c7803d8 - 2 earlier commits
    • 18f4f9d5 - tests/hlsl: Add tests for valid dst types of atomic ops.
    • f1f5535a - vkd3d-shader/hlsl: Implement InterlockedAdd() intrinsic.
    • f96679b5 - vkd3d-shader/hlsl: Implement InterlockedAnd() intrinsic.
    • 9ed3a646 - vkd3d-shader/hlsl: Implement InterlockedCompareExchange() intrinsic.
    • 72311aae - vkd3d-shader/hlsl: Implement InterlockedCompareStore() intrinsic.
    • 519799c3 - vkd3d-shader/hlsl: Implement InterlockedExchange() intrinsic.
    • b39eb5a3 - vkd3d-shader/hlsl: Implement InterlockedMax() intrinsic.
    • be95ddc9 - vkd3d-shader/hlsl: Implement InterlockedMin() intrinsic.
    • e996f555 - vkd3d-shader/hlsl: Implement InterlockedOr() intrinsic.
    • 1023f16c - vkd3d-shader/hlsl: Implement InterlockedXor() intrinsic.

    Compare with previous version

  • Elizabeth Figura approved this merge request

    approved this merge request

  • Shaun Ren added 48 commits

    added 48 commits

    • 1023f16c...a082daeb - 32 commits from branch wine:master
    • a082daeb...9d4ddc84 - 6 earlier commits
    • 2fd2b1cc - tests/hlsl: Add tests for valid dst types of atomic ops.
    • 4aef5580 - vkd3d-shader/hlsl: Implement InterlockedAdd() intrinsic.
    • 74eaaba2 - vkd3d-shader/hlsl: Implement InterlockedAnd() intrinsic.
    • 46b5b92a - vkd3d-shader/hlsl: Implement InterlockedCompareExchange() intrinsic.
    • 180be172 - vkd3d-shader/hlsl: Implement InterlockedCompareStore() intrinsic.
    • fd3f1fe4 - vkd3d-shader/hlsl: Implement InterlockedExchange() intrinsic.
    • 2455f61f - vkd3d-shader/hlsl: Implement InterlockedMax() intrinsic.
    • e6d9f24c - vkd3d-shader/hlsl: Implement InterlockedMin() intrinsic.
    • ac7e3950 - vkd3d-shader/hlsl: Implement InterlockedOr() intrinsic.
    • e5663342 - vkd3d-shader/hlsl: Implement InterlockedXor() intrinsic.

    Compare with previous version

  • Author Contributor

    In the latest revision:

    • Rebased.
    • Minor changes to some tests to avoid the problem of dxc emitting i32 undef in the value field for negative floating numbers.
    • Always cast floating numbers to signed int for InterlockedMax/Min.
    Edited by Shaun Ren
  • Shaun Ren resolved all threads

    resolved all threads

  • Henri Verbeet approved this merge request

    approved this merge request

  • Henri Verbeet added 20 commits

    added 20 commits

    • e5663342...10edcec0 - 4 commits from branch wine:master
    • 10edcec0...b3c13b44 - 6 earlier commits
    • 5f943e91 - tests/hlsl: Add tests for valid destination types of atomic operations.
    • 660a71ef - vkd3d-shader/hlsl: Implement the InterlockedAdd() intrinsic.
    • 3b19a4aa - vkd3d-shader/hlsl: Implement the InterlockedAnd() intrinsic.
    • 22ab08f4 - vkd3d-shader/hlsl: Implement the InterlockedCompareExchange() intrinsic.
    • 3fb47373 - vkd3d-shader/hlsl: Implement the InterlockedCompareStore() intrinsic.
    • b447fdce - vkd3d-shader/hlsl: Implement the InterlockedExchange() intrinsic.
    • 63fbe161 - vkd3d-shader/hlsl: Implement the InterlockedMax() intrinsic.
    • 5b9634a7 - vkd3d-shader/hlsl: Implement the InterlockedMin() intrinsic.
    • cbd504e8 - vkd3d-shader/hlsl: Implement the InterlockedOr() intrinsic.
    • 802d7317 - vkd3d-shader/hlsl: Implement the InterlockedXor() intrinsic.

    Compare with previous version

Please register or sign in to reply
Loading