Skip to content

feat(shader): link callees of calls whose target is loaded from a table - #4706

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
TripyFox:fix/3274-indirect-calls
Oct 14, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
TripyFox:fix/3274-indirect-calls

Conversation

@TripyFox

@TripyFox TripyFox commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

What

Fixes #3274: a compute shader's s_swappc_b64 whose target pair is loaded from a table (s_buffer_load_dwordx4 of a buffer whose descriptor comes from user data, indexed by a shifted offset) failed at compile time with "computed/data-dependent s_swappc_b64 call target ... is not statically resolvable".

  • Table callees (FindTableCalls, LinkCalls, driver ReadTableCallees): at dispatch the driver reads the descriptor and the buffer, collects the distinct non-zero targets at the loaded offset plus multiples of the offset's stride, reads each callee the same way user-data callees are read, and links one copy per call after the caller. The call becomes a branch to a compare chain (s_cmp_lg_u32 with a literal + s_cbranch_scc1 per dword) that s_call_b64s the matching copy. There is one chain per value of SCC at the call, and the chain restores SCC before the callee runs, so the callee sees the caller's SCC. Table contents stay out of the shader and pipeline keys; only the linked code (the target set) is keyed, as with user-data callees.
  • Unlinked calls: when a table call's target matches no linked copy, the call's fallback records an UnlinkedCall fault (target and PC) through the existing fault buffer and stops the invocation, or continues after the call when faults don't stop invocations. The driver reports it like a BDA fault. The linker lists these fallbacks in the request (GuestContext::linkedCallFallbacks, request format 25). Any other s_swappc_b64 whose target is not resolved is still refused at compile time, as on main.
  • Link-register model: outside the call regions, the link pair may now be used freely wherever no return address can reach it. A dataflow over the decoded instructions tracks each dword from the calls that write it until an instruction is known to overwrite it. Before, any non-overwrite use anywhere threw. The R&C caller reuses s14/s15 as scratch all over the program. VALU mask writes count as one SGPR unless the request is wave64. SOPC compares may read the pair anywhere.
  • Callers that read their PC: these can now have callees linked. Their PC-relative reads go to the captured guest code, which linking doesn't change. A caller with an in-code jump table still throws, because the CFG reads that table from the recompiled words.
  • SRT planning (second commit): PlanBuilder::Collect walked the IR recursively and kept visited values in a vector. A linked program with 30 callees (about 29k words) overflowed the stack. The walk now uses an explicit stack and hashed sets, and records reads in the same order.

Sources: AMD RDNA 2 ISA reference guide. The compare chain uses s_cmp_lg_u32/s_cmp_eq_u32 (§12.4, opcodes 6 and 7, encoding §13.1), s_cbranch_scc1/s_branch (§4.2, §12.5) and s_call_b64 (§12.2), and it keeps SCC as §3.8 describes it. The SGPR-mask width used by the link-pair dataflow comes from three places: §2.1 ("The upper half of EXEC and VCC are ignored for wave32 waves"), §3.9 (in wave32 only the low 32 bits of VCC are used) and §6.2.3 (a VALU mask in SGPRs takes two consecutive SGPRs in wave64). From these, a wave32 VALU mask write is counted as one SGPR. That is an inference from the guide, not a measurement. When the wave size is unknown, the dataflow assumes the narrower overwrite and the wider read. The s_buffer_load range is taken as the descriptor's whole size (stride × num_records, or num_records bytes). That size covers both possible range interpretations. Which one the PS5 uses was not measured, and this is recorded in TechnicalDebt. None of the new behaviour was measured on hardware. s_call_b64 itself was already measured on gfx1036, as TechnicalDebt records.

Data from R&C (PPSA01474): one table with 348,156 16-byte records {target, arg} and 30 distinct leaf callees, 85–834 instructions each (about 92 KB in total). Each callee returns through s_setpc_b64 on the link pair and makes no further calls.

