Repository navigation
feat(agc): store to images selected through image tables - #6402
Conversation
|
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.
|
|
Heads-up on an interaction with #6243 (selecting the record of a T# whose offset the CPU evaluates), which is still open. The two merge without conflicts, but on a merge of both, With that one expectation changed to Whichever of the two merges second needs that one-line test change. If this one lands first, I'll make it on #6243. |
|
@Pipodi I went through this against our local store work, and the model fits #757/#758 nicely. Keeping one element domain with the storage element in 260 and the sampled one in 261, faulting only on use for poisoned or rejected elements, and never reusing tables with written entries are all what I'd have done. A few things I think need fixing before it goes in, plus some I'd be fine leaving to a follow-up. The two I'd treat as blocking:
Smaller ones:
Happy to send the depth-plane part and our two extras (pipeline replay with storage tables, and 260 sized in classes of the element count so 233-element sets stay in chain pools) on top once this settles. I'll run it on Yōtei as part of our next game build. Details and notes
|
42ef89c to
9161151
Compare
|
@Celegans12 thanks for the careful pass. Pushed 085fa565b on top of a rebase onto current
The limits in your details block are in the same debt entry now too, along with the ones I should have listed in the first place: the storage capacity heuristic and that Tested on 9161151 (rebased onto One thing the new check caught: |
|
Tested on Linux with lavapipe, the CI device: llvmpipe (Mesa 26.2.4, LLVM 23.1.1), CachyOS, GCC 16.2.1, CMake 4.4.3, default build type, full build with This PR (
Merged together with #6416 (on
Apart from those, the failed and skipped lists match Not tested: NVIDIA, AMD, Intel, PPSA26344. AI-assisted: yes |
|
Data from 007 First Light (PPSA11386 v01.018.000), a title that stores to images selected through image tables in compute. Setup: Linux 7.2.10, RTX 3080 10 GB, NVIDIA 615.78.08, GCC 16.2.1, Release, bubblewrap sandbox. Stack: Without this PR, the title's compute dispatches fail their deferred write-back at five program sites with
AI-assisted: yes (Claude Code) |
|
Thanks, that's the first run on a title that stores through image tables: four of the five sites going from hundreds of failures to zero is what this PR is for. You're right about the fifth site's message. It comes from one check in
The two need different fixes. Case 2 means giving storage tables their own index space; case 1 is a recompiler bug. 0229b5f (pushed here) gives each case its own message:
It changes only those messages and corrects the debt entry, which said the index counts within the table rather than the set. It cherry-picks on its own, so a run with it would show which case your eleven failures are. AI-assisted: yes (Claude Code) |
|
Reviewed at 0229b5f. Looks good. Two small things for your next rebase:
Full CI is running on this head. It'll also go through our game gates before merge. |
An IMAGE_STORE whose T# comes from a runtime-indexed image table used to poison the whole table as an unsupported operation. Written, non-atomic table images now get storage modes, a STORAGE_IMAGE table binding (260, the sampled image table moves to 261 and keeps the variable count), and storage views of tracked storage textures at the same element indices. Every bound entry is marked written, so each dispatch writes back all of the table's storage images. Atomic, packed, BY2/BY4 and dynamic-mip table stores still fault. Tables with written entries are not reused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… elements The shader memory registration test pinned the old refusal. It now checks that the store's table is supported, does not fault, and binds every element as written on the storage table binding, and that a store-only table binds no sampled elements. The driver no longer requires a written flag per table element, so bindings that carry none read as unwritten. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lements like direct ones The non-split descriptor set's pool now counts the storage table binding's descriptors, as the split path already did. A table storage element whose minimum LOD clamps above the level it addresses is rejected per element, as the direct path refuses it, and the element is looked up as written so a depth surface under it is retired. The registration test checks again that image_store_mip, image_store_pck and image_atomic_add through a table stay unsupported operations that fault on use. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…st its capacity Both cases printed that the image storage table exceeds its capacity. They need different fixes, so each now has its own message with the set, the element's index in the set and the capacity. The debt entry now says that the index counts every table element before it in the set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
0229b5f to
b7e21ca
Compare
|
@oneandonlydean thanks. Rebased onto On #6243: agreed. EagleBearerYS offered to make the change on #6243 if this one lands first. If #6243 lands first, I'll flip the Tested on b7e21ca: full AI-assisted: yes (Claude Code) |
Main's #6402 runs image stores through an image table, so a T# keyed per lane keeps its table with a supported image_store and an unsupported mark for the other operations.
Main's #6402 stores to image tables: a table with a written entry takes the fresh path, as it does on main, and so does one without a sampled table binding, whose entries the fresh path rejects. The shared place step uses the binding as a pointer. Main's #6614 frees descriptor sets from shared pools one by one, while this branch keeps only dedicated pools, so both stay. #7101's bulk key read counts its unmapped-page skips as unmapped, and its key words sit next to this branch's write stamp and compared runs.
Brings combo19 (async lane, gpuind, chain-A, 1a f32 mix, boykopovar#7489, S1, bindless images, staging diff, HTILE work, epoch 5) up to upstream main of 2026-10-19. 118 conflicted files, resolved by area; notes in work/research/structural/merge19/notes-*.md. - Versions: FormatVersion 48, RuntimeAbi 18, request serialization 37 (upstream's 31..36 as upstream writes them, our single-workgroup flag is 37, gpuind's trailing block last; includes b75c4e6's fix). - Image tables: upstream's boykopovar#6454 reuse model, scratch indexes, selected records and pointer chains, with our bindless columns and resolve memo in front. - Pairs kept with ours as the default: boykopovar#6210 presenter epoch only with APS5_PRESENT_EPOCH=1; boykopovar#6402 table stores (our path); pending HTILE fills only with APS5_PENDING_HTILE_FILLS=1. boykopovar#6208 takes upstream's final code with DMA pipelined by default (APS5_NO_PIPELINE_DMA=1 off). AI-assisted: yes
What
An
IMAGE_STOREwhose T# comes from a runtime-indexed image table used to mark the whole table as an unsupported operation. The dispatch's deferred write-back then failed. This follows #757/#758's table model:AssignTablesnow accepts written, non-atomic table images and gives them storage modes. Atomic, packed, BY2/BY4 and dynamic-mip table stores still fault, as before.STORAGE_IMAGEtable binding (260). The sampled image table moves to 261 and keeps the variable descriptor count. The runtime ABI goes to v14, so existing shader caches are invalidated.RunStoreTestsis now a positive test: a lane stores to the key-1 image, while a null-key store and an EXEC-cleared store write nothing.verifyImageTableRegistrationexpects the bound written elements, a store-only table binds no sampled elements, andimage_store_mip,image_store_pckandimage_atomic_addthrough a table still poison the table with an unsupported operation and bind no storage elements.Native pipeline replay with storage tables, reuse of table layouts across size classes (which also lets storage-table sets back into chain pools), and rejecting the second writer of a depth plane are follow-ups. @Celegans12 will add them on top (#675).
Technical debt
New entry "Image storage tables" in
docs/dev/debt/libSceAgcDriver.md:0x80001e5f00binds the stencil plane0x505f350000as R8_UINT and R8_UNORM);maxPerStageDescriptorStorageImages), and the sampled capacity isn't reduced by it;PresyncSurfaceswalks only the sampled table;Tested
Windows 11, NVIDIA GeForce RTX 4090, driver 617.42.
main6e7ba92), fullBUILD_TESTING=ONbuild: 1364/1366 pass.agc_driver_pixel_lds(vkWaitForFences timeout in the wave64 unbounded case) also fails alone on plainmain6e7ba92;select_testsfailed in the parallel run and passes alone.0x80001e5f00ends with threeIMAGE_STOREs through runtime-indexed tables (pc 0x783c/0x785c/0x7868).image table: shader 0x80001e5f00 stage 4 pc 0x783c ... unsupported operation0x80001e5f00compiles (1,457,658 words, 63 s) and dispatches; workers keep running until ~1240 s0x80001e5f00v2 dispatched with 150-169 storage images bound (resources 108-343 ms); loading continues to the first in-game scene (~25-30 min)The game runs predate the review changes, which affect descriptor pool sizing, minLod rejection and depth-surface retirement for table storage elements.
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