Repository navigation
fix(shader): follow a conditional pointer chase through a V# behind a pointer-keyed record pointer - #9633
fix(shader): follow a conditional pointer chase through a V# behind a pointer-keyed record pointer#9633EagleBearerYS wants to merge 26 commits into
Conversation
…mers A T# read from a nested child table stayed a runtime read when one of its words also fed another instruction. That read goes through a V# that exists only in the snapshot, so the dispatch read a garbage address; lavapipe crashed in the shader. The nested branch now refuses a child whose descriptor words are used by anything but equal handles, as it already does for a V# with another consumer. (cherry picked from commit 0df7d32)
An S# read from a V# that a scalar record selects through a pointer was refused, so recompiling failed with "GetSamplerResource dword 0 is not a valid runtime value". The nested branch now takes sampler columns: they get child tables per distinct V# like nested T#s, share equal children, count against the nested key budget and the S# entry budget, and fault on an unmapped, non-buffer or uncaptured parent. The plan decoder accepts a nested sampler column; the plan encoding is unchanged.
An S# or T# loaded through a pointer that a scalar record holds was refused when a Phi merged the pointer's low word with constants, so recompiling failed with "GetSamplerResource dword 0 is not a valid runtime value". Such a column is now pointer-keyed: the snapshot captures every record's pointer and the descriptor behind it, the map holds boykopovar#7223's sorted pointer dictionary with descriptor records as its codes, and the GPU looks up the actual 64-bit pointer. A pointer that was not captured faults as outside the snapshot, and a null one as unmapped. The plan gains TableColumn::pointerKeyed and the snapshot gains ImageTableColumnSnapshot::pointerKeyed, so the disk-cache format moves to 43 and the image-table ABI to 4.
A T# loaded through a record pointer whose low word passes nested Phis was refused when one Phi arm held a word of a Ballot lane mask, as in a waterfall loop that reuses the pointer's SGPR for an s_and_saveexec or v_cmp mask, so recompiling failed with "GetImageResource dword 0 is not a valid runtime value". A pointer-keyed column now accepts such a word as an arm that is not a record pointer, as it does for constants, and still refuses any other foreign arm. At run time a pointer built from a lane mask is not a captured pointer and faults as outside the snapshot. A pointer-keyed column also accepts low-word reads, and a high-word read, of the same record word under different keys: same table, record offset, stride and addend. The GPU looks the pointer up by its value, so the key does not matter there. The column takes its table and record offset from the high-word read, which dominates the descriptor load. A shader that planned before plans the same way, so the disk-cache format stays at 43.
…ed record pointer A T# loaded through a record pointer was still refused with "GetImageResource dword 0 is not a valid runtime value" when the pointer's low word passed ANDs with lane masks, as a waterfall loop that copies EXEC into the pointer's SGPR builds them, or when its high word was a Phi of the record's high word read under several keys. For a pointer-keyed column, the walk of the low word now takes an AND with a lane-mask operand (a Ballot word, or an AND or Phi of only Ballot words and immediates) and keeps walking its other operand, so its record reads must still match the column and anything else still refuses. The high word is walked the same way: its record reads must be the record word at the low word's offset plus 4. A high word that is one read still gives the column its table and record offset, as before. Otherwise the table comes from a record read whose V# words are defined before the descriptor load, and the record offset from the high word, which the GPU does not read for a pointer-keyed column. A high-word arm read from another buffer, such as one whose V# is loaded through a pointer, stays refused. A shader that planned before plans the same way, so the disk-cache format does not change.
… a pointer-keyed record pointer A T# loaded through a record pointer was still refused with "GetImageResource dword 0 is not a valid runtime value" when a Phi of the pointer's low word merged the record read, a zero, and the waterfall's lane mask with the handled lane's bit cleared, BitwiseAnd32(CompositeExtractU32x4(Ballot(...), 0), BitwiseNot32(ShiftLeftLogical32(1, index))), as s_ff1, s_lshl and s_not with s_and, or s_andn2, build it in the SGPR the pointer reuses. The walk took only Ballot words and ANDs of them as lane masks, so it walked into the NOT and refused. A lane-mask operand is now a Ballot word or the loop's lane bit, a shift of 1 by an index from FindILsb32, FindUMsb32 or ReadFirstLane (through the AND, add, subtract and select that s_ff1_i32_b64, s_bitset0 and the shift's own mask put around it), or an AND, OR, NOT or Phi of only those and immediates. An arm of the pointer that is entirely such an operand is skipped. An AND with a record read in it still has the read matched against the column, and anything else still refuses. agc_driver_pointer_phi_lane_not_table builds that shape in a wave64 waterfall: the low word's Phis merge the record read at +0, a zero and the lane mask without the handled lane's bit, cleared by s_not and s_and and by s_andn2, and the high word is one read at +4. Each lane samples the T# of its record, and every masked word faults outside the snapshot. agc_driver_pointer_vsharp_select_table reads a record keyed by ReadFirstLane(SelectU32(LogicalAnd, IAdd32, Phi)), a pointer from it, a V# through the pointer and the T# through the V#; it passes without this change and keeps that chain checked, including a record pointer moved to the other V# without a recompile. Shaders that planned before plan the same way, and a refusal is never cached, so the disk-cache format does not change.
…keyed records A pointer-keyed record pointer now also takes lane-mask ANDs, the waterfall loop's lane bit and its OR and NOT forms, and a high word merged from the record's next word. A high word read from a second buffer stays refused.
The low and high words of a pointer-keyed record pointer were walked separately, so a high-word arm on a path whose low word is a lane mask refused the plan even though that pair is no pointer. Ratchet & Clank: Rift Apart (PPSA01474) stops this way at LoadAddressU32@pc=0x9a30: one high-word arm reads a second buffer, and on that edge the low word is a lane mask, the waterfall loop reusing the SGPR pair. Walk the two words together. Where both are Phis of one block, each low-word edge takes the high word of the same predecessor. An edge whose low word is a constant or a lane mask is skipped with its high word. A record low word still needs every high word it can meet to be a constant, a lane mask or the same record column at offset + 4, so a second-buffer high word paired with the record word stays refused. agc_driver_pointer_phi_high_second_lane_table adds the lane-mask low word to the second-buffer arm: it plans, each group samples the T# its record selects, and that arm faults outside the snapshot.
…nter The two words of the pointer are now walked edge by edge, and an edge whose low word is a constant or a lane mask is skipped with its high word. A high word read from a second buffer stays refused on an edge whose low word is the record word.
Brings boykopovar#8991 (match image-table T# halves offset before and after readfirstlane). ResourceTracker.cpp: main's FirstLaneOffset, ParseFirstLaneOffset and SameFirstLaneOffset go before TryMakeTableColumn, which keeps this branch's signature with the buffer parameter. Main's first-lane base check in the column loop merged on its own.
…ane-masks Brings boykopovar#7395's merge of main bd4d801, with boykopovar#8991. It merged without conflicts.
TryMakeIndirectColumn refused every pointer hop once the record's pointer words were merged, so a T# read through a pointer loaded from a pointer-keyed record pointer stayed unplanned (Gran Turismo 7's World Map compute shaders, issue boykopovar#6191). Only buffer handles still refuse hops: the materializer keys the records by the innermost pointer before it follows the hops, and the plan's parent pointer is that innermost one.
…one-key child From hugodearaujo's change on boykopovar#8939 (Marvel's Spider-Man 2, PPSA03016).
Spider-Man 2's shape: a record keyed by a first-lane read, its dwords 2-3 a pointer, a V# loaded at that pointer plus 0xa0, and the T# read at offset 0 or 0x40 of that V#'s buffer with no second key. Two records point at V#s of different T#s; a moved record pointer and a moved V# must change the result without recompiling.
… pointer-keyed record pointer A T# read at a pointer that a shader chases conditionally from a pointer-keyed record pointer, loading a V# at a fixed offset of the pointer and reading the next pointer from that buffer, stayed unplanned: the merged pointer's chase arms read another buffer at another offset than the record word, so PointerRecordRead refused them (Ratchet & Clank: Rift Apart stops at compute LoadAddressU32 with this shape). The walk now takes such an arm when its V# is read from a pointer that is the record pointer or another chase arm, records each distinct chase as a hop of the column, and refuses a chase that reaches itself or a depth over 4. The snapshot then also captures every record pointer's chase closure up to that depth as pointer-keyed entries, each with the T# at its own offset, so the GPU's lookup finds the pointer the shader reached.
|
@Tamalero @oneandonlydean this draft follows a conditional pointer chase through a V# behind a pointer-keyed record pointer, the shape we read behind the 0x10b84 refusal on #8939. Its branch, |
|
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.
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.
|
|
@EagleBearerYS thanks, I'll build your branch at 6a72841 as is and run PPSA01474 on it. If it stops at or near 0x10b84 again, I'll post the stop and the stderr lines around the refusal here. It's queued behind today's PR runs, so it'll be a few hours. |
jotitadev
left a comment
jotitadev
left a comment
There was a problem hiding this comment.
Run on NVIDIA, since the tests so far are llvmpipe only.
Machine: Ubuntu 26.04, i7-8700, GCC 15.2, Release, full build with BUILD_TESTING=ON. GPUs: NVIDIA GeForce RTX 3070 (driver 595.99.02, Linux) and lavapipe (Mesa 26.0.3, LLVM 21.1.8). Head 6a72841 (with its stack) merged into main da54c65.
agc_driver_pointer_chase_table(agc_driver_image_table_tests --pointer-chase) passes 3 of 3 on both devices, including the moved link, the link outside the snapshot, and the refused loop and five-step chase.- Control: the same tree with this PR's own
ResourceTracker.cppchange reverse-applied (the diff from #9457's head to this head) fails on both devices the way the title does:GetImageResource dword 0 is not a valid runtime value; chain: LoadAddressU32@pc=0x000000c0(GetAddressResource(Phi(Phi(ReadConstBuffer@pc=0x00000074(.... ^agcsuite, excluding the flip, display DCC andlds_atomics64tests: 875/878 on the 3070 and 877/878 on lavapipe. The failures are the same as onmainon this machine: the threesample_ordertests on the 3070 (no 2x2 sample-location grid at 2 samples) andmultisample_retirementon lavapipe.
So the chase also works on a real GPU with subgroup 32, not only on llvmpipe's subgroup 8.
GPUs tested: NVIDIA GeForce RTX 3070 (595.99.02, Linux), lavapipe
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
@jotitadev thanks for the RTX 3070 run with the reverse-applied control. A real GPU at subgroup 32 was the gap in this draft's testing. It's in Tested and on the GPUs tested line now. |
|
The current evidence is strong for the modeled pointer-chase path: the focused test has a reverse-applied negative control, Linux validation-layer A/B shows no new VUIDs, and an independent RTX 3070 run confirms subgroup-32 behavior. The remaining title-specific gate is the promised PPSA01474 run on this exact stack; the body correctly says |
|
@EagleBearerYS ran it: PPSA01474 (content 01.005.003) gets past the 0x10b84 stop with this PR, and stops on the next T# load, whose pointer goes through a third chased read. Setup: Linux (CachyOS, kernel 7.2.9), GCC 16.2.1, Release, RTX 4090 (NVIDIA 615.78.08). The same relinked executable for both builds, a cold shader cache, no input (the stop is in the black load after the logos).
Same shader on both builds: a second control run with
So the chase is followed on the title's own shader, not only on the test one. The next stop: in both refusals (0x13bc0 in game, 0x128d4 in the replay), the pointer Phi has the two chase reads you modelled plus a third read:
In the replayed shader that third read is an Full refusal, in game (compute 0x116bb67100)Tested and written by Claude (AI assistant) on a contributor's machine. |
|
@Tamalero thanks for running it on Ratchet & Clank: Rift Apart (PPSA01474), with the parent as a control and the offline replay of the sibling shader. Getting past 0x10b84 on the title's own shader is the run this draft was waiting for, and it's in Tested and on the Unblocks line now. The third chased read at 0x13bc0 is the next step. The IR blocks around it ( |
github-actions
Bot
added
the
unblocks title
|
@EagleBearerYS here is the third read. It's none of the three you listed: the pointer's two words are not adjacent. The V# offset differs too, but the per-site Same shader, both builds: a dump run of this PR stopped in compute 0x116bb67b00 at
The sibling has the same shape: H1 0xea9c/0xeaa8, H2 0xf04c/0xf064, H3 0x11afc/0x11bb0 (V# at P+0x110, words 0x54/0x6c).
So the step is structurally the one you modelled, with two different parameters: a V# immediate of 0x110, which per-site chases already cover, and a high word at its own immediate. A possible change: keep the high read's own immediate in After that: in the relaxed scratch build, both shaders get past the T# and stop on The IR stays local (it's the title's code), but I can trace any value you need. Tested and written by Claude (AI assistant) on a contributor's machine. |
PointerRecordPairs found an existing chase site by its V# alone. A second chase arm that reads its next pointer through the same V# at other words therefore kept the first arm's PointerChase, and the snapshot never followed the second arm: a T# behind a node whose V# holds two links (words 0 and 1, words 2 and 3), each taken in its own lanes, faulted as outside the snapshot when the second link was taken. Sites are now keyed by the V# and the whole PointerChase, so each word pair of one V# is its own chase in the column and in the snapshot's closure. This follows the chase work for Ratchet & Clank: Rift Apart (PPSA01474); its trace has not shown this shape yet.
|
@Tamalero thanks for tracing the third read in both shaders, down to the |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The latest report is unusually thorough: it includes the exact nested-pointer regression, positive and negative controls, cache and test coverage, validation-layer comparison, and a real NVIDIA run. I verified that the linked Game bring-up discussion #2172 is the Ratchet & Clank thread and that it now records this PR's later 0x13bc0 stop. I found no additional code issue in this review pass.
Before merge, this stack needs a current base. The PR targets da54c65, while main is 95 commits ahead at 8b3a6ba and those commits touch shader recompiler files changed here. Since this PR depends on open #9457, coordinate landing order, then rebase onto current main with that prerequisite and refresh overlap/dependency metadata. After rebasing, rerun the focused chase-word/table cases and the CONTRIBUTING.md validation-layer comparison on the same GPU for the current base and the updated head; the detailed existing comparison predates the latest head. Product jobs are skipped by path filters.
I independently ran exact-head git diff --check, tools/check_conventions.py against current main, and tools/check_test_registration.py --scripts; all passed (171 scripts registered). I could not configure the full project locally because the host lacks SDL2 development files and the SDL2 submodule is uninitialized, so I did not run guest_ajc/AGC tests here. The author and reviewers report the focused Linux and NVIDIA/lavapipe runs described in the PR.
|
Correction to my review text: “guest_ajc” was a typo; I meant the AGC test suite. |
|
@MarcZX10 thanks. The stack is refreshed: this PR has #9457's head 2a5db7a merged in (#9457 itself carries
All of it is in the body. |
|
@EagleBearerYS we ran PPSA01474 on your branch at 6a72841 (cold shader cache, The 0x10b84 refusal is gone. The title now stops 195.9 s in, in a compute shader with the same pointer Phis as before (IR ids %82054/%82055 match the 0x10b84 dump): The pointer, in the Walking the two words edge by edge, the leaves are the record read R (%8580/%8581, pc 0x1a88), the waterfall lane-mask edges, the two chase arms from the 0x10b84 case (V# at pc 0xfd84 and pc 0x1032c, which your change now follows), and one more chase site: That site is the join you said the PR still refuses: its pointer's two words come from Phis in different blocks. The low word goes through two extra Phis, at pc 0x5b7c. Their only new arm is What we think would cover it: before pairing P's words, resolve a word through Phis whose other arms are lane-mask values (the masks #8939's edge pairing already treats as dead) until it reaches the block of the other word's Phi. Then P is We can run the title again on a head that takes this case. The 1.4 M-line IR dump of this shader is available if you want any other block. |
|
@oneandonlydean thanks for the run and the trace. #9749, stacked on this PR and ready, is the follow-up for exactly this #9749 doesn't add the Phi resolution you propose. Its only tracker change is in If you're willing, a run of #9749's head b2d4afc with your setup would settle it. If it still stops at 0x13bc0, we'll add your proposal as a commit on #9749, with a test built from your IR, and then the blocks around 0x12de8 and 0x5b7c from the dump would be what we'd ask for. If it passes, |
|
@EagleBearerYS thanks, that settles it: the single-commit control is the evidence that counts, and our Phi-resolution reading was not what stopped the walk. In our dump the two words of that third chase ( In our next test window (within the next day) we'll run the title on |
|
@oneandonlydean one change before you spend that test window. @Tamalero ran |
|
@EagleBearerYS a correction to my last comment: we won't run |
|
@oneandonlydean here's the branch I mentioned: |
This branch has not been deployed
7 participants
What
On Ratchet & Clank: Rift Apart (PPSA01474), with #8939, a compute shader stops with
GetImageResource dword 0 is not a valid runtime valueatLoadAddressU32@pc=0x10b84. #8939 itself got it past its earlier stop at 0x9a30. The sources are a retail run on #8939 (comment), and two IR readings of the shader (1, 2).The descriptor pointer is a Phi of the record's pointer R and up to two chased pointers. The shader conditionally replaces P by the 64-bit pointer read at offset 0 of a buffer whose V# sits at P+0x60, up to twice. It then loads the T# at the final P+0. So the pointer is R, H(R) or H(H(R)).
PointerRecordReadthen refuses the chased arms as reads of another buffer at offset 0.The change:
GetAddressResource(P)at an immediate offset, where P resolves to R or to another chase arm.PointerChase {offset, immediate, readOffset, readImmediate}). Since 2c1a650, two links read through one V# at different words each get their own site, and so their own snapshot entry. No serialized field changes there.Overlap: checked with
git merge-treeagainst every open PR touching these files, onmainda54c65, and rechecked at 7928543 (withmain3ca07d0) against the PRs below: the conflicts are the same. #9508's other conflict there, inGuestMemory.cpp, is withmainitself.main'sTryMakeIndirectColumn. The stack's walk takes Phis whose arms differ, so the two compose. Whichever lands second carries the other's case through the same loop.ShaderDiskCache.hpp. This PR moves it 46 → 47, on top of feat(shader): resolve nested image tables through runtime pointers #7223's 45 and feat(shader): read samplers through nested and pointer-keyed descriptor tables #7395's 46. Whichever lands later takes the next number, and the format this PR ends at is settled when its base merges.Tested
agc_driver_pointer_chase_tablefails ×2 with fix(shader): follow pointer hops and fixed-offset V# reads behind a record pointer #9457's code, and passes ×5 with this change.agc_driver_pointer_hop_tableandagc_shader_disk_cachepass. GitHub's Windows runner builds it, and its Vulkan tests skip there.agc_driver_pointer_chase_words_tablefails 3 of 3 without the change ("pointer table with two chase links in one V#: the T# behind one of the two links faulted") and passes 10 of 10 with it. Withagc_driver_pointer_chase_table,_hop_table,_phi_high_table,_vsharp_fixed_tableandagc_driver_nested_image_table, it passes 10 of 10 on Linux; on Windows they build and pass, with the Vulkan tests skipping. libc'sprecise_waitfailed on both arms of that run.VUID-VkDeviceCreateInfo-pNext-pNext, and the commit adds noVUID-message.main3ca07d0 merged in (both clean; this PR's own diff is unchanged), on GitHub's runners (run): the 18nested_/pointer_table tests,agc_driver_image_table_vouched_lookups,agc_driver_image_table_vouch_queued_drawsand the disk-cache tests pass 10 of 10 on Linux, none skipped. On Windows they build and pass, with the Vulkan tests skipping.precise_wait, failed withmain's implementation too. On Windows libkernel's one failure isguest_raise_in_system_call, a 30 s timeout in a libkernel test this PR doesn't touch.mainagainst this head, in the same run: one GitHub Linux runner, Ubuntu 24.04.5, llvmpipe (Mesa 25.2.8, LLVM 20.1.2), withANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1,VK_LAYER_KHRONOS_validationandctest -V.main's arm runs the 2 image-table tests and the disk-cache tests it has, and this head's arm runs all of them. The layer loads in both runs, with no "was not found" line. Both log onlyVUID-VkDeviceCreateInfo-pNext-pNext, and this head adds noVUID-message.llvm-mc -arch=amdgcn -mcpu=gfx1030. A record pointer is keyed by a first-lane read, then two conditional chase steps each load a V# at P+0x60 and a pointer at offset 0, then a T# at the final P+0.GetImageResource dword 0 is not a valid runtime value; chain: LoadAddressU32@pc=0x000000c0(GetAddressResource(Phi(Phi(ReadConstBuffer….^agcand^shadershow the same failure set on fix(shader): follow pointer hops and fixed-offset V# reads behind a record pointer #9457's head and on this change.VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation,ANYPS5_NO_SHADER_CACHE=1,ANYPS5_REQUIRE_VULKAN=1andctest -V, withVK_LOADER_DEBUG=layerto show the layer loading. The device was llvmpipe (LLVM 20.1.2) on Mesa 25.2.8, Ubuntu 24.04.5, Khronos validation layers 1.3.275.agc_driver_pointer_chase_table, and fix(shader): follow pointer hops and fixed-offset V# reads behind a record pointer #9457'sagc_driver_pointer_hop_tableon the same pointer-keyed path. fix(shader): follow pointer hops and fixed-offset V# reads behind a record pointer #9457's tree has only the second.was not found.VUID-VkDeviceCreateInfo-pNext-pNext. This PR adds noVUID-message.mainda54c65.agc_driver_pointer_chase_tablepasses 3 of 3 on both devices, including the moved link, the link outside the snapshot, and the refused loop and five-step chase.ResourceTracker.cppchange reverse-applied, it fails on both the way the title does:GetImageResource dword 0 is not a valid runtime value; chain: LoadAddressU32@pc=0x000000c0(GetAddressResource(Phi(Phi(ReadConstBuffer….^agc, without the flip, display DCC andlds_atomics64tests: 875 of 878 on the 3070 and 877 of 878 on lavapipe. The failures are the same as onmainon that machine: the threesample_ordertests on the 3070, andmultisample_retirementon lavapipe.check_conventions.pypasses, with no comment lines added.LoadAddressU32@pc=0x10b84, the stop from fix(shader): take lane-mask arms and a merged high word in a pointer-keyed record pointer #8939.LoadAddressU32@pc=0x13bc0: a T# load whose pointer goes through a third chased read.agc_shader_replay, the parent refuses at the same site (pc=0xf898). This PR passes it and refuses at a later T# load (pc=0x128d4), so the chase is followed on the title's own shader.s_buffer_load_dwordx16.ChaseHighReadtakes a high word only atreadImmediate + 4, so the walk refuses there. That step needs a follow-up. In Tamalero's scratch build with only that check relaxed, both shaders then stop onsampler resource limit exceeded(MaxSamplers = 32).Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)