Repository navigation
test(agc): run the recorder tests on AMD and Intel GPUs - #1210
Conversation
importWindowTests pinned watched imports even where the driver decides to compare them. On amdgpu (RADV) and Intel (ANV) the import probe finds the pages written after a GPU access, so the driver never watches imports there; under the pinned watch the storage image keeps the CPU's blocks and the test fails. It fails the same way at d8c7c0c, where it was added. It now reports itself as not run when imports are compared, like the unit shadow test. minLodTests expected MIN_LOD 1.5 to blend levels 1 and 2. With VK_EXT_image_view_min_lod the view minimum LOD is minLodFloat (preferred) or minLodInteger (alternative); ANV takes the integer part and reads level 1. Either result is accepted and the alternative is reported. The test device ranks Vulkan devices as VulkanDevice does instead of taking the first one, so on a host with an iGPU it runs on the same GPU as the runtime. Like VulkanDevice, it destroys the unit shadows before the device: a test that failed with live slabs segfaulted at exit when the registry freed them after vkDestroyDevice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Dextroid17
left a comment
There was a problem hiding this comment.
Built and ran it. Note the target is not registered as a CTest test on this branch — registration comes from #1046, as your description says — so I ran the binary directly rather than through ctest.
Linux (CachyOS, GCC, Release), NVIDIA RTX 3080 Ti (discrete, the only device here):
cmake --build build --target agc_driver_recorder_tests --parallel 16
APS5_WRITE_WATCH_IMPORTS=watch ./build/tests/agc_driver_recorder_tests
→ "Recorder read tracking and label tests passed", rc=0
The device ranking matches the driver's, including on the edge cases. rank <= selectedRank → continue means the first enumerated device wins a tie, which is what VulkanDevice does when it walks the device list, and a device below 1.1 is skipped before ranking so it cannot be chosen and then fail later in the shared helper. With every candidate skipped, Require(context.physical != VK_NULL_HANDLE, "no Vulkan 1.1 device") gives a real message instead of a null dereference further down. The default arm ranks OTHER and CPU at 0 rather than excluding them, so a machine with only such a device still runs instead of requiring a discrete GPU — consistent with the rest of the suite.
DestroyShadows before vkDestroyDevice is the most valuable line in the diff. Turning a failure into a segfault is worse than the failure: it destroys the assertion message that says what went wrong, and a reviewer cannot tell a real crash from a leak at exit. Calling it on the same path VulkanDevice does keeps the two in step.
The LOD change is the right diagnosis with one thing left on the table. VK_EXT_image_view_min_lod lets a device take minLodInteger = ⌊minLod⌋, so ANV reading level 1 for a 1.5 minimum LOD is permitted behaviour and the old expectation was device-specific. Accepting it is correct. But the accepting branch only prints; it does not assert. On a device that takes the alternative the value is still deterministic — you quote 0.250980 = 64/255 on ANV, which is unorm(1) — so the branch could assert that instead of reporting and moving on, and the case would keep its coverage on both kinds of device rather than being unverified on one. expectRed(fractional, unorm(1), "minimum LOD clamp: the device reads the integer part of a fractional view minimum LOD") would do it. Reporting which behaviour was taken is good, but assert and report is better than report only.
Minor: the title undersells the change — it also fixes the segfault-on-failure and the LOD assumption, both of which a reader would care about more than the device-enumeration line. Not a problem for the one-topic rule, since it is all "make this suite device-independent"; just worth a title that matches.
What I could not verify: anything AMD or Intel. I have one NVIDIA device, so the RADV/ANV behaviours your table covers — the import probe deciding to compare rather than watch, and the integer view minimum LOD — are not exercised here. My run shows that the selection, the teardown and the LOD tolerance leave the NVIDIA path working, not that the AMD and Intel paths now pass.
This is a comment, not an approval — I have no merge rights here.
Written with AI assistance.
What
#1046 registers
agc_driver_recorder_testsand makes it pass on lavapipe. On amdgpu and Intel the suite still fails in two tests that assume one GPU's behaviour, and on any device a failure with live unit shadows ends in a segfault at exit. This fixes those three in the test file only; no driver code changes.importWindowTestspinsImportWatch::Watcheven where the driver decides to compare imports. On RADV and ANV the import probe finds pages written after a GPU access, so the driver never watches imports there. Under the pinned watch the storage image logsguest memory changed while GPU results were pending; keeping the CPU's blocksand the test fails with(w) results pending over the import window did not reach guest memory. It fails the same way at d8c7c0c, where it was added, on both GPUs, so this is not a regression. It now reportshost imports are compared, not watched: the import window not tested, as the unit shadow test does.minLodTestsexpectsMIN_LOD1.5 to blend levels 1 and 2. WithVK_EXT_image_view_min_lod, the view minimum LOD isminLodFloat(preferred) orminLodInteger= ⌊minLod⌋ (alternative); see Image Level(s) Selection in the Textures chapter. ANV takes the alternative and reads level 1 (0.250980 = 64/255). Either result is accepted and the alternative is reported.VulkanDevicedoes instead of takingdevices.front()(same change as test(agc): run agc_driver_bda_device_tests under ctest #1187 for the BDA test). LikeVulkanDevice, it callsDestroyShadowsbeforevkDestroyDevice. Without it, a test that throws with live slabs makes the static shadow registry free them after the device is gone, and the failure shows up as a segfault instead of the assertion message (seen on lavapipe onmain, whereunitShadowTestsfails).With imports compared on lavapipe after #1046, the import window test is not run in CI either. Under the pinned watch it passed there, but that is a configuration the driver no longer picks on lavapipe.
Tested
Linux, Ubuntu 24.04.5, GCC 13.3, Mesa 25.2.8; i3-12100 with RX 7600 (RADV) and UHD 730 (ANV).
agc_driver_recorderwith #1046 and this PR,ANYPS5_REQUIRE_VULKAN=1,--repeat until-fail:3:GALLIUM_OVERRIDE_CPU_CAPS=avx)On
mainwith this PR alone, lavapipe still fails inunitShadowTests(fixed by #1046), now with exit code 1 and the assertion message instead of a segfault, 3 of 3 runs.No title was run.
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code