Repository navigation
build(agc): share a precompiled header across the execution tests - #10164
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed head 3fe7aaf against the current main. The change is scoped to the execution-test target loop, and the PR reports measured rebuild, ccache-hit, full build, and AGC test results with the three known AMD failures and three skips clearly called out. I also confirmed the PR checks: conventions, eligibility, intake, labels, and report pass; full Linux/Windows build jobs are skipped by the repository PR policy. No blocking issue found in the patch. AI assistance is disclosed in the checklist.
|
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.
|
THEMANnt
left a comment
THEMANnt
left a comment
There was a problem hiding this comment.
Reviewed head 3fe7aaf3b4ca738f1dca58073d6897f6206012a6. No actionable findings in this focused shared-PCH build change.
The first generated execution-test target owns the PCH and subsequent targets reuse it. The existing loop applies common includes, linked dependencies and language settings before selecting reuse. The ccache launcher branch supplies the required PCH sloppiness settings through cmake -E env, preserving the configured launcher command.
Validation: Windows/MinGW GCC 15.2.0: exercised the exact changed loop with three representative executable targets and a small stand-in header. One .gch is generated and all three targets build/run, with invalid-PCH warnings treated as errors. A selected-test configuration also builds one shared PCH: the selected target exits 0 and both excluded-test stubs exit the expected 77. A ccache configuration verifies the generated launcher property contains CCACHE_SLOPPINESS=pch_defines,time_macros. Conventions and whitespace pass. Hosted relinker/ASan and policy checks pass; product jobs are skipped.
Scope: CMake/PCH mechanics and selected-test behavior, not compilation of the actual Vulkan header graph or all AGC fixtures. Full-driver build, real ccache hit rates, GPU execution and the reported speedup were not independently measured.
What
#10163: each AGC execution test object spends most of its compile time parsing the same headers. 463 of the 478
tests/executionsources includeVulkanTestDevice.hpp, which pulls inVulkanDevice.hpp,Draw.hpp,Recompiler.hppand about 35 more project headers.The loop that defines the execution tests now gives them one shared precompiled header. The first test builds it from
tests/execution/VulkanTestDevice.hpp,<array>,<iostream>,<span>,<string>and<vector>. The other tests reuse it throughtarget_precompile_headers(... REUSE_FROM ...). They are all defined in this loop with the same include directories, flags andCXX_EXTENSIONS OFF, so one PCH fits every one. No source file changes. CMake compiles with-Winvalid-pch, so a PCH that does not match a target is reported rather than silently skipped.When
CMAKE_CXX_COMPILER_LAUNCHERis ccache, these targets run it throughcmake -E env CCACHE_SLOPPINESS=pch_defines,time_macros. Without that setting ccache refuses every object built with a PCH ("Could not use precompiled header": 2 of 2 calls uncacheable here). That would make the CI and local caches miss on all 478 objects.cmake -E envworks with any ccache version. Theccache key=valuecommand-line form needs 4.8 or later.Tested
Ryzen AI 7 350, Radeon 860M (AMD driver 32.0.13058.8001), Windows 11 26300, WinLibs GCC 15.2.0, Release,
BUILD_TESTING=ON. Built onmaind339082d2e, same build directory for both sides. The branch is the same change onmainc928aeb0a2: thelibSceAgcDriver/CMakeLists.txtcommits in between add tests to lists and do not touch the execution test loop.Rebuild after a header edit. Touch
tests/execution/VulkanTestDevice.hpp, then build everytests/executionobject with ccache disabled, at-j 4, below-normal priority:mainThat is 49% less time for the same objects. All 478 now depend on the PCH, so the 15 sources that did not include
VulkanTestDevice.hppalso recompile when it changes. They are counted above.Why the PCH helps: five sources taken at even intervals from the target list, each compiled alone on an idle machine, parse in 2.1 to 4.6 s with
-fsyntax-onlyand compile in 2.4 to 4.1 s at-O3. Parsing is most of each compile, and-O1is no faster.ccache. After a populated build, I deleted 40 execution test objects and rebuilt them: 40 of 40 were direct ccache hits, in 4.9 s.
Tests.
cmake --build buildbuilds cleanly. One PCH is built (199 MB). Recompiling two objects withninja -vprints no PCH warning. WithANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1,ctest -R '^agc_' -j 4passes 939 of 942. The 3 failures areagc_driver_sample_order,agc_driver_sample_order_grid_referenceandagc_driver_sample_order_recipe_depth_locations, the AMD Windows failure of #9507. They fail the same way without this change. The 3 skipped tests (agc_host_import_sparse,agc_driver_ngg_vertex_wave_reduction,agc_driver_sparse_dma_buf_import) also skip onmainon this machine. The validation layer is not installed here. The change only affects how these objects compile, not what they record.Not run here: Linux and macOS. GCC and Clang both support
target_precompile_headerswithREUSE_FROM, and CI builds both.tools/check_conventions.py --base origin/mainpasses.tools/pr_overlap.py --branchreports no conflicting lines. 60 open PRs that conflict withmainshare thisCMakeLists.txt. Seven of them edit near the execution test loop: #6169, #6788, #7642, #8014, #8874, #9508 and #9892. Their changes there are to theforeach(agcTest IN ITEMS ...)list below it, not to the lines this PR adds.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)