Skip to content

feat(shader): implement image_atomic_fmin, fmax and fcmpswap - #562

Merged
oneandonlydean merged 3 commits into
boykopovar:mainfrom
adriadam10:feat/image-atomics-float
Oct 4, 2026
Merged

oneandonlydean merged 3 commits into
boykopovar:mainfrom
adriadam10:feat/image-atomics-float

Conversation

@adriadam10

Copy link
Copy Markdown
Collaborator

What

This implements the three MIMG float atomics, which threw as unknown opcodes. The ISA sweep found them.

Opcode Semantics (measured on an RDNA2 GPU)
image_atomic_fcmpswap (0x1d) texel = (texel == cmp) ? src : texel, compared as floats: −0 equals +0, NaN equals nothing. vdata[0] = src, vdata[1] = cmp, dmask must be 0x3 like image_atomic_cmpswap
image_atomic_fmin (0x1e) / image_atomic_fmax (0x1f) IEEE minNum/maxNum: −0 < +0. A signalling NaN operand gives that NaN quieted (src first). A quiet NaN loses to a number. Two quiet NaNs give src

These are the same rules the float buffer atomics follow in #332. On the hardware the image forms gave the same results as the buffer forms (800/800 rows), and the same results on a 32_UINT and a 32_FLOAT T#. Without glc the data VGPR is left untouched, as for the other image atomics.

Implementation:

  • Decoder, IR opcode and dispatch entries next to the integer image atomics.
  • The emitter runs AtomicUpdate's compare-exchange loop on the existing u32 texel pointer, as image_atomic_inc/dec already do. The value is computed on the raw bits through feat(shader): implement the float and 64-bit inc/dec buffer atomics #332's AtomicFloatBits, which is reached through two helpers, AtomicFloatMinMax and AtomicFloatCompareSwap. So the driver's float controls (denormals, NaN payloads) don't matter, and no Vulkan float-atomic feature is needed.
  • Image atomics are still only accepted on 32_UINT textures (ResourceMaterializer, "atomic image descriptor uses an unsupported format"). A float-format texture keeps throwing there. Accepting one needs a uint view of a float texture in the driver, which belongs in a separate PR.

Tested

Arch Linux, RDNA2 iGPU (Radeon 660M), RADV and lavapipe. No title.

New execution test agc_driver_image_atomics_float_tests (ImageAtomicsFloat.cpp):

  • 800 rows, measured on hardware: every pair of 20 special values (±0, ±1, ±inf, qNaN/sNaN with payloads, ±denormal min/max, ±π, max finite, min normal), once with cmp = src and once with cmp = old.
  • Each row checks the returned value and the resulting texel for fmin, fmax and fcmpswap with glc, and fmin without glc. Everything is compared bit for bit. The emitted code only uses integer ops on the bits, so NVIDIA's NaN canonicalisation doesn't apply.
  • It runs as wave32, as wave64 and as wave64 split into two 32-wide halves (ComputeTarget(32)).
  • 0 s cold on CI's lavapipe (Mesa 25.2.8).

The test fails without the change in each of these cases (each checked separately):

  • fmin and fmax swapped
  • a signed-integer min/max instead of the float rule
  • fcmpswap comparing bits instead of floats
  • src and cmp swapped
  • the comparator not read

Release build: full ctest passes except agc_driver_pixel_helper_lanes, which also fails on main on local RADV.

Checklist

adriadam10 and others added 3 commits October 4, 2026 22:56
…buffer atomics

buffer_atomic_inc, buffer_atomic_dec, buffer_atomic_cmpswap_x2 and
buffer_atomic_{add,sub,smin,umin,smax,umax,and,xor}_x2 threw in the decoder.
The 64-bit ones use native 64-bit SPIR-V atomics like the existing
swap_x2/or_x2; inc and dec wrap at the data operand through the existing
compare-exchange helpers.

Co-Authored-By: Claude <noreply@anthropic.com>
buffer_atomic_fcmpswap, fcmpswap_x2, fmin_x2, fmax_x2, inc_x2 and
dec_x2 are compare-and-swap loops on the dword or qword. fcmpswap
compares as floats (-0 equals +0, NaN equals nothing) and fmin/fmax are
IEEE minNum/maxNum with -0 < +0, a quieted result for a signalling NaN
and the other operand for a quiet one, all with integer ops so the
driver's float controls do not matter. The existing 32-bit fmin/fmax
kept the old value whenever a NaN or two zeros were involved; it now
uses the same rule.

