Skip to content

Float add atomics are kept: RED .ADD.F32.FTZ.RN was dropped, which washed out Ocarina of Time's outdoor scenes - #1359

Closed
lunks wants to merge 2 commits into
seedds:mainfrom
lunks:pr/float-add-atomics
Closed

Float add atomics are kept: RED .ADD.F32.FTZ.RN was dropped, which washed out Ocarina of Time's outdoor scenes#1359
lunks wants to merge 2 commits into
seedds:mainfrom
lunks:pr/float-add-atomics

Conversation

@lunks

@lunks lunks commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

What and why

Ocarina of Time (Switch 2, 0400486024DEC000, v131072; profile in #1299): outdoor scenes are washed out to near white, with fog over everything and a huge glare at doorways, while interiors look right. This is the overexposure on #1299's Open list ("the translator drops RED.F32.FTZ.RN, 34 shaders"); #1347 and #1326 also mention overexposed lighting.

Cause: the translator logs Invalid reduction type: F32FtzRn and drops RED.E.ADD.F32.FTZ.RN. 416 instructions in 34 shaders are lost. 12 of those shaders are agl's analysis compute shaders (sky_analyze, probe_analyze, cube_map_probe_analyze, shadow_analyze, inner_mask_analyze, local_depth_shadow_analyze), dispatched every frame outdoors, so every sum they make stays 0. The IR has no float atomic.

Fix: EmitAtomicAddF32 (InstEmitMemory.cs) adds through a compare-and-swap loop of the word, flushing denormals as .FTZ does (FlushDenormal, already in main from 84dea69). The load and the exchange go through GlobalToStorage like any other access. The translator is rebuilt from these sources and the chain version changes.

Scope: only this. The commit it came from also took 84dea69 (already in main) and #1343 (open); neither is in this PR.

stan69b's oot-b-wip branch has the same fix (their A-4); it has no PR, and they said on #1326 to go ahead with ours. Thanks to them for the comparison.

Reference entry: Float add atomics.

Checked

  • Tests: dm/gfx/tests/test_dmsh_ampere_mem.py (new RedF32: the add is kept as a compare-and-swap loop, with the value and the .FTZ bound; 3 tests, fail on main's translator), every other test_dmsh_*.py with the rebuilt translator, test_nsp_chain.py, test_docs.py. The GPU test suites (make -C dm/gfx/tests) were not run for this change.
  • Games run: Ocarina of Time on our stack. Overlaying only the 12 rebuilt agl analysis shaders on the old app turned the washed-out scene normal at Link's porch (same save, same spot). Not yet confirmed in a full build, and other scenes were not rechecked. How the game turns the sums into its exposure was not traced. No other game was run.
  • Devices and OS: Apple M5 Pro, macOS 27.
  • Chain change: the translator rebuilt from this branch's sources (dotnet publish, the 5 dlls copied, tool_lock.py --update-lock, dmsh_chain.py --header). Every game's shaders are rebuilt, and shaders without a float add atomic should be unchanged, but I did not compare other games' MSL. regress.py was not run.

Docs

  • The finding is in docs/reference/shaders.md and python3 dm/tools/docs_symptoms.py was run (the symptom index)
  • A chain tool changed: dmsh_chain.py --header was run and dm/gfx/nvn/dmsh_chain.h is part of the change
  • No game files, keys or data extracted from a game in the change or the attachments

@lunks
Float add atomics are kept: RED and ATOMG .ADD.F32.FTZ.RN were droppe…
34d04ef 
…d, which left Ocarina of Time's outdoor scenes washed out; chain 7f07eb715d7d

Outdoors the picture was near white, with fog over everything and a huge glare at doorways, while interiors looked right. The translator logged `Invalid reduction type: F32FtzRn` and dropped RED .ADD.F32.FTZ.RN: 416 instructions in 34 shaders. 12 of them are agl's analysis programs (sky_analyze, probe_analyze, cube_map_probe_analyze, shadow_analyze, inner_mask_analyze, local_depth_shadow_analyze), all dispatched every frame outdoors, so every sum they make stayed 0.

EmitAtomicAddF32 adds through a compare-and-swap loop of the word, flushing denormals as .FTZ does (FlushDenormal, from 84dea69). The load and the exchange go through the global-to-storage pass like any other access. With only those 12 shaders rebuilt on the old app, the same save on the same spot has its colours back. How the game turns the sums into its exposure was not traced; not confirmed in a full build, and other scenes were not rechecked.

Checked: test_dmsh_ampere_mem.py RedF32 (3 tests, fail on the previous translator), every test_dmsh_*.py with the rebuilt translator, test_nsp_chain.py, test_docs.py. regress.py was not run.
@github-actions github-actions Bot added nvn NVN on Metal and its tests (dm/gfx/nvn, dm/gfx/tests) docs Documentation shader chain The shader translation chain (DMSH_CHAIN): a change rebuilds every game's shaders labels Nov 5, 2026
@lunks lunks mentioned this pull request Nov 5, 2026
Open
1 task done
stan69b pushed a commit to stan69b/nxnative that referenced this pull request Nov 5, 2026
@seedds
OoT1 A-6: merge seedds#1359 (lunks) float add atomics, replacing A-4
9b24bf1 
lunks' seedds#1359 (34d04ef) keeps RED and ATOMG .ADD.F32.FTZ.RN through a
compare-and-swap loop that flushes denormals, the same instruction and the
same approach as our A-4 (d0952dc), which this merge reverts: our
EmitAtomicAddF32FtzRn, its RedF32 test and its reference entry go; seedds#1359's
EmitAtomicAddF32, RedF32 tests (the game's encoding, agl's sky_analyze) and
"Float add atomics" entry stay. seedds#1359 found what the dropped adds did:
Ocarina of Time's outdoor scenes washed out to near white. A-4 handled no
form seedds#1359 does not, so nothing of it is kept; the game note now links
seedds#1359's entry.

seedds#1359's dlls, lock and chain header are not taken (ours kept; the
translator is rebuilt from the merged source in a later commit).
test_dmsh_ampere_mem passes with a translator built from this source.
stan69b pushed a commit to stan69b/nxnative that referenced this pull request Nov 5, 2026
@seedds
OoT1 A-9: the translator rebuilt from the merged source, chain cd1da6…
84b1356 
…7c132d

dotnet build -c Release of dmsh_xlate from this branch's source: A-2
(ATOMG.CAS), seedds#1359's float add atomics (A-6), lunks' 574a38f pool table
(A-8), upstream main (A-5). seedds#1359's and lunks' own dlls are not taken.
dmsh_xlate.dll, its pdb and Ryujinx.Graphics.Shader.dll copied into
dmsh_xlate_bin (launcher kept; Ryujinx.Common.dll and Spv.Generator.dll,
whose sources did not change, kept as committed). tool_lock.py
--update-lock (Metal unchanged at 32023.922), dmsh_chain.py --header:
chain a5bef9d6ad60 -> cd1da67c132d.

Checked with this binary: every dm/gfx/tests/test_dmsh_*.py, t_gmem_raw
(61 checks on the GPU), t_pool_chunks (94), test_nsp_chain, test_dmsh_abi,
test_docs, tool_lock.py --check-tools. Not merged: seedds#1357 (-fbracket-depth
on every compile; A-3b keeps the compile cache), seedds#1360 (see A-8).
gnosticgeek pushed a commit to gnosticgeek/nxforge that referenced this pull request Nov 5, 2026
@claude
Merge upstream PR seedds#1359 (translator binaries and chain lock to …
4a55f18 
…be rebuilt)
gnosticgeek added a commit to gnosticgeek/nxforge that referenced this pull request Nov 5, 2026
@gnosticgeek @claude
Translator rebuilt with upstream PRs seedds#1359, seedds#1360 and see…
28a8672 
…dds#1376 (float add atomics, pool table count, LDSM); chain lock and header regenerated (65ed9f9547f9)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016qteK29T18U8p1c9LsVY8g
@david-on-spring david-on-spring mentioned this pull request Nov 5, 2026
Draft
@PaulEins PaulEins mentioned this pull request Nov 5, 2026
Open
1 task done
@deanpektas27 deanpektas27 mentioned this pull request Nov 6, 2026
Closed
3 tasks done
@lunks
Merge origin/main into pr/float-add-atomics
86b0c57 
Main's translator changed (8feddfe: F2FP, I2FP, BindlessToArray and others); this branch's sources merge without a text conflict. The translator was rebuilt from the merged source (dotnet publish, .NET 10.0.403), dmsh_chain.lock rewritten by tool_lock.py --update-lock and dmsh_chain.h by dmsh_chain.py --header: chain 7b82ada1189f.

