Repository navigation
feat(agc): implement image and sampler tables indexed at runtime offsets - #757
Conversation
|
Thanks for this. The Linux job fails in the new I don't think the table path is at fault here. The check just before it, that record 4 gives the same words as a direct binding of the same T# and S#, passed in that run, so the direct binding returns 0x00000001 on lavapipe too. Record 4 pairs Either of these should make it pass everywhere:
The first also fixes direct bindings on lavapipe, so I'd prefer it, but it's your call. The other rows pass, and Windows is green. |
7c75f0e to
b9c9cf0
Compare
|
Thanks, that was it. I went with the first option. #757, #758 and #759 are force-pushed onto
Windows: ctest 335/335 (#757), 335/335 (#758) and 336/336 (#759). |
|
Thanks. The Uint rule now covers direct bindings, table entries and both mixed pairings (direct S# with a table image, table S# with a direct image), and the sampler-word checks don't depend on the driver. Two things:
|
b9c9cf0 to
734ef85
Compare
|
Rebased the chain onto
|
|
Correction to the lavapipe note above. Under identical load, with main's and this chain's test binaries running side by side on Mesa 26.2.4 llvmpipe, this chain is about 45% slower in both slow tests:
NVIDIA shows no difference, and neither test uses image tables, so this is a per-dispatch or per-compile cost that #757 adds everywhere. It is not the environment. I'm tracking it down and will fix it on this branch before asking for another look. |
|
Thanks for the rebase. I went through the range-diff against the previous heads: the #668 merge keeps Thanks also for the lavapipe correction. It matches CI: #758's Linux job (run 37406080556) timed out in |
734ef85 to
1219876
Compare
|
Fixed and force-pushed. #757, #758 and #759 are on
|
|
Thanks for tracking that down. The range-diff against the previous heads only moves the fault-word conversions into the I'll run the chain through PPSA21564 next (cold and warm shader cache) and report back here. |
|
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.
|
adriadam10
left a comment
There was a problem hiding this comment.
Re-review of d9e47d1 (#757) with #758's 34525fd on top, merged onto main 84b01f9 (clean merge). Linux, RADV, -DANYPS5_ENABLE_SPIRV_TOOLS=ON, libSceAgc + libSceAgcDriver scope: 572/573 ctest.
agc_driver_image_table,_cache,_cache_bounds,_reuse,agc_driver_bindless_images,agc_shader_bindless,agc_shader_memory,agc_shader_disk_cache,agc_driver_graphics,agc_driver_key_miss,agc_sparse_run_importand_copiedpass. Inagc_driver_image_tablethe poisoned entries (outside the snapshot, not an image, unmapped surface, sRGB decode, table written by its dispatch) each reach the host as a thrownimage table:fault. Only the per-wave wave32 case is skipped, since RADV's subgroup is 64.- The one failure,
agc_driver_tessellation(SIGSEGV), fails the same way onmain84b01f9 built alone, so it isn't this PR.
All four points from my last review are addressed: the ARCHITECTURE.md section, both fold-ins (with their tests in agc_shader_bindless), the order (#4757 closed, #4883 rebased on) and the final numbers (disk format 32/33, runtime ABI 12). Selected poison entries throw through BdaResources::CheckFault, so nothing that main refused now returns a plausible value, and every unbound case has a debt line.
On @oneandonlydean's memory question: tibzejoker's 4dcf657 run shows both caches sitting at their new caps (dispatch cache ~254 MiB, resource cache 1,024 entries, only 20 table T# misses per 10 s). The ~1 GB/min that remains in GTA isn't in the structures this PR adds, and Guardians stays flat at 11.6 GiB. The CPU copies of sampled textures are bounded by the VRAM-derived budget, which is a follow-up that isn't specific to tables. The snapshot cost (Village 4.5 ms, GTA ~11 % of worker 0) is a performance follow-up (SleglJan's snapshot reuse plus the unwritten-range skip), not a correctness issue.
Approving both. #758 goes in after #757. The full Build (Windows) is dispatched on the final heads before the merge.
|
@Celegans12, Guardians of the Galaxy (PPSA01748) on Better than before. All the menu text now renders, which it didn't on the older heads. The title goes through the language, brightness and adjustments screens, plays its intro video, and starts loading the first level. It still stops during that load, on a shape I'd separate from the 0x4378/0x4380 one you described: The key is a On the request: I'd rather not attach it. The serialized request carries the title's shader binary, and I keep game code off public threads. If you open a follow-up for either shape, I can run it on this title the same day and post the result, or paste more of AnyPS5's own planner output from the log. Details
Tested and written by Claude (AI assistant) on a contributor's machine.
Note, there seems to be bug unrelated to this thread on the language availability. Will be reported later (chunks)
|
|
@Celegans12, Village numbers for the snapshot-reuse follow-up. Resident Evil Village (PPSA01556) on From one run from boot to first control with
Each table has 32,768 keys. In a Tracy capture of the title with local zones, the draw stage's capture, which reads each column's records, is 55% of the worker's own time, about 100k words read per capture. Without any profiling (only So keeping a column while its words are unchanged, and skipping its read while I can test SleglJan's |

What
Image and sampler descriptors that
s_buffer_loadreads at runtimekey * stride + addendoffsets fail planning (GetImageResource dword 0 is not a valid runtime value). This snapshots such tables per invocation and selects the entry in the shader; table contents stay out of shader, pipeline and disk-cache keys.ANISO_OVERRIDEpairings, and a T# whose surface has no mapped byte.(key << n) & maskkeys (@tibzejoker's fix) keep the whole table. Records are read in runs (@SleglJan'sShaderMemory::readWords).docs/dev/ARCHITECTURE.md; runtime ABI 12, disk format 32 (stored plans carry table columns); limits indocs/dev/debt.#758 adds tables loaded through 64-bit pointers. Table FMASK entries still fault. #5461 and #5472 also change the disk format, and #5620 extends the bindless-table path this replaces. #5550's T# that is both queried and sampled is one column here (its test is ported), as is #5188's T# read at a user-data offset, whose test now checks the record each key selects. #4783's key reservation and #5408's key-miss trace count the image shapes this adds to content keys.
Tested
Based on
main066b58f. Windows 11 (MinGW-w64 GCC 15.2.0) and Linux: configure -DBUILD_TESTING=ON, full build, --target libs, ctest.log_macrosalso fails onmainhere.main84b01f9: 1171/1173;agc_driver_bc_block_copyandagc_driver_depth_compare_copyalso fail onmainthere.agc_driver_image_table,_cache,_reuse,_cache_bounds(new: both bounds),agc_driver_bindless_images,agc_shader_bindless(new: shared keyed columns, masked offsets, #5550's queried and sampled T#),agc_driver_key_missandagc_driver_graphics(new: table words and shapes in content keys) andagc_shader_disk_cache(plans with table columns and key domains round trip) pass on both. The table tests don't build onmain;_cache_boundsfails without the two bounds.Not tested: native RADV, Linux with write tracking, console behaviour.
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)