Repository navigation
feat(shader): link raw fragment epilogs through user data - #4840
Conversation
|
The overlaps with #1313 and #4271 are inherited from #4821. I have opened one coordination thread on the prerequisite to keep the order and resolution in one place. The proposal is #1313 and #4271 first, then #4821, then this PR; #4715 remains required for the title bring-up. #4821 now records the two proposed merge-order dependencies. This PR continues to depend on #4821 and #4715. At the checked heads, the conflicts are the adjacent AI-assisted: yes (Codex). |
e206901 to
7ba0462
Compare
7ba0462 to
5d1d434
Compare
|
Rebase handoff — rebased onto current One finding — independent of the rebase: Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release,
To pick it up: |
5d1d434 to
e229262
Compare
|
Checked on lavapipe: |
What
XashPS5 (PPSA19111, v1.3 / content 01.030.000, bring-up #4711) uses a separate RADV fragment epilog. On the main baseline
43e3a70f8, it first stops at the point-size check addressed by #4715. With #4715 and #4821, the next stop is:The captured fragment body preserves
s4from user data, uses the originals5as flags, then writess5 = 2befores_setpc_b64 s[4:5]. Its epilog at0x200290a00packs and exports the computed colors. Reading the original user-data pair would produce the wrong address.Extend the bounded tail proof to accept an unchanged user-data low half with a high half whose last writer is an unconditional immediate
s_mov_b32. Keep all preceding instructions and register state, including SCC and EXEC, and link raw fragment tails through the same mechanism as vertex tails. Track the epilog snapshot in draw-cache validation so a changed address or rewritten epilog cannot reuse stale pixels. The high half does not require a second user-data word.Unknown/computed target halves, branching bodies, registered shader tails, mesh tails and continuations reading their relocated PC remain unsupported. This adds no shader bypass or title-specific address.
Semantics follow AMD RDNA2 ISA sections 12.3 (
S_MOV_B32,S_SETPC_B64) and 12.5 (S_BRANCH). The convention is explicit in XashPS5's pinned Mesa: fragment epilog jump, 32-to-64-bit pointer conversion, and lowering tos_setpc_b64. Console hardware has not been measured; shader debt records that limitation.Tested
Windows: Windows 11 Pro 26100, WinLibs MinGW-w64 GCC 15.2.0 r7, Ninja, Release,
BUILD_TESTING=ON, GTX 1660 SUPER (driver 32.0.15.5585). Fulllibs,relinkerand focused targets build ate229262449157afcc1544448c9724a80bdb2636d, based on maindacd78525e537a779682032c0ace7f7af120af05. LocalANYPS5_ENABLE_SPIRV_TOOLS=OFF. All 19 selected tests pass with Vulkan required: graphics, continuation clip/cull exports, layered rendering, shader memory/control flow, driver, scalar set-PC, compute user-data calls and prepared graphics.Portable vertex state regression, following review: both SCC outcomes export a valid position and a defined vertex color. The EXEC cases save the active mask, perform a masked color write across the tail boundary, and restore the mask before exporting color and position. Full-image comparisons check linked versus monolithic shaders, identical triangle coverage and white versus black pixels. No assertion relies on vertices that omit a position export. The parent feat(shader): link raw vertex prologs through user-data tails #4821 validation separately corrupts only the linked SCC or EXEC and confirms each fails the pixel comparison on Windows NVIDIA and Linux lavapipe; restoring that source passes. AMD RADV hardware has not been retested here.
Fragment regression (wave64): full-image comparison with a monolithic reference, five versus six user SGPRs, changed epilog addresses, rewritten/restored code and carried SGPR/VGPR/SCC/EXEC state. The supplied high user-data word is flags, not the pointer high half. The fragment test fails at
unsupported dynamic s_setpc_b64against both main and the updated feat(shader): link raw vertex prologs through user-data tails #4821 alone (7092b01b0daab6afc52ce67d79f5d0f2537b7279), on Windows and Linux. The vertex fault-injection checks are documented in feat(shader): link raw vertex prologs through user-data tails #4821's validation.Linux: exact-source validation passes on Ubuntu 24.04, GCC, Release, default
ANYPS5_ENABLE_SPIRV_TOOLS=OFFand lavapipe, with Vulkan required. Fulllibs/relinkerand focused targets build. Of 19 selected tests, 18 pass andagc_driver_vertex_clip_cull_distancesskips; zero failures. That test passes on the Windows GPU. The workflow verifies the negative controls, restores the exact source, rebuilds and retests. Conventions, test registration and patch whitespace pass. These are focused checks, not the full suite. The validation workflow is kept off the PR source branch.Title, Windows: with fix(agc): accept inactive vertex point size on non-point draws #4715 and feat(shader): link raw vertex prologs through user-data tails #4821, local integration
82b64a3dastops at the fragment error above. Adding this change passes it. Retesting exact heade2069019eplus fix(agc): accept inactive vertex point size on non-point draws #4715 on the earlier main baseline90c683946(clean local integration1de643a65) with shader/frame capture enabled stops after 5.61 s at:The captured request is now a mesh-stage draw with a different continuation (
0x200291300), which is outside this change's supported stages. GDB independently reaches the same stop. An earlier uncaptured run exited with0x20474343and no diagnostic; its cause is not established. No presented frame, menu, gameplay or performance improvement is claimed.Conventions, test registration and patch whitespace checks pass. This is focused validation, not the full test suite.
Reproduction uses the public XashPS5 v1.3 release,
relinker --windows --windows-diagnostics --registry input/eboot.elf run/app.exe, bundled guest libc, matching host libraries inrun/libs, release resources and owned Half-Life 1valvedata inrun/app0/valve, thenrun/app.exefromrun. Captures use only the existingAPS5_DUMP_SHADERS/APS5_DUMP_FRAMESdiagnostics. No game assets are included.Dependencies and overlap
Restacked on #4821
7092b01b0daab6afc52ce67d79f5d0f2537b7279, including its portable SCC/EXEC pixel-test revision, then fragment-tail commite229262449157afcc1544448c9724a80bdb2636d. Maindacd78525includes merged #3763 and #4271. The inherited shader-debt conflict is resolved, retaining main's cached-IR entry and both tail entries, as well as continuation-aware clip/cull layouts with routed layer bits.Merge #4821 first, then this follow-up. #4715 remains the title prerequisite; its commits are not included. Inherited open overlaps #1313 and #4943 are tracked in #4821's description and coordination thread. Their proposed orders are not claimed as agreed. Preserve all draw-cache fields and export-layout behavior during integration, then rerun the relevant tests. Those open implementations are not carried here and have not been tested together with this branch. #4706's compute calls are a different operation.
Additional overlap: #494 and its stacked follow-ups #1191, #1193, #1201 and #1202 share draw-cache changes. Coordination proposes #494, then #4821, then #4840 for this overlap, subject to the existing prerequisites. Agreement is pending; the direct
Depends onentry will be updated once the order is agreed. Pairwise merge simulations at #4945fe165d37and #1202fda7dbf04conflict inDrawCache.hppandDrawDecode.cpp. Integration must preserve continuation snapshots and their validation, refresh the continuation after data-only user-word reloads, and retain clip/cull layouts. Combined build/runtime validation remains outstanding; the standalone results above do not cover this stack.The revised SCC/EXEC test is also covered by the RADV retest request; AMD hardware validation is still pending.
The title evidence above is from the pinned earlier runs, not a fresh title run on this rebase. No presented frame, menu, gameplay or performance improvement is claimed.
Checklist
main, stacked on the declared prerequisite; no other open PR found implementing this fragment-tail fix