[billythemenace2]

What

Before: ds_add_f32 and ds_add_rtn_f32 were a plain host OpFAdd. Denormal operands and results were not flushed when the shader's FLOAT_MODE flushes f32 denormals. That includes 0xc0, the mode most titles use and the mode assumed for a shader without a known float mode. A signalling NaN was stored unquieted, and which NaN was stored, and the NaN from +inf + -inf, were whatever the host produced. The technical debt listed ds_add_f32 as not measured.

After: the add follows the rule measured on hardware:

1. A NaN data operand is stored quieted. Otherwise a NaN old value is stored quieted.
2. Both operands are flushed to a signed zero when bit 0 of the f32 denormal field is clear.
3. +inf + -inf stores 0xffc00000.
4. The sum is rounded to nearest even, whatever the round mode.
5. The result is flushed to a signed zero when bit 1 of the field is clear.

MemoryInfo gets a flushDenormalResults flag next to flushDenormals, because input and result flushing come from different bits.

Hardware semantics, measured on RDNA1 (gfx1010, AMD Radeon RX 5700 XT) with the tools/hw-oracle template. The kernels were assembled with the Windows driver's amd_comgr_2.dll and launched with its HIP runtime.

• 1574 operand pairs: signed zeros, denormals, the smallest normals, infinities, quiet and signalling NaNs with payloads, the largest normals, random pairs at the denormal boundary, and pairs that nearly cancel.
• They ran in all 16 combinations of the f32 denormal field and the f32 round mode, plus IEEE 1 and DX10 clamp 0: 28,332 rows. All of them match the rule above. IEEE_MODE and DX10_CLAMP have no effect.

The measurement write-up is attached.
measurement-results-ds-add-f32-gfx1010.md

Remaining gap, recorded in technical debt: the sum is still the host OpFAdd between the flushes. Where the shader keeps f32 denormals, the result also depends on the host preserving them, and the module does not request DenormPreserve for 32-bit floats. The Windows AMD driver used here flushes them.

Tested

• Windows 11, AMD Radeon RX 5700 XT, Adrenalin 32.0.21045.5002, MinGW GCC 15.2.0. The full build is clean.
• New test agc_driver_lds_add_f32 covers 64 lanes in two waves, with ds_add_rtn_f32 and ds_add_f32, in five float modes (denormal field 0-3 with round 0, and field 3 with round 3). The expected values are the RX 5700 XT's measured results, not a model. The test checks the 209 of the 320 rows that do not depend on the host preserving f32 denormals, and it also checks the value returned by ds_add_rtn_f32. It passes. Without the change it fails (a signalling NaN is stored unquieted).
• ctest -R "lds_|shader_memory": everything passes except agc_driver_lds_mixed_width_atomics, which also fails on unchanged main (dacd785) on this machine.
• python3 tools/check_conventions.py --base origin/main passes.

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 under Silent stubs in their library's technical debt file
• 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 (fix(shader): update per-invocation LDS without atomics #5160 edits the same technical-debt line, and its SpirvMemoryInstructions.cpp changes merge cleanly with this one; whichever merges second rebases that line. feat(shader): translate the 64-bit and f32/f64 min/max DS src2 forms #5474 merges cleanly with this one)
• Unblocks: none
• GPUs tested: AMD Radeon RX 5700 XT (gfx1010), Adrenalin 32.0.21045.5002, Windows 11
• AI-assisted: yes. Claude wrote the change, the test and the measurement kernels; I reviewed it and ran the measurements and tests on my machine.

[xXPBSXx]

Windows test of #5492: Windows 10 (19045), MinGW-w64 GCC 15.2, Release, ninja -j 4, ctest called directly, GPU selected by the tests: NVIDIA GeForce RTX 4080 SUPER (Selected GPU log line). Tree: main d1670009f + merge of PR head 2b9f7aad4d52695559ae5f45495c76e0eecf56e5 (local merge a4b19949d).
agc_driver_lds_add_f32 and agc_driver_lds_addtid: exit code 0 in 3 of 3 runs each (6 of 6). Logs: rtx5492_*_N.log.
Not tested: that the new test fails without the change (no baseline run), the AMD GPU (also present, not selected), other ctest targets, Linux, a title run.

AI-assisted (Claude Code).

[CnRJay]

A good, measured change. Two notes, one about cost and one about where the measurement comes from.

Hoist the data-only work out of the CAS loop. SharedAtomicFAdd32 goes through AtomicUpdate, so the replacement lambda runs on every CAS retry. Three of its parts depend only on data: flush(data), nan(data) and data | quiet. Under contention (the usual case for ds_add_f32: histograms, reductions, light-list counters), every extra instruction between the OpAtomicLoad and the OpAtomicCompareExchange widens the window in which another lane can change the value, so more iterations fail and retry. Driver compilers may hoist this by LICM, but values that feed an atomic loop aren't always treated as invariant. Computing the flushed data, the data-is-NaN flag and the quieted data once before AtomicUpdate, and keeping only the old-dependent selects inside the loop, makes the loop body as short as main's plus the old-value checks.

RDNA1 against RDNA2. The other LDS float atomics in the same debt entry were measured on gfx1036 (RDNA2), which is the family the console's GPU belongs to. This one was measured on gfx1010 (RDNA1). The denormal and round-mode handling of LDS float atomics did change between some GCN/RDNA generations, so the debt entry should say that the gfx10.3 behaviour is assumed from gfx10.1. A rerun of the oracle on any RDNA2 card would settle it.

The test design is good: the expected values are the measured results rather than a model, and the 111 rows that depend on the host preserving denormals are left out.

[xXPBSXx]

Correction and update to my Windows test above: (1) I wrote that the GPU was shown by a Selected GPU log line, but that line was not in the ctest logs of those runs (ctest without -V does not print the output of passing tests); I could not re-verify it for this PR, see (2). (2) On current main 1633843382ecb1434ac76ac0c4d4655d88bb241f the PR (head 2b9f7aad4d52695559ae5f45495c76e0eecf56e5) merges without a textual conflict but does not build: core/shader/recompiler/ShaderDiskCache.cpp:142:40: error: static assertion failed: MemoryInfo changed: update the plan encoder. The PR adds a field to MemoryInfo, and the shader disk cache on main pins the field count of the structures its plan encoder writes (assertion added in 664ad7703, 13 October); the encoder and the assertion need updating. My earlier passes (agc_driver_lds_add_f32 and agc_driver_lds_addtid, 3 of 3 each) were on main d1670009f, before that assertion was on the merge base. Not tested: the tests of this PR on current main (they cannot be built).

AI-assisted (Claude Code).

[oneandonlydean]

This doesn't build on current main: ShaderDiskCache.cpp:142: static assertion failed: MemoryInfo changed: update the plan encoder (the field counts were pinned in 664ad77). Please add flushDenormalResults to the plan encoder, bump FormatVersion, and rebase.

CnRJay's two points are worth taking too: compute the data-only parts once before AtomicUpdate, and say in the debt entry that it was measured on gfx1010 (RDNA1).

[billythemenace2]

Rebased on main. flushDenormalResults is now in the plan encoder (field count 31, minimum record 79 bytes, FormatVersion 33). The data-only parts (flushed data, its NaN test and the quieted data) are computed once before AtomicUpdate, so the CAS loop only holds the work that depends on old. The debt entry now says ds_add_f32 was measured on gfx1010 and gfx10.3 is assumed to match. agc_driver_lds_add_f32, agc_driver_lds_float_atomic_modes and agc_shader_disk_cache pass on an RX 5700 XT.

[thuanquang]

RDNA2 rerun of the measurement, as CnRJay asked: on gfx1031 every row follows the rule in the description.

Setup: AMD Radeon RX 6700 XT (gfx1031), Windows 11 26200, Adrenalin 26.8.1 (driver 32.0.21045.5002). Same method as the attached gfx1010 write-up: tools/hw-oracle/template.s from main e34c47921, assembled and linked by the driver's amd_comgr_2.dll (comgr 2.9) and launched through its amdhip64_6.dll. Body, one LDS word per lane:

  v_lshlrev_b32 v8, 2, v0
  ds_write_b32 v8, v4
  ds_write_b32 v8, v4 offset:4096
  s_waitcnt lgkmcnt(0)
  ds_add_f32 v8, v5
  ds_add_rtn_f32 v11, v8, v5 offset:4096
  s_waitcnt lgkmcnt(0)
  ds_read_b32 v10, v8
  ds_read_b32 v12, v8 offset:4096
  s_waitcnt lgkmcnt(0)

Operands: 1574 pairs of the classes the write-up lists (24 x 24 specials, 400 random pairs with exponents 0-3, 598 pairs that nearly cancel or are 30 binades apart; the random pairs are mine). Modes: the same 18 (denormal field 0-3 x round mode 0-3 at IEEE 0 and DX10 clamp 1, plus IEEE 1, plus DX10 clamp 0). Each stored value is compared with the six-step rule, with the add done as an exactly rounded f32 add between the flushes.

Wave	Rows	Stored value follows the rule	ds_add_rtn_f32 returns the old value	_rtn and plain forms store the same
wave32	28,332	28,332	28,332	28,332
wave64	28,332	28,332	28,332	28,332

Rows that differ between denormal modes at round 0 on this set: 0/3 346, 0/1 242, 0/2 71, 1/3 143, 2/3 275.

So gfx1010 and gfx1031 agree, and the debt sentence "RDNA2 (gfx10.3, the console's family) was not measured for ds_add_f32: its behaviour is assumed to match gfx10" can say it was measured on both. Not tested: this branch's build and agc_driver_lds_add_f32 (the branch conflicts with main now), and gfx1013 itself.

AI-assisted: yes (Claude Code).

[billythemenace2]

@thuanquang Thank you for the RDNA2 run. The debt entry now cites it instead of assuming gfx10.3 matches gfx10.1: "thuanquang measured the same rule on RDNA2 (gfx1031, RX 6700 XT) in #5492, with the same operand classes and 18 modes in wave32 and wave64; the PS5's gfx1013 is not measured."

Rebased onto main ea30c9274 and squashed into one commit (848ce1af8). That also drops the intermediate commit that still had conflict markers in the debt file. FormatVersion is now 34, since main took 33. agc_driver_lds_add_f32, agc_driver_lds_float_atomic_modes and agc_shader_disk_cache pass on an RX 5700 XT, and tools/check_conventions.py passes.

AI-assisted (Claude Code).

[adriadam10]

Checked on gfx1035 with the HW oracle: ds_add_f32 and ds_add_rtn_f32 over 1393 operand pairs (your 64 test rows, 23x23 edge classes, 800 random pairs near the denormal range and near-cancelling pairs) in IEEE 0/1 x every f32 denormal field x every f32 round mode, 44,576 rows: all match the rule in the description, and your test's measured values are bit-identical on gfx1035. Build and the AgcDriver/Agc tests pass, the new test fails without the change, and it passes on CI's lavapipe.

Added one commit on your branch after rebasing onto main:

• dropped the code comment in EmitSharedAtomicFAdd32 (CONTRIBUTING: no comments except technical debt).

Merging once the full Build is green.