Repository navigation
Conversation
…s reason Since boykopovar#9898, a cached variant whose runs only queued stores overlap is recaptured instead of drained, and validateVariant reported that as Differing, so the draw and dispatch caches counted those misses as changed content. It now returns QueuedStores. The draw cache counts it as DrawMiss::QueuedStores and the dispatch cache as queuedStores, both report it on its own, the dispatch trace names it, and the dispatch differing classification leaves it out. Nothing else changes. agc_driver_queued_constant_ram_dump checks that the draws count such a miss, and adds a dispatch whose cached variant reads a descriptor table that a queued DUMP_CONST_RAM rewrites. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
12 tasks done
Contributor
✅ 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 (3, not counted against this PR)Validation IDs already present on main (13 IDs in 544 tests)Skipped (47)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. |
Contributor
✅ AMD 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 (8, not counted against this PR)Validated but not compared (55, not in the main baseline's validation pass)Validation IDs already present on main (8 IDs in 534 tests)Skipped (57)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
|
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.
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A follow-up to #9898, from @XairRrR's two non-blocking points in their last review. #9898 merged before they were addressed.
validateVariantreported this asDiffering, so both caches counted these misses as changed content: as front, fragment or other stage differing in the draw cache, and as differing ("captured memory changed" in the trace) in the dispatch cache.validateVariantnow returnsEntryOutcome::QueuedStores. The draw cache counts it asDrawMiss::QueuedStoresand the dispatch cache asqueuedStores. Both 10 s reports print it as "recaptured over queued stores", the dispatch trace says "queued stores over the captured memory", and the profile-only differing classification of dispatch misses leaves it out.Differing, so nothing else changes.agc_driver_queued_constant_ram_dumpgets a dispatch case: a cached dispatch variant reads a buffer V# from a table, and aDUMP_CONST_RAMqueued behind the held committer rewrites that table. The case checks that the dispatch counts through the dumped descriptor, and that it doesn't drain the pipeline for capture validation.validateVariant. On queue 0, the pipeline is drained at the dispatch packet before the lookup. That happens atPacketExecution.cpp:469after an opened gate, and otherwise atDispatch.cpp:160, or at:338for a pipeline candidate whose cached reads overlap a queued write. On other queues,DrawPipeline::Active()is thread-local and set only on queue 0's worker (PacketExecution.cpp:175), sovalidateVariantdoesn't look at queue 0's pipeline at all.0x20, a dispatch resolved ahead counted through the old descriptor while the dump was still queued on queue 0. Hardware doesn't order work across queues without a wait either, so the test doesn't check that case.Tested
Windows 11 Pro 26300, WinLibs GCC 15.2.0, Release. AMD Radeon RX 9070 XT, driver 32.0.31041.1004 (Adrenalin 26.8.1). Environment:
ANYPS5_GPU=AMD ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, withctest -V. Neither log haswas not found but was requested by env var VK_INSTANCE_LAYERS, and everySelected GPUline names the RX 9070 XT.Focused set, 26 tests. It covers
queued_constant_ram_dump, thedraw_cache*,draw_data_hit*,mesh_draw_relocation,mesh_index_cache,dispatch_data_hit_words,dispatch_ahead*,gated_dispatch_capture*anddraw_pipeline_retired_itemstests.VUID-linesmain5b1d381d5aAPS5_VERIFY_KNOWN_VALUES=1:agc_driver_queued_constant_ram_dumpalso passes. The draws drain in this mode, as before, so the counter check is skipped.Mutations. Each was applied to this branch, rebuilt, and run 3 times. "Dispatch case only" means
main()ran only the new dispatch case, because the draw case fails first under some of these changes.validateVariantreturnsDifferingagainPacketExecution.cpp:469,Dispatch.cpp:160), dispatch case onlyAfter restoring, the test passes, and
git statusshowed only this change.Not run: Linux (no struct under the size guards changed, but nothing was compiled there), NVIDIA, the rest of the suite, and any title. The counters change no timing, so there is no speed claim.
Conventions.
tools/check_conventions.py --base origin/mainpasses.Overlap.
tools/pr_overlap.py --branch feat/agc-queued-store-miss-reasonfailed closed: 13 pull requests share files but conflict withmain, so it couldn't compare them. I read each diff, and none adds a queued-store outcome or counter.DeviceImageFallbackto the sameEntryOutcomeline, so whichever lands second needs a one-line merge that keeps both values.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)🤖 Generated with Claude Code