Tested

  • Windows 11, MinGW GCC 15.2, RTX 5090: full ctest on main 23e93c0 + this PR, 1146/1146 passed. New and updated cases are in ScalarCall.cpp (matcher, linker layout, CFG of the linked program, translation, unlinked fallbacks, wave32/64 mask writes, link-pair dataflow, jump-table refusal, s_call range), ShaderMemory.cpp (request format 25 and the new field's round trip), and the new execution test ComputeTableCalls.cpp (agc_driver_compute_table_calls). That test runs a linked two-record table on Vulkan in wave64 and wave32, with SCC 0 and 1 reaching the callee, and checks that a record past the table reports the UnlinkedCall fault.
  • tools/check_conventions.py --base origin/main: passed.
  • Ratchet & Clank: Rift Apart (PPSA01474):
  • Astro Bot (PPSA21567): it aborts about 1 minute in with storage image access to depth/stencil surface ... is not implemented. Plain main (d592d54) without this PR does the same, so it is not caused by this change.

Checklist

🤖 Generated with Claude Code

@kr4y4

kr4y4 commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

A second-vendor run, to go with the RTX 5090 one.

Setup: Windows 11 (26200), WinLibs GCC 15.2.0 r7, Release, CI configure flags, an RTX 4070 Laptop GPU and an Intel Raptor Lake-S iGPU. The head 7c7b22d merged into main 584cc4c.

  • scalar_call, scalar_swappc, agc_driver_scalar_swappc and agc_driver_prepared_shaders pass on both GPUs with --repeat until-fail:3.
  • Full suite (ctest --timeout 120 --parallel 4): 1018 of 1018 on both GPUs. That includes the user-data callee tests from feat(shader): link compute shaders' user-data callees at dispatch #3294, agc_driver_compute_user_data_calls and its --registered mode.

Written with AI assistance.

@Tamalero

Copy link
Copy Markdown
Contributor

In-game confirmation on Linux: with this PR, Ratchet & Clank: Rift Apart (PPSA01474) no longer aborts with the #3274 error. It plays the PS logo, the raytraced logo and the Insomniac logo, then compiles about 240 new shaders. It stops about 6 minutes in at the table shape you predicted from the offline replay.

  • No Recompiler: a deferred compute shader is dispatched and aborts: s_swappc_b64 through a per-lane function table (Ratchet & Clank: Rift Apart) #3274 abort. No UnlinkedCall fault was reported. The linked programs are big: about 4.5 MB each in the disk cache, with about 230 MB written per 10 s while they compile behind a black screen after the Insomniac logo.
  • No aperture refusal. I had a local stub answering operands 235–238 with values measured on gfx1030 (see shader: reading a scalar aperture source as a value refuses (Ghost of Yōtei PPSA26344, shader 0x8000356b00) #3803), so this doesn't show whether that shader would otherwise stop at src_private_base on this stack.
  • The stop is in compute shader 0x116bb65f00, stage 0: GetImageResource dword 0 is not a valid runtime value. The T# is loaded through a pointer that is read from a buffer at a waterfall index (ReadFirstLane inside a loop):
    LoadAddressU32@pc=0x6340(GetAddressResource(ReadConstBuffer@pc=0x1aa8(GetBufferResource(V# from the SRT),
      ShiftLeftLogical32(ReadFirstLane(SelectU32(LogicalAnd(INotEqual32(BitwiseAnd32, Immediate), INotEqual32(Immediate, Phi)), IAdd32(Phi(Phi, SelectU32), ...
    
    That matches the per-record pointer you describe. Marvel's Guardians of the Galaxy (PPSA01748) has the same shape in two shaders (pcs 0x4378 and 0x4380), so both titles now wait on it.
Details

Setup: CachyOS (Linux 7.2), RTX 4090 (NVIDIA 615.78), GCC 16, Release. The title is relinked with --to-intel from the same tree.

Stack: main 90c683946, plus this PR at 7c7b22dfe, plus #757 (tv7's rebase/757, 8945fe208), plus #758 (cad61e9a7). Local changes on top:

Merge notes against 90c683946:

Tests on the combined stack, all passing:

  • scalar_call, scalar_swappc
  • agc_driver_scalar_swappc and _fetch
  • agc_driver_compute_user_data_calls and _registered
  • agc_driver_prepared_shaders and its deferred, deferred_undecodable, registered_workgroup(_sync), indirect, uncached, debug and debug_failure variants
  • the image-table, bindless, runtime-image, shader-memory and disk-cache tests

Tested and written by Claude (AI assistant) on a contributor's machine.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Please rebase: core/shader/recompiler/ControlFlow/src/RequestSerializer.cpp conflicts with main in the trailer after version >= 21u (main now requires the float16 byte from version 22). Main is still on request format 23, so 24 for linkedCallFallbacks stays free. The Silent Hill: Townfall shape (descriptor at a constant address) is a separate follow-up, not for this PR.

@TripyFox

Copy link
Copy Markdown
Contributor Author

Rebased onto main (23e93c0). Main took request format 24 in #4201 in the meantime, so linkedCallFallbacks is now format 25; the float16 byte check and the trailing-data check from main are kept. Full ctest 1146/1146 on Windows/MinGW, conventions check passes. Agreed on Silent Hill: Townfall, that will be a separate follow-up.


Generated by Claude Code

@oneandonlydean
oneandonlydean dismissed adriadam10’s stale review October 14, 2026 10:47

Addressed in 10acd0c: compile-time refusal kept for non-table calls, Vulkan test added, PreparedShaders.cpp back to main, debt line moved.

@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 14, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Rebased on main: linked call fallbacks moved to request format 26 (25 is now dithered coverage), and the vertex user-data tail link fills linked.code. Full ctest passes.

@oneandonlydean
oneandonlydean force-pushed the fix/3274-indirect-calls branch from 10acd0c to 638413c Compare October 14, 2026 12:14
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 14, 2026
TripyFox and others added 2 commits October 15, 2026 06:44
An s_swappc_b64 whose target pair comes from an s_buffer_load of a buffer
whose descriptor is loaded from user data now calls the targets that buffer
holds: at dispatch the driver reads the buffer, links one copy of each
distinct callee after the caller and rewrites the call into a compare chain
that s_call_b64s the matching copy. A target that matches no copy, and any
other computed call, records an UnlinkedCall fault that the driver reports,
instead of failing the shader at compile time.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
An s_swappc_b64 whose target is not resolved compiles to a faulting call
only at the fallback the table-call linker emits; the linker lists those
calls in the request (GuestContext::linkedCallFallbacks), and every other
computed call is refused at compile time again, as on main. The new
agc_driver_compute_table_calls test runs a linked two-record table on
Vulkan in wave32 and wave64, with SCC 0 and 1 reaching the callee, and a
record past the table reporting the UnlinkedCall fault.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Rebased onto main again: UnlinkedCall now comes after ImageTable, and the SRT walk commit was dropped since main already has the same fix. Full ctest passes.

@oneandonlydean
oneandonlydean force-pushed the fix/3274-indirect-calls branch from 638413c to 3c1f439 Compare October 14, 2026 17:50
@github-actions

github-actions Bot commented Oct 14, 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
#5176 docs/dev/debt/shader.md L126-127
#5200 core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp L2905, L3017, L3133-3135
core/shader/recompiler/tests/RequestSerializerTests.cpp L205-207, L242-244, L265-266, L387-389, L401, L403
#5304 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp L35
#5314 core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp L2905, L3017, L3135
core/shader/recompiler/tests/RequestSerializerTests.cpp L205-207, L242-244, L265-266, L387-388, L401, L403
#5884 core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp L2905, L3017

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
@fma-rc

fma-rc commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

1348 Ex Voto (PPSA24249) is a second title with the Silent Hill: Townfall shape. On main 32e810d with this PR at 3c1f439 plus #5176, the call and ordered-append tests pass (18 of 18) and the title stops at 119 s with a warm shader cache:

compute shader 0x1680648b00 ... computed/data-dependent s_swappc_b64 call target at program counter 0x00000130 is not statically resolvable

From the dumped request: the shader loads its descriptor with s_load_dwordx4 from the constant address 0xF_E0040000 (vcc set to 0xe0040000 / 0xf), reads the target with s_buffer_load_dwordx2 at offset 0x20, skips the call when that is null, and otherwise calls it with s_swappc_b64. Nothing in AnyPS5 maps that address, so the descriptor is in the title's own memory and could be read when the dispatch is recorded.

Is anyone taking the follow-up? If not, I'd like to write it on top of this PR.

@tv7

tv7 commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

@fma-rc that would be great, please go ahead. Silent Hill: Townfall has the same shape at 0x134, and I can test your head on it. One thing to know first: the descriptor isn't in the title's memory. I looked at this for Silent Hill, and 0xF_E0040000 belongs to Sony's libSceAgc, so reading it at record time on main reads nothing the title wrote.

What the shipped libSceAgc does (from the backport copy that ships with some dumps; I only read it, nothing copied):

  • At init it takes the reserved direct-memory block (other PS5 projects pin it at 0xF_E0000000, 2 MiB), places its "FS table" at base + 0x40000, zeroes the first 0x60 bytes (six 16-byte V# slots), and aborts with [Agc] FS Table offset has shifted. This is not survivable. if the table is not at exactly 0xF_E0040000.
  • Four exports access it: -pjHhph0ZUc writes V# slot i (0..5), OXDc0KCnUhs reads it, 86rJnbZUpr4 writes a qword at 0xF_E0040010 + 8·i, and NbiQSQ-UL+M reads one. Silent Hill does not import any of them.
  • During init the library loads a handful of its own built-in GPU routines and writes {routine address, 0} pairs into a 128-byte buffer. It then sets slot 0 to a V# over that buffer (stride 16, 8 records). So the qword at +0x20 that the title loads is one of Sony's routines, not null. The one Silent Hill reaches is 0xd8 bytes long. It starts with s_getreg_b32 of HW_ID2 and reads trap temporaries (ttmp7), which AnyPS5's decoder rejects today (unsupported scalar source operand code 115).

So on hardware the call does happen. In Silent Hill the shader reads nothing back from it: after the call it builds M0 from its own registers and never reads s[16:17]. That fits an ordering step before ds_ordered_count, which #5176's debt entry already lists as not modeled.

What I measured on Silent Hill with a local stub that skips the call (details in #5176): with #5176 as it is, the title hangs within minutes. With the counter addressed by offset0 (the review's suggestion), it runs the full 900 s. So skipping this call did not cause the hang; #5176's M0 handling did.

A direction that would match this: AnyPS5 commits the table at 0xF_E0040000 itself at init, with slot 0 pointing to a buffer whose routine entries are an AnyPS5-written routine that only returns (s_setpc_b64 s[14:15]). Your follow-up then links that target like any other table callee. It would be worth adding a debt entry saying the routine's ordering work is not modeled, and possibly the four exports. Your call on the design, of course.

AI-assisted: yes

@oneandonlydean
oneandonlydean merged commit 4c1de0c into boykopovar:main Oct 14, 2026
1 check passed
@TripyFox
TripyFox deleted the fix/3274-indirect-calls branch October 14, 2026 19:19
@fma-rc

fma-rc commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

@tv7 thanks, that saved me a lot. I checked it against the PS5 12.00 libSceAgc and libSceAgcDriver modules and it matches: the driver allocates 2 MiB of direct memory and maps it at 0xF_E0000000 at init, libSceAgc takes its part through Um_jkyDy9rI and places the table at 0xF_E0040000 (aborting otherwise), zeroes the six 16-byte slots, and sets slot 0 to a V# with stride 16 over eight {routine, 0} records, the third of which is what the title calls. The slot accessors return 0x8a6c000b for a slot of 6 or more and 0x8a6c002f when the table is elsewhere.

AnyPS5 maps nothing there yet, so the follow-up will map that block, fill the table at init with routines that only return, and link that callee on top of this PR. It is up as #6160 (branch fma-rc:feat/agc-fs-table-calls). On main 2559e96 Ex Voto stops at 57 s on this call; with it, no stop in 300 s cold or warm. @tv7 Silent Hill: Townfall on that head would be the second title, if you still have time.

Edit: an earlier version of this comment said the block was not at 0xF_E0000000; the driver module shows it is.

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

Labels

agc libSceAgc* build docs shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Recompiler: a deferred compute shader is dispatched and aborts: s_swappc_b64 through a per-lane function table (Ratchet & Clank: Rift Apart)

7 participants