Repository navigation
fix(shader): map retained SPIR-V from shader cache entries - #4806
Conversation
|
In a plain build the two new tests don't run. Setup: Windows 11 (26200), WinLibs GCC 15.2.0 r7, Release, CI configure flags, an RTX 4070 Laptop GPU and an Intel Raptor Lake-S iGPU. The head 501f197 merged into
Also, the new block sits between Written with AI assistance. |
501f197 to
4c82d90
Compare
|
Rebased onto main The default full Windows Release build now builds the executable without naming its target. Heap/file storage, shader memory and shader disk cache all pass without skips (four tests, 0.78 s). The 64 MiB private-byte deltas are 67,244,032 bytes for heap storage and 159,744 bytes for file backing. Separate cache-disabled heap/file replay processes emit identical 134,780-word SPIR-V for the captured title shader. Conventions and whitespace checks pass. No new game-runtime claim is made for this rebase. |
cebe922 to
3e38138
Compare
|
|
|
A Demon's Souls (PPSA01341 v1.07) data point for the route @oneandonlydean suggested. @ThatsLeo, your Setup: Linux, RTX 5070 Ti, NVIDIA 595.91, 30 GiB RAM,
A heap profile of the run without it puts the growth in SPIR-V decoded from the disk cache and kept in This PR's file-backed storage doesn't help on this machine, because If you open the branch as a PR, I can rerun it there. AI-assisted: yes |
A cached variant kept its SPIR-V in memory for the whole run. On Linux and macOS the words are now a view into the mapped cache file, and a freshly compiled variant is written and mapped back, so the kernel can drop them.
3e38138 to
4a0489a
Compare
|
Replaced the opt-in temporary-file implementation with the suggested cache-entry mapping route. The branch now carries ThatsLeo's original change with its authorship preserved, plus Windows mapping support at Rebased onto |
|
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.
|
…d pointer Retained SPIR-V is a span now, and SpirvTools::Validate takes a vector or a pointer and a size.
|
The Linux build with SPIRV-Tools failed in |
|
Demon's Souls (PPSA01341 v1.07) on Linux with this head (4a0489a), as offered above. With it, the title loads the tutorial level and stays in it, where it was OOM-killed without it. Setup: Linux, RTX 5070 Ti, NVIDIA 595.91,
One caveat: the run without the mapping was on an older base ( The Linux side works like ThatsLeo's original branch did here (numbers in my earlier comment). AI-assisted: yes |
Retained SPIR-V is a span on main since boykopovar#4806.
* perf(agc): start no collect epoch at a wait its own queue's stored label satisfies waitMemory starts a new collect epoch when it returns, so the next CollectWrites of every surface walks the write watch again. The label-table path already keeps the epoch for a non-late hit whose store came from the waiting queue: such a label orders only that queue's own work, no store by the guest or another queue. The two storedSince returns (at entry and while polling) answer the same question from the label-store history but kept the bump for a store of the waiting queue. They now keep the epoch in that case too. (cherry picked from commit 17984f8) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * perf(agc): bind the address space's BDA table for writes inside writable imports A build mapping only the cached address space but writing through V#s got a table keyed by its writes: a new key for nearly every set of written addresses, so each such build merged the space's imported ranges, stored them, then hashed and compared them in BdaResources. AddressRanges splits only ranges without write permission around the written intervals. When no written interval overlaps such a range of the space, its result is the space's own ranges, so the build binds the space's table by the space serial, as a build without writes does. A write overlapping a range without write permission keeps the per-writes table. Test: builds with one and two writes inside a writable import bind the serial and the ranges of the build without writes, equal to AddressRanges(), and keep the read-only import without write permission. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit d95634b) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * perf(agc): extend pending write snapshots incrementally (cherry picked from commit ac145e9) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * feat(agc): count which key component differs on resource-content cache misses Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 5cf0aa4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * feat(agc): render into SW_64KB_Z_X color targets SW_64KB_Z_X (0x18) color targets use the texture equations and 64 KiB block geometry of that swizzle mode, including the equation bits above the block that R_X targets already apply. A title renders the R32 float copy of its depth into one. Fork: the debt note goes into docs/dev/TechnicalDebt.md. On Metal the shared decode maps the mode to kZ64KBX, which the Metal texture path already detiles through its equation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit f6aa98b) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): label the device words that end a dispatch content key Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit e88b5c2) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * perf(agc): compare key components only when APS5_TRACE_RESOURCE_KEY_MISSES is set The per-miss component comparison allocates and walks the whole key, and it ran under APS5_PROFILE_DRAW as well, where it cost queue time on every resource-content miss. It now runs only with the trace variable, which is also the only one that prints its [keymiss] lines. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 7e00eda) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): accept 2D-array image loads and stores on 3D T#s A 2D-array image_load or image_store bound to a 3D T# addresses the slice as z. Add a 3D runtime mode for these accesses and drop loads and stores whose slice is past the volume depth. (cherry picked from commit 945b970) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * perf(agc): take a dword fill of any inline constant as a buffer fill The zero fill kernel (v_mov_b32 v1, 0; buffer_store_format_x) is also emitted with other inline integer constants as its source. Run as compute dispatches, those writes are pending writes of unknown value, so a later read of DCC keys over their range waits for the dispatch. A constant of 0 to 64 or -1 to -16 is now matched like the zero fill and stored as a buffer fill of that value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Fork: the fork has no MatchZeroDwordFill or memory-value buffer fills (an earlier upstream series), so MatchConstantDwordFill is added on its own and tried after MatchDwordPatternFill. Vulkan driver only: the Metal backend does not use BufferFill.cpp. (cherry picked from commit b83755a) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): select coherent buffer heaps before allocation (cherry picked from commit ffc6c89) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * test(agc): verify buffer budget placement before allocation (cherry picked from commit 53acbf0) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): accept alpha-to-coverage from a 32_AR MRT0 export without a color target radeonsi exports MRT0 as 32_AR when alpha-to-coverage is on and no color buffer is bound, so a depth-only draw can depend on that alpha. The decoder now accepts 32_AR and an absent MRT0 target, and reads the MRT0 export format from SPI_SHADER_COL_FORMAT, which it masked to 0 for draws without a color target. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Fork: the debt note is updated in docs/dev/TechnicalDebt.md. State.cpp is shared with Metal, so Metal draws with alpha-to-coverage from a 32_AR MRT0 export or without an MRT0 color target are no longer refused at decode. The Metal draw path does not pass alphaToCoverage to the pixel stage (unchanged by this commit). (cherry picked from commit c8662b6) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * test(agc): check both registration layouts against isolated compiles Add a --layout-pair mode to agc_shader_memory_tests, registered as agc_shader_layout_pair and agc_shader_layout_pair_off (the latter with APS5_NO_PERF_LAYOUT_PAIR=1), next to the frontend-pair tests. A pixel shader with a buffer load is prepared with a 128-byte push range and with none through one context, in both orders: without registration modules, with them at push offset 0 and at the fragment slot offset 128 that devices with graphics pipeline libraries use, and with a registration word limit that forces the fallback to plain modules. Each pair must build one program with reuse and two without, and each layout must match a compile of the same code made on its own source without a context: SPIR-V, binding layout, push range and push-data use, artifact fields and compiled shader information. The materialized result must also match an uncached Recompile. The context must not reuse a program translated at another code address or one built for another source at the same address, and it must release the kept program when it builds another one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 0d7b4b4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * test(agc): compare every artifact field of both registration layouts The layout-pair check compared a subset of the prepared artifact: the fragment parameters only by count, and not the barycentric emulation, the runtime ABI version or the vertex interface fields. Its pixel fixture also had no interpolated inputs, so the fragment parameter list was empty on both sides. Compare every artifact field except the variant id, the fragment parameters element by element, and add a fifth configuration: registration modules at the fragment slot offset with two interpolated inputs, which must declare two fragment parameters in both layouts. With this, flipping the barycentric emulation flag, a fragment parameter's flat flag or the runtime ABI version on the reused path fails agc_shader_layout_pair, where the previous check passed all three. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 60e564a) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * test(agc): compare the copied compiled information with the moved one The reused program hands out its compiled information through IrProgram::CompiledInfo() const&, while compileVariant still moves it out with TakeCompiledInfo(). The layout-pair fixtures are pixel shaders whose user-data base, scratch size and parameter export mask are all zero, so the comparison with isolated compiles could not tell whether the copy kept those fields: a copy that zeroed them still passed. Translate a compute program with a user-data base of 2 and 64 scratch dwords and require the copied and the moved information to agree on every field. Zeroing those fields in the copy now fails agc_shader_layout_pair and agc_shader_layout_pair_off. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 49de394) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * perf(agc): draw stages use their shared compiled result instead of a copy Driver::draw copied every stage's compiled result (RecompileResult, with its binding, push-constant and attribute vectors) out of the shared result the capture materialized, once per stage per draw. The stages now point at the shared result. Only the CPU path of an indirect draw rewrites a stage (each record's patched user data is materialized again), so that path copies the stages it may rewrite first, on a cache hit or a miss. agc_driver_indirect_patched_stage draws through that path: a record that patches the user data word a vertex stage writes as its depth must draw with the record's value, not the register's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 73a808e) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * test(agc): validate the narrow constant store fixture through its word pointer Retained SPIR-V is a span now, and SpirvTools::Validate takes a vector or a pointer and a size. (cherry picked from commit 759cc67) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * perf(agc): eliminate DCC register clear fallback during color metadata pass (cherry picked from commit 2061625) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * test(agc): keep the pixel LDS read-modify-write slots within 1 KiB Each pixel used its own 16-byte slot across 512 pixels, so the per-invocation LDS array held 8 KiB for a test that only needs one slot per lane of a wave. 64 slots and the counter right after them keep the array at 320 dwords, which software rasterizers compile in a fraction of the time. (cherry picked from commit 3b8bf7e) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * test(agc): take the LDS update check's words as a span Retained SPIR-V is a span on main since boykopovar#4806. (cherry picked from commit bfca92e) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * perf(agc): reserve resource keys for their serialized payload (cherry picked from commit 00fe89c) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * test(agc): pin that the content key reserves exactly its serialized word count (cherry picked from commit 6d7b1d1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * feat(libSceAgcDriver): support secondary LOD bias and relax unnormalized coordinate filters Fork: the updated unnormalized S# note moves from docs/dev/debt/libSceAgcDriver.md to docs/dev/TechnicalDebt.md. GuestSamplerResource.cpp is shared with Metal: MetalSampler now receives equal min/mag filters and clamp-to-edge for X/Y clamp mode 0 on unnormalized S#s, which its pixel-sampler contract accepts, so no Metal change is needed. (cherry picked from commit e1d402c) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * feat(agc): store pending storage-image results before each GPU label with APS5_STORE_AT_LABEL Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Fork: the debt note is updated in docs/dev/TechnicalDebt.md. Vulkan driver only: the label hook is in VulkanDevice::WriteLabelOnGpu; the Metal backend does not keep StorageTexture pending results. (cherry picked from commit d756f52) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * perf(agc): note an address space's in-place reads by reference Every address-based build copied the cached address space's imported ranges (about 9000 in Sword of the Sea gameplay) into its batch as pending reads, only for the copy HLE's reader check to scan them. The space now builds that list once, and a build that took the space whole notes it by reference (Recorder::NotePendingReadList); its own ranges follow as before. The space is immutable while its serial lives, the list equals the space's part of InPlaceReads in its order, and the batch scans plain reads and lists in note order, a list by bisection, so every PendingReadOverlaps and DescribePendingRead answer and reader kind stay the same. A capture trace or APS5_FLIP_READ_CHECK expands the list as before. Tests: lists noted by reference answer like the same ranges noted one by one (open and in-flight scans, kinds, a list noted twice, empty and null lists); the space's list is shared while the registry stands and rebuilt when a range is added or removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 48cdce9) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * docs(agc): keep one unnormalized S# note in TechnicalDebt.md TechnicalDebt.md already carried the unnormalized S# note that upstream e1d402c updates, so the updated note replaces it in place instead of being listed twice. The title named in the upstream note is left out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * test(agc): run ImageAddressDimension on the Metal driver Upstream 945b970 extends ImageAddressDimension with 2D-array loads and stores on 3D T#s, which the shared ResourceMaterializer now lowers as volume accesses. The test runs unchanged through the Metal driver semantics shim beside the other image addressing tests. ImageStoreArrayThroughVolume, ImageSampleUnnormalized and Uint8x4ColorTarget are not added: they use VulkanDevice members the shim does not provide. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj --------- Co-authored-by: TideOPL <obinewman101@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: AlxcBTW <187763517+AlxcBTW@users.noreply.github.com> Co-authored-by: Evgeniy Kazarin <evgkaz0@gmail.com> Co-authored-by: codingreapermaps <233954859+codingreapermaps@users.noreply.github.com> Co-authored-by: MidnightApora <midnightaporaismybro@gmail.com> Co-authored-by: Scramblrrrr <joey.purkis@gmail.com> Co-authored-by: Niko <nikobilger2000@gmail.com> Co-authored-by: raghav314 <samraghav2@gmail.com> Co-authored-by: Dean Galvin <dean.galvin@live.com> Co-authored-by: Kikoarl <7354010+Kikoarl@users.noreply.github.com> Co-authored-by: Adrià Franch <adriadam10@gmail.com> Co-authored-by: pushkarreddyy <212133856+pushkarreddyy@users.noreply.github.com> Co-authored-by: Sasha Papo <sasha.papo@gmail.com>
What
Replace this PR's opt-in temporary-file backing with read-only mappings of the shader disk-cache entries themselves.
APS5_FILE_BACKED_SPIRVand the extra temporary files are removed from the implementation.This adopts ThatsLeo's
perf/shader-spirv-on-diskimplementation (0bc9120bb), preserving its authorship, and adds the Windows mapping path. Cache-loaded variants retain a shared mapping owner; newly compiled variants are persisted to the normal cache and mapped back before retention. Reading or copying a module shares that storage; mutation detaches into a private vector.Words()returns a const span, and explicit vector consumers useCopy().Windows mappings share read, write and delete access, so atomic replacement/removal of a cache file does not invalidate its existing readers. Failed cache persistence/mapping and disabled caches retain the existing vector path. No dedicated backing-enable switch, temporary-volume dependency, memory-budget knob or cache-version bypass is introduced. This does not add cache eviction; normal cache files still use disk space.
The cache payload is aligned and its format advances from 30 to 31. Length, key and payload validation remain in force.
Tested
Windows 11 Pro 10.0.26300, GCC 15.2, Release, SPIRV-Tools OFF, current main
f87799aa3plus this PR alone.The cache regression verifies actual Windows
MEM_MAPPEDstorage, round-trip words/metadata, copy-on-write isolation, and retained word access after the original cache pathname is atomically replaced and removed. Existing cross-process, corruption, cache-invalidation, binding, vertex specialization and invocation-isolation cases also pass.Full Windows build passed on the final rebased branch. Four focused cache/shader/rendering checks passed on NVIDIA RTX 5070 / driver 617.14 in 6.81 s; graphics and prepared-shader checks passed on AMD Radeon integrated gfx1036 / driver 32.0.21036.18 in 5.26 s. The cache regression also exercises loading while a published writer retains write/rename access. Conventions and whitespace checks pass. Linux/macOS execution and full-suite CI are not claimed for this revised head.
The previous opt-in implementation's memory measurements are superseded and are not evidence for this implementation. The independent Demon's Souls report in #4806 (comment) applies to ThatsLeo's original POSIX branch: it reached the tutorial where its comparison run exhausted the memory cap. That report used a warm cache and test-only format/source-version overrides, which are not included here. It is external Linux title evidence, not a Windows title validation of this revised head. No new Windows title-memory or speed improvement is claimed yet.
Checklist
main; no equivalent open cache-mapping PR foundRelated: #5552 changes the fallback file reader. Its head �f2b86928 merges cleanly with this branch in a pairwise merge simulation; that simulation is not a combined build/test claim.