Skip to content

64-bit LDS atomics under the workgroup lock lose concurrent 32-bit atomics on the same dwords, which the hardware serialises #2297

Closed

Description

@SleglJan

Measured

RDNA2 (gfx1036, Ryzen 7950X3D iGPU) with tools/hw-oracle, Linux (CachyOS, kernel 7.2.9): 256 lanes in one workgroup (8 wave32 or 4 wave64 waves) on one LDS slot initialised to 0; 128 lanes ds_add_u64 with 0xffffffff and 128 lanes ds_add_u32 with 1 on the slot's high dword. The hardware gives exactly 0xffffffff80 (= 128 · 0xffffffff + 128 · 2^32) in 6 of 6 runs per wave size: the LDS serialises 32-bit and 64-bit atomics on the same address.

The same kernel through the recompiler (main d70b899 with my pending LDS float-atomics change on top, which touches neither the lock nor the integer paths; run through the replay harness oracle-audit/KernelReplay.cpp on the linked branch), 20 runs per host and wave size, counting the lost 32-bit increments of 2^32 each:

RADV (Mesa 26.2.4) RTX 3080 (615.71.09)
wave32 64 lost in 19 runs, 96 in 1 lost in 18 of 20 runs: 16 to 80, most often 32
wave64 64 lost in 16 runs, 32 in 2, 96 in 2 0 in 17 runs, 16 in 3

Every loss is a multiple of 16 and the low dword is 0xffffff80 in every run, so no 64-bit add was lost; the loss is timing-dependent. The pure 64-bit case is exact on both hosts: 256 lanes of ds_add_rtn_u64 on one slot sum to 0xffffffff00 and the returned values form one serial order.

This is the limitation TechnicalDebt records for the lock ("atomic with respect to each other only: 32-bit accesses to the same dwords issued concurrently are not ordered by the lock"); the measurement shows that the hardware does order them, and how much is lost. Bodies, rows, outputs and the per-run finals: https://github.com/SleglJan/AnyPS5/tree/audit/oracle-2026-10-08/oracle-audit/lds64 (mixed-repeats.json, contention.json, hosts-report.json, mixed-*-body.s).

Possible fix

#573 named the alternative: VK_KHR_workgroup_memory_explicit_layout to alias the LDS block as 32-bit and 64-bit arrays, and shaderSharedInt64Atomics for native 64-bit atomics on it. Both features are reported here by RADV and by the RTX 3080. Native atomics would also remove the lock and its elect loop (#826, #833); the lock would stay as the fallback for devices without the features. #1776 (open) moves large LDS and the lock dword to a storage buffer, so a native path has to coordinate with it. I can take this if nobody is on it.

AI-assisted: yes (Claude Code).

