Repository navigation
fix(agc): take a unified heap as the host heap for the import budget - #11600
oneandonlydean merged 1 commit into
Conversation
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
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.
|
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed head 269fb645e30d2ba243f566ff0ac29c1d714fbad9. The fallback only runs when no non-device-local heap exists, and selects a valid heap referenced by a host-visible, host-coherent memory type. The tests cover a unified heap under usage, live-import subtraction, missing budget report and absent coherent type, plus a discrete device that must keep its separate host heap; the expected 64 MiB rounding matches HostImportBudgetFor. I checked the notable #6548 overlap: it selects a coherent memory type for an actual host import, while this PR selects the heap whose budget constrains imports; these are distinct decisions. The PR reports the mocked heap tests and a macOS/MoltenVK title run; I did not rerun the tests locally. Hosted eligibility, conventions, intake, labels and report pass; product/relinker builds are skipped by path policy. No blocking issue found.
jotitadev
left a comment
There was a problem hiding this comment.
Linux run, including the main run that macOS couldn't do.
Linux run: Ubuntu 26.04, GCC 15.2, Release. RTX 3070 (NVIDIA 595.99.02) and lavapipe (Mesa 26.0.3), one at a time via VK_DRIVER_FILES. Khronos validation layer 1.4.341 with ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1; VK_LOADER_DEBUG=layer shows the layer inserted, no "was not found but was requested", 0 VUID- lines.
Base main 808a557ac; the merge of 269fb645e is clean.
- Red:
main+ only this PR'stests/Graphics.cpp:agc_driver_graphicsfails witha unified heap used by other allocations did not lower the import budget. - Green:
agc_driver_graphics,agc_host_import*andagc_driver_host_import*(11 tests) pass 3/3 on each device. 0VUID-. Both devices have a separate host heap, so this is the discrete path. ^agcsuite (no flip/display/lds64,-j1): 3070 1052/1052, lavapipe 1051/1052. The one failure isagc_driver_multisample_retirement, which also fails onmain.- Builds on Linux/GCC;
public_headerspasses.
GPUs tested: NVIDIA RTX 3070 (Linux), lavapipe.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
Windows run of
A data point for integrated GPUs on Windows: AMD's driver doesn't report this iGPU (Ryzen 9 7900, AI-assisted: yes (Claude Code) |
|
Windows + NVIDIA result for head This PR's own tests: Host: Windows 11 26300, Intel i7-12700K, NVIDIA GeForce RTX 2070 SUPER (vendor 0x10de, device 0x1e84; Vulkan loader 1.4.357 from the NVIDIA driver 621.01), Khronos Validation Layers nightly 1.4.366 (b2b44c0d37) through Same focused run on
The tree also carried the open PRs #11948, #12062, #12132, #12070, #11680, #11679, #11678, #11677, #11675, #11674, #11605, #11604, all merged cleanly, so the run is a joint one. Not run on a PS5. AI-assisted: yes (Claude Code), the runs and the numbers above are from this machine. |
✅ NVIDIA GPU test: passedPR head
Intermittent validation messages (1, seen once in the parallel run, not reproduced when the test was re-run alone 2 times; not counted against this PR)Unstable on main (3, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (4, not counted against this PR)Validation IDs already present on main (5 IDs in 476 tests)Skipped (44)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
What
VK_EXT_memory_budgetfeedback for host imports looks at the host heap.hostHeapIndexfinds it as the largest heap that is notDEVICE_LOCAL. A device with one unified heap (Apple silicon with MoltenVK or KosmicKrisp) has no such heap. The function returned no heap, so the budget feedback never applied and the import budget stayed at the configured value (max(6 GiB, physical / 2), 12 GiB on a 24 GiB Mac).When no heap without
DEVICE_LOCALexists,hostHeapIndexnow takes the largest heap that has aHOST_VISIBLE | HOST_COHERENTmemory type. A discrete GPU keeps its separate host heap. A heap with no host-coherent type is still not a host heap.HostImportBudgetandeffectiveBudgetare not changed.Tested
unifiedHeapImportBudgetTests(agc_driver_graphics_tests), with mocked heaps:mainand passes with the change.host heap 0 reports a VK_EXT_memory_budget budget of 18186 MiB and a usage of 9643 MiB, and imports were held to 11200 MiB. This line did not appear before the change. MoltenVK counts the imports in the heap usage.ANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation(layer v1.4.363, x86-64, MoltenVK). The log has nowas not found but was requested by env var VK_INSTANCE_LAYERSand noVUID-line.agc_driver_graphics_testscreates no Vulkan instance (VK_LOADER_DEBUG=allprints nothing), so the layer did not check any call. The change adds no Vulkan call.main(maindoes not build on macOS).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)mesh/nativebuild, Add VK_EXT_mesh_shader support KhronosGroup/MoltenVK#2842), macOS 27.0. The validation layer was enabled but the test makes no Vulkan call (see Tested).🤖 Generated with Claude Code