Repository navigation
Conversation
…measured The sizes are those of libstdc++ with std::atomic<std::shared_ptr>, which GCC 12 added. On GCC 11's libstdc++ AtomicSharedPtr falls back to a mutex and a shared_ptr, so RecompileResult's draw key memo is 56 bytes instead of 16 and the build stopped at the RecompileResult check, although EncodeResult never writes the memo and the stored format is the same. The round-trip test now checks that the memo is not stored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
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.
The compile-time size guard now runs only where its measured libstdc++ layout applies, and still enforces the same sizes for std::atomic<std::shared_ptr> builds. The added round-trip test verifies that a populated process-local draw-key memo is neither serialized nor restored. The reported GCC 11/Windows runs and all current hosted build/relinker checks pass. No blocking issue found.
|
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 #12298, #12272, #12262, #12256, #12234, #12228, #12211, #12210, #12209, #12208, #12207, 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. |
|
@maskym Recorded your Windows/NVIDIA comparison for This is your external test evidence, including the other PRs listed in that joint tree; I have not rerun that configuration or attributed its full result solely to this patch. The PR's AI-assisted: Codex. |
|
Manual follow-up to the skipped comparisons in the overlap report (comment6461320271). Target head: Reviewed the listed shared-file patches for all 20 candidates, including refreshed #9852 at The important merge seam is the platform/sizeof assertion block shared with #5609/#5925/#6412/#6970/#7642/#9356/#9387/#9508/#9852/#10049/#10193. Preserve each candidate's correct size values within the new atomic_shared_ptr guard; retain field-count and encode/decode checks outside that ABI-only guard. Test fixtures and verifyResultRoundTrip must keep both new metadata coverage and the rule that drawKeyMemo is not persisted. #11551 remains the separately reported compared overlap; this reply does not dismiss that finding. The cache source/test patches in this review are complete. The ShaderResources.cpp full-source reconstruction used for the separate #12246 review is outside this PR's listed shared-file scope. No pairwise merge or combined build/test is claimed. Existing CI, current-head validation, dependency coordination and re-review obligations remain separate. Candidate heads, shared files and manual dispositions
AI-assisted: yes (Codex). Bounded shared-cache refresh (bot snapshot 2026-10-29 23:48:58 UTC)This supplements the historical 20-row table above for published #12250
For #7689, the reviewer and author explicitly identify FormatVersion 49 reuse as outstanding. #12250 also has 49; #10049 proposes 50 and #7642 proposes 62. A composition must reserve the appropriate distinct version after reconciling the actual landed schema; this review does not reserve a number or establish an order. The separate #7689/#12039 sequence agreement is not an agreement with #12250. All three source changes are distinct from #12250's ABI guard and transient-memo exclusion. Keep the existing seven-line populated-drawKeyMemo regression (including unchanged encoded bytes and empty decoded memo) alongside persistent-field coverage. This is a bounded source comparison, with complete shared-patch hunk spans and change counts verified; no pairwise build, runtime, GPU, performance, author-consent or landing claim. The other retained rows' current heads were not refreshed here. The private #9508 CPU comparison does not clear them or these three integrations, and this source snapshot does not approve future heads or the whole bot table. AI-assisted: yes (Codex). October 30 bounded bot-delta source follow-upThe 02:06:13 UTC capture of bot comment6461320271 has 23 skipped rows. Its refreshed 02:28:36 capture has the same 23 candidates and shared paths, now one compared #9508 cache-size conflict plus 22 skipped rows. Against the preceding 20-row refresh, the new rows are #6959/#10846/#12764; #7642 also moved. The published target remains Here “production” means
The separate #9508 receipt already pins its current I also inspected the proposed target No build, GPU run, pairwise merge, performance, console, author-consent or landing result is claimed by this bounded source follow-up. The whole table and future heads are not cleared, and no mandatory implementation dependency follows merely from sharing these files. AI-assisted: yes (Codex). Bounded graphics shared-source follow-upOwned #12250 remains
Earlier7642 CMake/core-cache source dispositions do not establish this newly listed AGC test-file coverage. This is a test-source composition finding at the frozen pin, not execution of its device-table path or reuse of the old CPU/private-composition result. A single end metadata check found the four candidate heads and both owned heads unchanged/open. Actual selected patches were reconstructed from immutable merge-base/head source; no unshared paths were reviewed. AI-assisted: yes (Codex). Follow-up, 2026-10-31 - source disposition for new compared rows #13928 and #13929 The current overlap report lists #13928 at head In the shared Linux/x86_64/libstdc++ size-sentinel block, these heads change |
|
Follow-up to the actual compared conflict in overlap comment 6461320271, for #12250 at head I reviewed the fresh #11551 patch at head #11551 changes the Linux/libstdc++ ABI size expectations for I propose #11551 → #12250: first integrate the new cache fields and revised size expectations, then apply #12250's compiler/library guard to those updated assertions. Keep the guard limited to ABI-size checks; retain the #12246 also changes the same file's @imellag, I propose landing #11551 before #12250 so the updated sizes and codec are in place before narrowing the ABI guard. Please confirm or suggest another order; I will record the dependency only after agreement. These notes do not establish a clean merge or passing combined build/roundtrip test. Combined Linux/GCC 11 and supported-toolchain checks remain outstanding. The separate #12246 re-review block is not cleared by this comparison. Existing prerequisites and orders remain untouched, including #11551's body-reported #11507-first order, #10773 before #9884, #7085's #6280/#9999 requirements, and #10064's recorded prerequisites. The 49-field calculation above applies only to #11551 + #12246; composing #7085's additional levelUses field as well gives 50 on that same 45-field baseline. Neither count is a claim that the combined branch has been built or tested. AI-assisted: yes (Codex). October 29 current-main and #9508 compared-scope follow-upCurrent integration The live bot now reports #9508 as the compared cache-file conflict. I read its complete listed @Raghav314 For this shared cache/sizeof seam, I propose #9508 before #12250 so the later guard retains the resulting supported-toolchain sizes and codec. Please confirm or revise that order. This is a proposal, not consent or a declared runtime dependency. The prior #11551-before-#12250 proposal remains unconfirmed; #11551 stays at A bounded refresh of all 20 current bot heads identifies changed retained #7642 ( AI-assisted: yes (Codex). Private #9508 cache/guard integration check — October 29I composed frozen main The same focused cache fixture on both arms retains the seven-line populated-drawKeyMemo omission regression and adds nondefault heap artifact fields, heap-slot words and binding-layout fields with comparisons. Windows 11 / WinLibs GCC15.2 r7 / Release: control and composition each build the actual cache target and pass three The selected fixture exercises CPU cache codecs and creates no Vulkan instance/device. Required Vulkan environment flags were retained, and Windows used the existing exclusive lock, but no actual GPU validation or performance result follows. #9508's exact-head GPU/performance review5705690478 remains separate. Independent main buffer checks and both test insertions were retained when composing newer main conflicts; those GPU paths were not run by this cache fixture. Public branches remain unchanged. Private passing tests do not establish author consent or a mandatory implementation dependency. The earlier #9508 order proposal remains unanswered; #11551's separate seam and other current overlaps remain outside this result. No all-overlaps-clear, readiness or full-suite claim. AI-assisted: yes (Codex). |
|
Focused qualification for Windows 11 Pro 26300 / WinLibs GCC 15.2.0 r7 / RTX 4080 SUPER 621.01: main with an identical test-only fixture overlay and candidate each passed the cache CPU case and existing real-GPU compute-pipeline companion 3 times (6/6 per arm, 10.15/9.35 s). Required Vulkan/no-cache env and shared GPU lock used; actual Khronos instance/device insertions and NVIDIA selections are present, with zero missing-layer/VUID messages and the same loader warning/error sequence. Ubuntu 22.04 / GCC 11.4: actual main translation unit fails This is focused main-refresh evidence, not #9508 GPU/performance qualification or blanket current-overlap approval. Historical results keep their original pins. SPIRV-Tools OFF; install/export not tested. Independent AI review passed for this focused change; maintainer review remains pending. AI-assisted: Codex. Follow-up, 2026-10-30 — fresh #11551/#12250 codec qualification Ran a bounded private codec check for candidate Windows: Windows 11 / WinLibs GCC 15.2.0 r7, Release. Linux: Ubuntu 22.04 / GCC 11.4.0, Release. At exact #11551 control These are focused CPU codec checks, not GPU qualification or a new hosted-CI result. This evidence applies to the frozen v51 pair only. #10351's current author-order request may consume v51 and move #11551 to v52; a v52 composition is outside this qualification and would need its own source review and checks. No claim is made that this private candidate is the current public head or that the PR is ready/merged. Follow-up — frozen-main v50 qualification, 2026-10-31 Candidate Windows control (c12e with the identical fixture overlay) and candidate each passed the default cache fixture, Linux GCC 11.4 candidate build and four CPU checks passed. The Linux guard-overlay control and candidate have identical tree Follow-up — frozen de824/c392 refresh, 2026-10-31 Candidate The three codec modes and pack-ownership CTest remain historical c12e/bad0 evidence. This refresh is not a later-live-main, hosted-CI, title/game, or combined-#11551 qualification. The exact-path review found no duplicate fixture assertions; the one shared guard line with #12246 follows the agreed #12250-first/rebase-and-drop sequence. AI-assisted: Codex. |
✅ NVIDIA GPU test: passedPR head
Unstable on main (3, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (2, not counted against this PR)Validation IDs already present on main (4 IDs in 390 tests)Skipped (41)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. |
|
@B-Howell Fine with #11551 → #12250, as you proposed. #11551, rebased today onto |
✅ NVIDIA GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (2, not counted against this PR)Validation IDs already present on main (4 IDs in 390 tests)Skipped (40)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. |
✅ 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 (4, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (2, not counted against this PR)Validation IDs already present on main (3 IDs in 384 tests)Skipped (41)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
On Linux with GCC 11/libstdc++ lacking
__cpp_lib_atomic_shared_ptr,AtomicSharedPtrhas a different in-memory representation from thestd::atomic<std::shared_ptr>layout used for the existingRecompileResultsize sentinel. The change gates only thatRecompileResult == 552assertion on the feature macro, inside the existing Linux/x86_64/libstdc++ block. All other 29 native-size sentinels and the explicit field-count and encode/decode checks remain active.The cache fixture verifies that the process-local
drawKeyMemois excluded from serialized result bytes and is absent after decode. It also round-trips a non-defaultmeshWindowBufferand verifies that the corresponding request key changes. The wire format remains version 50 with 46 serializedImageResourcefields; the existing 96-byte native-size sentinel remains in place. No cache schema or format bump is made.This update publishes candidate
e450f9a69a94a64097353826562bc46a601a31ca(tree8cff0750750eb9fb373bddf58d64a44f3e7e582f), an ordinary merge of public PR headc39227975df7b075570266202f9ff2dfac32bf49followed by frozen main snapshot0836dfdf4fecb1e0326100c490476e15d3a74da5. The selected PR changes remain limited to the cache test andShaderDiskCache.cpp; the main delta overlaps neither path. The paired qualification below is CPU-only and does not claim Vulkan instance, validation-layer loading, or GPU behavior. The #11551 author agreed to the preferred #11551 → #12250 sequence and discussed reverse composition (comment); this change includes no #11551 commit and makes no combined-v51 claim. #12246 shares only the narrowRecompileResultguard; author coordination is #12250 first, followed by #12246 rebasing and dropping that duplicate line. This is sequencing, not a code prerequisite, soDepends on: noneremains accurate.Tested
Frozen main0836 / candidate e450 focused qualification - 2026-10-31
Windows 11, WinLibs GCC 15.2.0 r7, Release/Ninja,
BUILD_TESTING=ON, cacheb12250a. Frozen main0836dfdf4fecb1e0326100c490476e15d3a74da5and candidatee450f9a69a94a64097353826562bc46a601a31caeach built the existingagc_shader_disk_cache_teststarget with two jobs and passed CTestagc_shader_disk_cacheonce (1/1; child durations 2.81s and 2.74s). Both runs usedANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1, andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation. Their logs contain noVUID-or requested-layer-missing marker. The executable is CPU-only and creates no Vulkan instance; this pair does not prove positive layer loading or GPU behavior. The CTest source-version labels aref25d44b2596fec4ffor main and579d0de06cfff348for the candidate; these are test output labels, not commit IDs. The control executable was not archived separately.Raw build and CTest logs, numeric exits, the candidate executable hash, the final cache hash, and the object-transfer receipt are retained under the private qualification receipt. This is a focused Windows CPU pair only: no Linux result, GPU qualification, hosted-CI result, title/game result, or full-suite claim is made. The earlier de824/c392 results below retain their original pins.
Earlier frozen-main de824 qualification - 2026-10-31 (c392; historical)
Candidate
c39227975df7b075570266202f9ff2dfac32bf49(treeebba59a7ec71ff21b56c6ef1baeb4f9fe19b19f3) is an ordinary merge of the previously prepared candidatebad0e6b2f515b7c3caa552a3f214ad02d400f996followed by the frozen current-main snapshotde824c2919ca65086c6324a1e0388866a7327eea. Main had advanced 26 commits fromc12e7e61; the relevant compiled-source refactor inShaderInfoCollector.cppchanges generatedSourceVersion, so the affected candidate target was rebuilt on each toolchain. The cache format remains 50, with 46 serializedImageResourcefields and the existing 96-byte native-size sentinel.be914c6613250040ffc8a75f84da47723248d4c8) and candidate c392 each configured and builtagc_shader_disk_cache_testsandagc_driver_prepared_shaders_testssuccessfully. Each default cache-fixture run passed once. Eachagc_driver_prepared_shaders_indirectCTest passed once on the RTX 4080 SUPER under the existing GPU lock and pinned Khronos validation environment. Logs show three Khronos instance and three device insertions per arm, no missing requested-layer diagnostic, noVUID-, no skip, and a released lock. The two-Wstringop-overreadwarnings are present in both build logs; they are not candidate-only. An unrelated extern warning also appears in both logs.agc_shader_disk_cache_testsand the default fixture passed once. Its tree is byte-identical to Linux overlay-control commit11359a93130643f3f0194cd776101010ce9359ce; with the same compiler/build settings and 31 initialized submodule pins, this one execution serves both source roles, not two independent runs. The generatedSourceVersionis0xe3df2eaa97cb77be.drawKeyMemoomission ormeshWindowBufferequality/key-sensitivity fixture assertions. Thirteen patches mentionmeshWindowBufferserialization already present in de824. The one shared production line is the narrowRecompileResultassertion guard in feat(agc): sample 1D arrays past maxImageArrayLayers as rows of a 2D image #12246; agreed coordination is fix(recompiler): check the encoder struct sizes only where they were measured #12250 first, then feat(agc): sample 1D arrays past maxImageArrayLayers as rows of a 2D image #12246 rebases and drops that duplicate guard. This is scoped duplicate-behavior evidence, not blanket overlap clearance.Depends on: noneremains the code-dependency declaration.Earlier frozen-main v50 qualification — 2026-10-31 (c12e/bad0; historical)
Candidate
bad0e6b2f515b7c3caa552a3f214ad02d400f996(tree23c9807f5bfe405e5940c831f6a036043c1d6599) is an ordinary two-parent merge: prior public PR headaf8bc35724893cd0a084cafab2d068ce866c26ee, then frozen mainc12e7e61a185261d837b6fb2ff1c92a07a00e5ff. This qualification is bound to that frozen main snapshot, not a later livemaintip.c0c6c1b852297cfd1388c23705c4a12cc87fd017) and the candidate each passed four CPU checks: default fixture mode,--artifact-codec,--resource-plan-codec, andagc_shader_disk_cache_pack_ownership. Each arm also passed oneagc_driver_prepared_shaders_indirectCTest on the RTX 4080 SUPER under the existing GPU lock and pinned Khronos validation setup. Actual instance/device layer insertion was captured; no skip, missing requested-layer message, orVUID-was found. Total: 5/5 checks per arm.agc_shader_disk_cache_testsbuild passed. Default, artifact-codec, resource-plan-codec, and pack-ownership checks each passed once. The Linux guard-overlay control (56e21714287a3904d4e90bc97bc87f0062a9f304) has the identical tree as the candidate, so this was one Linux build and four executions serving both source roles, not two independent runs. Its 31 initialized submodule pins matched the recorded checkout baseline.Recompiler.hppandShaderDiskCache.cppblobs are identical at that recorded head andc12e7e61. The current Linux control applies only the reviewed fixture and single-assertion guard overlays. No new full-tree build of unmodifiedc12e7e61or fresh592-byte measurement is claimed.There were 14 unique local executions across both Windows arms and the single Linux tree. No Linux GPU test, hosted-CI result, title/game result, or blanket overlap/readiness clearance is claimed. The Windows control configure transcript has no numeric exit capture and its standalone build stdout is absent; generated targets and the saved numeric build exit are successful. The full inherited Linux process environment was not snapshotted; exact argv/cwd and explicit mode environment changes are recorded. Historical results below remain tied to their original source pins.
Earlier pinned evidence (unchanged)
The sections below preserve their original source pins and scope. Statements about then-current heads, overlap, approvals, or proposed ordering are historical snapshots, not current status; the current #11551 order agreement is recorded above.
Current integration
881bfa72460361eb2ed747a2ae7ba932d37aea6b(tree0fb4edb9c9423e0c07cdf6657526bd68306b5024), ordinary merge of frozen mainfdd1753204a4732e1011539cc9d3bb69f70b27a6into publishedf2210862935e033d3eed2c5b37f2fbc07e60a6f3:ShaderDiskCache.cppmakes its actual translation-unit build fail at line 90'ssizeof(RecompileResult) == 552assertion. Restored the exact candidate, built the realagc_shader_disk_cache_teststarget with--parallel 2, thenctest --test-dir build -R '^agc_shader_disk_cache$' --repeat until-fail:3 -Vpasses all three runs (1.97 seconds). This directly exercises the affected compiler; no fresh 592-byte measurement or Linux GPU result is inferred.agc_shader_disk_cache_testsandagc_driver_compute_pipeline_outside_gpu_mutex_testson both frozen main and the candidate. Both use the identical memo fixture bytes (488f99a7d60d858e923f1bef4ef1de1122c9bb04cb8f8f2d7fd8201d04166f19); main's original fixture was restored byte-for-byte afterward and its checkout was verified clean.32.0.16.1742, both arms pass two selected CTest cases three times each:ctest --test-dir build -R '^(agc_shader_disk_cache|agc_driver_compute_pipeline_outside_gpu_mutex)$' --repeat until-fail:3 -V, under the existing exclusive GPU lock,ANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1,VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, the existingbuild-runtime/claude-vvlsetup and loader diagnostics. Each arm proves six instance and six device insertions of the Khronos layer, no missing-requested-layer message and zero VUIDs. Both logs retain the same six ancillary AMD implicit-layervkGetDeviceProcAddrdiagnostics. The CPU cache case itself creates no Vulkan device; the companion case compiles and executes actual shaders on the selected NVIDIA GPU. This is fixture coverage, not a title result or runtime GPU fix.A private #9508 cache/guard composition on frozen main
d65bff2679is now tested atc9dbc8d046(tree55b8b322a2601f0ac5f938eec1ac57581e8119f3): Windows/GCC15.2 control and composition each pass three cache runs; Linux/GCC11.4 control fails the actual unguarded 624-byte assertion and composition passes three runs. The identical fixture retains populated-memo omission and adds nondefault heap-field round trips. This is CPU cache/compiler coverage, not GPU/performance validation or a new measurement of the supported Linux ABI size table. Public branches are unchanged. At this historical snapshot, #9508 and #11551 sequencing were unconfirmed. Later, the author of #11551 agreed to the preferred #11551 → #12250 sequence and discussed reverse composition (comment #6519481893). That agreement does not import #11551 into this candidate or make the two tested source pairs equivalent. Other changed/new bot scopes remain pending. Historical approval and maskym's joint-tree report below retain their original pins and limits; no current-head approval or all-comments-complete/readiness claim is made.Earlier integration and tester evidence follows:
Published integration
f2210862935e033d3eed2c5b37f2fbc07e60a6f3(treece9a76a231ea220019510f042af5fcb6616d16c0), based on frozen main319621ba2e578bd053b0baba2877fe70520e0737:agc_shader_disk_cache_testsbuilt on both main and this integration with--parallel 2. Main used the identical seven-line memo fixture overlay, preserving all incoming main tests. The original main test source was restored byte-for-byte afterward.ctest -R '^agc_shader_disk_cache$' --repeat until-fail:3 -Vpassed all three executions withANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1,VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validationand loader diagnostics. Both CPU logs contain no missing-requested-layer message and no VUID. This cache test does not exercise a Vulkan instance/device; no real-device validation or GPU-rendering result is claimed.External tester provenance remains separate: maskym's report applies to old head
572692c05e, main3b979d42a9, and the other PRs listed in that joint tree. Its NVIDIA comparison and shared baseline failure/validation messages are the tester's evidence, not a fresh isolated run of this integration; our acknowledgment retains that qualification.Earlier published-head evidence follows:
BUILD_TESTING=ON:libsstops at the assertion above.libsbuilds andagc_shader_disk_cachepasses 3 of 3 runs.BUILD_TESTING=ON:libsbuilds, andagc_shader_disk_cachepasses 3 of 3 runs onmainand on this change.agc_shader_disk_cachecreates no Vulkan device, and neither machine had the validation layer installed during those historical runs, so no Vulkan validation run was made then.python tools/check_conventions.py --base origin/mainpasses.Checklist
main; no other open PR implements the same functions0836dfdf4fecb1e0326100c490476e15d3a74da5. The earlier exact-path comparison at de824 covered 1,558 open PR metadata entries and 90 selected file patches. The refreshed source-delta review for this candidate found zero overlapping paths between the PR delta and main0836; no newer global overlap census is implied. The narrow guard line shared with feat(agc): sample 1D arrays past maxImageArrayLayers as rows of a 2D image #12246 remains subject to the fix(recompiler): check the encoder struct sizes only where they were measured #12250-first/rebase-and-drop sequence.NotImplemented_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)agc_driver_prepared_shaders_indirectcheck at the previously published c392 source pin, with actual Khronos instance/device insertion. The new e450 qualification is CPU-only and creates no Vulkan instance; it adds no GPU or validation-layer-loading result. Driver and OS build were not captured in the historical run.🤖 Generated with Claude Code