From a130ec081ccf23b28e11c90dd0e0d10878da60e4 Mon Sep 17 00:00:00 2001 From: Yunus Shaikh <75296019+EagleBearerYS@users.noreply.github.com> Date: Fri, 30 Oct 2026 07:55:06 +0000 Subject: [PATCH] perf(agc): copy a descriptor region behind pending GPU work on the GPU whatever its size A misaligned descriptor region inside a host import is copied out of the import by the GPU only up to APS5_GPU_COPY_MAX_KIB (1 MiB). A larger one was read on the CPU, and when a pending GPU write, pending storage results or a unit shadow covered it, that read waited in the flush hook for the producer's batch (the [hooksync] "buffer-upload" syncs). Such a region now takes the GPU copy whatever its size: the copy is recorded after the producer in batch order, behind the barrier and the pending-results flush the GPU path already records, so the bytes are the same and the CPU does not wait. UploadFinish asks the recorder under the GPU mutex; UploadPrepare, which holds no lock, asks the published pending-write snapshot. Larger regions behind no GPU work keep the CPU copy, which never waits for them. Completion write-backs and labels over the range are still synced first by CompletePendingCpuWrites, so a pending write the copy sees has landed in the import on the GPU. The test (agc_buffer_upload_pending_gpu_copy) uploads a 3 MiB misaligned region over a GPU write still in the open batch: the upload makes no flush-hook wait and no submission, and a GPU read of the bound buffer holds the producer's bytes. --- core/libs/prx/libSceAgcDriver/CMakeLists.txt | 6 + .../Graphics/include/GuestBufferMemory.hpp | 1 + .../Graphics/src/GuestBufferMemory.cpp | 14 ++- .../prx/libSceAgcDriver/tests/Recorder.cpp | 106 ++++++++++++++++++ docs/dev/ENVIRONMENT.md | 2 +- 5 files changed, 126 insertions(+), 3 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/CMakeLists.txt b/core/libs/prx/libSceAgcDriver/CMakeLists.txt index 2dd6c8cf56ee..4da0eac2762c 100644 --- a/core/libs/prx/libSceAgcDriver/CMakeLists.txt +++ b/core/libs/prx/libSceAgcDriver/CMakeLists.txt @@ -1843,6 +1843,12 @@ if(BUILD_TESTING) endif() add_test(NAME agc_storage_typed_fill_clear COMMAND agc_driver_recorder_tests --typed-fill-clear-only) set_tests_properties(agc_storage_typed_fill_clear PROPERTIES ENVIRONMENT "APS5_NO_PIPELINE_JOURNAL=1" SKIP_RETURN_CODE 77) + add_test(NAME agc_buffer_upload_pending_gpu_copy COMMAND agc_driver_recorder_tests --pending-upload-gpu-copy-only) + set_tests_properties(agc_buffer_upload_pending_gpu_copy PROPERTIES SKIP_RETURN_CODE 77) + if(WIN32) + set_tests_properties(agc_buffer_upload_pending_gpu_copy PROPERTIES ENVIRONMENT_MODIFICATION + "PATH=path_list_prepend:$") + endif() add_test(NAME agc_storage_integer_clear COMMAND agc_driver_recorder_tests --integer-clear-only) set_tests_properties(agc_storage_integer_clear PROPERTIES SKIP_RETURN_CODE 77) if(WIN32) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp index 72e07d86648b..13e37619f6c8 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp @@ -497,6 +497,7 @@ class GuestBufferMemory { // instead of the CPU (see Region::gpuCopy): live guest bytes, not sparse, at most the size // APS5_GPU_COPY_MAX_KIB allows, and APS5_CPU_COPIES unset. bool gpuCopyEligible(const Region& region) const; + bool copyBehindGpuWork(const Region& region, bool locked) const; // Whether a region inside a host import is staged in device memory instead of bound in place // (see GuestBufferMemory.cpp): staging allowed, not address-based unless an overlay region, a // written element inside, and an atomic element or a size within the written-shadow window. diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp index 03b1d4c207ef..f3a2a4ef0739 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp @@ -5194,6 +5194,16 @@ bool GuestBufferMemory::gpuCopyEligible(const Region& region) const { return region.end - region.begin <= gpuCopyLimit(); } +bool GuestBufferMemory::copyBehindGpuWork(const Region& region, bool locked) const { + if (!gpuCopiesEnabled() || region.sparse || !(region.hostBacked || region.writable)) return false; + const auto* recorder = Recorder::Active(); + if (recorder == nullptr) return false; + const auto bytes = static_cast(region.end - region.begin); + const std::array, 1> range{{{region.begin, region.end}}}; + const bool written = locked ? recorder->PendingWriteOverlaps(region.begin, bytes) : Recorder::SnapshotWriteOverlaps(region.begin, bytes); + return written || StorageTexture::AnyPendingOverlaps(range) || AnyShadowedOverlaps(region.begin, bytes); +} + bool GuestBufferMemory::bindableInPlace(std::uint64_t offset, bool addressable) const { if (adjustedRegions && !addressable) return offset % 4 == 0; return offset % context.limits.minStorageBufferOffsetAlignment == 0; @@ -5385,7 +5395,7 @@ void GuestBufferMemory::UploadPrepare(bool addressable) { } if (misaligned || staged) { if (!staged) { - if (!gpuCopyEligible(region)) continue; + if (!gpuCopyEligible(region) && !copyBehindGpuWork(region, false)) continue; const auto allocateStart = std::chrono::steady_clock::now(); region.buffer = std::make_shared(context, static_cast(region.end - region.begin), gpuCopyUsage(addressable), copyBufferProperties(false)); if (profile) allocateUs.fetch_add(microsecondsSince(allocateStart), std::memory_order_relaxed); @@ -5586,7 +5596,7 @@ void GuestBufferMemory::UploadFinish(bool addressable) { // region in place (Descriptor prefers `direct`), and a kept buffer would count as copied. region.buffer.reset(); region.deviceLocal = false; - } else if (entry != nullptr && recorder != nullptr && (staged || gpuCopyEligible(region))) { + } else if (entry != nullptr && recorder != nullptr && (staged || gpuCopyEligible(region) || copyBehindGpuWork(region, true))) { // Misaligned in the import (or staged): the GPU copies the sub-range out of it. The // handle and base are taken now, under the import lock: a reconcile by the flush // below could retire the entry (the batch opened below keeps a retired import's diff --git a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp index 9d3fb79f85ea..c7057d180d33 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp @@ -4221,6 +4221,108 @@ bool sparseRunImportTests(const Device& device, bool copied) { return true; } +bool pendingUploadGpuCopyTests(const Device& device, Recorder& recorder) { + const auto& context = device.GetContext(); + const auto alignment = context.limits.minStorageBufferOffsetAlignment; + if (alignment < 8 || alignment > 256) { + std::cout << "storage buffer offset alignment " << alignment << ": uploads behind a pending GPU write not tested\n"; + return false; + } + constexpr std::size_t bytes = 4u << 20u; + void* block = context.hostImportAlignment != 0 ? AllocateWatched(bytes, 65536) : nullptr; + if (block == nullptr) { + std::cout << "host imports or write watching unavailable: uploads behind a pending GPU write not tested\n"; + return false; + } + struct Release { + void* block; + ~Release() { ReleaseWatched(block, bytes); } + } release{block}; + auto* guest = static_cast(block); + for (std::size_t at = 0; at < bytes; ++at) guest[at] = static_cast(at * 5u + 1u); + const auto address = reinterpret_cast(block); + { + GuestAllocations::Mutation mutation; + mutation.Add(block, bytes, true, true, true); + } + struct Unregister { + const Context& context; + void* block; + ~Unregister() { + { + GuestAllocations::Mutation mutation; + mutation.Remove(block); + } + HostImportFor(context, reinterpret_cast(block), bytes); + } + } unregister{context, block}; + const auto* import = HostImportFor(context, address, bytes); + if (import == nullptr) { + std::cout << "host import of the watched block refused: uploads behind a pending GPU write not tested\n"; + return false; + } + constexpr std::size_t viewOffset = 4096 + 4; + constexpr std::size_t viewBytes = 3u << 20u; + constexpr std::size_t producedOffset = 1u << 20u; + constexpr std::size_t producedBytes = 1u << 20u; + const auto view = address + viewOffset; + const auto produced = address + producedOffset; + Require((view - import->base) % alignment != 0 && (view - import->base) % 4 == 0, "the test view is not a DWORD-aligned view off the storage buffer offset alignment"); + std::vector expected(guest + viewOffset, guest + viewOffset + viewBytes); + auto source = std::make_shared(context, producedBytes, VK_BUFFER_USAGE_TRANSFER_SRC_BIT); + auto* pattern = reinterpret_cast(source->Bytes().data()); + for (std::size_t at = 0; at < producedBytes; ++at) { + pattern[at] = static_cast((at * 7u + 3u) ^ 0xa5u); + expected[producedOffset - viewOffset + at] = pattern[at]; + } + { + std::lock_guard lock(GpuMutex()); + recorder.Sync(); + const auto commands = recorder.Commands(); + CopyBuffer(context, commands, source->Handle(), 0, import->buffer, HostImportOffset(*import, produced), producedBytes); + RecordMemoryBarrier(context, commands, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_TRANSFER_READ_BIT); + recorder.Keep(source); + recorder.NotePendingWrite(produced, producedBytes); + } + Require(recorder.OpenWriteOverlaps(produced, producedBytes), "the producer's GPU write is not pending in the open batch"); + { + GuestBufferMemory memory(context); + memory.AddReadable(view, viewBytes); + const auto waitsBefore = Recorder::ThreadHookWaits(); + const auto submissionsBefore = recorder.Submissions(); + { + std::lock_guard lock(GpuMutex()); + memory.Upload(false); + } + const auto waits = Recorder::ThreadHookWaits() - waitsBefore; + const auto submissions = recorder.Submissions() - submissionsBefore; + Require(waits == 0 && submissions == 0 && recorder.OpenWriteOverlaps(produced, producedBytes), "the upload of a misaligned " + std::to_string(viewBytes >> 20u) + " MiB region over a pending GPU write waited for it on the CPU (" + std::to_string(waits) + " flush-hook waits, " + std::to_string(submissions) + " submissions)"); + auto readback = std::make_shared(context, viewBytes, VK_BUFFER_USAGE_TRANSFER_DST_BIT); + { + std::lock_guard lock(GpuMutex()); + std::uint32_t adjustment = 0; + const auto info = memory.Descriptor(view, viewBytes, adjustment); + const auto commands = recorder.Commands(); + RecordMemoryBarrier(context, commands, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_ACCESS_SHADER_WRITE_BIT | VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_TRANSFER_READ_BIT); + CopyBuffer(context, commands, info.buffer, info.offset + adjustment, readback->Handle(), 0, viewBytes); + RecordMemoryBarrier(context, commands, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_HOST_BIT, VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_HOST_READ_BIT); + recorder.Keep(readback); + recorder.Submit(); + } + device.WaitQueue(); + { + std::lock_guard lock(GpuMutex()); + recorder.Sync(); + } + const auto* read = reinterpret_cast(readback->Bytes().data()); + const auto mismatch = std::mismatch(expected.begin(), expected.end(), read); + Require(mismatch.first == expected.end(), "the region uploaded behind the pending GPU write misses the producer's bytes at byte " + std::to_string(mismatch.first - expected.begin()) + " of the view"); + Require(std::memcmp(guest + producedOffset, pattern, producedBytes) == 0, "the producer's GPU write did not reach guest memory"); + } + std::cout << "uploads behind a pending GPU write tests passed\n"; + return true; +} + void misalignedRegionTests(const Device& device, Recorder& recorder) { const auto& context = device.GetContext(); const auto alignment = context.limits.minStorageBufferOffsetAlignment; @@ -12400,6 +12502,9 @@ int main(int argc, char** argv) { if (argc == 2 && std::string_view(argv[1]) == "--typed-fill-clear-only") { return denormalClearTests(device, recorder) ? 0 : 77; } + if (argc == 2 && std::string_view(argv[1]) == "--pending-upload-gpu-copy-only") { + return pendingUploadGpuCopyTests(device, recorder) ? 0 : 77; + } if (argc == 2 && std::string_view(argv[1]) == "--integer-clear-only") { integerClearTests(device); return 0; @@ -12654,6 +12759,7 @@ int main(int argc, char** argv) { fmaskDecompressMsaaTests(device, recorder); misalignedSnapshotTests(device, recorder); misalignedRegionTests(device, recorder); + pendingUploadGpuCopyTests(device, recorder); drawSnapshotReuseTests(device, recorder); drawDescriptorWriteTests(device, recorder); drawSnapshotTableSetTests(device, recorder); diff --git a/docs/dev/ENVIRONMENT.md b/docs/dev/ENVIRONMENT.md index ea3cc4465f4f..8e67c5fb10f6 100644 --- a/docs/dev/ENVIRONMENT.md +++ b/docs/dev/ENVIRONMENT.md @@ -174,7 +174,7 @@ The code reads environment names through `getenv`, `IsEnvironmentSet`, `kibSetti | `APS5_FRAME_CAPTURE_TRIGGER` | string | `core/libs/prx/libSceAgcDriver/Execution/src/FrameRecorder.cpp` | | file whose appearance starts the frame capture at the next frame boundary; a number in it replaces the frame count | | `APS5_FULL_BARRIERS` | presence | `core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp` | | records every barrier instead of merging redundant ones | | `APS5_GPU_CHECKPOINTS` | presence | `core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp` | | attaches device diagnostic checkpoints through VK_NV_device_diagnostic_checkpoints, or buffer markers through VK_AMD_buffer_marker on a device without it | -| `APS5_GPU_COPY_MAX_KIB` | integer | `core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp` | 1024 | size cap in KiB on a descriptor sub-range copied through the GPU path, larger ones staying on the CPU | +| `APS5_GPU_COPY_MAX_KIB` | integer | `core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp` | 1024 | size cap in KiB on a descriptor sub-range copied through the GPU path, larger ones staying on the CPU unless a pending GPU write, pending storage results or a unit shadow overlaps them (the GPU then copies them whatever their size, so the CPU does not wait) | | `APS5_GPU_NO_WRITEBACK` | presence | `core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp` | | skips writing GPU results back into guest memory | | `APS5_GPU_WAIT_TIMEOUT_MS` | integer | `core/libs/prx/libSceAgcDriver/Execution/src/Driver/Synchronization/WaitMemory.cpp` | 1000 | milliseconds a GPU wait may run before a warning is printed | | `APS5_GROUP_CAPTURE_ALL_QUEUES` | presence | `core/libs/prx/libSceAgcDriver/Execution/src/Driver/Dispatch/DispatchAhead.cpp` | | resolves group-ahead capture on queue 0 as well |