Activity

  1. husseinkad commented on Oct 10, 2026

    @husseinkad
    Contributor

    I have a change for this: #2675 (branch husseinkad:fix/lds-atomic-mixed-width-lock). In a program with a 64-bit LDS atomic, every 32-bit LDS atomic now takes the same workgroup lock; programs without one emit the same SPIR-V as before. It adds a deterministic SPIR-V structure test that also runs on lavapipe CI and an execution test with a divergent case, and the TechnicalDebt entry records that only ds_add_u32 against ds_add_u64 was measured. The intake check closed the PR automatically, so the branch is here for a collaborator to carry. I'm working on this one, so please say here before starting the same change.

    AI-assisted: yes.

  2. SleglJan commented on Oct 10, 2026

    @SleglJan
    ContributorAuthor

    Cross-test of #2675 (husseinkad:fix/lds-atomic-mixed-width-lock at ad49d36) with this issue's reproducer. Its two commits cherry-pick onto main b5f5362 without conflicts. The kernel is 128 lanes of ds_add_u64 and 128 lanes of ds_add_u32 on the slot's high dword, the same assembled kernel and rows through KernelReplay as above, 20 runs per host and wave size. Linux (CachyOS, kernel 7.2.9, GCC 16.2.1), Release with ANYPS5_ENABLE_SPIRV_TOOLS=ON.

    Lost 32-bit increments (of 128) main b5f5362 with #2675
    RADV (Ryzen 7950X3D iGPU, Mesa 26.2.4), wave32 64 in 19 runs, 96 in 1 0 in 20 runs
    RADV, wave64 64 in 19 runs, 32 in 1 0 in 20 runs
    RTX 3080 (NVIDIA 615.71.09), wave32 16 to 64 in 19 runs, 0 in 1 0 in 20 runs
    RTX 3080, wave64 0 in 18 runs, 16 in 2 0 in 20 runs

    With #2675, every run ends at 0xffffffff80, the value the hardware gives (6 of 6 runs per wave size, above).

    • agc_spirv_lds_atomic_lock passes. With only the change to SpirvMemoryInstructions.cpp reverted, it fails with "elect 1, lock acquire 1, lock release 1, expected 2 each", as in the PR.
    • agc_driver_lds_mixed_width_atomics fails on both GPUs in Recompile of its first (uniform) kernel, so neither the sum check nor the divergent case runs. The message is the same with APS5_SPIRV_OPT=full and =none; it is word=231 on RADV and word=234 on NVIDIA:
      ShaderRecompiler::Recompile: SPIR-V validation after optimization failed:
      level=2 word=231: The continue construct with the continue target '23[%23]' is not structurally post dominated by the back-edge block '165[%165]'
      
    • With the change to SpirvMemoryInstructions.cpp reverted, the same kernel fails the same way (back-edge block 106[%106]). On main the kernel's only lock is the one ds_add_u64 takes, so the failure is not from fix(shader): LDS 32-bit atomics take the workgroup lock of the program's 64-bit LDS atomics #2675's change.
    • Recompiler: SPIR-V validation fails on a continue construct its back-edge block does not post-dominate #1921 reported the same validator message from validation before optimization; 33603bf fixed that case (runtime image mode switches). Here, validation before optimization passes and the optimized module fails.
    • fix(shader): LDS 32-bit atomics take the workgroup lock of the program's 64-bit LDS atomics #2675's own build and test jobs were skipped when the intake check closed it.
      • The Linux job builds with ANYPS5_ENABLE_SPIRV_TOOLS=ON and runs the execution tests on lavapipe with ANYPS5_REQUIRE_VULKAN=1, so this test runs there with validation. I have not tried it on lavapipe here.
      • Builds with the default ANYPS5_ENABLE_SPIRV_TOOLS=OFF neither validate nor optimize.

    AI-assisted: yes (Claude Code)

  3. husseinkad commented on Oct 10, 2026

    @husseinkad
    Contributor

    Thank you @SleglJan for the cross-test, the per-GPU table (0 lost increments in 20 runs on RADV and the RTX 3080 at both wave sizes, against 16 to 96 lost on main) is exactly the hardware evidence the branch lacked, and it also matches the 0xffffffff80 hardware value from your first run.

    You found a real problem in my execution test: I only built with ANYPS5_ENABLE_SPIRV_TOOLS off, so I never saw that its hand-assembled uniform loop fails SPIR-V validation after optimization (continue construct ... not structurally post dominated by the back-edge block), and CI's Linux job builds with validation on. The structural test agc_spirv_lds_atomic_lock does not have that problem. I'll reproduce it with validation enabled, change the test kernel so it validates (the failure is independent of the lock change, as you showed), and push the fix to the same branch.

    AI-assisted: yes.

  4. husseinkad commented on Oct 10, 2026

    @husseinkad
    Contributor

    @SleglJan the validation failure is fixed on the same branch, husseinkad:fix/lds-atomic-mixed-width-lock (2e0fd5db, one test-only commit on top of ad49d36a; the lock implementation is unchanged).

    Cause, reproduced here with ANYPS5_ENABLE_SPIRV_TOOLS=ON on an RTX 4050 (Windows, the same message as yours: continue construct ... not structurally post dominated by the back-edge block). The recompiler's module validates before optimization. After DeadBranchElim removes the constant-true selections, the SPIRV-Tools BlockMergePass (the submodule at 7f2d9ee, v2025.4) merges the loop header with its multi-block continue target, the header then names itself as the continue target, and the validator rejects the result. It is independent of the lock change, as you showed. I reproduced it with a module that has a plain block at the start of a loop body and a nested loop, running only CreateBlockMergePass(): valid input, invalid output. So it can hit any loop of that shape, not just this test.

    The test kernel no longer has the scalar loop; the 16 iterations are emitted inline (same 256 threads, same expected sums, same divergent case), which validates. Results with the change, SPIRV_TOOLS on: ctest -R "lds_mixed|agc_spirv_lds" 2 of 2 pass; default build ctest -R "lds_|ds_|agc_spirv_lds" 23 of 23 pass. With only SpirvMemoryInstructions.cpp reverted both tests fail (elect 1, lock acquire 1, lock release 1, expected 2 each and lane 0 read 0x0000167ffffff000, expected 0x00001ffffffff000). I have not run lavapipe or Linux; could you re-run agc_driver_lds_mixed_width_atomics on RADV and the RTX 3080 with this commit?

    AI-assisted: yes.

  5. SleglJan commented on Oct 10, 2026

    @SleglJan
    ContributorAuthor

    Rerun with 2e0fd5d, with the three commits cherry-picked onto main cf8b129 without conflicts. Linux (CachyOS, kernel 7.2.9, GCC 16.2.1), Release with ANYPS5_ENABLE_SPIRV_TOOLS=ON.

    agc_driver_lds_mixed_width_atomics (wave32), 20 runs per GPU:

    RADV (Ryzen 7950X3D iGPU, Mesa 26.2.4) RTX 3080 (NVIDIA 615.71.09)
    with the change passes 20 of 20 passes 20 of 20
    SpirvMemoryInstructions.cpp reverted passes 20 of 20 fails 20 of 20 in the uniform case: lane 0 reads 0x0000165ffffff000 to 0x000016dffffff000, varying between runs, where 0x00001ffffffff000 is expected
    • The validation error is gone on both GPUs, and both the uniform and the divergent case run and pass.
    • agc_spirv_lds_atomic_lock, which uses no GPU, passes with the change and fails with it reverted.
    • On RADV, the unrolled kernel does not show the race: with the change reverted it passes in all 20 runs. By contrast, this issue's replay kernel lost 32 to 96 increments in every run on the same driver (my previous comment, main b5f5362, wave32 and wave64). On this GPU, only agc_spirv_lds_atomic_lock catches a revert.

    AI-assisted: yes (Claude Code)

  6. husseinkad commented on Oct 10, 2026

    @husseinkad
    Contributor

    Thank you @SleglJan, that confirms both things I could not test: the validation error is gone on RADV and the RTX 3080, and with the lock change the test passes 20 of 20 on both. You also found the weak spot: with the change reverted my execution test only fails on the NVIDIA GPU (20 of 20) and passes on RADV, while this issue's own kernel (128 lanes of ds_add_u64, 128 lanes of ds_add_u32 on the slot's high dword) loses 32 to 96 increments every run there. The structural test agc_spirv_lds_atomic_lock is what catches a revert on every host, but an execution test that fails on RADV as well would be better. I'll try to rebuild the execution test around the issue's kernel shape (separate lanes for the 64-bit and the 32-bit add) and push it to the same branch if it discriminates on this machine's GPU; I'll post here either way.

    AI-assisted: yes.

  7. husseinkad commented on Oct 10, 2026

    @husseinkad
    Contributor

    @SleglJan I added the case you pointed at to the same branch, husseinkad:fix/lds-atomic-mixed-width-lock (7775d6c7, a test-only commit on top of 2e0fd5db): a single-pass kernel in the issue's shape, even lanes run ds_add_u64 and odd lanes run ds_add_u32 at offset +4 (parity split with s_and_saveexec, unrolled, no loop), with the issue's constants and the expected 0xffffffff80; Run compiles it once and dispatches it 32 times. (I had guessed lanes 0..127 against 128..255 first; that variant never failed on my GPU, so it is not included.) The uniform and divergent cases are unchanged.

    On this machine (RTX 4050, Windows) with SpirvMemoryInstructions.cpp reverted the new case fails 32 of 32 dispatches (the old cases already failed every run on NVIDIA), and with the change ctest -R "lds_|ds_|agc_spirv_lds" passes 23 of 23 in the default build and lds_mixed|agc_spirv_lds 2 of 2 with ANYPS5_ENABLE_SPIRV_TOOLS=ON. I only have NVIDIA and Intel here, so whether the new case fails on RADV with the change reverted is unverified; could you run agc_driver_lds_mixed_width_atomics on RADV with and without the SpirvMemoryInstructions.cpp change? That is the missing piece for a test that catches a revert on AMD as well as NVIDIA.

    AI-assisted: yes.

  8. SleglJan commented on Oct 10, 2026

    @SleglJan
    ContributorAuthor

    Rerun with 7775d6c, with the four commits cherry-picked onto main cf8b129 without conflicts. Same setup as before: Linux (CachyOS, kernel 7.2.9, GCC 16.2.1), Release with ANYPS5_ENABLE_SPIRV_TOOLS=ON.

    agc_driver_lds_mixed_width_atomics, 20 runs per GPU RADV (Ryzen 7950X3D iGPU, Mesa 26.2.4) RTX 3080 (NVIDIA 615.71.09)
    with the change passes 20 of 20 passes 20 of 20
    SpirvMemoryInstructions.cpp reverted passes 20 of 20 fails 20 of 20, in the uniform case

    So on RADV the split-lane case does not catch a revert either. On NVIDIA the test stops at the uniform case, so the split-lane case's own result is not visible there.

    In this issue's replay kernel, what decides it is how the operands reach the registers. I replayed that kernel on RADV with main's SpirvMemoryInstructions.cpp, which is unchanged between 197ffac and cf8b129. It uses the same parity split, 256 threads and wave32, and every lane's row holds the same operands (0xffffffff, 0, 1).

    • With each lane loading its own row (buffer_load_dwordx4 at a lane-dependent offset), 32 to 96 of the 128 increments are lost in every run, with LDS sizes of 4, 8, 64 and 1,024 dwords. That holds with either exec split: v_cmp + s_and_saveexec_b32 as in the replay, or s_mov_b32 exec_lo, 0x55555555 / 0xaaaaaaaa as in the test.
    • With every lane loading from one address (buffer_load_dwordx4 ... off, or s_buffer_load_dwordx4), none are lost in 10 runs each.
    • With v_mov_b32 constants, as in the test, none are lost in 10 runs (LDS 8 dwords).

    A version of the split-lane case that loads its operands per lane, from a lane-dependent offset in an input buffer (Unused holds 4 dwords; per-lane rows need 256 × 16 bytes), may therefore catch a revert on RADV as well.

    AI-assisted: yes (Claude Code)

  9. husseinkad commented on Oct 10, 2026

    @husseinkad
    Contributor

    Thank you @SleglJan, that isolates the ingredient I missed: the race shows on RADV only when the operands reach the lanes through per-lane buffer loads (lane-dependent offsets), not v_mov constants or a scalar load. That explains why both of my kernel shapes pass there with the change reverted. I'm pausing work for today, so I won't rework the test right now; the branch's correctness story is unchanged (the structural test agc_spirv_lds_atomic_lock catches a revert on every host, and the execution test catches it on NVIDIA), and your measurement is on record here for whoever reworks the execution case with per-lane operand rows (256 x 16-byte input, buffer_load_dwordx4 at a lane-dependent offset). If you want to push that variant to the branch yourself, I'd welcome it; otherwise I'll pick it up when I'm back.

    AI-assisted: yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions