Skip to content

Catch translator and host changes that break games nobody on the PR owns #272

Closed

Description

@jthrilly

This issue is fundamentally about how this project manages the deluge of changes that are landing in such a way so as fixes for one game don't break others. Please read the below @seedds! Cheers.

What happened

c3a0880 (from #178, "Guard Metal image atomics") was validated thoroughly on Pokémon Legends: Z-A: all 28,603 shaders, GPU probes and a 240 s run. Even so, it broke at least two games outside that set:

Neither failure is a mistake in the testing of #178. The project has one shader chain (DMSH_CHAIN) for every game, so each translator or host change affects all of the roughly 70 profiles. But it can only be checked on the games its author owns, and no contributor owns them all. CI can't help either, since it can't hold game files or run Metal 4 (#112).

What the existing tests miss

  • dm/tools/tests/test_image_atomic_bounds.py already builds synthetic Maxwell shaders from InstTable.cs and runs them through the translator, spirv-val and spirv-cross, with no game inputs. In CI, though, the translator cases are skipped: DMSH_TEST_XLATE is unset, and spirv-cross and spirv-val aren't installed.
  • The same test asserts that an atomic whose result is used must be rejected (require IGN with a discarded result). That is the pattern Yoshi uses, so the regression was locked in as intended behaviour. Nothing told the author that shipping games depend on it.
  • When shaders fail to translate, dmsh_build.py exits 0 and the NSP script packs the app anyway with "N of M shaders ready; K failed". The first sign is a crash at run time, often long after the change, once a rebuild picks up the new chain.

Proposal

Every item works for any contributor and in CI. None needs game files, keys, Metal or a particular contributor owning a particular game.

  1. Run the translator fixture tests in CI. Point DMSH_TEST_XLATE at the committed dm/gfx/tools/dmsh_xlate_bin, and install the .NET runtime and spirv-cross/spirv-val in tool-tests. The Metal compile step (DMSH_TEST_METAL) stays local. This is small: about 15 lines of workflow.
  2. A synthetic fixture for each pattern a game turns up. When a game hits a translator gap, the fix comes with a hand-encoded shader reproducing the instruction pattern, as test_image_atomic_bounds.py already does. For this case, that means a SUATOM/SURED with a used result, plus a buffer-image binding, replacing the assertion that requires rejection.
  3. Report shader regressions against the contributor's own previous build. dmsh_build.py compares the new _b_build.json with the previous one in the same build folder. It prints the shaders that used to translate and now fail, with reasons, and the NSP script stops unless told to continue. Nothing is committed: every contributor gets the check for the games they own.
  4. A per-profile instruction census (for discussion). shader_split.py already decodes every shader to make the profile's tables. It could also write per-shader counts of the instruction forms the translator treats specially (for example SUATOM with a used result, by clamp mode). Like shader_facts.tsv, it would hold only hashes and counts, no game code. The translator would declare the forms it rejects, and a CI test would list every profile whose census contains one. A PR that rejects a form used by a shipping game would then name those games, even when its author doesn't own them. Profiles without a census report "unknown" rather than pass, so it can be filled in gradually by people who own each game.

Ruled out: golden MSL outputs and committed per-game translation baselines. Both need the game to verify, and golden MSL is code derived from the game.

A smaller question: should the image-atomic admission skip the one dispatch with a [RED] line instead of aborting the app? I understand the abort is deliberate, since an unchecked atomic must never reach a dummy buffer. Skipping writes nothing, so it keeps that guarantee while a game keeps running.

Offer

I'm happy to open PRs for items 1 to 3 now, as separate small PRs (2 together with the Yoshi translator fix). Item 4 changes what a profile holds, so I'd like your view first. Does the census sound acceptable, and would you prefer it in shader_facts.tsv or a separate table?

Related: #178, #263, #173, #218, #112.

Activity

  1. JMRBDev commented on Oct 26, 2026

    @JMRBDev
    Contributor

    One more data point for the same cause, from Super Mario Bros. Wonder (010015100B514000, update v327680, chain 2647452f4522): 2 of 16,268 shaders are rejected at translation with

    NotSupportedException: MSL 2.3 image atomics require IGN with a discarded result; TRAP/returning semantics are not implemented.
    

    The other 16,266 build, the split finds them all, and the IPA packs (4.0 GB) — the game itself was not run yet, so what those two pipelines do at run time is unknown. It reads like the same semantic as Yoshi's three rejections (a shader that uses the value an image atomic returns), with a different message. The build log is at hand if the keys help.

  2. JMRBDev commented on Oct 26, 2026

    @JMRBDev
    Contributor

    Follow-up on the Wonder case I reported above: on current main (chain a3b77ce4602d) those two shaders compile — 16,268 of 16,268 — so whatever moved between 2647452f4522 and now (the translator binary included) covers that pattern. The rejection was real on 2647452f4522.

  3. v0lumehi commented on Oct 26, 2026

    @v0lumehi
    Contributor

    Good point @jthrilly and I have the same observation. Some of my new features introduced side effects that I could not have tested thoroughly because testing all games requires a heavy amount of compute and storage.

    I am trying to come up with a solution to this that doesn't break ToS of the well known Japanese company.

    I think at this state the project deverses a proper release cycle and an agent harness to test against to find regressions and bugs.

    Compute and storage are a cost factor also

    I would also appreciate some way of communicating with each other to plan and design more

  4. jthrilly commented on Oct 26, 2026

    @jthrilly
    ContributorAuthor

    Agree completely @v0lumehi - some sort of coordination is essential.

    I do think that the ideas proposed here could work. If the four proposals in my PR were implemented, the regression I experienced couldn't have happened. Is the same true of your regressions? Or is something more needed?

    @seedds - would really appreciate your thoughts!

  5. v0lumehi commented on Oct 26, 2026

    @v0lumehi
    Contributor

    Good question. Your four proposals would have caught c3a0880, and seedds has since landed 1 and 3 (and part of 2) in b117c91. Mine were a different kind, though, and none of the four would have caught them:

    1. pso_offline: metal-tt fails on ToTK's pso_pairs.tsv pairs whose pixel shader reads an output the vertex shader does not write #283 (TotK's offline pipeline archive): every shader translated fine. The break was between shaders: 11 pipeline pairs whose pixel shader reads an input the vertex shader doesn't write. Only an interface check across pairs finds that.
    2. Odyssey: "Preparing pipelines" deadlocks at ~6,000 of 20,740 (handoff-cap hits during the freeze) — DM_PSO_PREPARE=0 works around #294 / Mac app compile is failing for Mario odyssey #276 (Odyssey's prepare step): a run-time hang. One 31 MB uber shader makes Apple's compiler loop, and a lock held across the compile turns that into a deadlock (thanks to @mikesweeneyca for pinning it down). Only a boot run under a watchdog shows it.

    So I think something more is needed, and that's what #328 and #329 try to be, building on b117c91:

    I'd really value your eyes on both, especially #329, since the census was your idea. I'm tracking all of this in #313, which builds on this issue, so feel free to add or reorder things there.

  6. seedds commented on Oct 28, 2026

    @seedds
    Owner

    The original safeguards are implemented: translator fixture tests in CI and shader-regression checks, native image atomics with synthetic coverage, the cross-game regression harness, and usage manifests / impact analysis. Closing this proposal as completed; continuing regression infrastructure work stays tracked in #313. Thanks for identifying the gap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions