Repository navigation
Conversation
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the ownership sequence and both regression paths. region->buffer is retained before the first recorded copy, and the diagnostic readback is retained before the later copy uses it; this covers the allocation-failure unwind in the issue without changing normal copy semantics. The deferred-fence mock directly checks pool reuse before completion, and the reported RTX 3080 Ti run exercises the successful and injected-failure Vulkan paths. I did not independently run the GPU tests; the hosted platform build jobs are not included in the current check results.
|
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.
|
github-actions
Bot
added
fix error
|
Independent Windows run of this PR with a baseline (data only). Environment: Windows 10 Pro 10.0.19045, Ryzen 7 5800X3D, NVIDIA GeForce RTX 4080 SUPER (the log shows Results (3 runs each):
Not run: other GPUs (no AMD card for this PR), other platforms, the other tests, the full repository CTest, and any title or console run. AI-assisted (Claude Code). |
adriadam10
left a comment
adriadam10
left a comment
There was a problem hiding this comment.
The fix itself reads correctly (recorder->Keep(region->buffer) before the copy and Keep(readback, ...) before its use). Two things need fixing before it can merge:
core/libs/prx/libSceAgcDriver/CMakeLists.txt:2390callsset_tests_properties(agc_driver_copy_input_lifetime ...)but nothing defines that test or its executable: there is noadd_executable/add_test, andtests/execution/CopyInputLifetime.cppis not referenced anywhere. CMake errors at configure ("Can not find test to add properties to"), so the real-Vulkan regression is never built. Add the target andadd_testthe way the othertests/execution/*Vulkan tests are registered, with the skip-77 handling the CIcheck_skips.pyexpects.agc_copy_input_failure_lifetimeis only registered underif(WIN32)(and the test body throws elsewhere), so Linux CI exercises neither regression. Please make the mock-based test run on Linux too (the guest registration can use theGuestAllocations::Mutationhelper thatCopyInputLifetime.cppalready uses), or explain why it cannot.
|
Additional integration validation on base Windows, Ryzen 9 3900X / RTX 3080 Ti, WinLibs GCC 15.2, Release. All 25 focused CTest selections qualified. The 23 GPU executions positively loaded Khronos validation 1.4.363 and identified the RTX 3080 Ti; full stdout/stderr contained no VUID errors, synchronization hazards, missing-layer warnings or skipped tests. The other two selections were non-GPU controls. The real GPU-copy lifetime regression passed with Focused correctness testing on this configuration; not a full repository suite or a performance result. AI-assisted testing and report: Codex. |
What
With
APS5_CAPTURE_GPU_COPIES=1, diagnostic readback allocation can fail after the input copy has been recorded. The destination is kept by the batch later, so unwinding can return it to the buffer pool while the recorded command still refers to it.Keep the destination before recording the copy, and retain the diagnostic readback before recording commands that use it. Add deferred-fence mock and real Vulkan regressions, including a successful-readback control. No experimental memory policy is included.
Fixes #8659. This PR replaces the standalone patch-review issue.
Tested
Windows 11 Pro build 26300, WinLibs MinGW-w64 GCC 15.2.0, NVIDIA GeForce RTX 3080 Ti, driver 610.88.
a6a4101e190ec408c3ed471d20f06872cae1b4a3. Before tests used this production code with the final regression fixtures added; after tests used the corrected source tree. The regression and CMake sources are identical on both sides.agc_copy_input_failure_lifetimeandagc_driver_copy_input_lifetime: both fail before the fix because the destination is reusable before its fence; both pass afterward. The successful-readback control passes on both sides.agc_buffer_pool,agc_driver_graphics,agc_driver_bda_device,agc_driver_bda_span_reads,agc_driver_guest_memory_lifetime,agc_driver_host_import_windowsandagc_copy_input_kept_bytes; 7/7 pass on each side.ANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation. Full logs positively show the RTX 3080 Ti and validation instance/device insertion, zeroVUID-messages, no missing-layer warnings and no GPU skips.The separate native allocation failure during completion-callback registration was not fault-injected. Linux and other GPUs were not tested. These are synthetic correctness tests; no game compatibility or performance improvement is claimed.
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)