Repository navigation
fix(agc): capture direct-write order under the tracker lock - #15551
rickyalvarez185 wants to merge 2 commits into
Conversation
Keep write-watch proof separate from record order for untracked direct writes. Reserve a fresh order after failed-collect unwatch interleavings and test both normal and suspended-lock paths.
Initialize the optional order output under the lock before the original zero-byte return. Assign the new test its own Windows PATH property instead of editing the shared test list.
✅ NVIDIA GPU test: passedPR head
Unstable on main (4, 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)Validated but not compared (1, not in the main baseline's validation pass)Validation IDs already present on main (4 IDs in 517 tests)Skipped (48)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. |
|
Tested on a second machine, merged onto current Windows 11 Pro 10.0.26340, WinLibs GCC 15.2.0 r7, Ninja, Release, Each tree ran
The cause I traced on this machine matches the PR description. The GDS is mapped when the device is created, so it takes the first arena block. The test's surfaces then start at AI-assisted: yes (Claude Code) |
|
Windows run of head Windows 11 Home 10.0.26200, WinLibs GCC 16.2.0 (
AI-assisted: yes (Claude Code) |
|
Windows AMD check of head Setup: Windows 11 Enterprise 22621, WinLibs GCC 15.2 UCRT, Release, Radeon RX 6700 XT 12 GB (discrete), AMD driver 26.10.1. Run with
So on discrete RDNA 2 too, the drop traced in #15564 is gone with this change. That makes three GPUs with this result, together with the NVIDIA reports on the issue. AI-assisted: yes. |
✅ AMD GPU test: passedPR head
Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (5, not counted against this PR)Validated but not compared (1, not in the main baseline's validation pass)Validation IDs already present on main (4 IDs in 589 tests)Skipped (61)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. |
|
Windows check of head Setup: Windows 11 Pro (10.0.26200), WinLibs GCC 15.2.0 r7 UCRT, Release,
So this closes #15564 on another machine, discrete Ada and integrated RDNA 2 alike. AI-assisted: yes. |
|
Tested on a Radeon 860M (RDNA 3.5,
Focused tests only, not the full GPU suite. AI-assisted: yes. |
What
Capture the direct-write record order while holding the write-tracker lock, independently of whether the destination has valid write-watch proof.
MarkDirectWritespreviously reused the zero proof value for untracked destinations as its record order; completion could then discard image results recorded after the buffer write once the destination became tracked again.The existing two-argument
MarkPossiblyWrittenstill returns zero when proof is unavailable. The ordered overload also reserves a distinct order after a failed fresh collect temporarily releases the lock for an unwatch hook.Adds
agc_write_tracking_mark_order, covering empty, tracked and untracked ranges, concurrent generation advances, and a deterministic failed-collect interleaving. No GDS allocation/layout workaround is included.Tested
Windows 11, WinLibs GCC 15.2.0 UCRT, Release/Ninja; NVIDIA GeForce RTX 4080 SUPER, driver 621.01; Vulkan validation layer 1.4.363.1. GPU tests run at medium integrity with
ANYPS5_GPU=4080,ANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1,VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, and verbose loader/layer logging. The layer's actual insertion and absence of missing-layer warnings were checked, not just its installation.mainat1e658521a0a599872612bb91949311e2956b5283;agc_driver_direct_write_completionfails. The regression was also bisected to merge8009be68d7602c728b7d8f7c70880035b4ec343f(perf(agc): back the GDS with anonymous shared memory on integrated GPUs and keep its DMA_DATA on the GPU #13159), while its parent passed on the same GPU.a failed fresh collect reused the intervening writer's mark order; restoring it passes. The pre-update candidate passed the direct-write and write-tracking focused set three consecutive times without VUIDs.agc_storage_rewritten_blocks,agc_storage_rewritten_stale_units,agc_driver_prepared_exportsandagc_driver_srgb8_color_target. Baseline: 9 pass / 1 fail; change: 11 pass / 0 fail (the additional case is the new ordering test). Both logs have exactly 18 occurrences ofVUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605from the existing validation controls, with identical VUID-ID multisets, six validation-layer insertions each and no missing-layer warning. The direct-write regression adds no VUID; the focused comparison introduces no new validation error.libstarget succeeded. Full verbose CTest run after the final overlap-avoidance changes: 3925 cases, 3912 passed, 12 skipped, 1 failed. The only failure isagc_driver_store_over_untracked(the first result was not stored), reproduced on cleanmainat the base above and intentionally left to a separate fix. The full run reports only the same 18 occurrences of the validation ID already present in the focused baseline. The previously intermittent no-timeline test and Windows relinker fixture pass in this completed run; interrupted earlier runs are not counted as completed validation. Relocating the new test's Windows PATH property preserves all 3925 generated test command/property configurations exactly.pr_overlap.py --branch fix/agc-direct-write-record-order --repo boykopovar/AnyPS5 --base main --jobs 4reports no direct overlap but exits 2, not clean: 33 other PRs already conflict withmain, so its comparisons fail closed. Every one was checked with paired base+PR and change+PR merge trees: identical conflict paths and identical modes/source after reversing only this contribution, ignoring only left-side commit labels on conflict-marker lines. fix(agc): stop frame capture deltas from collecting the previous delta's pages again #14137 is also checked and merges without the former registration collision. All 34 audited heads and the base remained unchanged in the final official rerun. These are source/merge checks, not claims that the other branches build or pass tests.Validation logs, full-suite summary and per-head overlap evidence: https://gist.github.com/rickyalvarez185/bb546d1d2140addcb2a91bd52ccba2a4
Final head:
30a0b806bf71da658f0e5231bfa79b1c4fdc1b39. Independent static review approved both the runtime patch and final overlap-avoidance changes; final-head focused rerun: 11/11 pass.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)