Repository navigation
fix(agc): save the bytes a GPU-side store writes outside the write watch - #5760
Conversation
github-actions
Bot
added
needs rebase
|
@oneandonlydean While refreshing the PPSA99902 validation stack, StoreOverUntracked access-violated on Windows NVIDIA. I reproduced it on unchanged main3e (initial failure, then nine passes followed by another failure); AMD passed20. Run can return while a Store is still queued, then destroy its local surfaces. #5789 records the reproduction. The separate three-line cleanup wait in #5790 passed100 repetitions on each GPU, both before and after rebase onto main82. It leaves the GPU mutex before waiting and retains the original store assertions. Your new RunSecondStore also has early returns after submission, so it may need the same lifetime ordering. I have not tested #5760 or established that its new case crashes. This report concerns fixture lifetime, not validation/rejection of your production correction. AI-assisted: Codex. |
8f1ffed to
84a41e2
Compare
A write-back stored on the GPU over memory outside the write watch left its units without saved guest bytes (only the CPU path saved them), so the next store of results landing there threw (boykopovar#5711), and every refresh re-uploaded the image. The stored ranges are now copied from the retile scratch into a host-visible buffer behind the store and become the saved bytes when the batch completes; a store or refresh that needs them first syncs that batch. Units without a write generation pending when their range leaves the watch save their bytes there too. The second-store fixture waits for the GPU before its surface is freed on every return, as boykopovar#5790 does for the other cases.
84a41e2 to
69702ef
Compare
|
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.
|
What
A write-back stored on the GPU over memory outside the write watch left its units without saved guest bytes. Only the CPU path saved them. The next store of results there threw "storing storage image results pending over memory outside the write watch without the guest bytes saved before them is not implemented" (#5711, #5693), and every refresh re-uploaded the image.
originalwhen the batch completes (OnComplete). A write-back or refresh that needs them first syncs that batch. Anything that replaces or drops a unit's saved bytes in the meantime cancels the save for that unit.saveBeforeUnwatch, as the watch would store it whole.StoreOverUntracked: a surface outside the watch from the start is stored twice, with a CPU write to another block in between. On main the first store leaves no saved bytes (the test fails); with this change the CPU block is kept and the second result is stored.Stacked on #5707 (review only the top commit). Once #5679 is in, its capture at landing has to skip units whose save is still pending (
savedTicket[layer] == 0).Tested
BUILD_TESTING=ON:ctest -R "agc_|guest_memory|write_watch"504 of 504 (one skipped); the newStoreOverUntrackedcase fails on fix(agc): keep the saved guest bytes of units a fill or image copy does not cover #5707's head and passes with this, 10 of 10 repeats.main+ fix(agc): save pending storage results in every 64 KiB block that leaves the write watch #5671 + fix(agc): save the guest bytes of units without a write generation when results land outside the write watch #5679 + fix(agc): keep the saved guest bytes of units a fill or image copy does not cover #5707 + this (with thesavedTicketskip in fix(agc): save the guest bytes of units without a write generation when results land outside the write watch #5679's capture): 509 of 509 (one skipped).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)