Skip to content

fix: compile deeply nested NS2 Metal shaders offline - #1849

Merged
ly05010419 merged 1 commit into
ly05010419:mainfrom
1395490241lhj-code:codex/metal-bracket-depth-20261106
Nov 9, 2026
Merged

ly05010419 merged 1 commit into
ly05010419:mainfrom
1395490241lhj-code:codex/metal-bracket-depth-20261106

Conversation

@1395490241lhj-code

@1395490241lhj-code 1395490241lhj-code commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Generated Switch 2 MSL can exceed Metal/clang's default bracket nesting limit of 256, for example when translation turns an unrolled early-exit loop into nested blocks. Those sources currently fail offline compilation.

Select -fbracket-depth=1024 per deep source in the Mac app shader step and the translation-cache compiler. The app step applies this only to registered NS2 versions. Ordinary source commands, safe-math flags, SHA-based library names and compiler cache stamps remain unchanged, so existing libraries are reused. Compiler errors retain the existing reporting and cleanup behavior.

The scanner and synthetic walk are adapted from seedds' nxnative work at 56b73f21; attribution is recorded in THIRD_PARTY_NOTICES.md under the existing MIT notice.

Validation against main 9fe3fdc1d08ce5e1a64d120f257d79cc2dc7c0ca:

  • Four focused regressions pass, covering the 256/257 boundary, both production compilation entry points, ordinary NS1/NS2 sources and cache reuse. These exposed missing flags before the fix.
  • The real macOS Metal compiler rejects a synthetic depth-300 kernel with the default limit and compiles it successfully with the selected flag. No game shader is included.
  • python -m unittest discover -s tests: 1,541 tests run, 169 skipped, no failures (201.175 seconds). Existing subprocess ResourceWarnings were emitted.
  • git diff --check passed; read-only review found no blocking issue.

Draft validation limits: no full game app or extracted shader corpus was rebuilt for this isolated patch. The bracket scan is intended for generated MSL, counts comments/literals too, and raises the limit only to 1024. This does not establish gameplay or performance acceptance.

Final upstream CI: run 42323956367 was cancelled at the existing 10-minute job limit. All four new bracket-depth tests passed on the hosted runner, including the real Metal compiler test. Before cancellation, the log recorded four failures in test_settings_panel.SettingsPanel.test_current_modes_and_refresh (TOTK v1.4.3, compact/corners, overrides off/on). A control run on unchanged main at 9fe3fdc1 completed all 1,537 tests and failed those same four subcases on the settings-window height assertion. These PRs do not change that code. Full upstream CI is therefore not passing; the timed-out runs do not establish complete-suite acceptance.

Independent CI repair: #1850 corrects the baseline settings height assertion and splits source tests while retaining the final test gate and all eight UBSan checks. Its upstream run completed all 1,540 discovered tests without failures (8 skips) and passed all eight UBSan checks. That successful run is for the CI repair branch, not this PR: this PR's previous run remains cancelled and needs a rerun after the CI fix is available on its branch.

Integration validation (2026-11-07): all three jobs passed on the exact combined commit 360180ae85f263e3fa57ffe6bceddfcab3855e63 (fork CI run). This separate validation branch merges the unchanged heads of #1841, #1849, #1850, #1852 and #1856; none of these PR branches was rebased or expanded.

  • Hosted core: 1,544 tests, 11 skips, no failures; settings: all 3 tests passed; final gate and all 8 UBSan checks passed.
  • The hosted runner has no usable Metal residency-set device: the registration test and both pool-retirement modes were skipped. Their real-Metal cases passed locally in the complete combined suite: 1,547 tests, 169 environment-dependent skips, no failures; all 8 UBSan checks also passed locally.
  • This is combined-source evidence, not a green status for each original PR head, a packaged-app test, or gameplay/performance acceptance. The independent diffs remain unchanged; the four runtime/build fixes still need their own heads refreshed and validated after the CI repair in Fix compact settings probe assertions and split source CI #1850 lands.

@ly05010419
ly05010419 merged commit 9d69999 into ly05010419:main Nov 9, 2026
1 of 2 checks passed
ly05010419 added a commit that referenced this pull request Nov 9, 2026
ly05010419 added a commit that referenced this pull request Nov 9, 2026
…(1395490241lhj-code)

Preserve the contributor's original head.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants