Repository navigation
feat(recompiler): plan waterfall loops over per-lane T#s as non-uniform tables - #5089
adriadam10 merged 6 commits into
Conversation
4e5643e to
2e27fa0
Compare
|
Heads-up about a conflict that neither git nor the overlap check shows. My #4883 (open) stores the resource plan in the shader disk cache, and it writes The three bools this PR adds (ControlFlowInfo.hpp L36-L38) fit in the padding after #4883 uses a plan loaded from the disk cache in place of the front end (Recompiler.cpp L423-L425). If both PRs are merged, a warm-cache run gets the plan with
Example: a table V# with stride 48, the T# at record offset 16, and 3 records.
Whichever PR lands second has to add the three fields to EncodePlan/DecodePlan. If this one lands first, I will do that in #4883 and also make its guard catch fields added inside padding. AI-assisted: written with Claude Code; I read both diffs and compiled the size checks described above. I did not build or run either PR's tests. |
…ec writes The plan and ShaderInfo encoders write these structures field by field, and the sizeof asserts are their only reminder to update the encoders. A field that fits in existing padding keeps the size: main's layerExported landed in ShaderInfo with sizeof still 232, and the three bools boykopovar#5089 adds to DescriptorSource::IndirectImage keep it at 28. A warm cache would then hand back entries without the new field and nothing would fail. Count the fields of every aggregate the plan codec reaches (IrResourcePlan, GuestRegister, MemoryInfo, DescriptorSource and IndirectImage, ResourceBlock, SrtRead, UniformFill, UniformFillPlan, and ShaderInfo with its resource and stage I/O records) by brace-initializing each with more and more convertible-to-anything values until the initializer no longer compiles, and assert the counts. Unlike the size asserts this does not depend on the ABI, so it is checked on every platform. IrValue is a class with private state and keeps only its size assert. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Following up on the note above: #4883 now pins the field count of every structure its plan codec writes (664ad77). With this PR's three bools in |
|
Thanks for checking this and adding the field-count guard. I agree that Please keep #4883's codec guard. If #5089 merges first, your proposed codec update in #4883 covers the three fields. If #4883 merges first, I will rebase #5089, add the fields to AI-assisted: yes. |
c18d269 to
2adf364
Compare
2adf364 to
9007648
Compare
9007648 to
8416241
Compare
|
For reference, a title this does not cover, so nobody counts on it: Guardians of the Galaxy (PPSA01748). Tried on One more thing for sequencing: this PR conflicts with #757 + #758 in Tested and written by Claude (AI assistant) on a contributor's machine. |
… one table A table entry was a 32 byte descriptor heap slot. A game that keeps a record of its own layout -- an S# at +0 and an R128 T# at +16, 48 bytes apart -- indexes the same way, so the plan carries an `entryStride` next to its `entryOffset` and the descriptor's own width bounds the keys instead of the entry's. A heap whose descriptors start at an entry boundary still has to end on one; behind the last descriptor of a larger record lies the record's payload, which says nothing. The S# follows: when the shader loads its sampler out of the same entries, the sampler source carries the table too and materialization reads it at the keys the image bound. The slots are one image array against one sampler, so entries whose samplers disagree are rejected rather than silently sampled with the first. The slot count per table becomes a function of the shader's own image count, so a shader with several tables gets fewer slots each rather than no table at all. ShaderMemory covers the record array end to end: 0xfeedface around the descriptors, which no slot may pick up. The plan codec writes these structures field by field, so the new `entryStride` is written and read with them, the field-count and size asserts that guard the encoder move to 8 and 32, and the minimum bytes a stored source occupies to 66. A plan from a warm cache would otherwise come back with a zero stride, which the materializer rejects -- the failure boykopovar#5089's field-count assert was added for. The round-trip test's braced table gains the field too: without it the trailing `true` bound to `entryStride` and `hasMaterial` silently fell back to false.
|
Thanks for checking this. Guardians' access is outside this PR's per-lane descriptor waterfall fold, and I have not tested that title. The public branches still need a merge-order composition with #757/#758; a private integration does not establish Guardians coverage. The #4883 plan-codec overlap is now handled at 8416241: all three flags are encoded and decoded, the ten-field guard is retained, and format31 rejects older entries. The CPU round-trip and selective-red checks described in the PR do not claim a fresh combined native or title run. AI-assisted: yes. Co-Authored-By: Codex noreply@openai.com |
…rm tables A waterfall loop (s_ff1 or v_readfirstlane picks a lane, v_readlane copies its T# to SGPRs, v_cmpx_eq keeps the lanes whose T# VGPRs match, the image instruction runs, the matched lanes leave the loop) reads the T# from VGPRs, which no descriptor source evaluates, so planning failed with "GetImageResource dword 0 is not a valid runtime value". When every T# dword of an image read, sample, gather or query comes from v_readlane_b32 or v_readfirstlane_b32 and the EXEC its results are written under implies that each of them equals the lane's own VGPR, the access takes each lane's own T#. A T# that each lane loaded with buffer_load_dword* (IDXEN) from one evaluable V# becomes a bindless image table keyed by the lane's index, addressed by the V# stride, and its slot is decorated NonUniform. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…mixed modes A lane whose IDXEN load was skipped keeps the T# VGPRs it had before, and a waterfall that reaches it uses that value as its T#. The fold refused only a value that was itself a uniform descriptor or a table read, so a default T# copied into the VGPRs under EXEC, or merged by a phi, was accepted and those lanes read zeros. The skipped lanes' values are now followed through the masked selects and phis that all eight dwords share, and the fold is refused when an arm is a descriptor or a table read. Every lane of a per-lane table runs the access with its own record, and the runtime image mode picks the SPIR-V path per lane, so lanes of one quad whose records need different modes take different paths around an implicit-derivative sample or LOD query, where derivatives are undefined. Such a table now throws at dispatch; accesses of one table share the check, so they keep one table. The test also covers v_readfirstlane and v_cmpx_eq_u64 waterfalls and refuses loops that compare seven T# dwords, run the image instruction after EXEC is restored, or store. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…bound modes only A per-lane table V# with num_records 0, a null V# included, threw at dispatch on its stride (0 is less than the record offset plus 32) although no lane can hold a record, also when no lane enters the waterfall. With no records every OOB_SELECT puts each load out of range (the GFX10 table in Mesa's ac_descriptors.c), so such a table now binds no record and every lane that runs the access reads zeros; the swizzle, ADD_TID, OOB_SELECT and stride checks apply only to a V# with records. The implicit-derivative mode check compared every bound record with record 0, whose mode is 0 when it is null, so a table with a null record 0 and bound records of one other mode was refused. Only bound records are compared now. Lanes whose record is null or outside the table take the zero path under record 0's mode, so their quad neighbours' derivatives can still be undefined; the debt entry records this and the 16-record limit of the whole table. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The disk resource plan codec added on main must retain whether an indirect image uses a per-lane key, an indexed record stride, and implicit derivatives. Encode and decode those fields, update the aggregate field guard, and reject the older resource-plan format. Cover all eight flag combinations so a loaded plan cannot silently lose the stride or bound-mode checks. Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
|
Rebased onto
Checked: scoped build + ctest on RADV ( |
8416241 to
270eb06
Compare
|
Doing the same 2×N reshape (and the index-derived IDs) whenever a compute shader needs derivatives would also make each derivative quad four consecutive lanes, as on RDNA. Today a quad-compatible 8×8 group pairs lanes 0, 1, 8 and 9. I can send that as a PR if you'd like, or leave it to you since it's your test. AI-assisted: yes |
…rfall-descriptors" agc_driver_waterfall_images fails on main since fd438f6: its compute shader runs an LOD query with a local size of 8x1, and the module gets DerivativeGroupQuadsKHR, which SPIR-V validation rejects unless both dimensions are even (VUID-StandaloneSpirv-DerivativeGroupQuadsKHR-10151). This reverts commit fd438f6, reversing changes made to 5103174. Co-Authored-By: Claude <noreply@anthropic.com>
Revert #5089: agc_driver_waterfall_images fails SPIR-V validation on main
|
Reverted in #5824: after the merge, |
… tables A waterfall loop over a T# that each lane loaded with IDXEN buffer loads (s_ff1/v_readfirstlane, v_readlane of the eight dwords, v_cmpx_eq against the lane's VGPRs, the image instruction) stopped planning with "GetImageResource dword 0 is not a valid runtime value". Bring back the fold of boykopovar#5089 (reverted by boykopovar#5824 for a derivative-group validation error that boykopovar#5829 fixed) on the image table model of boykopovar#757: the T# dwords are the lane's own VGPRs under the loop's EXEC, and the access reads an image table column keyed by the lane's buffer index, with the V#'s stride and NUM_RECORDS addressing of the recompiler's buffer loads, a record count in the table header, NonUniform slots, and whole-table faults for V# layouts and for mixed image modes under implicit derivatives. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Codex <noreply@openai.com>
What
Image accesses inside a waterfall loop over a per-lane T# failed planning with
GetImageResource dword 0 is not a valid runtime value; chain: ReadLane(SelectU32(..., CompositeExtractU32x4(LoadBufferU32x4(...)), ...), ...). The loop is the one compilers emit for a divergent resource: pick a lane (s_ff1_i32_b32/b64of the remaining mask, orv_readfirstlane_b32), copy that lane's eight T# VGPRs to SGPRs withv_readlane_b32, narrow EXEC withv_cmpx_eq_u32to the lanes whose eight VGPRs equal the copy, run the image instruction with the SGPR T#, remove those lanes (s_andn2), repeat. The SGPR T# comes from VGPRs, so no descriptor source could evaluate it.This plans such accesses as one access per lane with the lane's own T#:
ResourceTracker, before descriptor-phi splitting). For an image read, sample, gather or query, every T# dword that is aReadLane/ReadFirstLaneof a valuexis replaced byxwhen the EXEC its results are written under (the condition of everySelectU32that takes its result) impliesReadLane(x) == x. The facts are collected fromLogicalAnd/LogicalNot/LogicalOrofIEqual32/INotEqual32, and ofIEqual64of twoCompositeConstructU64(thev_cmpx_eq_u64form). Lanes outside that EXEC still run the SPIR-V access (thes_cbranch_execzaround it is wave-uniform) but their results are discarded, so which T# they use does not matter; stores and atomics are not folded. The fold is kept only when it produces something the tracker can bind: dwords that are all valid runtime values, or the table below. Otherwise the handle is left as it was, so planning that worked before (ReadFirstLaneof a phi of uniform T#s through the descriptor web) is unchanged.buffer_load_dword*with IDXEN (immediate VOFFSET and SOFFSET) from one V# that the SRT walker evaluates, at consecutive offsets, the access becomes a bindless image table (DescriptorSource::IndirectImage) keyed by the lane's buffer index (laneKey,indexed). The loads stay; they also feed the loop's compares. A lane that the load skipped (the T# VGPRs are a maskedSelectU32) gets key0xffffffff, which is never mapped, and reads zeros. This is exact when the VGPRs it keeps are zero (a null T#). If they hold another descriptor or another table read, directly or through masked writes and phis that all eight dwords share (a default T# copied into the VGPRs under EXEC, or merged at a branch), the fold is refused and planning throws as before; any other stale value reads zeros and is recorded indocs/dev/debt/shader.md.image_get_lod), where SPIR-V leaves the derivatives undefined, so such a table throws at dispatch withbindless image table keyed per lane holds images of different runtime modes under an implicit-derivative access, which is not implemented. Only bound records take part in the check, so a null record 0 is not compared. Lanes whose record is null or outside the table take the zero path of the per-lane mode switch under record 0's mode, the first mode when record 0 is null, so around such an access the derivatives of quad neighbours whose records need another mode are still undefined; this gap is recorded indocs/dev/debt/shader.md. Accesses of one table share the check; reads, gathers and explicit-LOD samples of a table nothing derives through still take mixed modes.kis atbase + entryOffset + k * strideand the table hasnum_recordsrecords, matching the structured buffer addressing of the load. A V# withnum_records0, a null V# included, binds no record and is not checked further: with no records everyOOB_SELECTputs each load out of range, so no lane can hold a record, every lane that runs the access reads zeros, and a dispatch over an unbound table whose waterfall no lane enters does not throw. A V# with records and swizzling,ADD_TID,OOB_SELECT2 or 3, or a stride smaller than the record offset plus 32 bytes throws at dispatch.NonUniformeven where wave-uniform keys are not (one wave held by one host subgroup), since the slot differs per lane.Sources: AMD RDNA 2 ISA reference guide, chapter 12:
V_READLANE_B32(VOP3 opcode 864; lane selected by SSRC1, ignores EXEC),V_READFIRSTLANE_B32(VOP1 opcode 2),S_FF1_I32_B32/_B64(SOP1 opcodes 19/20),V_CMPX_EQ_U32(VOPC opcode 210; writes EXEC),V_CMPX_EQ_U64(VOPC opcode 242),S_ANDN2_B32/_B64(SOP2 opcodes 20/21; SCC = result != 0),S_CBRANCH_SCC1(SOPP opcode 5) andIMAGE_GET_LOD(MIMG opcode 96). Range checks withnum_records0: the GFX10OOB_SELECTtable in Mesa'ssrc/amd/common/ac_descriptors.c(0: index >=NUM_RECORDSor offset >= stride, 1: index >=NUM_RECORDS, 2:NUM_RECORDS== 0, 3: offset >=NUM_RECORDS). The loop shape is LLVM'semitLoadScalarOpsFromVGPRLoop/loadMBUFScalarOperandsFromVGPRinllvm/lib/Target/AMDGPU/SIInstrInfo.cpp(readfirstlane of each operand piece,V_CMP_EQ_U32/_U64against the VGPRs,S_AND_SAVEEXEC, the instruction,S_XOREXEC,S_CBRANCH_EXECNZ), andAMDGPURegisterBankInfo::executeInWaterfallLoopfor GlobalISel. Mesa'snir_lower_non_uniform_access.cbuilds the same loop for RADV before ACO forNonUniformdescriptor indices; this PR undoes it in the other direction, from the guest loop back to aNonUniformaccess. Derivatives: the Vulkan specification's derivative operations leave implicit derivatives (implicit-LOD samples,OpImageQueryLod) undefined when the invocations of a quad are not in the same control flow. The fold is a logical consequence of those semantics; nothing here was measured on hardware.Limits (recorded in
docs/dev/debt/shader.md): only IDXEN loads from an evaluable V# are planned; waterfalls over other per-lane T#s, over S#s, and around image stores or atomics still throw. The equalities are read from the EXEC lane condition the translator builds forv_cmpxand, in wave32, fors_and_saveexecof av_cmp/VCC result; compares combined withs_and_b32in other SGPRs, ands_and_saveexec_b64in wave64, leave EXEC as a mask, so those loops still throw (not tested). Onmaina per-lane table goes through the whole-table mode ofmain's bindless table (there are no material records to enumerate its keys from), so any table V# with more thanRuntimeAbi::BindlessTableSlots(16) records is rejected at dispatch withbindless image table has N entries (0 materials), limit 16. #758 (stacked on #757) lifts this with per-invocation table snapshots; whichever lands second ports the lane-keyed table to the other's model (#757 rewritesDescriptorSource::IndirectImage,TryMakeTableImage,resolveTableImageand the SPIR-V table selection that this PR extends).Merged #4883 persists resource plans. This branch now encodes and decodes all three additional
IndirectImagefields (laneKey,indexed,implicitDerivatives), retains the strict ten-field guard, and uses fresh disk format 31 after main format 30. Fresh and loaded plans must preserve indexed table addressing and derivative-mode checks.Tested
Windows Server 2025, MinGW-w64 GCC 15.2.0, NVIDIA GeForce RTX 3060 (subgroup 32). The first two commits were tested on
mainc8eb1f5; the third (empty tables, bound modes) and the rebase onmain2f6ae9f as noted below.agc_driver_waterfall_images(tests/execution/WaterfallImages.cpp): hand-assembled compute shaders that load a per-lane T# from a 4-record table with twobuffer_load_dwordx4 ... idxen(keytid & 3, one record a null T#, records of anR32_UINTand anR32_SFLOATtexture) and run the waterfall loop aroundimage_loadat a per-lane coordinate; three dispatches with different tables must reuse one compiled artifact, and the SPIR-V must decorate the table slotNonUniform. Variants that must read each lane's texels: wave32 over the whole wave, wave32 under a partial EXEC (lanes skipped by the load stay 0), wave64 under a partial EXEC (when the host subgroup holds 32 lanes), a loop that picks lanes withv_readfirstlane_b32, and a loop that compares the T# with fourv_cmpx_eq_u64. Variants that must still throw: lanes whose T# VGPRs keep another descriptor, lanes whose T# VGPRs got a descriptor written under EXEC, a loop that compares seven of the eight dwords, an image load after the loop restores EXEC, and an image store in the loop. A loop behinds_cbranch_execzover the lanes whose key is below the table V#'snum_recordsmust read each lane's texels over the 4-record tables, and must neither throw nor change the value each lane holds before the loop (-1) over a null table V#, a V# with stride 0 and no records, and a swizzledADD_TIDV# withOOB_SELECT2 and no records. Animage_get_lodwaterfall compiles over a table ofR32_SFLOATrecords and over a null record 0 withR32_UINTrecords, and must throw over a table that mixesR32_UINTandR32_SFLOAT, with or without a null record 0.main, the test kept): fails withShaderRecompiler::Recompile: GetImageResource dword 0 is not a valid runtime value; chain: ReadLane(CompositeExtractU32x4(LoadBufferU32x4(GetBufferResource(GetUserData....wave32 waterfall over the whole wave: the per-lane table slot is not decorated NonUniform.a waterfall over a table T# and a T# written under EXEC to the VGPRs of lanes the load skipped was accepted; dispatched, lane 7 of that shader read 0 instead of texel 4097 of the default texture.an LOD query in a waterfall over a table of float and integer images was not refused.ShaderRecompiler::Recompile: bindless heap record is smaller than its descriptor, the swizzledADD_TIDOOB_SELECT2 V# withbindless image table indexed through a V# with swizzling, ADD_TID or an unbounded range check is not implemented, and the null record 0 withR32_UINTrecords withbindless image table keyed per lane holds images of different runtime modes under an implicit-derivative access, which is not implemented; the mixed table with a null record 0 is refused with and without the fix.agc_shader_memory,agc_shader_bindless,agc_shader_srt_read_patching,agc_shader_frontend_pair(_off),agc_structurizer*,agc_spirv_module,agc_masked_select,agc_shader_disk_cache,agc_shader_control_flow*,agc_driver_bindless_images,agc_driver_buffer_descriptor_runtime,agc_driver_exec_host_lanes,agc_driver_read_lane,agc_driver_runtime_images,agc_driver_wave32_subgroup, everyagc_driver_image_*and everyagc_driver_prepared_*test, andagc_driver_waterfall_images(first two commits). With the third commit, rebased on 2f6ae9f:agc_driver_waterfall_images,agc_shader_bindless,agc_shader_memory,agc_driver_bindless_images,agc_driver_buffer_descriptor_runtimeandagc_driver_runtime_imagespass.python tools/check_conventions.py --base upstream/main(upstream/main 2f6ae9f, all three commits): passed.agc_shader_replay, first two commits) of 139 captured requests from six titles,main2a2a3dc and this branch on it, both the recompile and the registration (--prepare) path: 132 requests give the same result on both builds (101 compile to the same number of SPIR-V words, 31 fail with the same message); only the seven GTA V requests below change.0x25cb2dc00that stopped the title at a draw and six compute shaders refused at registration (0x25cbd6c00,0x25cbe7800,0x25cbf0000,0x25cc00a00,0x25cc08400,0x25cc0ac00) fail planning onmain, on feat(agc): implement image tables loaded through 64-bit pointers #758 (rebased onmain3779597) and on feat(agc): image-table records that hold a pointer to the T# or split it over two loads #4475 (the six compute requests; its serializer cannot read the pixel request) with the error above. With this change all seven pass planning and the registration path compiles them (259514, 975997, 1812923, 1918841, 469346, 696742 and 709414 SPIR-V words, in the order above; the pixel shader and0x25cbd6c00give the same counts after the rebase). The requests carry zero user data, so the recompile path, which now has to read the table V# to materialize the table, stops later atSrtWalker::EvaluateDescriptorSources(the SRT pointer is null); for the same reason the table contents are not in the requests, so whether the tables fit in 16 records, and whether the table behind the pixel shader'simage_get_lodwaterfall holds records of one runtime image mode, was not checked.Not tested: in game, on lavapipe/Linux, on AMD hardware or RADV, console behaviour.
Latest source rebase: main
0df6e295c, with original commit authors/messages and complete rebased trees independently verified. The added codec compatibility commit is tested with the exact public production sources and matching IR units in a CPU-only fixture: all eight combinations of the three flags have distinct encodings and round-trip correctly. Removing only those field reads/writes fails the encoding assertion; changing only format 31 back to 30 fails the old-entry refusal assertion. The latest rebase preserves these tested production/fixture blobs exactly. This does not claim a fresh native or title run of the rebased branch.Checklist
main; no other open PR implements the same functions (feat(agc): implement image and sampler tables indexed at runtime offsets #757, feat(agc): implement image tables loaded through 64-bit pointers #758 and feat(agc): image-table records that hold a pointer to the T# or split it over two loads #4475 plan scalar-loaded table T#s; none foldsv_readlanewaterfalls or plans T#s loaded per lane bybuffer_load)NotImplemented_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)0x25cb2dc00that stops the title, and of six compute shaders refused at registration; not run in game, and dispatch still needs the tables to fit in 16 records)🤖 Generated with Claude Code
Co-Authored-By: Codex noreply@openai.com