Co-Authored-By: Claude <noreply@anthropic.com>
The three MIMG float atomics threw as unknown opcodes. They now run as a
compare-and-swap loop on the 32-bit texel with the bit-level rules the
float buffer atomics already use (NaN, signed zero and sNaN quieting as
measured on RDNA2), so no float atomic feature is needed from the driver.

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown

Progress report (d7f0c49 → d1f7710)

📈 GPU shader instructions: 81.99% (+1.72%, +20 implemented instructions)

✅ 20 implemented
Encoding Name
MIMG IMAGE_ATOMIC_FCMPSWAP
MIMG IMAGE_ATOMIC_FMAX
MIMG IMAGE_ATOMIC_FMIN
MUBUF BUFFER_ATOMIC_ADD_X2
MUBUF BUFFER_ATOMIC_AND_X2
MUBUF BUFFER_ATOMIC_CMPSWAP_X2
MUBUF BUFFER_ATOMIC_DEC
MUBUF BUFFER_ATOMIC_DEC_X2
MUBUF BUFFER_ATOMIC_FCMPSWAP
MUBUF BUFFER_ATOMIC_FCMPSWAP_X2
MUBUF BUFFER_ATOMIC_FMAX_X2
MUBUF BUFFER_ATOMIC_FMIN_X2
MUBUF BUFFER_ATOMIC_INC
MUBUF BUFFER_ATOMIC_INC_X2
MUBUF BUFFER_ATOMIC_SMAX_X2
MUBUF BUFFER_ATOMIC_SMIN_X2
MUBUF BUFFER_ATOMIC_SUB_X2
MUBUF BUFFER_ATOMIC_UMAX_X2
MUBUF BUFFER_ATOMIC_UMIN_X2
MUBUF BUFFER_ATOMIC_XOR_X2

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Thanks. I checked the encodings with llvm-mc for gfx1013: image_atomic_fcmpswap, image_atomic_fmin and image_atomic_fmax are MIMG opcodes 0x1d, 0x1e and 0x1f, and the RDNA2 ISA guide gives the same semantics you measured (fcmpswap stores src when tmp == cmp as a float compare, with DATA[0] = src and DATA[1] = cmp, so dmask 0x3 like cmpswap). The emitter reuses #332's AtomicFloatBits through the existing AtomicUpdate loop on the u32 texel pointer, the same way image inc/dec already work, so the result never depends on the driver's float controls. equal treats -0 and +0 as equal and a NaN as equal to nothing, and minMax is the rule I already checked on #332.

The first two commits are patch-identical to #321 and #332, so this lands after them. Image atomics are still only accepted on 32_UINT views, and float-format textures keep throwing in the materializer, which is the right place to stop for now.

agc_driver_image_atomics_float passes on my NVIDIA machine on current main (with #321 and #332), and fails on main's code.

One note for the merge order: #506 also edits SpirvMemoryInstructions.cpp next to the same helpers and conflicts textually with this one, so whichever lands second needs a small rebase.

This only adds encodings that main refused, so it's ready to merge from my side once #321 and #332 are in.

@oneandonlydean
oneandonlydean merged commit 3fe9068 into boykopovar:main Oct 4, 2026
3 checks passed
@adriadam10
adriadam10 deleted the feat/image-atomics-float branch October 5, 2026 06:47
thuanquang added a commit to thuanquang/AnyPS5 that referenced this pull request Oct 21, 2026
…mics

On RDNA2 the image, buffer and global float atomics (fmin, fmax,
fcmpswap and the _x2 forms) flush both operands to a signed zero when
bit 0 of the shader's denormal field is clear, as the LDS forms do: the
f32 field for the 32-bit forms, the f16/f64 field for _x2. Only the
LDS forms flushed here. floatAtomicFlush (was sharedFloatFlush) now
sets MemoryInfo::flushDenormals for every float atomic, and
AtomicFloatBits applies it in minMax and compareSwap.

The float atomic tests from boykopovar#332, boykopovar#506 and boykopovar#562 were measured with
denormals kept, so they now set FLOAT_MODE 0xf0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants