Skip to content

fix(agc): reuse a recycled descriptor set only for a table of its own size - #6567

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
SleglJan:pr/rewritten-set-variable-count
Oct 19, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
SleglJan:pr/rewritten-set-variable-count

Conversation

@SleglJan

@SleglJan SleglJan commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

What

DescriptorCache::AllocateRewritten handed out any recycled set of the requested layout. PrepareDrawBindings takes that path for the snapshot sets of recorded draws (every draw unless APS5_SYNC_DRAWS is set).

  • The variable descriptor count of an image-table binding is fixed when its set is allocated.
  • PrepareDrawBindings writes the table's present elements, up to its current element count, into whichever recycled set it gets.
  • A draw whose table has more elements than the table the recycled set was allocated for can write descriptors past the set's allocation. That is undefined behaviour in Vulkan, and the validation layer reports it as VUID-VkWriteDescriptorSet-dstArrayElement-00321.

A set from a chain pool now records the variable count it was allocated with, as a dedicated-pool set already does since #6454. Recycled sets are kept per layout and variable count, so a draw reuses a set only for a table of the same size, and allocates otherwise.

  • A recycled set is reused only on an exact count match, not for a smaller table.
  • The sets kept for reuse are now counted per layout and table size, instead of per layout.

Tested

Linux 7.2.10 (CachyOS), RTX 3080 with NVIDIA 615.78.08, GCC 16.2.1, Release, BUILD_TESTING=ON.

Related

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt; none added
  • Unimplemented paths throw; no stubs
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source: n/a
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: none
  • GPUs tested: NVIDIA GeForce RTX 3080, driver 615.78.08, Linux 7.2.10 (CachyOS)
  • AI-assisted: yes (Claude Code)

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

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.

Pull request Shared files not compared
PR 1395 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 4040 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 5441 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 5499 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 5500 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 5925 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6146 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6177 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 6188 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6192 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 6194 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6202 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 6223 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 6235 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6292 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 6340 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 6548 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6563 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6612 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6615 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 6639 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6647 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6683 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6718 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 6762 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6801 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6808 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6820 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 6937 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 6946 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 6947 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 6970 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 6972 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 7002 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 7085 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 7154 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 7254 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 7256 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 7258 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 7332 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 7340 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 7390 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 7410 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
PR 7484 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 7618 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 7642 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 7650 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
PR 7689 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp

@fma-rc

fma-rc commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Demon's Souls (PPSA01341) reaches this. On main 5d9dcc4 it crashes on the driver thread in 4 of 6 runs once it reaches character creation, as a SIGSEGV in the decode memo walk or double free or corruption, so something was overwriting the heap.

I ran it with a small LD_PRELOAD allocator that ends every 0x1f0..0x210-byte block at a PROT_NONE page. It runs slower that way, and both runs on main faulted earlier, in the same scene of the opening cinematic (~1360 s): a store in libnvidia-glcore 615.78.08 16 bytes past a 0x208-byte block, inside vkUpdateDescriptorSets called from ShaderResources::PrepareDrawBindings (from recordDraw / Graphics::Draw). That is the write past a recycled set's variable count you describe.

With this PR on the same main, two runs with the same steps went through that scene to the end (1550 s) with no fault. agc_driver_graphics passes on an RTX 5080 (NVIDIA 615.78.08), a Radeon 610M (RADV) and lavapipe.

@SleglJan

Copy link
Copy Markdown
Contributor Author

Thanks @fma-rc, that's the first title reported to reach this, and the guard-page runs pin it to the same write. I added your result to Tested.

AI-assisted: yes (Claude Code)

@fdfawef324

Copy link
Copy Markdown
Contributor

A Windows data point to go with fma-rc's: Demon's Souls (PPSA01342), RTX 3060, NVIDIA 591.86, Windows Server 2025 (26100), MinGW-w64 GCC 15.2.0, Release. The per-run table is on #6537. Every run with the fix had both this PR and #6537 cherry-picked (patch-identical to 544e7af and 9800f3a), so these runs can't say which one matters for this title.

Stack: main a5e2a80 + other PRs and a local change (listed on #6537) Runs Result
without #6537 / #6567 6 all died between 190 s and 1491 s: 4× 0xC0000374 (heap corruption), 2× access violation
with both 4 2 ran to the 3000 s limit with no fatal, in-game; 1 texture out-of-memory at 683 s (#6481 now faults such an entry); 1 VK_ERROR_DEVICE_LOST at 778 s (still looking into it)
  • One access violation (d4) is a write fault inside nvoglv64.dll, at the same call as fma-rc's guard-page fault. The first libSceAgcDriver return address on the stack is ShaderResources::PrepareDrawBindings+0x1df9, right after its only vkUpdateDescriptorSets call. This PR's cause and fix(agc): keep each set's image table writes in its own draw snapshot… #6537's both write through that call, so the site doesn't pick between them.
  • The other access violation (d1) is a read fault in VulkanDevice::PrepareDispatch; we can't tie it to either PR.
  • The crashes without the PRs don't look like commit running out. Windows logged no low-memory event (2004) during any of these runs. Commit stayed at least 41.7 GB free until the last 25 s before each crash, and in those last 25 s it fell as low as 15.2 GB.

AI-assisted: yes

@SleglJan

Copy link
Copy Markdown
Contributor Author

Thanks @fdfawef324, a second platform and driver on Demon's Souls: with #6537 and this PR, 2 of 4 runs reached your 3000 s limit, where all 6 without died. I added your result to Tested, with your note that these runs can't tell the two apart.

AI-assisted: yes (Claude Code)

@fma-rc

fma-rc commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Death Stranding 2 (PPSA02015) reaches this too, Linux, RTX 5080. On my DS2 stack without this PR, runs died on glibc heap errors (corrupted double-linked list at 470 s, munmap_chunk(): invalid pointer at 390 s with MALLOC_CHECK_). With the PR on the same stack, three runs went 460, 536 and 1280 s and none hit a heap error. They ended on device memory (#6890) and later stops.

The test also holds on current main 55b968a, RADV on a 680M: main plus only your test hunk fails 3/3 with a recycled descriptor set served a larger variable image table than it was allocated for, main plus the PR passes 3/3, main alone 3/3.

@SleglJan

Copy link
Copy Markdown
Contributor Author

Thanks @fma-rc. Death Stranding 2 is the second game reported to reach this, after Demon's Souls, and the RADV runs show that the test holds on current main too. I've added both to Tested.

AI-assisted: yes (Claude Code)

@fisherjoey

Copy link
Copy Markdown
Contributor

Demon's Souls (PPSA01341 v1.07) on Linux hits this too. Here the base has neither #6537 nor this PR, so the runs separate the two causes.

Setup: Nobara 44 (kernel 7.2.0), RTX 5070 Ti, NVIDIA 595.91.07, GCC 16.2.1, Release. The base is main 93e54ce plus #6280's driver-side commit b9badf4 and a local one-file stencil patch that doesn't touch descriptors. That main predates #6537 (merged 2026-10-18). Every run used the same route (title, character creation, the first tutorial level), the same warm shader cache and the same stop at 900 s, with no compiler running on the host.

Without the change, 5 of 5 runs died in the level between 366 s and 680 s on glibc heap checks: corrupted size vs. prev_size twice (680 s, 452 s), free(): invalid pointer once (366 s) and SIGSEGV twice (415 s, 393 s). In 4 of the 5, the frame content stopped changing first (at about 275 s in three runs, 366 s in one) while [present] and the draw counters kept going, and the abort came 85 to 140 s later. Four of the five runs carried one extra change each (no b9badf4 in one, then APS5_STORE_AT_FLIP=1, #6926 or #6931 cherry-picked). None of them changed the outcome.

With this PR's commit 544e7af cherry-picked onto that base, and only libSceAgcDriver.prx swapped, the run reached the 903 s stop and exited 0: no heap message, no core, no device loss. A patch of ours with the same ShaderResources.hpp/.cpp hunks, written from the same vkUpdateDescriptorSets write before we found this PR, did the same in 3 of 3 runs (two crash checks and one profiled run, 900 s each). So on this base it's 4 of 4 clean with the fix and 5 of 5 crashed without it, same night, same procedure. The level frame rate was 0.80 fps median in both arms (the title is CPU-bound here), so the change costs nothing we can measure.

Your agc_descriptor_cache case passes on lavapipe (Mesa 26.2.1 llvmpipe) with 544e7af cherry-picked onto main ccb0c09, along with agc_driver_graphics, agc_descriptor_pool_reset_failure and agc_compute_descriptor_layout_flags.

AI-assisted: yes (Claude Code)

@SleglJan

Copy link
Copy Markdown
Contributor Author

Thanks @fisherjoey, I've added your runs to Tested. Like fma-rc's earlier Demon's Souls runs, yours are on a base without #6537, so the fix arm has this change and not #6537's. The Windows runs had both and couldn't tell them apart.

AI-assisted: yes (Claude Code)

@fisherjoey

Copy link
Copy Markdown
Contributor

#6454 has landed on main, so this is now the "whichever lands second" case from the description. The bug is still there on main 40393b0: SetAllocation has std::optional<std::uint32_t> variableCount and the new kept-set path compares it, but recycled is still keyed by layout alone, and the two chain-pool returns in Allocate leave variableCount empty. A recycled set can still go to a draw whose table is larger than the one it was allocated for.

Here is how we resolved it on our local stack, keeping main's struct:

  • recycled becomes std::map<std::pair<VkDescriptorSetLayout, std::uint32_t>, std::vector<SetAllocation>>.
  • AllocateRewritten looks up {layout, variableCount.value_or(0u)}, and Recycle files the set under {layout, allocation.variableCount.value_or(0u)}.
  • Both chain-pool returns in Allocate fill in the count: return {set, pool.handle, index - 1, updateAfterBind, VK_NULL_HANDLE, {}, variableCount, {}}; and the same for chain.size() - 1. The dedicated-pool return on main already records it.

With that on main 40393b0 (plus #6280 and #6926, which don't touch descriptors), lavapipe passes 1535 of 1536 tests. The one failure is application_heap_lazy, which also failed on our previous stack. We haven't run Demon's Souls on this rebased build yet, so the game result in my earlier comment is from the older base.

AI-assisted: yes

… size

DescriptorCache::AllocateRewritten handed out any recycled set of the requested layout. The
variable descriptor count of an image-table binding is fixed when its set is allocated, and
PrepareDrawBindings writes the table's present elements, up to its current element count, into
the set it gets. A draw whose table had more elements than the table the recycled set was
allocated for could write descriptors past the set's allocation
(VUID-VkWriteDescriptorSet-dstArrayElement-00321).

A set from a chain pool now records the variable count it was allocated with, as a dedicated-pool
set already does, and recycled sets are kept per layout and variable count, so a rewritten draw
reuses a set only for a table of the same size.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@SleglJan
SleglJan force-pushed the pr/rewritten-set-variable-count branch from 544e7af to 04aef89 Compare October 19, 2026 02:59
@SleglJan

Copy link
Copy Markdown
Contributor Author

@fisherjoey thanks. The rebase is pushed as 04aef89 on main 8a526d4, and it is the resolution you describe:

  • main's std::optional variableCount stays;
  • recycled is keyed by layout and the set's variableCount.value_or(0u) in both AllocateRewritten and Recycle;
  • the two chain-pool returns in Allocate now fill in the count.

On a Linux container with lavapipe, main 8a526d4 plus only the test hunk fails agc_driver_graphics and agc_descriptor_cache with a recycled descriptor set served a larger variable image table than it was allocated for. Both pass with the change. With the change, the wider agc|shader|recompiler|request|tess set fails the same 18 of 765 tests as main built the same way. Details are in the description.

AI-assisted: yes (Claude Code)

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 19, 2026
@adriadam10 adriadam10 self-assigned this Oct 19, 2026
@adriadam10
adriadam10 merged commit fdf64db into boykopovar:main Oct 19, 2026
11 of 12 checks passed
@Capmare

Capmare commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Confirming this on a title: Marvel's Wolverine (PPSA03671), Linux, NVIDIA GeForce RTX 4070 SUPER (driver 615.71.09), on a tree with #7395. Before this change the game died at about present 580, during the first build of an image table with 12,447 entries. The host heap was corrupted, and the crash site changed from run to run. With VK_LAYER_KHRONOS_validation the cause shows as VUID-VkWriteDescriptorSet-dstArrayElement-00321: pDescriptorWrites[106].dstArrayElement (12438) + descriptorCount (1) is larger than pDescriptorCounts (12435) for a recycled set.

I found it independently and made the same change locally: the recycled sets are keyed by layout and variable count. With it, 6 of 6 runs of 240 seconds survive. Before, 6 of 6 crashed at about 110 seconds. So this PR fixes the crash that stops Wolverine there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants