Skip to content

test(agc): give the storage image budget test a device-local heap as large as its reported budget - #11293

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
LeandroLP23:test/agc-storage-budget-small-device-heap
Oct 27, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
LeandroLP23:test/agc-storage-budget-small-device-heap

Conversation

@LeandroLP23

@LeandroLP23 LeandroLP23 commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

What

agc_driver_sampled_cache_budget_memory fails on a GPU whose largest device-local heap is small, such as an APU's 512 MiB carve-out. storageMemoryTests reports a 64 GiB VK_EXT_memory_budget budget on the largest device-local heap, but keeps that heap's real size. StorageTextureCeiling caps the storage image cache at half the heap, with a 512 MiB floor, so on a 512 MiB heap the cache budget is 512 MiB. Both checks then fail:

Check Expected Got on a 512 MiB heap
idle, 12 GiB in use (== half, from #10551) min(heap / 2, budget / 2) = 256 MiB 512 MiB (the floor)
under pressure, 55 GiB in use 1024 MiB + cached storage images 512 MiB (the ceiling)

The driver behaves as intended here: storageTextureBudgetTests in tests/Graphics.cpp keeps the 512 MiB floor and the half-heap cap. The test's reported budget is what doesn't fit the heap. The test now raises the largest device-local heap in its copy of the device's memory properties to at least the reported budget. Both expectations then hold whatever heap the device has. On a discrete GPU the heap it reports also becomes 64 GiB, and the two checks still expect half the budget (32 GiB) idle and the 1 GiB headroom result under pressure. The case where half the heap is below half the budget remains covered by the unit tests in tests/Graphics.cpp.

Tested

Windows 11 10.0.26300, AMD Ryzen AI 7 350 with Radeon 860M (largest device-local heap 512 MiB), AMD 24.30.58.08 (LLPC), WinLibs GCC 15.2, Release. Head 3fe0cd9b8b, on main 204c6747d0 (tested on 3d2b7c0673; Graphics/ and SampledCacheBudget.cpp are unchanged since).

The 15 tests of agc_driver_sampled_cache_budget_tests (ctest -R 'sampled_cache|resource_cache_|recover_'), on main 3d2b7c0673 and on this head, with VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation (Vulkan SDK 1.4.363.0), ANYPS5_NO_SHADER_CACHE=1 and ANYPS5_REQUIRE_VULKAN=1 via ctest -V:

main 3d2b7c0673 this head
passed 14 of 15 15 of 15
agc_driver_sampled_cache_budget_memory fails: "52 GiB free of a 64 GiB budget does not leave the storage image cache half of the heap or of the budget" passes
VUID- lines 0 0
was not found but was requested by env var VK_INSTANCE_LAYERS 0 0
Insert instance layer "VK_LAYER_KHRONOS_validation" with VK_LOADER_DEBUG=all (_memory run) yes yes

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
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: AMD Radeon 860M (integrated, 512 MiB device-local heap), AMD 24.30.58.08 (LLPC), Windows 11 10.0.26300
  • AI-assisted: yes (Claude Code)

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the one-line fixture change. Raising only the copied test context's selected heap makes its synthetic 64 GiB budget internally consistent; I did not find a code defect in that adjustment.

I cannot approve this head yet because the hardware comparison in the report uses main at 47f9cee2cd, while current main is ef1ddae699cb. Since that tested base, main gained AGC changes, including dbda360df4 (depth-bounds emulation) and its focused tests. The PR's product/GPU jobs are skipped, so there is no hosted run against current main. CONTRIBUTING requires AGC changes to run the same focused tests on main and the change with the validation layer and to compare validation output.

Please update the branch to current main and rerun the same agc_driver_sampled_cache_budget* focused tests on both current main and the updated head on the Radeon 860M, with VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, ANYPS5_NO_SHADER_CACHE=1, and ANYPS5_REQUIRE_VULKAN=1. Include the paired results and confirm both logs loaded the requested layer and the change adds no VUID- lines. This review environment has no Vulkan device, so I could not independently repeat the hardware run.

@github-actions

github-actions Bot commented Oct 26, 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 5021 core/libs/prx/libSceAgcDriver/tests/execution/SampledCacheBudget.cpp
PR 9012 core/libs/prx/libSceAgcDriver/tests/execution/SampledCacheBudget.cpp
PR 9508 core/libs/prx/libSceAgcDriver/tests/execution/SampledCacheBudget.cpp

@github-actions github-actions Bot added the agc libSceAgc* label Oct 26, 2026
@LeandroLP23
LeandroLP23 force-pushed the test/agc-storage-budget-small-device-heap branch from b499fc2 to b6c323d Compare October 26, 2026 07:34
@LeandroLP23

Copy link
Copy Markdown
Contributor Author

@MarcZX10 thanks. Rebased onto main 3d2b7c0673 (head b6c323df8a, the same one-line change) and reran on the Radeon 860M (AMD 24.30.58.08, LLPC, Windows 11 10.0.26300, Vulkan SDK 1.4.363.0).

All 15 tests of agc_driver_sampled_cache_budget_tests (ctest -V -R 'sampled_cache|resource_cache_|recover_'), on main 3d2b7c0673 and on this head, with VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, ANYPS5_NO_SHADER_CACHE=1 and ANYPS5_REQUIRE_VULKAN=1:

main 3d2b7c0673 this head
passed 14 of 15 15 of 15
agc_driver_sampled_cache_budget_memory fails: "52 GiB free of a 64 GiB budget does not leave the storage image cache half of the heap or of the budget" passes
VUID- lines 0 0
was not found but was requested by env var VK_INSTANCE_LAYERS 0 0

To confirm both runs loaded the layer, agc_driver_sampled_cache_budget_memory was run again through ctest -V with VK_LOADER_DEBUG=all on each build. Both logs show Insert instance layer "VK_LAYER_KHRONOS_validation", and neither has a VUID- line. No test was skipped on either side. The PR description is updated with these results.

AI-assisted: yes (Claude Code)

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your 07:35 update satisfies the earlier AGC validation request on the Radeon 860M: the focused baseline/change matrix is 14/15 versus 15/15, both validation layers were confirmed loaded, and neither run added VUIDs. I checked current sync: head b6c323df8a93a040b6dd90c844efedad052bc77b is 1 ahead / 42 behind current main 204c6747d0a1cbaf932def3dea0778810cc3935e, with a clean merge preview. The libSceAgcDriver/Graphics tree and this test file have not changed since the tested base 3d2b7c06, so the reported test results still cover the relevant code. Please bring the branch up to current main before merge; a repeat GPU run is only needed if the rebase changes relevant code.

…large as its reported budget

agc_driver_sampled_cache_budget_memory reports a 64 GiB budget on the
largest device-local heap but kept the heap's real size. On an APU that
heap is 512 MiB, so the storage image cache ceiling is the 512 MiB
floor, and both checks failed: half of the heap or of the budget
(256 MiB) and the budget under pressure (1024 MiB). The test now raises
the heap it reports to at least the reported budget, so its
expectations hold whatever heap the device has.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@LeandroLP23
LeandroLP23 force-pushed the test/agc-storage-budget-small-device-heap branch from b6c323d to 3fe0cd9 Compare October 26, 2026 08:13
@LeandroLP23

Copy link
Copy Markdown
Contributor Author

@MarcZX10 rebased onto current main 204c6747d0; head is now 3fe0cd9b8b, the same one-line change. Between the tested base 3d2b7c0673 and 204c6747d0, libSceAgcDriver only changed in the frame recorder (#11197: Execution/src/FrameRecorder.cpp, FrameRecorder.hpp, Driver/EntryPoints.cpp, tests/FrameReplay.cpp and the test registration in CMakeLists.txt). Nothing changed in Graphics/ or in SampledCacheBudget.cpp, which the storage image budget tests exercise, so I didn't repeat the GPU run. check_conventions.py --base origin/main passes.

AI-assisted: yes (Claude Code)

@SeaSunPT

Copy link
Copy Markdown
Contributor

A discrete-GPU run, since the description notes the reported heap also changes there.

Windows 11 Pro 26200, Intel Core i5-14600K, NVIDIA GeForce RTX 5070 Ti (16 GiB, driver 610.88), WinLibs GCC 15.2.0 r7, Release. Head 3fe0cd9b8 merged onto main cbaacd2910 (clean), compared with that main.

ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation ctest --test-dir build -R "sampled_cache|resource_cache_|recover_" -V, 3 runs each:

  • This PR: 15 of 15 pass every run, agc_driver_sampled_cache_budget_memory included.
  • main: 15 of 15 pass every run, as expected with a heap this large.
  • 0 VUID- lines and no missing-layer line in any log. The validation layer is Khronos vulkan-sdk-1.4.363.0, built from source with the same GCC (mimalloc off).

Full ctest -j 8 with ANYPS5_REQUIRE_VULKAN=1: 2450 of 2452 pass. The two failures, agc_host_import_cut_lifetime (#10746) and agc_driver_depth_plane_partial_store (#10664), fail on main on this machine too.

@adriadam10 adriadam10 self-assigned this Oct 27, 2026
@adriadam10
adriadam10 dismissed MarcZX10’s stale review October 27, 2026 08:13

Dismissing a stale review: the requested changes were addressed after it (new head) and the merge queue blocks on it. Please request changes again if anything is still open.

@adriadam10
adriadam10 merged commit c547dea into boykopovar:main Oct 27, 2026
12 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the one-line fixture change against StorageTextureBudget (min(heap / 2, budget / 2)): raising the copied heap size to the reported budget makes both expectations hold on any heap, and the driver is untouched. The 860M (14/15 → 15/15) and RTX 5070 Ti (15/15 both) runs cover small and discrete heaps with no new VUIDs; merged.

@LeandroLP23
LeandroLP23 deleted the test/agc-storage-budget-small-device-heap branch October 27, 2026 16:16
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.

4 participants