Repository navigation
feat(shader): read samplers through nested and pointer-keyed descriptor tables - #7395
EagleBearerYS wants to merge 9 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.
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 38 and the image-table ABI to 4.
|
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.
|
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 38.
|
Rebase handoff — rebased onto current
One combination to review when you pick it up: when a Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release, To pick it up: Head is |
The conflicts in the image table, disk cache and resource tracking files are resolved as in blawesom's rebase of this branch onto main (fdcf0d5). Co-authored-by: blawesom <9946064+blawesom@users.noreply.github.com>
|
@blawesom thank you for the rebase and for reconciling all eight files, including the auto-merge's non-compiling |
|
@blawesom a follow-up on the case you flagged, where a
A Linux run of the |
|
@EagleBearerYS thanks for b91f050. Ratchet & Clank: Rift Apart (PPSA01474) data with it:
I can capture the shader ( AI assistance: the run and this summary were made with Claude Code. |
|
@ps54all thanks for running it. On your branch, b91f050 clears the stop on compute shader 0x116bb5f500 ( No need for the |
main's boykopovar#7296 took disk-cache format 38, so this branch's format change moves to 41.
|
Tested on real hardware, with no title run. Head
The Intel failures, run as The two values are swapped between the two tests. The tests choose between a wrap and a clamp point sampler, so ANV may return the result of the other sampler. I have not found the cause. The existing Tested and written by Claude (AI assistant) on a contributor's machine. |
|
@Capmare thank you for running all four devices, and for repeating the ANV failures three times. The tests can't confuse the two samplers. Every lane samples past the right edge, so a wrap sampler reads texel x (0x3e800000, 0.25, at lane 0) and a clamp sampler reads texel 63 (0x427d0000, 63.25). Both failures come from the first check of the first dispatch. That check walks the workgroups in order and stops at the first one that differs, so your two lines say this:
So on ANV the second workgroup gets the first workgroup's sampler. Workgroups 2 and 3 never ran through the check. The If you have time for one more run: the same two tests with the Khronos validation layer ( |
|
@EagleBearerYS I ran the two tests with the Khronos validation layer, and I found a trigger on ANV that does not involve our SPIR-V. Details below, including what I could not prove. Summary
Setup
Results of the two tests
The two Intel failures are the lines from your comment:
I added the group number to the failure message locally. It confirms your reading: the first mismatch is workgroup 1, and workgroup 1 gets the sampler of workgroup 0. The failure does not depend on the validation layer, on Validation layer
The SPIR-V of the failing dispatchI captured the pipeline with
What I tried on the real test, with the real buffersI added a local diagnostic hook that replaces the compute module with an edited file, so the driver, the buffers and the descriptors stay real. Results on the Intel GPU:
The row with the workgroup id is the important one. It means the key, the record and the pointer chain are not the cause. Standalone reproduction (no AnyPS5 code)A C program (about 300 lines) and one GLSL compute shader. It makes two samplers (REPEAT and CLAMP_TO_EDGE) and a 64x1 R32_SFLOAT texture with texel On the Intel GPU:
NVIDIA, RADV and llvmpipe pass every variant. The validation layer reports nothing for the program. The trigger is therefore: required subgroup size 32, a workgroup of exactly 32 threads (one SIMD32 thread), and a sampler array with more than 2 elements indexed per workgroup. The driver creates our compute stages with The compute shader of the reproduction#version 450
#extension GL_EXT_nonuniform_qualifier : require
#extension GL_KHR_shader_subgroup_ballot : require
layout(local_size_x = 32) in;
layout(set = 0, binding = 0) uniform texture2D tex;
layout(set = 0, binding = 1) uniform sampler samplers[]; // compiled with -DRUNTIME_ARRAY
layout(set = 0, binding = 2) readonly buffer Indices { uint indices[]; };
layout(set = 0, binding = 3) writeonly buffer Results { float results[]; };
void main() {
uint index = indices[gl_WorkGroupID.x];
float value = textureLod(sampler2D(tex, samplers[index]), vec2(1.2578125, 0.5), 0.0).x;
results[2u * gl_GlobalInvocationID.x] = value;
}Workarounds I tested on the Intel GPU
Effect of
|
| Run | pass | skip | fail |
|---|---|---|---|
| Intel, default | 217 | 3 | 15 |
Intel, APS5_SPANNED_WAVE=1 |
219 | 3 | 13 |
| RTX 4070 SUPER, default | 220 | 2 | 13 |
| RX 7600, default | 218 | 3 | 14 |
Between the two Intel runs only agc_driver_nested_sampler_table and agc_driver_pointer_phi_sampler_table change, from fail to pass. No test goes from pass to fail.
Tests that fail on every device
These 13 tests fail on Intel, NVIDIA and RADV alike, with and without the spanned-wave switch: agc_buffer_pool, agc_driver_stencil_replay_no_gpl, agc_host_import_second_mapping, agc_import_settle_deferred, agc_import_settle_deferred_other_family, agc_import_settle_earlier_import, agc_import_settle_first_use, agc_import_settle_retire, agc_import_settle_storage_image, agc_shader_bindless, agc_shader_frontend_pair, agc_storage_cpu_snapshot, agc_vertex_barrier. RADV also fails agc_pipeline_cache_file. I did not investigate them. They may come from my merge of origin/main, from my local patches, or they may exist upstream. I did not run them on a clean checkout of the PR head.
Caveats
- This is not a clean PR head. My tree has local patches for a different goal (running a game). They do not touch the sampler-table code, but the local build has a diagnostic fallback in the resource tracker that replaces a sampler reached through a runtime pointer chain. With it,
--nested-samplerfails on every device witha V# with another buffer consumer was not refused by resource planning. I turned it off withAPS5_NO_DESCRIPTOR_SUBSTITUTE=1for all runs above. A clean PR head does not have this fallback. - I tested only the Intel iGPU of this PC. I did not test other Intel GPUs or other Mesa versions.
- I did not read the Mesa source. I cannot say which ANV code path causes the behaviour.
- The test comparison covers the registered unit tests. I did not test game workloads with the spanned-wave mode.
Suggestion
I did not change the tests or the driver. If you want a driver change, enabling the spanned-wave mode on ANV is the smallest one I found. It changes how every single-wave wave64 compute shader runs on that driver, so I leave the decision to you. Separately, the standalone program is a clean bug report for Mesa. I can attach the source and a short write-up, or file the Mesa report myself if you prefer.
|
@Capmare thank you, this settles it, and the standalone program is the strongest part. With no AnyPS5 code, ANV gives workgroup 1 workgroup 0's sampler whenever the stage has A correction to my last comment: the module carries no The two tests stay as they are, since they're what caught it. If you want to report it, your program looks ready to post as a Mesa issue as it stands (https://gitlab.freedesktop.org/mesa/mesa/-/issues). That's entirely up to you. |
|
@EagleBearerYS thank you for the careful review and for the clear questions. They pointed me to the validation layer and to the exact failing dispatch, which is how I found the cause. I posted the standalone program as a Mesa issue: https://gitlab.freedesktop.org/mesa/mesa/-/work_items/16622 I agree with leaving the two tests as they are and with not changing the driver yet. If Mesa answers, I will post the result here. |
Resolves the image-table record columns against boykopovar#6232's pointer chains: TableColumn keeps buffer, pointerKeyed and pointerHops (20 fields, all compared and encoded), the materializer records a buffer or pointer-keyed column's record pointer before following hops, and a chain whose record read is merged across lanes or feeds a buffer column is refused, as main refuses it, since neither side plans that combination. The image-table tests keep this branch's nested and pointer-keyed cases and boykopovar#6232's pointer-chain case.
|
#7684 adds conservative table-key ranges and sentinel handling, with 3 CPU suites and a 7-case regression batch (5 real Vulkan, 2 CPU/mock) passing on its exact head. The overlap check found a textual conflict with this PR's capture refactor. I propose #7684 first, then this PR can retain I left #7684 as a draft pending agreement on ordering. If the #7223 → #7395 sequence should go first, I can rebase the bounded change afterward and rerun the combined fixtures. AI-assisted: yes |
|
@fdfawef324 thank you for checking the overlap and listing what to keep. #7684 first works for us. On a trial merge of this PR's head (59b319a) over #7684's df6aa65, the conflicts are small: six hunks in
If #7684 lands first, we'll merge |
|
Tested #7395 head 59b319a (including #7223) offline against a captured Wolverine compute request, with SPIRV-Tools enabled. Needed a local iterative Tarjan walk to avoid the previously observed stack overflow; used APS5_FORCE_STRUCTURIZER_FALLBACK=1 and disabled shader caching. This is a combined experiment, not a stock-PR title test. The request loads, but still stops in resource analysis at A diagnostic traversal of the failing word's Phi edges finds 129 Phi nodes and 43 ReadConst nodes referring to 30 distinct runtime-read slots, plus arithmetic and lane-operation leaves. This is only the Phi surface, not a full dependency graph; it does not prove that the live sampler path combines different values. Attached synthetic DescriptorMerge.cpp isolates the validator boundary: equivalent-slot Phi inputs are accepted, while distinct-slot Phi inputs are individually valid and rejected after merging, at depths 1/2/9. All six controls pass. This is a unit-level capability reproducer, not a minimized full resource-tracker/title reproduction and not evidence that the conservative rejection should be removed. No title code, capture or dump is included. Is a synthetic full-tracker fixture retaining the incoming control-flow correlation the most useful next step for this shape? I have kept the title capture and memory dump local. AI-assisted: yes. No compatibility or performance claim. |
|
Little Nightmares Enhanced Edition (PPSA10737, version 01.004.000, on Windows 11 / RTX 5090 with #7413 In With a local change in AI-assisted: yes. Written with AI assistance and reviewed by me. |
|
Following the agreed #7684-first order, #6488 is now rebased and verified at At your reviewed I propose #7684, then #6488, then your #7395 rebase with both conventions and the masked-invalid-table controls retained. Only the #7684-first part is already agreed; this extension is a proposal. I am keeping #6488 in draft while that order is coordinated. Current #6488 CPU3 and Vulkan3 pass, including the new controls, but no combined #7395 runtime result is claimed. Let me know if the opposite order for these two is preferable. AI-assisted: yes (Codex) |
|
Marvel's Wolverine (PPSA03671) on this PR's head 59b319a, as asked on #6191. Rocky Linux 10.2, Release build of the head as it is (no local patches), NVIDIA GeForce RTX 4070 SUPER (driver 615.71.09), cold shader cache, one run of about 9 minutes, then I stopped it.
So the next stop of the title on this head is not a refusal but this build storm. I think the cause is the one-image-per-table-element build of surfaces that appear at several extents (dynamic-resolution variants of one surface). On my local tree, substituting one representative per family of variants and building table entries lazily avoids it, and the same title then reaches the 3D scene and the Initial Setup menu. That is a separate topic from this PR. I can open it as its own PR if you want it. |
|
Update on the ANV failure of the two sampler tests: the Mesa developers answered on Mesa issue 16622. It is fixed by Mesa merge request !37394 ("nir/brw: Fix texture/sampler accesses with EU fusion on Gfx12.X HW"). On Gfx12 two fused threads run in lock step and cannot have different sampler handles, which matches workgroup 1 getting workgroup 0's sampler. The fix is in Mesa 25.3.0 and later, and not in the 25.2 branch. I checked it on the same Intel RPL-S with the freedesktop Flatpak runtimes' Mesa, using the standalone program from the issue. Mesa 25.2.7 (the distribution package) gives 18 failing runs. Mesa 26.2.2 and 26.2.4 give 0. I did not run |
What
Depends on #7223. With #7223, Marvel's Wolverine (PPSA03671) and Ratchet & Clank: Rift Apart (PPSA01474) stop on the sampler half of the descriptor chain that #7223 resolves for images (#6191):
#7223 resolves record → pointer → V# → T# and leaves samplers refused. Its nested table representation, pointer dictionary and limits are what this PR builds on. The PR extends them to samplers and to a one-hop pointer whose low word is merged through a Phi. Three commits:
fix(shader): refuse nested table entries whose words have other consumers. On feat(shader): resolve nested image tables through runtime pointers #7223's head, a nested child whose descriptor words another instruction also reads crashes lavapipe inside the shader. The child's V# words are planning-only loads and are never emitted, while the other consumer keeps its runtime read through a V# that exists only in the snapshot. The nested branch now refuses such a child, as feat(shader): resolve nested image tables through runtime pointers #7223 already refuses a V# with another consumer.feat(shader): resolve S# reads through nested descriptor tables. This is Wolverine's shape: record → pointer → V# → S#.feat(shader): look up a Phi-merged record pointer to an S# or T#. This is Ratchet & Clank's shape: record → pointer → S# through an address, with the pointer's low word merged through a Phi.mained4f148, after fix(shader): bind an unnormalized S# normalized when it samples only null images #7296 took 38 onmain), and the image-table ABI from 3 to 4.fix(shader): accept a lane-mask word in a pointer-keyed record pointer. Ratchet & Clank: Rift Apart's next stop (ps54all, #6191):GetImageResource dword 0 is not a valid runtime value. A T# is read through a record pointer whose low word passes six levels of Phi, and one arm is a word of the waterfall loop'sBallotlane mask.Ordering. This PR is stacked on #7223, so until that merges its diff shows #7223's two commits too.
main: this branch mergesmain9caecfd (aed137c),main61d3d30 (ed4f148) andmainff2fc57 (59b319a). The conflicts in the image table, disk cache and resource tracking files are resolved as in blawesom's rebase of this branch ontomain(comment), and the merge credits them as co-author. The second merge resolves the shader debt file, keeping this PR's new image-table bullet and fix(shader): keep a shader's selected image-table records within the image and pair limits #7527's sentence in the next one, and moves the disk-cache format to 41. The third merge combines this PR's V# and Phi-merged record pointers with feat(shader): follow pointer chains from image-table records to the T# #6232's pointer chains from a record to its T#. Both stay, and a chain whose record pointer is merged through a Phi or leads to a V# is refused, asmainrefuses it, since neither PR plans that combination. feat(shader): resolve nested image tables through runtime pointers #7223 on its own still conflicts withmain.mainis at 38 since fix(shader): bind an unnormalized S# normalized when it samples only null images #7296, so this branch is at 41.main). feat(recompiler): run waterfall loops over per-lane T#s through image tables #6488 also moves the disk-cache format, to 37, belowmain's 38, so whichever of the two lands second takes the next free numbers.Tested
main9caecfd (aed137c, the head), on GitHub's runners, at 3730bb8, the same merge with the same tree before the co-author line was added (run): the^agctests pass 3 runs in a row on Linux (715, 0 failed), and the libc and libkernel suites pass. On Windows the only failures areagc_driver_pm4_replay_toolandagc_shader_replay_tool, which fail the same way on that runner for an unrelated branch (run).agc_driver_nested_image_table,agc_driver_nested_sampler_table,agc_driver_pointer_phi_sampler_table,agc_shader_disk_cacheandagc_shader_disk_cache_large_filepass 10 of 10 runs on both runners. Three of them skip on the Windows runner.agc_driver_nested_sampler_table: S# through record → pointer → V#. It covers independent keys, wave32 against wave64, an edited S#, unmapped and non-buffer parents, shared children, the consumer refusals, captured and uncaptured Phi pointers, the S# budget and the nested key budget. Red:GetSamplerResource dword 0 is not a valid runtime value.agc_driver_pointer_phi_sampler_table: S# through a Phi-merged record pointer. It covers the direct control, captured and uncaptured Phi constants, a constant equal to another record's pointer, a null pointer, a non-buffer record table, an S# with another consumer, and a T# through the same path.agc_driver_nested_image_tablegains "a T# with another consumer" (refused). Red is a SIGSEGV on feat(shader): resolve nested image tables through runtime pointers #7223's head.agc_shader_disk_cacheround-trips both new plan shapes and rejects a malformed pointer-keyed column.agc_driver_pointer_phi_ballot_tableandagc_driver_pointer_phi_copies_table: synthetic wave64 waterfall loops shaped like Ratchet & Clank: Rift Apart's chain, where the lane-mask pointer faults as outside the snapshot. On GitHub's runners (run) both fail in all 3 runs on Linux with the third commit's code (4a4db4b) and pass 10 of 10 with the fourth, with the other nested and pointer tests andagc_shader_disk_cache; the libc and libkernel suites pass. On Windows they skip without a Vulkan device andagc_shader_disk_cachepasses. A run that mirrors CI's Linux job gives 1378 tests against 1376 on 4a4db4b, with only the two new tests changing.main82f0c9a with perf(agc): draw finisher pipeline (APS5_DRAW_PIPELINE) #6946, fix(agc): skip a periodic pipeline cache save while a pipeline creation holds the cache #6958, fix(recompiler): give a dynamic-mip storage image a slot for every level #7238, feat(shader): resolve nested image tables through runtime pointers #7223, fix(recompiler): read lane spills back through relative-addressing window registers #6277, fix(shader): plan image tables through invariant descriptor phis #6417, feat(agc): link calls through the AGC FS table #6160 and fix(ampr): return EBUSY when a command does not fit and EPERM before a buffer is bound #7300) plus this PR's three commits, cold shader cache, one run:GetSamplerResourcestop no longer occurs (0 hits);PhibeforeLoadAddressU32(GetImageResource dword 0 is not a valid runtime value). That is the title's next stop. The fourth commit (b91f050) addresses that chain; Tamalero has since run the title with it (below).main66702f1 (feat(shader): resolve nested image tables through runtime pointers #7223's base) with no local patches, fresh shader cache, one run each:GetSamplerResource dword 0(pc=0x6b74, 3 refusals);GetImageResource dword 0(pc=0x99ec, 3 refusals). Its record pointer goes through a deeperPhinest, with a word of theBallotlane mask ANDed into it (BitwiseAnd32), a shape the fourth commit doesn't cover. That is the title's next stop.main82f0c9a with open PRs, already carrying this PR's earlier commits) plus the fourth commit (b91f050), one run each before and after:LoadAddressU32@pc=0x8fd4, six levels ofPhi) no longer occurs;GetImageResource dword 0 is not a valid runtime value,LoadAddressU32@pc=0x9a30): a seven-levelPhinest with aBallotlane-mask word ANDed in andReadFirstLane(SelectU32 …)arms. That is Tamalero's next stop (above) in a sibling shader: both chains read the record table at pc=0x1aa8, and the stop and the second buffer's reads sit 0x44 later than in Tamalero's (0x9a30, 0x908c and 0x900c against 0x99ec, 0x9048 and 0x8fc8).main61d3d30: Rocky Linux 10.2, Release,BUILD_TESTING=ON, 735 graphics tests per GPU, no title (comment):agc_driver_nested_sampler_tableandagc_driver_pointer_phi_sampler_tableamong them;agc_pipeline_cache_filefails, which Capmare reports as unrelated;requiredSubgroupSize = 32,local_size_x = 32and a sampler array of more than 2 elements indexed per workgroup), so it's an ANV issue, filed as Mesa issue 16622 (comment, comment). The tests stay as they are. The existing nested image-table tests pass on all three GPUs.main61d3d30 (ed4f148), on GitHub's runners (run): the^agctests pass on Linux (734, 0 failed, 49 skipped), and the suites' only failure is libc'sprecise_waittiming test. On Windows, where 551 of the 734 skip without Vulkan, the only failures areagc_driver_pm4_replay_toolandagc_shader_replay_tool(exit code 0xc0000139), which fail the same way on that runner for an unrelated branch (run).mainff2fc57 (59b319a, the head), on GitHub's runners (run): the^agcand^shadertests pass on Linux (739, 0 failed, 49 skipped), and the suites' only failure is again libc'sprecise_waittiming test. On Windows, where 551 of the 738 skip without Vulkan, the only failures are againagc_driver_pm4_replay_toolandagc_shader_replay_tool(exit code 0xc0000139).Checklist
main66702f1); 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)