Repository navigation
fix(shader): plan a table T# that several image operations share - #5550
Conversation
TryMakeTableImage planned a T# read from a bindless image table only when its eight scalar reads had no user other than that one GetImageResource. Descriptor-phi splitting gives each image operation its own copy of the T# over the same reads, so a table entry that is both queried and sampled was refused, and GetHandle then rejected the scalar reads. Accept reads whose every user is a GetImageResource built from exactly those eight reads; each copy is planned as the same table. Crimson Desert (PPSA25286) stops on such a shader at start-up: image_get_resinfo and image_sample_lz on one s_buffer_load_dwordx8 T#. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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.
|
|
Windows test of #5550: Windows 10 (19045), MinGW-w64 GCC 15.2, Release, AI-assisted (Claude Code). |
|
Thanks, @xXPBSXx. I've added the Windows run to the description. AI-assisted: yes |
|
Title evidence: Onimusha: Way of the Sword (PPSA27836 01.000.001, RE Engine), Windows 11 Pro 26200, RTX 5070. On A later program reads the same table T# for two image operations. With #5620 and #5550 both merged, both pass and the title stops next at AI-assisted: yes (Claude Code). |
|
Thanks for the Onimusha run. The later program, with one table T# used by two image operations, is the shape this PR handles (the copies of AI-assisted: yes |
Materialize runtime-indexed image and sampler records into descriptor tables while keeping descriptor selection in the shader. Report invalid active entries through the existing deferred fault buffer. Bind fragment tables in the fragment set when pipeline libraries split it, and reserve a physical descriptor for empty tables. Reject unsupported sampler, border, cube and anisotropic pairings per entry. Keep one column per heap access shape, one T# shared by several image operations included (milescoviello's boykopovar#5550 case), and narrow it to the union of its keys' domains when every key has one (SleglJan's key-domain changes). Plan columns read at (key << n) & mask over their whole domain (tibzejoker's fix). Read records in runs through SleglJan's bulk reader, and reject entries whose surface has no mapped byte, as Tamalero proposed. Store table columns in disk plans; update the runtime ABI and disk format. Bound table-sized entries in the resource cache (65,536 table elements) and the dispatch cache (256 MiB of captured words), evicting LRU.
What
Crimson Desert (PPSA25286) stops at start-up while it prepares one of its vertex programs:
The program reads a T# from a descriptor table at a wave-uniform key and uses it twice:
TryMakeTableImagealready plans this shape (32-byte entries, a uniform key) as a bindless image table. It refused this one because it requires the eight scalar reads to have no user other than the oneGetImageResourceit is planning.SplitDescriptorPhisruns first. When it finds a Phi in the chain behind a T#, here in the lane condition of thev_cndmask_b32that builds the key, it gives each image operation its own copy of theGetImageResourceover the same eight reads. The query and the sample therefore have two handles, each read has two users, and the table is refused.GetHandlethen rejects the scalar reads with the message above.A read now passes the check when every user is a
GetImageResourcebuilt from exactly these eight reads. Each copy is planned as the same table. Both get the same descriptor source, so the query and the sample share one table image and one set of slots.agc_shader_bindlessgains a case for this: a loop whose counter is part of the key, withimage_get_resinfoandimage_sample_lzon the same table T#. The loop Phi makes the split give the two operations separate copies, as in the title. The case checks that the plan has one table source and one image for both operations, that the capture binds the table slots, and that the program compiles to valid SPIR-V.Instructions of the new case, after the four
s_load_dwordx4the existingwholeCodestarts with:#5188 also targets
GetImageResource dword 0 is not a valid runtime value, for a T# read at an offset the CPU walk can evaluate. With #5188 applied, the title still stops on this shader, because its key is not one the walk can evaluate. #5089 changes the same file for per-lane T#s in waterfall loops, and #1184, #757, #758 and #4475 extend the table planning.tools/pr_overlap.py --branchfinds no overlap with any of them.Tested
Ubuntu 24.04, GCC 13.3 (CI's compiler), Release with CI's configure flags, on
main7a0c838:main'sResourceTracker.cppand the new case,agc_shader_bindlessfails with the title's message,GetImageResource dword 0 is not a valid runtime value; chain: ReadConstBuffer@pc=0x00000034(...). With the change it passes.libSceAgcDriverpass on an RTX 4070 Ti (NVIDIA 580.142), the shader and driver tests included.agc_driver_stencil_ref_exportis skipped, as onmain, because the device lacksVK_EXT_shader_stencil_export.agc_shader_replay, fails resource analysis with the message above onmain. With the change,--prepareproduces 36,023 SPIR-V words.--to-inteland run on Linux. Both runs include fix(libkernel): fail sceKernelStat with EFAULT for a null buffer #5464, fix(http2): allow completion waits without an event queue #3925 and fix(pad): accept vibration mode 5 #5523, which get the title past its earlier stops, and differ only inResourceTracker.cpp:Shader 0x20ac0f4100 could not be prepared after registration).VGT_SHADER_STAGES_EN=0x60212d: combined tessellation and geometry is unsupported by the reference path.-fsyntax-only -Wall -Wextracompile of both changed files has no errors. Its warnings,-Wclass-memaccessfromIrValue.hppand-Wmissing-field-initializersin the test file, are the same onmain.tools/check_conventions.py --base upstream/mainpasses.main34355eeea.agc_shader_bindlessandagc_shader_memorypass in 3 runs each; that run had no baseline without the change.Not checked on a console.
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)🤖 Generated with Claude Code