Repository navigation
Conversation
…unwatches them ImportWatched with staysWatched ran the unwatch hook only for imports that leave the watch. An import that stays watched over memory the watch covers only in part fails its write walk and unwatches the whole range without the hook, so storage images with results pending there lost their saved guest bytes and their store threw "storing storage image results pending over memory outside the write watch without the guest bytes saved before them is not implemented". The hook now also runs when a stays-watched import is not fully covered. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
unblocks title
…unwatch-partly-watched-import # Conflicts: # docs/dev/debt/libSceAgcDriver.md
|
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.
|
jotitadev
left a comment
jotitadev
left a comment
There was a problem hiding this comment.
Apple GPU / MoltenVK run. On macOS, agc_write_tracking stops at an earlier check, so CheckPartlyWatchedImportSavesFirst never runs there. Run on its own, it fails without the fix and passes with it.
Machine: Apple M2 Max, macOS 27.2, x86-64 build under Rosetta. Head 3724e31, PR diff applied cleanly. Drivers: MoltenVK 1.4.2 (Vulkan SDK 1.4.363.0) and MoltenVK main + SPIRV-Cross #2704.
Tested: the tree is main a373e85 + mugurc's macOS series (ci/macos-series-2) + the PRs needed to build on macOS (#6667, #6668, #7524, #6969, and two files of #7332), not plain main (it doesn't build on macOS yet). Also in the same build: #7707, #7698, #7687, #7691 and #7705 (they touch different files and applied cleanly together), plus a test-only patch that lets nine AGC fixtures load the Vulkan loader on macOS (branch jotitadev:fix/agc-tests-vulkan-loader-macos). Base on that tree: 40 ^agc failures with MoltenVK 1.4.2 and 26 with MoltenVK main + #2704.
-
agc_write_trackingalready fails on that tree before reaching the new check. It stops inCheckImportWindowwitha block the import window did not touch reads as written. The cause is how the macOS series'ImportWatchedopens the range on Apple, not this PR. The failure is the same with and without the PR, on both drivers. -
To reach the new check I patched the test locally to run only
CheckPartlyWatchedImportSavesFirst:- with the PR: passes 3 of 3;
- with
GuestMemory.cppback to the base: fails 3 of 3 withthe watched blocks of a partly watched import left the watch without the unwatch hook seeing them (0 calls).
So the fix also holds on the macOS path of
ImportWatched. Here the write watch reportshost imports resolve write protection: no, so imports stay watched. -
Full
^agcsuite with the stack: the base failures on each driver, plusagc_driver_pooled_buffer_rangeandagc_performance_overlay, which only run because of the loader patch. Nothing from this PR.
GPUs tested: Apple M2 Max (MoltenVK 1.4.2; MoltenVK main + SPIRV-Cross #2704)
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
Intel and AMD Windows data: with this PR's test changes, Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, every execution test built,
AI-assisted: yes (Claude Code) |
|
Closing as a duplicate of #6308, which fixes the same stop and covers more: it runs the unwatch hook whenever the write walk fails before the range is unwatched, while this PR only covered a stays-watched import over partly watched memory. #7505 takes a similar approach. I missed both when checking for overlap, sorry for the noise. For #6308: Marvel's Wolverine (PPSA03671) hits this stop on Windows with an RTX 5070 Ti, at a 0x18b74000-byte import at 0x11e128c000 with |
3 participants
What
GuestMemory::ImportWatchedran the unwatch hook (StorageTexture::SaveBeforeUnwatch) only for imports that leave the write watch. An import that stays watched over memory the watch covers only in part fails its write walk and unwatches the whole range, without the hook. Storage images with results pending in that range then had no saved guest bytes, and their store threwstoring storage image results pending over memory outside the write watch without the guest bytes saved before them is not implemented.The hook now also runs, before the import, when a stays-watched import is not fully covered by the watch. The debt entry for pending results outside the write watch names this case among the ones that save the bytes.
Tested
Windows 11 (10.0.26300), GCC 15.2.0 UCRT Release.
CheckPartlyWatchedImportSavesFirstinagc_write_tracking: unwatches the middle block of three, then imports all three withstaysWatched. Without the change the unwatch hook sees none of the watched blocks (0 calls) and the test fails; with it, it passes.ctest -R '^agc_'on this branch (rebased on main 8b8edc5): 737 of 737 passed.APS5_TEXTURE_CACHE_MIB=7168-11264: it stopped on the error above after about 60 s, after a 0x18b74000-byte import at 0x11e128c000 withstaysWatchedandcovered=0. With the change it ran on past that point (180 s and more) without the error. Not yet repeated on this branch's main.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