Skip to content

fix(recompiler): fill missing pixel interpolators as DEFAULT - #6604

Merged
oneandonlydean merged 5 commits into
boykopovar:mainfrom
notaxeltv:fix/recompiler-default-past-num-interp
Oct 21, 2026
Merged

oneandonlydean merged 5 commits into
boykopovar:mainfrom
notaxeltv:fix/recompiler-default-past-num-interp

Conversation

@notaxeltv

@notaxeltv notaxeltv commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

What

When the IR reads a pixel attribute at or past SPI_PS_IN_CONTROL.NUM_INTERP, fill the missing SPI_PS_INPUT_CNTL slots as DEFAULT (OFFSET 0x20) before shader info collection, and emit the same defaults from GetInterpolationParameter when the attribute has no SPIR-V binding (emit rebuilds input info from SPI and would otherwise throw undefined attribute). Component and mode bounds still throw.

PPSA03541 (UFC 5) prepares pixel shader 0x114073b400 at use with NUM_INTERP 0 while it still issues v_interp on attribute 0.

Tested

  • Windows
  • interpolation_reference_range_tests passed (collector coverage)
  • PPSA03541 stacked: past collector OOR for ~5h, then aborted on undefined attribute; this follow-up addresses that emit path (retest after deploy)

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
  • Unblocks: PPSA03541
  • GPUs tested: n/a (recompiler)
  • AI-assisted: yes

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
PR 7085 docs/dev/debt/shader.md L31
PR 9046 docs/dev/debt/shader.md L30-31

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 5021 docs/dev/debt/shader.md
PR 5371 docs/dev/debt/shader.md
PR 6235 core/shader/recompiler/SpirvBackend/src/SpirvModuleEmitter.cpp
docs/dev/debt/shader.md
PR 6280 docs/dev/debt/shader.md
PR 6412 docs/dev/debt/shader.md
PR 6488 docs/dev/debt/shader.md
PR 6879 docs/dev/debt/shader.md
PR 6924 docs/dev/debt/shader.md
PR 6947 core/shader/recompiler/SpirvBackend/src/SpirvModuleEmitter.cpp
PR 7201 core/shader/recompiler/SpirvBackend/src/SpirvModuleEmitter.cpp
PR 7484 docs/dev/debt/shader.md
PR 7642 core/shader/recompiler/SpirvBackend/src/SpirvModuleEmitter.cpp
PR 7684 docs/dev/debt/shader.md
PR 7922 docs/dev/debt/shader.md
PR 8004 docs/dev/debt/shader.md
PR 8077 docs/dev/debt/shader.md
PR 8538 core/shader/recompiler/Optimization/src/ShaderInfoCollector.cpp
PR 8625 docs/dev/debt/shader.md

@github-actions github-actions Bot added shader core/shader docs labels Oct 16, 2026
notaxeltv and others added 2 commits October 16, 2026 19:14
PPSA03541 prepares a pixel shader that still reads attribute 0 while SPI_PS_IN_CONTROL.NUM_INTERP is 0; extend the CNTL table with OFFSET 0x20 defaults instead of throwing.

Co-authored-by: Cursor <cursoragent@cursor.com>
After collector coverage, emit still rebuilt pixel input info from SPI and threw undefined attribute for the same UFC5 shader; return OFFSET 0x20 defaults from SPIR-V emission too.

Co-authored-by: Cursor <cursoragent@cursor.com>
@notaxeltv
notaxeltv force-pushed the fix/recompiler-default-past-num-interp branch from cd7db36 to c32c26e Compare October 16, 2026 17:15
@savvaniss

Copy link
Copy Markdown
Contributor

Intel and AMD Windows data: the new test passes on both GPUs with this change and fails on both without it.

Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, every execution test built, ANYPS5_NO_SHADER_CACHE=1, serial CTest, main 47a266cf9. Intel UHD Graphics 770 (8086:a780, 32-wide subgroups), driver 32.0.101.7088. AMD Radeon RX 9070 XT (1002:7550, 64-wide subgroups), driver 32.0.31041.1004, with VkCache emptied first.

UHD 770 RX 9070 XT
This head c32c26edb merged into main: interpolation_reference_range, ×3 pass pass
This PR's test file on main without the fix, ×3 fails fails

Full CTest (--parallel 4) with six open PRs merged into main (#6638, #6604, #6014, #6773, #6772, #6725): the same failures as main alone on both GPUs, nothing new. main alone: UHD 770 1392 of 1398, RX 9070 XT 1396 of 1398. The failures are image tables (#6647) and sample_multisample_target (#6971) on Intel, and quad_pixel_mask (#6255) and layered_volume_vertex_no_gpl (#4768) on AMD.

AI-assisted: yes (Claude Code)

@Neithennn

Copy link
Copy Markdown
Contributor

Windows NVIDIA run of the GPU tests, since the description lists the collector test only.

Windows 11 Pro 26200, NVIDIA GeForce RTX 4080 (driver 617.42), AMD Ryzen 7 5800X, WinLibs GCC 15.2.0 r7, Ninja, Release, bundled dependencies, BUILD_TESTING=ON, ANYPS5_NO_SHADER_CACHE=1. Head c32c26edb merged onto main f51724078; full build succeeds.

  • interpolation_reference_range, agc_interpolant_mapping, agc_driver_interpolation_modes, agc_driver_interpolation_f16: pass, with ANYPS5_REQUIRE_VULKAN=1.
  • Full ctest -j 8: 1398/1398. Unchanged main on the same machine: 1398/1398.

No title run.

AI-assisted: yes (runs done with Claude Code).

@klosixgaming-arch

Copy link
Copy Markdown
Contributor

Tested on Windows 11, MinGW-w64 GCC 15.2.0, NVIDIA GeForce RTX 5090 (driver 617.42), Ryzen 9 9950X3D. Head c32c26edb merged onto current main (a373e85); the merge is clean.

  • interpolation_reference_range passes with the change.
  • On main with only the updated test file it fails with attribute 0 with no SPI inputs becomes a default: shader info collection failed: interpolation parameter reference is out of range (attribute 0 component 0 mode 0, 0 inputs) (and the attribute 5 / 2 inputs case), so the test does cover the new behaviour.

Two things I could not settle from the diff:

  1. The description says the title result is a retest after deploy ("PPSA03541 stacked: past collector OOR for ~5h, then aborted on undefined attribute; this follow-up addresses that emit path"). Nothing here shows that UFC 5 gets past this shader with this change, so the "unblocks title" label is not backed by a run yet. I have no PPSA03541 dump to check.
  2. EnsurePixelInterpolatorCoverage modifies inputInfo.pixel through const_cast<ShaderPixelInputInfo&>, an object the caller passes as const. If that struct is hashed, serialized or used as part of a cache identity before Collect runs, filling in inputNum and interpolatorSettings afterwards could make the key differ from what was compiled. Is it only built per compile, or can it be shared? A copy passed down would avoid the question.

I am not approving until the title run is shown.

AI-assisted: yes. Written with AI assistance and reviewed by me.

@klosixgaming-arch

Copy link
Copy Markdown
Contributor

Tested on a PC with a real title, EA SPORTS UFC 5 (PPSA03541), as a controlled A/B. Both runs used the same base, upstream main 819c547, plus #6549, #6563 (resolved against main) and #6572, which UFC 5 also needs. The only difference was this PR.

Without this PR: the game aborts at +205 s of the run. The log shows Shader 0x114073b400 stage 5 ... preparing it at use, then presentation failed: shader 0x114073b400 stage 5: ShaderRecompiler::Recompile: shader info collection failed: interpolation parameter reference is out of range (attribute 0 component 0 mode 0, 0 inputs), then SIGABRT (exit 0xC0000409).

With this PR: the same shader 0x114073b400 stage 5 is prepared at use (+424.8 s), and the game does not abort. It kept running for the rest of a 1000 s session, with no abort, fatal or driver-failure line in the log.

Caveats: this is one run each, and the abort and shader prepare times differ because I played to the match by hand. The game is still very slow after this point (about 0.2 to 2 fps), which is a separate problem this PR does not touch. I did not run the unit tests as part of this check.

AI-assisted: yes. Written with AI assistance and reviewed by me.

@oneandonlydean oneandonlydean self-assigned this Oct 19, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Thanks, the fix is scoped well: it only changes shaders that fail to compile on main today. Two things before merge:

  1. Please remove the game title, the PPSA id and the shader address from the docs/dev/debt/shader.md line. Debt entries describe the behaviour, not a specific game. Title-specific notes belong in the Game bring-up discussion. Something like "a pixel shader can be prepared with NUM_INTERP 0 while it still issues v_interp on attribute 0" keeps the useful part.

  2. The emitter change in c32c26e (the default for an attribute past inputNum or without a SPIR-V binding) isn't covered by a test. Nothing fails if it's reverted. Could you add a case that runs SPIR-V emission of GetInterpolationParameter for an attribute past inputNum (and one with no binding) and checks the P0 default and zero deltas? In InterpolationReferenceRange.cpp, it would also help to assert the outcome (the filled slots read as DEFAULT and no parameter input is added) rather than only the absence of one error message.

Minor, no change needed: InterpolateHostP1/P2 are lowered before info collection, so those two cases in EnsurePixelInterpolatorCoverage never fire. And a 16-bit high-half read past NUM_INTERP still fails during emission (as it does on main).

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 19, 2026
# Conflicts:
#	docs/dev/debt/shader.md
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 20, 2026
…g a title

Name no title, title ID or shader address in the entry. The mechanism text is unchanged.
@oneandonlydean

Copy link
Copy Markdown
Collaborator

We merged current main and reworded the debt entry so it names no title; no code change.

@oneandonlydean
oneandonlydean merged commit 3594eee into boykopovar:main Oct 21, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs shader core/shader unblocks title Names a title ID on the Unblocks line

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants