From e6fce45d3afd399aba245c3f5be06c47be7cfeec Mon Sep 17 00:00:00 2001 From: nmzik Date: Mon, 5 Oct 2026 21:00:12 +0400 Subject: [PATCH] gpu: skip drains for CPU-only memory unmaps --- .../host_gpu/renderer/cache/textureCache.cpp | 5 ++ .../host_gpu/renderer/cache/textureCache.h | 1 + .../host_gpu/renderer/commandScheduler.cpp | 46 +++++++-------- .../host_gpu/renderer/commandScheduler.h | 3 + .../host_gpu/renderer/renderContext.cpp | 7 ++- tests/ShaderRecompilerComputeTests.cpp | 56 +++++++++++++++---- 6 files changed, 81 insertions(+), 37 deletions(-) diff --git a/src/graphics/host_gpu/renderer/cache/textureCache.cpp b/src/graphics/host_gpu/renderer/cache/textureCache.cpp index 2dc642e8..50d23fc9 100644 --- a/src/graphics/host_gpu/renderer/cache/textureCache.cpp +++ b/src/graphics/host_gpu/renderer/cache/textureCache.cpp @@ -1962,6 +1962,11 @@ bool TextureCache::TouchMeta(uint64_t address, uint32_t slice, bool is_clear) { return true; } +bool TextureCache::IsRegionRegistered(uint64_t address, uint64_t size) { + std::scoped_lock lock {m_lock}; + return !FindImagesInRegion(address, size, false).empty(); +} + void TextureCache::UnmapMemory(uint64_t address, uint64_t size) { if (!GuestRange {address, size}.Valid()) { EXIT("TextureCache: invalid unmap range\n"); diff --git a/src/graphics/host_gpu/renderer/cache/textureCache.h b/src/graphics/host_gpu/renderer/cache/textureCache.h index 1c943231..5be210e5 100644 --- a/src/graphics/host_gpu/renderer/cache/textureCache.h +++ b/src/graphics/host_gpu/renderer/cache/textureCache.h @@ -61,6 +61,7 @@ public: uint32_t packed_clear); void InvalidateMemory(uint64_t address, uint64_t size); void InvalidateMemoryFromGPU(uint64_t address, uint64_t size); + [[nodiscard]] bool IsRegionRegistered(uint64_t address, uint64_t size); [[nodiscard]] bool IsMeta(uint64_t address); [[nodiscard]] bool IsMetaCleared(uint64_t address, uint32_t slice); diff --git a/src/graphics/host_gpu/renderer/commandScheduler.cpp b/src/graphics/host_gpu/renderer/commandScheduler.cpp index 5d68954e..81e92156 100644 --- a/src/graphics/host_gpu/renderer/commandScheduler.cpp +++ b/src/graphics/host_gpu/renderer/commandScheduler.cpp @@ -226,43 +226,37 @@ void CommandScheduler::PopPendingOperations() { } void CommandScheduler::DeferOperation(Common::UniqueFunction&& operation) { + QueueOperation(std::move(operation), false); +} + +void CommandScheduler::DeferPriorityOperation(Common::UniqueFunction&& operation) { + QueueOperation(std::move(operation), true); +} + +void CommandScheduler::QueueOperation(Common::UniqueFunction&& operation, bool priority) { CheckActive(); EXIT_IF(!operation); std::unique_lock lock(m_operation_mutex); if (m_operation_state == OperationState::Open) { - m_pending_operations.push({std::move(operation), CurrentTick()}); - return; - } - if (g_deferred_callback_scheduler == this) { + auto& queue = priority ? m_priority_operations : m_pending_operations; + queue.push({std::move(operation), CurrentTick()}); lock.unlock(); - operation(); + if (priority) { + m_operation_available.notify_one(); + } return; } - m_operation_available.wait(lock, - [this] { return m_operation_state == OperationState::Closed; }); + if (g_deferred_callback_scheduler != this) { + m_operation_available.wait(lock, + [this] { return m_operation_state == OperationState::Closed; }); + } lock.unlock(); operation(); } -void CommandScheduler::DeferPriorityOperation(Common::UniqueFunction&& operation) { - CheckActive(); - EXIT_IF(!operation); - std::unique_lock lock(m_operation_mutex); - if (m_operation_state == OperationState::Open) { - m_priority_operations.push({std::move(operation), CurrentTick()}); - lock.unlock(); - m_operation_available.notify_one(); - return; - } - if (g_deferred_callback_scheduler == this) { - lock.unlock(); - operation(); - return; - } - m_operation_available.wait(lock, - [this] { return m_operation_state == OperationState::Closed; }); - lock.unlock(); - operation(); +bool CommandScheduler::HasPendingPriorityOperations() { + std::lock_guard lock(m_operation_mutex); + return !m_priority_operations.empty() || m_priority_active; } void CommandScheduler::PriorityOperationsThread(std::stop_token stop) { diff --git a/src/graphics/host_gpu/renderer/commandScheduler.h b/src/graphics/host_gpu/renderer/commandScheduler.h index f6d590f5..ebdaaa21 100644 --- a/src/graphics/host_gpu/renderer/commandScheduler.h +++ b/src/graphics/host_gpu/renderer/commandScheduler.h @@ -38,8 +38,10 @@ public: void PopPendingOperations(); void DrainPriorityOperations(); void WaitPriorityOperations(uint64_t tick); + // Guest-memory completions use the priority queue; normal callbacks maintain GPU resources. void DeferOperation(Common::UniqueFunction&& operation); void DeferPriorityOperation(Common::UniqueFunction&& operation); + [[nodiscard]] bool HasPendingPriorityOperations(); [[nodiscard]] static bool InDeferredOperation() noexcept; [[nodiscard]] bool Active() const noexcept { return m_command.m_registers != nullptr; } @@ -82,6 +84,7 @@ private: void BeginNext(); void PriorityOperationsThread(std::stop_token stop); + void QueueOperation(Common::UniqueFunction&& operation, bool priority); void RunOperation(Common::UniqueFunction&& operation); MasterSemaphore m_master; diff --git a/src/graphics/host_gpu/renderer/renderContext.cpp b/src/graphics/host_gpu/renderer/renderContext.cpp index 0f16dc47..431a56f4 100644 --- a/src/graphics/host_gpu/renderer/renderContext.cpp +++ b/src/graphics/host_gpu/renderer/renderContext.cpp @@ -99,7 +99,12 @@ void RenderContext::UnmapMemory(uint64_t vaddr, uint64_t size) { vaddr, size); } const auto unmap = [this, vaddr, size] { - if (m_command_scheduler.Active()) { + // Check cache ownership on the GPU thread. Guest-memory callbacks can still + // access a range with no cached data, so they must finish before it is unmapped. + if (m_command_scheduler.Active() && + (m_buffer_cache.IsRegionRegistered(vaddr, size) || + m_texture_cache.IsRegionRegistered(vaddr, size) || + m_command_scheduler.HasPendingPriorityOperations())) { const auto tick = m_command_scheduler.CurrentTick(); m_command_scheduler.Finish(); m_command_scheduler.WaitPriorityOperations(tick); diff --git a/tests/ShaderRecompilerComputeTests.cpp b/tests/ShaderRecompilerComputeTests.cpp index 32332873..5e247130 100644 --- a/tests/ShaderRecompilerComputeTests.cpp +++ b/tests/ShaderRecompilerComputeTests.cpp @@ -2341,6 +2341,7 @@ public: constexpr uint64_t base = 0x0000000200000000ull; constexpr uint64_t page = 0x4000; + const auto cpu_only_tick = scheduler.CurrentTick(); resources.MapMemory(base, page * 4); resources.MapMemory(base + page * 2, page * 4); Require("GpuMappedRangeLifecycle", "union", @@ -2373,8 +2374,9 @@ public: resources.MapMemory(new_prt, page * 6); Require("GpuMappedRangeLifecycle", "PRT replacement", !resources.IsMapped(old_prt, page * 4) && - resources.IsMapped(new_prt, page * 6), - "old-unmap/new-map did not replace full PRT coverage"); + resources.IsMapped(new_prt, page * 6) && + scheduler.CurrentTick() == cpu_only_tick, + "CPU-only unmaps submitted GPU work or lost full PRT coverage"); scheduler.Finish(); context.ShutdownGpu(); @@ -2818,10 +2820,20 @@ public: alignas(uint64_t) uint64_t interrupt_only_gds_label = UINT64_MAX; alignas(uint64_t) uint64_t cb_db_release_label = UINT64_MAX; bool parser_kept_nonblocking_boundaries = false; + bool host_resource_retired = false; + bool cpu_unmap_did_not_submit = false; std::jthread no_gpu_wait([&] { gpu.SendCommandSync([&] { processor->BufferInit(); const auto tick_before = gpu_scheduler.CurrentTick(); + gpu_scheduler.DeferOperation([&] { host_resource_retired = true; }); + constexpr uint64_t cpu_only_address = 0x0000000200400000ull; + context.MapMemory(cpu_only_address, 0x4000); + context.UnmapMemory(cpu_only_address, 0x4000); + cpu_unmap_did_not_submit = + gpu_scheduler.CurrentTick() == tick_before && + !host_resource_retired && + !context.IsMapped(cpu_only_address, 0x4000); processor->TriggerEvent(0x16u, 0u); auto packet = make_release_mem(0, 0, nullptr, 0); Pm4Execution execution; @@ -2887,6 +2899,10 @@ public: no_gpu_wait.join(); gpu.SendCommandSync([&] { gpu_scheduler.Finish(); }); m_runtime_context.device.destroySemaphore(blocked_timeline, nullptr); + Require("GpuCommandLane", "CPU-only unmap with pending host retirement", + parser_did_not_wait_gpu && cpu_unmap_did_not_submit && + host_resource_retired, + "CPU-only unmap submitted or waited for unrelated native work"); Require("GpuCommandLane", "nonblocking GPU packets", parser_did_not_wait_gpu && parser_kept_nonblocking_boundaries, "a cache event, GL2 writeback, DE counter, or interrupt boundary " @@ -3321,13 +3337,14 @@ public: "an unrelated unmap waited for a blocked PM4 submission"); auto &scheduler = context.GetCommandScheduler(); - std::atomic normal_completed{false}; - gpu.SendCommandSync( - [&] { scheduler.DeferOperation([&] { normal_completed = true; }); }); + std::atomic completion_published{false}; + gpu.SendCommandSync([&] { + scheduler.DeferPriorityOperation([&] { completion_published = true; }); + }); resources.MapMemory(empty_unmap_base, empty_unmap_size); resources.UnmapMemory(empty_unmap_base, empty_unmap_size); Require("GpuCommandLane", "unmap native completion", - normal_completed.load() && + completion_published.load() && !resources.IsMapped(empty_unmap_base, empty_unmap_size), "unmap returned before an earlier native guest-memory callback"); @@ -4100,6 +4117,13 @@ public: cache.IsRegionRegistered(index_begin - 1, index_span + 2), "indexed lookup mishandled a half-open boundary, gap, or broad " "overlap"); + const auto clean_unmap_tick = scheduler.CurrentTick(); + resources.UnmapMemory(index_begin, index_page); + Require(name, "clean cached buffer unmap drain", + scheduler.CurrentTick() == clean_unmap_tick + 1 && + !resources.IsMapped(index_begin, index_page), + "unmapping a clean cached buffer did not drain native work"); + resources.MapMemory(index_begin, index_page); const auto index_bridge = cache.FindBuffer(index_begin + index_page - 1, index_page + 2); Require(name, "registered-range merge", @@ -7138,12 +7162,24 @@ public: Prospero::ImageType::kColor2D, {2048, 1, 1}, 1, 4, 1); const auto partial_unmap_image_id = texture_cache.FindImage(partial_unmap_image); - texture_cache.UnmapMemory(partial_unmap_image.info.data.address, 0x1000); + const auto image_address = partial_unmap_image.info.data.address; + Require(name, "registered image boundaries", + texture_cache.IsRegionRegistered(image_address, 1) && + texture_cache.IsRegionRegistered(image_address + 0x1fff, 1) && + !texture_cache.IsRegionRegistered(image_address - 1, 1) && + !texture_cache.IsRegionRegistered(image_address + 0x2000, 1) && + texture_cache.IsRegionRegistered(image_address - 1, 0x2002), + "image lookup mishandled a partial overlap or adjacent range"); + scheduler.Finish(); + scheduler.DrainPriorityOperations(); + const auto image_unmap_tick = scheduler.CurrentTick(); + resources.UnmapMemory(image_address + 0x1000, 0x1000); Require(name, "partial image unmap tracking", partial_unmap_image_id && - !texture_cache.FindImageFromRange( - partial_unmap_image.info.data.address, 0x2000, false), - "partial unmap left the deleted image's mapped tail tracked"); + scheduler.CurrentTick() == image_unmap_tick + 1 && + !texture_cache.IsRegionRegistered(image_address, 0x2000) && + !texture_cache.FindImageFromRange(image_address, 0x2000, false), + "partial image unmap skipped its drain or retained the image"); constexpr uint64_t unformatted_alias_offset = 0x2500000; auto unformatted_alias =