Skip to content

feat(shader): implement the 64-bit LDS atomics - #573

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
CnRJay:feat/ds-atomics-64
Oct 5, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
CnRJay:feat/ds-atomics-64

Conversation

@CnRJay

@CnRJay CnRJay commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

What

The 35 64-bit DS atomics threw DS opcode is not supported:
ds_add/sub/rsub/inc/dec_u64, ds_min/max_i64, ds_min/max_u64, ds_and/or/xor/mskor_b64, ds_cmpst_b64/f64, ds_min/max_f64, their _rtn forms, and ds_wrxchg_rtn_b64 (DS 0x40-0x4c, 0x50-0x53, 0x60-0x6d, 0x70-0x73).

LDS is emitted as a 32-bit Workgroup array, and 64-bit atomics on it would need VK_KHR_workgroup_memory_explicit_layout plus shaderSharedInt64Atomics. Instead, each op reads and writes its two dwords under a lock dword placed after the guest LDS:

  • The lock is cleared at entry, followed by a workgroup barrier.
  • Each lane acquires it with a compare-exchange (acquire) and releases it with an atomic store (release) in the same loop iteration. Lanes of one subgroup can't deadlock, because the holder releases before the loop's back edge.
  • Stages whose LDS is a function variable (non-compute) update the dwords directly.
  • 64-bit GDS atomics throw.

Semantics follow the 32-bit forms (commit c8af889): cmpst stores data1 when memory equals data0, and mskor computes (old & ~data0) | data1. inc and dec wrap as in ds_inc_u32 and ds_dec_u32. The f64 min/max/cmpst ops are computed on the bits with the NaN and signed-zero rules measured for the f32 forms. This is recorded in TechnicalDebt together with the lock's scope: it orders 64-bit atomics against each other, not against concurrent 32-bit access to the same dwords.

tools/progress.py: shaders 1013/1166 (86.88%), +35.

Tested

Windows 11, MinGW GCC 15.2.0 r7, NVIDIA Vulkan, -DANYPS5_ENABLE_SPIRV_TOOLS=ON.

  • New agc_driver_lds_atomics64 runs one wave32 compute dispatch:
    • Each lane writes a value per op, applies all 35 ops, and checks the memory and returned values against a CPU reference. The inputs cover random 64-bit values, small values for inc/dec wrap, equal and unequal compare pairs, and f64 zeros, infinities, quiet and signalling NaNs and denormals.
    • All 32 lanes then hit one address with ds_add_rtn_u64 and ds_max_rtn_u64. The test checks the total and that the returned values form one serial order.
  • Mutation checks, both caught by the test: removing the lock fails the contended add total, and swapping ds_min_i64's compare fails its lane checks.
  • Full ctest: 212/213 pass. agc_shader_memory fails identically without this change (SPIR-V validation OpSelect vector size).

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#461 docs/dev/TechnicalDebt.md L78

ds_add/sub/rsub/inc/dec_u64, ds_min/max_i64/u64, ds_and/or/xor/mskor_b64,
ds_cmpst_b64/f64, ds_min/max_f64, their _rtn forms and ds_wrxchg_rtn_b64
threw in the decoder.

LDS is a 32-bit workgroup array, so the two dwords are read and written
under a lock dword placed after the guest LDS, which is cleared with a
workgroup barrier at entry. The lock is acquired and released in the same
loop iteration, so lanes of one subgroup cannot deadlock on it. Stages
whose LDS is a function variable update it directly. 64-bit GDS atomics
throw.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CnRJay
CnRJay force-pushed the feat/ds-atomics-64 branch from 1591f13 to d78eea1 Compare October 4, 2026 23:14
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Thanks, this is a careful one. I checked the 35 opcodes with llvm-mc for gfx1013: the test shader disassembles to exactly the ops in names, and the operand order matches the 32-bit forms (cmpst compares with data0 and stores data1, mskor is (old & ~data0) | data1, inc and dec wrap as in the ISA). The lock only exists in shaders that use a 64-bit LDS atomic, and the f64 rules are recorded in TechnicalDebt as not measured, which is what the project asks for.

One thing to watch when CI runs (it still needs approval): in the contended part, all 32 lanes write 0 to the two shared slots and then do the ds_add_rtn_u64/ds_max_rtn_u64 on them with only an s_waitcnt in between. That's correct for one wave on hardware and on a GPU with 32-lane subgroups. On a device whose subgroups are narrower than the wave (lavapipe in the Linux job, 8 lanes), the recompiled wave runs as several subgroups, so one subgroup's zeroing write can land after another subgroup's atomics and the total comes out short. The other cross-lane tests (LdsAddtid, DsPermute) skip there. If the Linux job fails on that check, an s_barrier after the zeroing writes (or skipping the contended part below 32 lanes) would fix it.

agc_driver_lds_atomics64 passes here on NVIDIA (32-lane subgroups) and fails on main ("DS opcode is not supported"), so only the narrow-subgroup case is open.

Small optional note: the lock dword adds 4 bytes, so a shader that already uses the device's whole shared memory would go over the limit. That's an edge I don't expect to see.

PPSA21564 has no 64-bit LDS atomics, so this doesn't affect my runs. Ready to merge from my side once CI is green.

