Repository navigation
Preserve shared-memory writes during shader AST optimization - #1487
Open
thehobbit85 wants to merge 1 commit into
Open
thehobbit85 wants to merge 1 commit into
thehobbit85 wants to merge 1 commit into
Conversation
javierdv7
pushed a commit
to javierdv7/nxnative
that referenced
this pull request
Nov 6, 2026
…#1462's CAS, seedds#1487, seedds#1329, seedds#1410, seedds#1411, seedds#1412, seedds#1357): lock and chain header; conflict leftovers removed from shaders.md and test_dmsh_ampere_alu.py
thehobbit85
force-pushed
the
codex/shader-optimizer-memory-effects
branch
4 times, most recently
from
November 9, 2026 02:38
1b92e34 to
72a05e0
Compare
Keep SPIR-V input argument storage addressable and keep effectful expressions at their definition. Cover optimized shared subword stores and discarded-result atomic add and exchange. Fixes seedds#1415
thehobbit85
force-pushed
the
codex/shader-optimizer-memory-effects
branch
from
November 10, 2026 02:48
72a05e0 to
ebf51ec
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1415 by preserving addressable shader call arguments and atomic writes whose result is discarded. The current main translator still reproduces both optimizer defects.
Current integration, 2026-11-10
Published commit
ebf51ecb4c1524b2a51c0a062b4fb9d60a2342b9rebases72a05e090e2d7e02ba7745c459f0382dc56afb8dontobbea4c90f136f478e25228bc3d4ab33c77420f2e. The optimizer patch and its assertions are unchanged. The rebuilt shader assembly combines them with main’s uniform-buffer bounds, vector loads, and other translator changes. It was published from source with .NET SDK 10.0.402. The main launcher, dependency graph, companion artifacts, and compiler pins are preserved. Combined chainb4168d6e0f71. The tool lock and apphost signature checks passed with Xcode 27.2 and Metal 32023.922.4 for both SDKs. AnyCPU metadata passed its fixture. Managed x64 and physical Intel execution remain open.Validation on this revision
dmsh_fetchfrom their extracted fixture scope, and the ABI literal ratchet counts a third"?"string innvn_mtl.m. The main bundle and the Pokopia and mesh fixture inputs are identical between5e1c1aecand the final main pin. The ABI failure was also reproduced on exact final main.dmsh_build.py. Exact main reproduces the same diagnostics. The baseline and affected source are unchanged. Generated documentation, chain verification, and whitespace checks passed.The impact scan identifies Breath of the Wild, Pokémon Scarlet, and Tears of the Kingdom through the shared NVN shader chain. Pokopia and Splatoon Raiders add shared-atomic coverage. Read-only discovery found no runnable Mac builds for these five under the inspected roots. Full game shader and gameplay comparisons and iOS-device validation remain open. Remote CI remains pending verification.
Historical published description, validation and original commit context
Fixes #1415 by preserving addressable shader call arguments and atomic writes whose result is discarded. Current main still reproduces both defects.
Current integration, 2026-11-09
Published commit
72a05e090e2d7e02ba7745c459f0382dc56afb8drebases1b92e34e9b4620cc7c7a11233a0382db5beddb97onto413e8808e0799eab8730688f082835aeeaca84c5. The optimizer patch and regression assertions are unchanged. The new main vector-load lowering, mesh scheduling, and shader builder changes are included. The combined shader assembly was rebuilt with .NET SDK 10.0.402. The universal apphost, runtime dependency graph, and compiler pins remain those from main. The chain is3ff28178f8d0.Validation on this revision
t_render_counter_overlapafter the four repaired presentation-crop modes passed 112 checks each. The follow-up targets also exposed nine failed assertions int_render_enable_queue. Exact main reproduces both failures with the same Mario profile, which setsDM_GAME_RENDER_COND=0. Under that profile the host deliberately draws through disabled predicates. The failing counter case's GPU bytes matched the expected zero. Both fixture recipes stopped before their second mode on this profile.dm/tools/unity_bucket.py. Exact current main produces the same diagnostic log. The lint baseline is unchanged. Generated documentation and whitespace checks passed.The impact scan names Breath of the Wild, Pokémon Scarlet, and Tears of the Kingdom through the shared NVN shader chain. Pokopia and Splatoon Raiders are additional comparison targets for shared atomics. Read-only game discovery found no runnable Mac builds for these five under the inspected game roots. Full game shader and gameplay comparisons and iOS-device validation remain open. Remote CI on this head is pending verification.
Historical published description and original validation
Fixes #1415. Preserve addressable shader call arguments and atomic writes whose return values are discarded. Main still lacks the optimizer changes; its translator source and binary are unchanged from the November 7 failing control. The original optimizer patch, assertions and commit message remain intact.
Current validation — November 8
Rebased the confirmed generated-header conflict onto main
555122a9069235441d5a88955105397ec6d53686; tested head1b92e34e9b4620cc7c7a11233a0382db5beddb97, chain07e6ae1e4d33. The combined source retains main's shader-output masks, handle-table correction and pipeline build changes. All five optimizer source changes, the translator binaries and lock fingerprints are byte-identical to the previous reviewed PR head. Only the generated chain header changes in the feature range-diff; all 87 upstream-only paths match main.Fresh validation on Apple M2 Max, Xcode 27.2 and supported Metal 32023.922.4:
DMSH_TEST_XLATE=$PWD/dm/gfx/tools/dmsh_xlate_bin/dmsh_xlate /opt/homebrew/bin/python3 -B -m unittest dm.gfx.tests.test_dmsh_ampere_mem dm.gfx.tests.test_dmsh_ampere_alu dm.gfx.tests.test_dmsh_shared_store dm.tools.tests.test_docs dm.tools.tests.test_nsp_chain: 143 passed, no skips. This includes optimized store/atomic regression cases and the native shared-byte/halfword Metal compilation and GPU fixture./opt/homebrew/bin/python3 -B -m unittest dm.tools.tests.test_dmsh_pool_arrays: seven passed, covering main's updated handle-table behavior.tool_lock.py --check-toolspassed with the unchanged supported pins.make -C dm/host GAME=010015100B514000 -j3passed.make -C dm/gfx/tests GAME=010015100B514000 -j3 rundid not complete: it stopped with SIGSEGV int_present_crop. The crash is atnvn_queue_present(nvn_queue.inc:207), where current main readsq->serial; the fixture callspresent(NULL, ...)att_present_crop.m:197. The fixture, runtime and build recipes match exact main; only the generated chain string differs. A separate build from an exact555122a9source export reproduced the same SIGSEGV under the normal recipe. With the optional Metal debug layer enabled, that main build instead stops earlier at Apple MetalFX’s_outputTextureBarrierStagesassertion; these are distinct failures. Earlier executed graphics cases passed, but later cases were not reached. No assertion, runtime guard or baseline was weakened.dm/tools/tests/test_patches.py, from main's new frame-step matching tests. No baseline growth, suppression or feature-branch workaround was added. This shared failure remains unresolved.The previous eight optimized/unoptimized synthetic SPIR-V/Metal readbacks and four additional native-store Mac/iOS compiler/readback cases remain historical on
e284a46b; translator/source/lock identity was verified before retaining that evidence. They were not repeated or relabeled as new game validation.The fresh impact scan identifies Breath of the Wild, Pokémon Scarlet and Tears of the Kingdom through the general shader rule; Pokopia and Splatoon Raiders also exercise shared atomics. The required
scripts/regress.py check --games 01007EF00011E000,0100A3D008C5C000,0100F2C0115B6000,0400A3A01EE4C000,0400380020056000stopped with exit 2 because no qualifying local Mac builds of those five are available. No gameplay, full game-shader rebuild or iOS device behavior is claimed. Remote CI on this replacement head remains pending verification.Earlier implementation, validation and preserved commit context (historical)
Fixes #1415. Shader optimization must preserve addressable call arguments and atomic writes even when their return values are discarded. These defects remain in main: its exact shipped translator fails both optimized subword-store cases and both discarded-result atomic cases. The original five-file optimizer patch and its assertions are retained.
Current validation — November 7
Tested
e284a46b1b791551a7c02550fdcd308ed0e6e22eagainst main01abf27977aa4a7366c4a14bc58d9eae75f1d8e6on Apple M2 Max with Xcode 27.2 and supported Metal 32023.922.4. RebuiltRyujinx.Graphics.Shader.dllfrom the combined sources, updated only that lock fingerprint, and regenerated chain2fd0305821c0. Main's other tools and compiler pins remain intact. The original commit message is preserved; range-diff and remaining-diff review passed.DMSH_TEST_XLATE=$PWD/dm/gfx/tools/dmsh_xlate_bin/dmsh_xlate /opt/homebrew/bin/python3 -B -m unittest dm.gfx.tests.test_dmsh_ampere_mem dm.gfx.tests.test_dmsh_ampere_alu dm.gfx.tests.test_dmsh_shared_store dm.tools.tests.test_docs dm.tools.tests.test_nsp_chain: 143 tests passed. Pinned Ruff/Pyright: 371 baseline findings, zero new and zero stale.0x25223344,0x00253344,0x11223349and0x00000005, respectively.make -C dm/host GAME=010015100B514000 -j3andmake -C dm/gfx/tests GAME=010015100B514000 -j3 runcompleted successfully. All executed cases passed; six optional Wonder reflection samples were unavailable, and two texture-upload poll cases were inapplicable to the selected profile. The former native-link failure is historical. Existing compiler warnings remain in unchanged main source; no warnings were suppressed.The impact scan identifies Breath of the Wild, Pokémon Scarlet and Tears of the Kingdom under the general shader rule; Pokopia and Splatoon Raiders also use shared atomics.
scripts/regress.py check --gamesfor those five stopped at inventory because none has a qualifying local Mac build. No game execution or full game-shader rebuild is claimed. Synthetic iOS compilation does not establish device behavior. Remote CI on this replacement head awaits verification.Earlier implementation and validation (historical revisions)
Fixes #1415. With shader AST optimization enabled, 8- and 16-bit shared stores need an addressable argument for the write helper. Atomic add and exchange must also execute when the shader discards their return value. This change preserves both properties and includes main’s Ampere global compare-and-swap support.
Current validation
Tested
28e2564eagainst mainb2d46b86on an Apple M2 Max. Built the shader assembly from the combined C# sources and regenerated the tool lock and shader-chain header (5264d54bea94). The supported compiler fingerprints matched. The exact main translator failed all four optimized regression cases. The rebuilt translator passed the focused Ampere memory, arithmetic, docs and chain suite (111 tests), and pinned Ruff/Pyright reported 371 baseline findings with zero new or stale findings.All eight synthetic shaders passed SPIR-V validation and Metal GPU readback. With optimization on and off, the observed shared word was
0x25223344for STS.U8,0x00253344for STS.U16,0x11223349for ATOMS.ADD, and0x00000005for ATOMS.EXCH. The Mac host built successfully. The full native graphics suite stopped while linkingt_gfx_suspendbecause its target omits thedm_vi_frame_queuedstub. The target, fixture, caller and stub sources match the tested main revision. Earlier full-suite and Python results are recorded below.The impact scan identified Breath of the Wild, Pokémon Scarlet and Tears of the Kingdom through the general shader rule. Pokopia and Splatoon Raiders also use shared atomics. The regression harness found no Mac build of these five games. This revision has synthetic shader and Mac GPU coverage. Gameplay, full game-shader builds and iOS behavior remain unverified.
Earlier implementation and validation
Fixes #1415.
With
debug: false, the shader AST optimizer could substitute a value for a local whose address the SPIR-V backend passes to a function. That broke the write helper used by 8- and 16-bit shared stores. It could also discard an atomic when the shader sent its return value to RZ, losing the write to shared memory.The optimizer now preserves those argument locals and keeps calls and atomic operations at their original execution point. The structured and lower-level optimizers share the same side-effect check. This includes the rebuilt translator assembly, its lock entry and shader chain
2070b9d404dc.Validation
Tested on an Apple M2 Max running macOS 26.6 with Xcode 27.2, against main
0f510fd5.Shared.test_optimized_*regressions against main's shipped translator. Both store widths failed translation and both discarded-result atomics disappeared. All four cases passed with the rebuilt translator. The fulldm/gfx/tests/test_dmsh_ampere_mem.pymodule passed all 66 tests withDMSH_TEST_XLATEset.spirv-val --target-env vulkan1.1. For GPU readback, the test harness added initialization and observation of one shared word around the generated Metal body. Starting from0x11223344, both settings produced0x25223344,0x00253344,0x11223349and0x00000005, respectively.Ryujinx.Graphics.Shader.csprojin Release with .NET SDK 10.0.402, then verified the shipped assembly and tool lock.make -C dm/gfx/tests GAME=010015100B514000 -j3 runpassed locally. The pinned Ruff and Pyright checks reported zero new findings.Reflect.test_table_of_uniform_handles, also fails on unmodified main. It expects a generated texture lookup without the existingdm_pool_markwrapper.Game coverage
The production shader build uses
XLATE_DEBUG=1by default. The reported defect requires the alternate optimization mode.regress.py impactlisted Breath of the Wild, Pokémon Scarlet and Tears of the Kingdom under the broad shader rule. Pokopia and Splatoon Raiders also use shared atomics. This pass used synthetic shaders only. Game builds, gameplay and iOS were untested.Preserved commit message