Checked: test_dmsh_ampere_mem (67) with DMSH_TEST_XLATE, test_nsp_chain, test_xlate_anycpu, test_docs, docs_symptoms.py --check.
gnosticgeek pushed a commit to gnosticgeek/nxforge that referenced this pull request Nov 6, 2026
@claude
Merge upstream PR seedds#1359 (newer commits): Float add atomics are …
3cce90e 
…kept: RED .ADD.F32.FTZ.RN was dropped, which washed out Ocarina of Time's outdoor scenes

Translator binaries, lock and chain header kept until the translator is rebuilt on the Mac.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016qteK29T18U8p1c9LsVY8g
gnosticgeek added a commit to gnosticgeek/nxforge that referenced this pull request Nov 6, 2026
@gnosticgeek @claude
Translator rebuilt from upstream main 16e3e24 and PRs seedds#1414 see…
c8ea716 
…dds#1300 seedds#1359 seedds#1360; chain lock and header regenerated (c5757270bc73)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016qteK29T18U8p1c9LsVY8g
@seedds

seedds commented Nov 6, 2026

Copy link
Copy Markdown
Owner

Thank you. Float add atomics are kept on main now (landed on main in b1d7c70 and 6121fd7), the same root cause you found, so I'm closing this. Final checks on the rebased tree (full shader build, walks, regression) are still running, so please report anything you see.

@seedds seedds closed this Nov 6, 2026
gnosticgeek pushed a commit to gnosticgeek/nxforge that referenced this pull request Nov 6, 2026
@claude
Merge upstream main 88c7e03: Ocarina of Time's landing (b1d7c70, 6121fd7
46d8e1f 
) replaces the pull request heads

Upstream landed Ocarina of Time in main (chain adec7d81d970: "holds 60 fps in
Kokiri Forest, 4 GB less") and closed the pull requests it folded in, with
reworded and partly different code. Mixing a pull request head with its landed
version hunk by hunk silently broke things (two GlobalToStorage tables, a
duplicate AF2ip, two mtk_pso definitions), so the rule here is one version per
feature:

- Pull requests upstream closed (seedds#1299, seedds#1300, seedds#1312, seedds#1343, seedds#1345, seedds#1353,
  seedds#1354, seedds#1359, seedds#1360, seedds#1373, seedds#1428, seedds#1430, seedds#1434): their hunks are taken out
  of our side before the three-way merge, so upstream's landed version stands
  alone. Gone with them: bindless pool table patching (reg_bump), the ghost
  table's per-submission hold, the growing raw pool table and its write-log
  sizing, seedds#1428's pool_buf spares (upstream's pool_table_take is the same
  idea), the ixc-based indirect mesh draw (upstream uses mdx), and their tests
  (t_pool_table, t_mesh_indirect, t_gmem_raw, finalized_bindless_pool,
  test_bnsh_nvn2, test_ndve).
- Open pull requests that upstream's landing supersedes: seedds#1394 (upstream's
  mtl_pool_addressed) and seedds#1393 pool_skip_retired (upstream's budgeted pool
  walk; the profile does not use it) are dropped with their tests and docs.
  seedds#1368's mesh pipeline prewarm is dropped too: upstream kept the background
  build in nvn_geometry.inc, and the prewarm's mtk_build / mtk_claim /
  g_warm_mps went with it.
- Kept, as upstream does not have them: NXForge's own features (GPU watchdog,
  present filters and anti-aliasing, aniso floor, quick settings, pipeline
  archive compaction, upload breakdown, memory report lines) and the still
  open pull requests seedds#1304, seedds#1305, seedds#1308, seedds#1320, seedds#1331, seedds#1332, seedds#1355, seedds#1356,
  seedds#1369, seedds#1376, seedds#1377, seedds#1379, seedds#1382, seedds#1390, seedds#1391, seedds#1392, seedds#1414, seedds#1420,
  seedds#1436, seedds#1439, seedds#1446, seedds#1453.
- Combined where both sides changed the same lines: GW_N 1 << 16 with seedds#1320's
  wide-write intervals (GW_PAGES = SBW_WPAGES, 128 slots), the CPU upload log
  with upstream's linear upload route, arch_open_end before the compaction
  limit, cbn_prog in geo_gs_one (seedds#1382).
- Translator: upstream's sources and binaries, plus seedds#1414's
  InstEmitAmpereMem diff and the LDSM SubgroupBarrier in
  InstEmitAmpereTensor.cs. The binaries and dmsh_chain.lock are upstream's
  until the Mac rebuilds them (dotnet publish, tool_lock --update-lock,
  dmsh_chain --header); dmsh_chain.h is regenerated for now.
- RED_ONCE keys: upstream took 153 and 154, so the present anti-aliasing lines
  move to 156 and 157 and seedds#1420's packaged-entry line to 158 (array 159).
- Host: hle_net.c and t_net_eventfd.c are upstream's (its eventfd poll waits
  out the whole timeout, which our 5d7920c fixed another way).
- Kept ours: dm/tools/romfs_image.py and its test, so the 26 GB RomFS image is
  not packed again.

dm/tools tests on Linux: same failures as on 875449e (Linux-only quirks),
plus test_nlg4 and test_dmsh_pool_arrays, which fail on upstream main too.
Not yet built on the Mac.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

docs Documentation nvn NVN on Metal and its tests (dm/gfx/nvn, dm/gfx/tests) shader chain The shader translation chain (DMSH_CHAIN): a change rebuilds every game's shaders

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lunks @seedds