@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Progress report (f83ee12 → 7907e87)

📈 GPU shader instructions: 86.88% (+3.0%, +35 implemented instructions)

✅ 35 implemented
Encoding Name
DS DS_ADD_RTN_U64
DS DS_ADD_U64
DS DS_AND_B64
DS DS_AND_RTN_B64
DS DS_CMPST_B64
DS DS_CMPST_F64
DS DS_CMPST_RTN_B64
DS DS_CMPST_RTN_F64
DS DS_DEC_RTN_U64
DS DS_DEC_U64
DS DS_INC_RTN_U64
DS DS_INC_U64
DS DS_MAX_F64
DS DS_MAX_I64
DS DS_MAX_RTN_F64
DS DS_MAX_RTN_I64
DS DS_MAX_RTN_U64
DS DS_MAX_U64
DS DS_MIN_F64
DS DS_MIN_I64
DS DS_MIN_RTN_F64
DS DS_MIN_RTN_I64
DS DS_MIN_RTN_U64
DS DS_MIN_U64
DS DS_MSKOR_B64
DS DS_MSKOR_RTN_B64
DS DS_OR_B64
DS DS_OR_RTN_B64
DS DS_RSUB_RTN_U64
DS DS_RSUB_U64
DS DS_SUB_RTN_U64
DS DS_SUB_U64
DS DS_WRXCHG_RTN_B64
DS DS_XOR_B64
DS DS_XOR_RTN_B64

On a device whose subgroups are narrower than the wave, the recompiled
wave runs as several subgroups, so one subgroup's zeroing write could
land after another's atomics, and a read could run before another
subgroup's atomics. An s_barrier after the zeroing writes and another
after the atomics order them on any subgroup width.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Thanks for adding the barriers. I owe you a correction on my earlier note: the Linux job had already passed on d78eea1 (agc_driver_lds_atomics64 passed all three repeats on lavapipe). The reason is that the recompiler already orders LDS accesses inside a single-wave compute workgroup: WaveLdsScope in SpirvEmitter.cpp puts a workgroup barrier between an LDS write and the next LDS access, and the new 64-bit atomics count as writes through SharedAccessOf. So the zeroing writes were never racing with the atomics there.

The two s_barriers are still fine to keep. An explicit barrier resets that tracking, so nothing is emitted twice, and the test no longer depends on it. I disassembled the new code with llvm-mc for gfx1013: the only change is an s_barrier after each of the two s_waitcnt lgkmcnt(0), one before the contended atomics and one before the reads.

agc_driver_lds_atomics64 passes here on the new head and still fails on main ("DS opcode is not supported"). The new push needs workflow approval again before CI runs. Ready to merge from my side once it's green.

@DotDebian DotDebian mentioned this pull request Oct 5, 2026
9 tasks done
@oneandonlydean
oneandonlydean merged commit 8387b4d into boykopovar:main Oct 5, 2026
3 checks passed
oneandonlydean added a commit to oneandonlydean/AnyPS5 that referenced this pull request Oct 5, 2026
Conflicts resolved:
- libSceAudioPropagation/Export.cpp: upstream's boykopovar#482 + boykopovar#489 version taken; it is
  the same no-propagation model as astrobot's stand-in, with SourceRender and the
  handle-only UnregisterMaterial already in it.
- State.cpp (boykopovar#453): astrobot's resident depth model kept; it already skips the
  tests of an absent plane (boykopovar#453's test is on astrobot since e087a4f).
- Graphics.cpp, State.hpp, ShaderResources.cpp: astrobot's DepthTarget model kept
  (exportIndex, no DepthSurface.hpp), upstream's ThreadOwned include added.
- BdaResources.cpp: astrobot's form with boykopovar#390's message.
- VulkanDevice.cpp, Context.hpp, PacketExecution.cpp: both sides kept
  (calibrated timestamps, host query reset, GDS, memory budget; pipeline stats).
- Spirv: astrobot's split-wave rules first, then boykopovar#608's wave-mask ballot and
  boykopovar#587's host subgroup lane; astrobot's function LDS sizing and boykopovar#573's LDS lock
  both kept; conditionalDepth kept in the atomic update loops; upstream's
  buffer atomics (boykopovar#321 boykopovar#332 boykopovar#594) and 64-bit LDS atomics taken.
- TechnicalDebt.md: both sides, duplicate lines dropped.


Build fixes folded in: the upstream boykopovar#453 test block duplicated next to
astrobot's adapted copy removed; a lost closing brace after the host
query reset block restored; lookupLog() is a function since boykopovar#590; the
duplicate MUTEX_TYPE_ADAPTIVE constant (astrobot's 93b2d0e equals boykopovar#156)
removed; the ops boykopovar#596 drops removed from the masked-select list and the
BDA test message updated (the gate-batch-2 integ commits 559e4fab6 and
2ec968fb5).
oneandonlydean added a commit to oneandonlydean/AnyPS5 that referenced this pull request Oct 5, 2026
The LDS array bound (3cbea50) counted every shared access that is not a
vector load or store as one dword. boykopovar#573's 64-bit LDS atomics read and write
two, so a pixel shader's last 64-bit atomic could reach past the array.
SharedComponentCount (boykopovar#573) already gives the dword count of every shared
op; the bound now uses it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants