Repository navigation
fix(shader): take lane-mask arms and a merged high word in a pointer-keyed record pointer - #8939
EagleBearerYS wants to merge 18 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.
|
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 for #8939. We ran PPSA01474 on The IR shows which check refuses it, and why that arm isn't a pointer:
So on the one path where the high word comes from the second buffer, the "pointer" is (a lane mask, a T#-table dword). That's the SGPR pair being reused by the waterfall loop, not an address. Tamalero's dump on #6191 (issuecomment-6255548496) agrees: the record's high word is Could the walk pair the two words by incoming edge? Concretely:
This decides it at plan time from the IR alone, with no run-time fault path and no new plan field. If you'd rather keep the walks separate, a run-time fault for pointers built from that arm (as lane-mask pointers already do) would also work for us. We can share the IR excerpt around 0x9a30, and we're happy to run the title on the result. |
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.
|
@oneandonlydean, thank you for reading the IR this closely. The edge-by-edge pairing is the better fix, so I've written it as you describe, with both cases: |
…walk red/green and Linux CI mirror
|
@oneandonlydean, thank you for the IR dump and the steps. a7d1e62 on this branch pairs the two words as you describe:
There's no new plan field and no new run-time path. Tested on GitHub's runners, on
One shape that planned before is now refused. Both Phis are in one block, one edge carries the record's low word with an immediate high word, and the other carries a lane mask with the record's high word. No edge carries a whole record pointer, so the old plan could only fault at run time there. If you'd rather keep it planned, falling back to the old separate high-word walk when the paired walk finds no high read keeps it as before. Could you run Ratchet & Clank: Rift Apart (PPSA01474) on a7d1e62? If it stops again, the IR excerpt around the new stop would help. |
|
Retail check of a7d1e62 on Ratchet & Clank: Rift Apart (PPSA01474, 01.005.003), Linux, RTX 4090 (NVIDIA 615.78.08), cold shader cache, one unattended run per build:
The new chain has the same overall shape one level deeper. The low word is a Phi of the record read at So a7d1e62 fixes the 0x9a30 stop on the retail title. No regression up to the new stop. I can dump this shader's IR or the runtime values of the Tested and written by Claude (AI assistant) on a contributor's machine. |
|
@Tamalero, thank you for the retail run. It's now in the description: this PR gets Ratchet & Clank: Rift Apart (PPSA01474) past the 0x9a30 stop. We'd like to look at the 0x10b84 stop next. If you have time, the IR dump would help most:
If it's easy, the runtime values would help too: on the dispatch that stops, the dword the We'll read the dump before saying what the fix is. |
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.
|
Marvel's Spider-Man 2 (PPSA03016 01.004.003, the US release of spiff637's PPSA08338) still stops on the #6191 chain with this PR, so #7395 with #7223 doesn't take this title's shape yet.
Linux 7.2.11, RX 9070 XT, RADV Mesa 26.2.5, Release. One run on The shape.
A local change for that part. A fixed offset under a pointer-loaded V# becomes a one-key child: key 0, stride the descriptor size, addend the immediate. It applies only on the nested path; a fixed offset under a runtime V# still refuses as before.
diff on a7d1e62@@ -660,7 +660,13 @@
if (address && immediateU32(offset, descriptorOffset)) {
return TryMakeIndirectColumn(handle, *heapHandle, sampler, words, descriptorOffset, first, plan);
}
- if (!MatchAffineOffset(offset, key, stride, addend)) {
+ std::uint32_t fixedOffset = 0;
+ const bool fixed = !address && !buffer && immediateU32(offset, fixedOffset);
+ if (fixed) {
+ key = &m_builder.Constant(0u);
+ stride = width * static_cast<std::uint32_t>(sizeof(std::uint32_t));
+ addend = fixedOffset;
+ } else if (!MatchAffineOffset(offset, key, stride, addend)) {
if (!MatchMaskedOffset(offset, key, stride)) {
return false;
}
@@ -668,7 +674,9 @@
}
DescriptorSource heapSource;
std::uint32_t heapIndex = 0;
- if (!MakeRuntimeSource(*heapHandle, address ? IrOpcode::GetAddressResource : IrOpcode::GetBufferResource, address ? 2u : 4u, heapIndex, heapSource)) {
+ if (MakeRuntimeSource(*heapHandle, address ? IrOpcode::GetAddressResource : IrOpcode::GetBufferResource, address ? 2u : 4u, heapIndex, heapSource)) {
+ if (fixed) return false;
+ } else {The next refusal, in the same shader (pc 0x8f54 and 0x8fa0):
I haven't tried to take this second shape; how a linked pointer should be planned is your call. I can share the request file and the IR around 0x8f54. |
|
@hugodearaujo, thank you for the runs and for tracing both refusals. Your fixed-offset change is right: a T# at a fixed offset of a V# loaded through the record pointer has one record (key 0, the descriptor size as stride, the immediate as addend), which the nested path already serves, and a fixed offset under a runtime V# still refuses as before. We'd like to carry it as your own commit, with you as its author, in a new PR stacked on this one, together with tests of ours for that shape. If you'd rather open it yourself, say so and we'll leave it to you. For the linked-pointer shape at 0x8f54 and 0x8fa0, the IR text around those reads would help. We don't need the request file, since it holds the title's shader code. |
|
@EagleBearerYS here's the IR reading for the 0x10b84 stop. The address changes per run (0x116bb5ad00, 0x116bb61300 and 0x116bb5c500 in three runs on a7d1e62), so I captured the failing request with Short version: the edge pairing works here. Every (low, high) pair the walk pops is consistent, and the case your PR still refuses (a record low word under a second-buffer high word) doesn't occur. The stop has a different cause: the pointer has three sources, and only the first is the record column.
H1 and H2 both fail So the shader chases the pointer conditionally, up to twice (P → V# at P+0x60 → dwords 0/1 → next P), and loads the T# at the final P+0. The final pointer is one of R, H(R) or H(H(R)). Since the column is pointer-keyed and the runtime looks the T# up by pointer value (
The existing I haven't checked whether the H(R)/H(H(R)) targets fall inside the captured regions. I can run a values pass on this shader if that helps. Tested and written by Claude (AI assistant) on a contributor's machine. |
|
@Tamalero thanks, this is the reading we needed, and the replay through It's new work rather than something #9457 covers. #9457's hops follow unconditional chains of plain address loads, while here the descriptor pointer is a Phi of R, H(R) and H(H(R)), and each hop reads the next pointer through a V# at P+0x60. A chase-hop closure along the lines you describe is planned as a separate PR stacked on #9457, built and tested with a synthetic chase shader. The If you can run the values pass, three things would help:
No need to send the capture itself. Once there's a candidate, a replay on your side is what we'd ask for. |
|
@EagleBearerYS I ran 8704f73 (stacked on #7395 and #7223) on PPSA01474. The edge pairing works: the 0x9a30 stop is gone, and the lane-mask edges there are skipped as intended. The same program now stops later, at The pointer is
The other edges are lane masks paired with stale high words (including a As far as I can tell, I don't think this belongs in #8939's current scope, which is one record column per pointer. I'm posting it so you know where the program goes next. Would you plan a multi-source pointer like this (one column per source, selected at run time), or does it need a general runtime T# fetch? I can share the IR block for $2037 and the pairing output if useful. |
|
@oneandonlydean thanks for running it on Ratchet & Clank: Rift Apart (PPSA01474) and for the IR read. Good to see the 0x9a30 stop gone. 0x10b84 is the stop Tamalero traced (reply): the same three arms, the record column and two chased pointers, each read from offset 0 of a buffer whose V# is loaded from the pointer before it. To your question: neither, quite. The plan is one pointer-keyed column whose entries cover every pointer the shader can reach from each record: the record's pointer R and the chased H(R) and H(H(R)), read from the snapshot when the dispatch is captured. The shader then looks its run-time pointer up in that column as it does today. So no general run-time T# fetch is needed while the shader fixes the chase depth (two sites here) and the chased memory is in the snapshot. A chase that loops would need a run-time fetch, and it would be refused instead. It's planned as a separate PR stacked on #9457, built with a synthetic shader, and we'll ask you to run it when it's up. Yes, please post the |
github-actions
Bot
added
the
unblocks title
|
@EagleBearerYS thanks for carrying the fixed-offset change in #9457 as my commit. I'm running Spider-Man 2 on #9457 now and will post the result there. Here is the IR around the 0x8f54 and 0x8fa0 reads, after
|
This branch has not been deployed
5 participants
What
Depends on: #7395, which is stacked on #7223. Until both merge, this PR's diff shows their commits too.
#7395 lets a T# or S# be read through a pointer that a scalar-buffer record holds, with the pointer's low word merged by Phis with constants, lone
Ballotwords and copies of the record word. Two titles' chains still refuse on the pointer's words:Ballotmask ANDed with theNOTof the loop's lane bit, thes_andn2that clears the current lane.main82f0c9a plus open PRs and feat(shader): read samplers through nested and pointer-keyed descriptor tables #7395's earlier commits:Ballotword is ANDed into the low word, and the high word is merged through a Phi. One arm of that Phi reads a second buffer, and on that edge the low word is a lane mask: the waterfall loop reusing the SGPR pair, as oneandonlydean read it from the IR (comment).The change, in
ResourceTracker.cpp, for pointer-keyed columns only:fix(shader): take lane-mask ANDs and a high-word Phi in a pointer-keyed record pointer.Ballotword, or an AND or Phi made only of those and immediates. The other operand is walked on, so a record read in it must still match the column. An AND with no lane-mask operand still refuses.fix(shader): skip lane-mask arms built from a waterfall's lane bit in a pointer-keyed record pointer.ShiftLeftLogical32(1, index), and ORs and NOTs of lane-mask operands.FindILsb32,FindUMsb32orReadFirstLane, throughBitwiseAnd32,IAdd32,ISub32, immediates and the arms ofSelectU32.s_ff1_i32_b32,s_ff1_i32_b64,s_bitset0_b32and the shift's ownAND 31.docs(shader): the image-table debt entry lists the new arms and the merged high word, and says that a high word read from a second buffer stays refused.fix(shader): pair a pointer-keyed record pointer's two words by Phi edge, from oneandonlydean's reading of Ratchet & Clank: Rift Apart's IR (comment).docs(shader): the debt entry says the two words are walked edge by edge, and that a second-buffer high word stays refused on an edge whose low word is the record word.At run time, a pointer built from a lane mask faults as outside the snapshot, as #7395's lane-mask arms do.
Disk cache: no plan field changes, so the format stays at #7395's number. The disk-cache format ladder along this stack is
main44, #7223 45, #7395 46 (which this PR keeps) and #9633 47. Whichever lands next takes the next number, and the ones above it renumber in their rebase.Marvel's Spider-Man 2 (PPSA08338), run by spiff637 (comment), stops on a record → pointer → V# → T# chain keyed by
ReadFirstLane(SelectU32(LogicalAnd, IAdd32, Phi)). The second commit'sagc_driver_pointer_vsharp_select_tablebuilds a record → pointer → V# → T# chain with the T# read under a second key, and it passes without this PR's tracker changes. The title doesn't take it yet: hugodearaujo's run of the US release, Marvel's Spider-Man 2 (PPSA03016), still stops on the #6191 chain with this PR and #7679 (comment). There the T# is read at a fixed offset of the pointer-loaded V#, with no second key, and the same shader then reads its next block through a linked pointer. The test stays as coverage of the keyed form.Tested
tests/execution/ImageTable.cpp, on lavapipe. They are synthetic wave64 waterfall loops, assembled withllvm-mc -mcpu=gfx1030 -mattr=+wavefrontsize64. The fourth commit's is the second-buffer shader with one instruction added (s_mov_b32 s44, exec_lo,0xbeac037e) and its three crossing branch offsets adjusted, checked by disassembly.agc_driver_pointer_phi_lane_and_table(lane-mask ANDs),agc_driver_pointer_phi_high_table(a merged high word) andagc_driver_pointer_phi_high_second_table(the second-buffer arm, which must refuse).agc_driver_pointer_phi_lane_not_table(Little Nightmares'BitwiseNot32ands_andn2arms) andagc_driver_pointer_vsharp_select_table(Spider-Man 2's chain).agc_driver_pointer_phi_high_second_lane_table(Rift Apart's edge: the second-buffer high word with a lane-mask low word, which must plan).ResourceTracker.cppreverted and the tests kept, on Linux with CI's configuration:lane_andandhighfail with the dword-0 refusal, andhigh_secondpasses (refused). Each half of the commit fixes only its own test.lane_notfails:GetImageResource dword 0 is not a valid runtime value; chain: LoadAddressU32@pc=0x000000c0(GetAddressResource(Phi(Immediate, Phi(ReadConstBuffer@pc=0x00000074(…), Phi(BitwiseAnd32(CompositeExtractU32x4(Ballot(LogicalAnd), Immediate), BitwiseNot32(ShiftLeftLogical32(Immediate, BitwiseAnd32))), ….vsharp_selectpasses (coverage).high_second_lanefails:GetImageResource dword 0 is not a valid runtime value; chain: LoadAddressU32@pc=0x00000108(GetAddressResource(Phi(Phi(ReadConstBuffer@pc=0x0000007c(…. The other 10 image-table cases pass,high_secondamong them (refused).agc_shader_disk_cache.agc_shader_disk_cachepass 5 runs in a row on Linux. On Windows the GPU ones skip and the rest pass. The libc and libkernel suites pass on both.agc_shader_disk_cache_large_filepass 5 runs each. Windows: builds; the Vulkan cases skip on the runner.^agc(run): 873 of 874 pass, 60 skip. The one failure isagc_driver_sgpr_pattern_fill_keys, a timing test, which passed in the worker's full run of the same head.GetImageResource dword 0,LoadAddressU32@pc=0x9a30.GetImageResource dword 0,LoadAddressU32@pc=0x10b84.pc=0xfd94and aBallotlane-mask word. The high word's other arm isReadConstBuffer@pc=0x10350, through a V# whose four dwords are loaded atpc=0x10338from another pointer.Checklist
main0c6a5bd); 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)