Skip to content

fix(recompiler): include implicit inputs in relative vector windows - #4057

Merged
oneandonlydean merged 3 commits into
boykopovar:mainfrom
jweidenh:codex/vector-window-stage-inputs
Oct 12, 2026
Merged

oneandonlydean merged 3 commits into
boykopovar:mainfrom
jweidenh:codex/vector-window-stage-inputs

Conversation

@jweidenh

@jweidenh jweidenh commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

What

Fix a shader compilation regression introduced by #3856. The relative VGPR window was sized only from decoded operands, before stage setup wrote implicit registers. A vertex program naming v0–v3 can still need v5/v8, a pixel program naming only v0 can receive position inputs in v1–v3, and a compute program naming v0/v1 can need its Z ID in v2. Each can throw “vector register … lies past the relative-addressing window.” External fetch calls have the same problem when their generated destination span extends beyond decoded operands.

Extend an active window from the actual prologue writes before translating the body, and pre-scan external fetch destinations using the same load generator as translation. Registers below the window stay in SSA; shaders without relative addressing keep their existing path. The measured out-of-range opcode behavior is unchanged.

Add CPU regressions for seven stage prologues in wave32/wave64, SSA prefixes, and external fetch spans ending at v255. Add native vertex, pixel and compute readbacks. Also keep the existing SPIR-V window-shape test observable under optimization with a dynamic index and buffer stores.

Tested

Current source: 5ec6cff3fae85661c2f69048a35c857b4e8c7826, rebased onto bab7b3285221495463a4a2494dc410cf137b4a92. The only rebase conflict was adjacent CMake test registration; both this PR's registrations and #3741's eight optimized-readback registrations were retained. The translator and both test source files are byte-identical to the preceding head.

Windows 11 x64, RTX 5090 driver 617.42, portable MinGW GCC 15.2, Release with SPIRV-Tools enabled: all six selected tests passed on the rebased candidate without skips or feature refusals:

Test Result CTest time
agc_driver_dpp16_operands Pass 2.875 s
agc_driver_relative_indexing Pass 1.118 s
agc_driver_relative_stage_inputs Pass 1.002 s
agc_driver_scalar_relative_indexing Pass 1.130 s
agc_vector_window Pass 1.621 s
agc_vector_window_stage_inputs Pass 0.013 s

The full libs target built successfully. Conventions and independent source/raw-evidence reviews passed. Shader caches were disabled and every test remained below 30 seconds.

The current Windows base still fails to compile in Socket.cpp because HostPoll is unavailable there. Validation therefore applies only the reviewed build fix from #4021 (121b16fb4bbd47ff4ce9302c40631bab858e3e71). Actual tested Windows pin: 840621b548c9be8699b19a9d0440fcf3ee64b408. It differs from the published candidate only in Socket.cpp; that separate fix is not included in this PR.

Historical negative controls remain at test-only baseline 359d0e0ec418fab8c600c8a73ca77023d1677684, based on 11d9c00cf60bf6242fe51c5711167d8ad7cfbbb1. Windows validation pin 7ab31c73af76b44ba4bb58a532f235a774dee512 includes the same #4021 build dependency. Both exact baseline regressions reproduced all expected assertions: nine CPU groups and three native shader compilation failures. The preceding candidate 5ee79a871b6b193ee22ef711606196b1fda222e6 (Windows pin a8c0c2557292f646028ff3b0072b2df44841a814) passed all six tests. The unchanged baseline failures were not repeated for this CMake rebase; current validation above covers the rebased candidate.

The native regression verifies every pixel of two 64×64 draws and all 16 compute output words. Vertex indexing depends on nonzero firstVertex and firstInstance; the fragment shader reads position W relatively; the compute shader reads implicit Z relatively. Other stage prologues and external-fetch spans have CPU translation coverage only.

Linux validation remains blocked at configuration because the local runner lacks the required SDL PipeWire/PulseAudio/ALSA development files. That known blocker was not rerun, and no Linux result is claimed for this head. Before the original rebase, all nine CPU groups passed on Linux at 78763696, and the existing relative/scalar GPU controls passed; these remain historical evidence. Linux DPP16 remains excluded for the existing eight-lane lavapipe subgroup limitation. These are synthetic correctness regressions, not retail-title startup or FPS measurements. The existing inferred-usage approximation for the window end remains. No driver settings or production LTO policy changed.

Checklist

@github-actions github-actions Bot added build shader core/shader agc libSceAgc* needs rebase Conflicts with main, rebase needed labels Oct 11, 2026
@github-actions

github-actions Bot commented Oct 12, 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
#4120 core/libs/prx/libSceAgcDriver/tests/VectorWindow.cpp L39, L121
#4150 core/shader/recompiler/Translation/src/InstructionTranslator.cpp L484

@Dextroid17

Copy link
Copy Markdown
Contributor

The mechanism matches the body's description: emitEntryPrologue's writes are now routed through a setVector helper that grows vectorWindowSize from the stage's actual implicit writes before the body loop runs, and translateExternalFetch is refactored so the same fetch-generating code pre-scans destination spans and feeds the window extension — both gated on a window already existing (vectorWindowSize != 0), so a program with no relative addressing is untouched. The register-count boundary (v255, NumVectorRegs = 256) is real and the pre-scan's clamp exercises it correctly.

There's a real conflict to flag before this merges: this diff predates #4120 and does not include it. Both PRs are open against the same function, InstructionTranslator::Translate, and the same files (InstructionTranslator.cpp, IrBuilderRegisters.cpp, VectorWindow.cpp). The hunks themselves don't overlap line-for-line, so a mechanical rebase will likely apply, but the two changes interact semantically: #4120 makes SetVectorReg emit a plain SSA register in addition to the window store when there are no indexed writes, while this PR's own RequireWindow assertion (per its test additions) expects SetVectorRegister to appear only below the window base. This PR's new stage-prologue fixtures use only v_movrels (no v_movreld/v_movrelsd/v_swaprel), so after a rebase onto #4120, vectorWindowIndexedWrites would be false for them — worth a specific re-check of whether the new CPU tests still pass once that flag exists, not an assumption that they will.

Two smaller things worth surfacing to the author: the diff removes the pre-existing agc_vector_window test's selects <= 4 assertion — a real regression guard being weakened, not just extended, and that's easy to miss in a 373-line diff; and the body's claim that "the current Windows base still fails to compile in Socket.cpp" is stale — the stated base (bca7a29d) already has #4021's fix merged into it.

On testing: the six named ctest targets are real and four pre-exist; two (agc_vector_window_stage_inputs, agc_driver_relative_stage_inputs) are genuinely new and registered correctly in the diff. But none of that can be confirmed to have actually run on this PR's head — no CI lane that builds core/shader/recompiler or runs these tests executes on a pull request (linux/windows are gated to push/workflow_dispatch; relinker returns before core/libs), and the body's own pinned commit for the tested Windows run (840621b5) differs from the published head (5ec6cff3). Given the size and correctness-critical surface here, I'd ask for the rebase against #4120 first, then a dispatched full build before this is approved on trust.

Written with AI assistance.

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

Labels

agc libSceAgc* build shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants