Skip to content

feat(shader): decode and implement ds_condxchg32_rtn_b64 - #1372

Closed
pandather wants to merge 1 commit into
boykopovar:mainfrom
pandather:feat/shader-decode-condxchg32-b64
Closed

pandather wants to merge 1 commit into
boykopovar:mainfrom
pandather:feat/shader-decode-condxchg32-b64

Conversation

@pandather

Copy link
Copy Markdown
Contributor

What

Decodes and implements ds_condxchg32_rtn_b64 (DS opcode 0x7E) — the last DS instruction in tools/rdna_isa.txt that the decoder did not recognize and that no open pull request covers. Shader instructions: 1139/1166 -> 1140/1166.

  • Decoding: operands per the machine-readable spec (amdgpu_isa_rdna2.xml) — VDST is a B64 pair, ADDR a B32, DATA0 a B64 pair, plus implicit DSMEM — read from the fields every other DS instruction uses; gds bit 17 is kept in instruction.gds. Classified as a DS atomic like ds_wrxchg_rtn_b64.
  • Semantics (measured on RDNA2 hardware, gfx1031): VDST gets the old LDS value pair; each 32-bit half of the pair is written independently iff that half's bit31 is set, storing data & 0x7FFFFFFF — bit31 is a write-enable flag. Seven oracle cases cover every write-enable combination (both halves, low only, high only, neither), bit31-only and bit30-only immediates, and the mask edges (0xFFFFFFFF, 0x80000000, 0x00000001 as data). A second conditional exchange fed back from LDS confirms the returned value is the state before each write.
  • Implementation: a new SharedAtomicCondxchg64 IR opcode lowered through the same locked 64-bit LDS path as ds_cmpswap_b64 (SharedAtomic64, workgroup spinlock via LockedLdsUpdate when analysis requires it), so the read-modify-write is atomic across the wave. The GDS variant throws "64-bit GDS atomics are not supported", consistent with every other 64-bit DS atomic.

Sources:

  • Encoding and operand fields: AMD machine-readable ISA spec (amdgpu_isa_rdna2.xml, DS_CONDXCHG32_RTN_B64, opcode 126 = 0x7E) cross-checked byte-for-byte against LLVM's assembler for gfx1031. The words in the tests are clang --target=amdgcn-amd-amdhsa -mcpu=gfx1031 encodings (ds_condxchg32_rtn_b64 v[16:17], v8, v[6:7] -> D9F80000 10000608, gds variant -> D9FA0000 ...).
  • Behavior: measured on RDNA2 hardware (gfx1031, RX 6700 XT) with the project's hardware oracle (tools/hw-oracle); not derived from documentation. The ISA guide describes the same conditional exchange; the measurements pin down the bit31 write-enable and the mask.

Tested

Windows 11, NVIDIA Blackwell desktop GPU (driver 616.92), MinGW-w64 GCC 15.2.0 (WinLibs r7): agc_ds_condxchg32_decode, agc_driver_ds_condxchg32_b64 and the full suite report 100% tests passed out of 406. Conventions check (tools/check_conventions.py) passes.

  • New agc_ds_condxchg32_decode: decodes three LLVM-encoded programs (plain, offset:255, gds) and checks opcode, length, operands, offset and gds flag.
  • New agc_driver_ds_condxchg32_b64: runs two chained conditional exchanges on the GPU through the recompiler and SPIR-V backend with 32 lanes whose data halves drive every write-enable combination, and checks all eight output dwords per lane against the measured semantics; also checks that the GDS variant is rejected.
  • Oracle measurements on gfx1031 (the behavior the test asserts):
init               data              returned/lds after op1      after op2 (data=init)
11111111 22222222 A0000001 B0000002 -> 11111111 22222222 | 20000001 30000002   (both write)
33333333 44444444 A0000003 10000004 -> 33333333 44444444 | 20000003 44444444   (low writes)
55555555 66666666 10000005 B0000006 -> 55555555 66666666 | 55555555 30000006   (high writes)
77777777 88888888 10000007 10000008 -> 77777777 88888888 | 77777777 88888888   (neither)
99999999 AAAAAAAA 80000000 FFFFFFFF -> 99999999 AAAAAAAA | 00000000 7FFFFFFF   (mask edges)
DEADBEEF FEEDFACE 40000000 00000000 -> DEADBEEF FEEDFACE | DEADBEEF FEEDFACE   (bit30: no write)
12345678 9ABCDEF0 FFFFFFFF 80000001 -> 12345678 9ABCDEF0 | 7FFFFFFF 00000001

Not tested: no title or homebrew (LLVM emits this instruction only from explicit assembly, never from intrinsics). Concurrent contention on one LDS address from several lanes is not exercised; the workgroup lock makes the pair access atomic with respect to the other 64-bit LDS atomics, which are not ordered against concurrent 32-bit accesses (TechnicalDebt).

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

Decodes DS_CONDXCHG32_RTN_B64 (DS opcode 0x7E), the last missing DS
instruction, with its encoding from the AMD machine-readable RDNA2 ISA
spec cross-checked against LLVM's assembler for gfx1031. Implements it
as a locked 64-bit LDS atomic like ds_cmpswap_b64: VDST returns the old
value pair and each 32-bit half is written iff that half's bit31 is set,
storing the data with bit31 cleared - measured on RDNA2 hardware
(gfx1031) with the hardware oracle over seven cases covering every
write-enable combination and the mask edges. A GDS variant throws:
64-bit GDS atomics are not supported, as for every other 64-bit DS
atomic.
@github-actions github-actions Bot added build shader core/shader agc libSceAgc* labels Oct 8, 2026
popcoder565 added a commit to popcoder565/AnyPS5 that referenced this pull request Oct 8, 2026
… shared lines

boykopovar#1372 adds its test after the vop1 decode tests and boykopovar#1314 its debt entry after s_setkill, the places where this branch added its own. The wave control decode test now precedes the vop1 decode tests and the s_sethalt entry follows sceRtcParseDateTime, so either pull request can merge first without a conflict.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 8, 2026
adriadam10 pushed a commit to popcoder565/AnyPS5 that referenced this pull request Oct 8, 2026
… shared lines

boykopovar#1372 adds its test after the vop1 decode tests and boykopovar#1314 its debt entry after s_setkill, the places where this branch added its own. The wave control decode test now precedes the vop1 decode tests and the s_sethalt entry follows sceRtcParseDateTime, so either pull request can merge first without a conflict.

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

Copy link
Copy Markdown
Collaborator

Closing as a duplicate: #1198 implemented ds_condxchg32_rtn_b64 and is already on main. Thanks for the hardware measurements. If they show something #1198 gets wrong, please open a fix against main.

@adriadam10 adriadam10 closed this Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* build needs rebase Conflicts with main, rebase needed shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants