Repository navigation
fix(recompiler): split image reads whose descriptor is a web of SRT Phis - #1274
Conversation
SplitDescriptorPhi (boykopovar#414) handles a T# or S# that is a Phi of SRT reads in one block. A descriptor picked inside a loop through a jump table reaches the read as a chain of Phis rooted at the loop header, so the plan threw "GetImageResource dword 0 is not a valid runtime value". When the one-block split does not apply, walk the descriptor dwords together through the Phi web, give each distinct SRT descriptor an arm, build a U32 selector Phi web carrying the arm index, and select between one copy of the read per arm. Paths whose dwords hold a program-counter, EXEC or ballot value fold onto the first arm (listed in TechnicalDebt); any other descriptor that cannot be recomputed still throws. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
adriadam10
left a comment
There was a problem hiding this comment.
Checked: build + ctest green locally (436/436) and CI green. The new verifyDescriptorPhis loop cases fail with ResourceTracker.cpp reverted to main and pass with it, so they test the change. A few things before this can land:
ResourceTracker.cpp:793-795: a leaf that carries a program-counter, EXEC, ballot or undefined value is folded onto arm 0. That turns a draw that threw onmaininto one that returns the first descriptor's result when the assumption ("that path never reaches the read") is wrong, which is the plausible-but-wrong case the exact-or-throw rule rejects (#333 #436 #469). The TechnicalDebt line records it but nothing checks it. Please scope the fold to what you can show is a non-sampling path (for example the leaf's edge only comes from blocks where the read is not reached under the structurized EXEC mask), and throw for any other control-value leaf. If you can't show that statically, say what in the shader guarantees it.- The PR body says the final version (folding only control values) wasn't rerun on the title; the earlier version that ran to the intro folded every unrecomputable leaf and ran together with #293. Please rerun the final version, or add a Linux test that reproduces the jump-table shape with the saved-exec-ballot registers, which is what a game log alone can't give us.
- Test coverage: only the
s_getpc_b64variant is pinned. Add rows for EXEC, ballot and undefined-register leaves (what you fold must be on purpose, not by accident), and a case that hits the 64-arm and 4096-state limits and still throws.
Happy to look again once these are addressed.
…criptors A path whose T# or S# dwords hold a program-counter, EXEC or compare-mask value is no longer folded onto the first descriptor: the web split gives up and the read throws as on main. Cache the invariant-Phi resolution per web, lower the state limit to 512 and test the program-counter, EXEC and compare-mask rows, unwritten registers, and the arm and state limits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
adriadam10
left a comment
There was a problem hiding this comment.
Checked the new head: build + ctest green on top of current main (550/550), and agc_shader_memory fails with ResourceTracker.cpp reverted, so the tests cover the change. CI is green.
The three points from the earlier review are addressed: leaves with a program-counter, EXEC or compare-mask value (and any leaf that can't be recomputed) now throw as on main instead of folding onto arm 0, the tests pin those cases plus the unwritten-register one, and the 64-arm and 512-state limits are tested to still throw.
Merging.
What
#414 splits an image read whose T# or S# is a Phi of SRT reads in one block. A descriptor chosen inside a loop or through a chain of branches reaches the read as a web of Phis, for example
Phi(first, Phi(first, second))across a loop back edge. The plan then throwsGetImageResource dword 0 is not a valid runtime value; chain: Phi(...Phi(...)).When the one-block split does not apply,
SplitDescriptorWebnow takes over:WebSelectorwalks all split descriptor dwords together, as one tuple, back through the Phi web. It handles one Phi block at a time, memoized on the tuple, so loop back edges close into a cycle. It stops at 512 tuples and 64 arms.U32selector Phi web that carries the arm index.IEqual32(selector, arm)andSelectU32.GetShaderBasearithmetic, which no descriptor holds), the read is left alone and throws as onmain. The selector Phis built so far are discarded. Nothing is folded or guessed.SplitDescriptorPhikeeps #414's one-block path (nowSplitDescriptorEdges) and calls the web only where that path used to give up. The copy-and-select emission is shared between the two paths (ReplaceWithArmCopies).Not covered: a web whose dwords are Phis of different blocks at the same step, such as a join that reloads only part of the T# while the rest comes from an earlier join's Phi. It throws as before.
UFC 6 (PPSA23566) is not fixed by this PR alone. In compute shader 0x363c84000, two
s_setpcjump tables are indexed by the same clamped light type,min(type, 32). Entry 32 of the first table jumps to the join without loading a T#, and entry 32 of the second jumps straight to the two samples at 0x808. So for a light type of 32 or more the samples read whatever the T# registers hold: the second table's address arithmetic from the previous loop iteration, or a saved compare mask. Nothing in the shader rules that path out; only the title's data (no light type ≥ 32) does. Those leaves therefore throw here. Handling them exactly needs a fault raised at run time when the read executes through such an arm, which could build on the deferred-fault buffer from #759. That would be a follow-up.Test (
verifyDescriptorPhisinagc_shader_memory_tests):Phi(first, Phi(first, second)). The plan has two images, the SPIR-V samples twice, and the snapshot holds both T#s.s_getpc_b64), by EXEC, or by av_cmpmask through VCC, throws.SsaPass::MakeInitialValuegives a never-written register the value 0, so the planner sees a constant, as it does for every unwritten SGPR read.Tested
Windows 11 Pro, NVIDIA GeForce RTX 5080, MinGW-w64 GCC 15.2.0,
ANYPS5_ENABLE_SPIRV_TOOLS=ON.ctest: 400/400 pass.ResourceTracker.cppreverted tomain,agc_shader_memoryfails at the loop row withGetImageResource dword 0 is not a valid runtime value; chain: Phi(ReadConst(GetSrtResource, Immediate), Phi(ReadConst(GetSrtResource, Immediate), ReadConst(GetSrtResource, Immediate))).s_getpc_b64row plans a third image from the program counter.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code