diff --git a/src/graphics/host_gpu/renderer/cache/bufferCache.cpp b/src/graphics/host_gpu/renderer/cache/bufferCache.cpp index 1a28c97b..c2258168 100644 --- a/src/graphics/host_gpu/renderer/cache/bufferCache.cpp +++ b/src/graphics/host_gpu/renderer/cache/bufferCache.cpp @@ -259,51 +259,36 @@ void BufferCache::ReadMemory(uint64_t vaddr, uint64_t size, bool is_write) { } void BufferCache::ReadMemoryOnGpu(uint64_t vaddr, uint64_t size, bool is_write) { - // CPU invalidation reaches this point only for a GPU-owned tracker page. Resolve the exact - // Buffer owner on the GPU thread so the cache index remains single-thread-owned. if (is_write && !IsRegionRegistered(vaddr, size)) { return; } + auto& buffer = m_slot_buffers[FindBuffer(vaddr, size)]; + + // Widen nearby CPU reads so they share one GPU drain. + constexpr uint64_t WindowSize = 512 * 1024; + const auto buffer_begin = buffer.CpuAddress(); + const auto buffer_end = buffer_begin + buffer.Size(); + const auto window_begin = std::max(vaddr & ~(WindowSize - 1), buffer_begin); + const auto window_end = std::min(std::max(window_begin + WindowSize, vaddr + size), buffer_end); + std::vector copies; m_memory_tracker.ForEachDownloadRange( - vaddr, size, + window_begin, window_end - window_begin, [&](uint64_t address, uint64_t bytes) noexcept { m_memory_tracker.ValidateGpuDirtyPages(m_gpu_modified_ranges, address, bytes, "memory invalidation"); }, - [&](uint64_t address, uint64_t bytes) noexcept { + [&](uint64_t address, uint64_t bytes) noexcept { for (const auto range: m_gpu_modified_ranges.Intersections(address, bytes)) { - for (uint64_t copied = 0; copied < range.size;) { - const auto copy_address = range.address + copied; - const auto* owner = m_page_table.Find(copy_address >> PageTable::kPageBits); - if (owner == nullptr || !*owner) { - EXIT("BufferCache: invalidation readback has no buffer owner\n"); - } - auto& buffer = m_slot_buffers[*owner]; - if (!buffer.IsInBounds(copy_address, 1)) { - EXIT("BufferCache: invalidation readback is outside its buffer owner\n"); - } - const auto copy_size = std::min( - range.size - copied, buffer.CpuAddress() + buffer.Size() - copy_address); - copies.push_back( - {&buffer, buffer.Offset(copy_address), copy_address, copy_size}); - copied += copy_size; - } + copies.push_back( + {&buffer, buffer.Offset(range.address), range.address, range.size}); } }); - if (copies.empty()) { - if (!is_write) { - return; - } - // A preceding read fault can consume the last GPU-owned copy after this write invalidation - // has already chosen to flush. Complete the CPU ownership handoff even though this callback - // no longer has bytes to download. - m_memory_tracker.MarkRegionAsCpuModified(vaddr, size); - return; + if (!copies.empty()) { + DownloadBufferMemory(copies); + // The enumeration covered whole dirty pages and every exact interval on them. + m_memory_tracker.UnmarkRegionAsGpuModified(window_begin, window_end - window_begin); } - DownloadBufferMemory(copies); - // The enumeration above covered whole dirty pages and every exact interval on them. - m_memory_tracker.UnmarkRegionAsGpuModified(vaddr, size); if (is_write) { m_memory_tracker.MarkRegionAsCpuModified(vaddr, size); } diff --git a/tests/ShaderRecompilerComputeTests.cpp b/tests/ShaderRecompilerComputeTests.cpp index 21a6cf09..7e75c938 100644 --- a/tests/ShaderRecompilerComputeTests.cpp +++ b/tests/ShaderRecompilerComputeTests.cpp @@ -3360,7 +3360,9 @@ public: resources.HandleFault(PageFaultAccess::Read, base + ring_fault_first_offset), "fault readback could not wrap a live download-ring tick"); - uint64_t expected_packing_offset = 128; + // The widened window collects the two earlier writes and both + // fault-adjacent writes. + uint64_t expected_packing_offset = 4 * 64; uint64_t expected_packing_alignment = 1; if (!download.IsCoherent()) { Require(name, "fault-ring atom policy", @@ -3377,14 +3379,22 @@ public: packing_offset == expected_packing_offset, "adjacent fault downloads did not reserve an atom-safe stride"); download.Commit(); + uint32_t widened_first_backing = 0; + uint32_t widened_second_backing = 0; uint32_t ring_fault_first_backing = 0; uint32_t ring_fault_second_backing = 0; + std::memcpy(&widened_first_backing, memory + first_offset, + sizeof(widened_first_backing)); + std::memcpy(&widened_second_backing, memory + second_offset, + sizeof(widened_second_backing)); std::memcpy(&ring_fault_first_backing, memory + ring_fault_first_offset, sizeof(ring_fault_first_backing)); std::memcpy(&ring_fault_second_backing, memory + ring_fault_second_offset, sizeof(ring_fault_second_backing)); Require(name, "fault-ring wrapped contents", - ring_fault_first_backing == ring_fault_first_value && + widened_first_backing == first_value && + widened_second_backing == second_value && + ring_fault_first_backing == ring_fault_first_value && ring_fault_second_backing == ring_fault_second_value && &BufferCacheTestAccess::DownloadBuffer(cache) == fixed_download && @@ -3392,6 +3402,64 @@ public: download.Size() == (32ull << 20), "wrapped fault batch published incorrect disjoint ranges"); + constexpr uint64_t window_size = 512 * 1024; + constexpr uint64_t window_owner_offset = 0x240000; + constexpr uint64_t window_owner_size = 0xc0000; + constexpr uint64_t window_fault_offset = window_owner_offset + 0x100; + constexpr uint64_t window_inside_offset = + window_owner_offset + window_size - sizeof(uint32_t); + constexpr uint64_t window_outside_offset = + window_owner_offset + window_size; + constexpr uint32_t window_stale = 0x13579bdfu; + constexpr uint32_t window_value = 0x2468ace0u; + constexpr std::array window_offsets{ + window_fault_offset, window_inside_offset, window_outside_offset}; + static_assert((base + window_owner_offset) % window_size == + window_size / 2); + for (const auto offset : window_offsets) { + Libs::LibKernel::Memory::WriteBacking(base + offset, &window_stale, + sizeof(window_stale)); + } + (void)cache.FindBuffer(base + window_owner_offset, window_owner_size); + for (const auto offset : window_offsets) { + MarkGpuWrite(base + offset, sizeof(window_value)); + cache.FillBuffer(base + offset, sizeof(window_value), window_value, + false); + } + cache.ReadMemory(base + window_fault_offset, sizeof(window_value)); + uint32_t window_inside_backing = 0; + uint32_t window_outside_backing = 0; + Libs::LibKernel::Memory::TryReadBacking(base + window_inside_offset, + &window_inside_backing, + sizeof(window_inside_backing)); + Libs::LibKernel::Memory::TryReadBacking(base + window_outside_offset, + &window_outside_backing, + sizeof(window_outside_backing)); + Require(name, "widened-window boundary", + window_inside_backing == window_value && + window_outside_backing == window_stale && + !cache.HasGpuDirtyBytes(base + window_inside_offset, + sizeof(window_value)) && + cache.HasGpuDirtyBytes(base + window_outside_offset, + sizeof(window_value)) && + !cache.IsRegionGpuModified(base + window_inside_offset, + sizeof(window_value)) && + cache.IsRegionGpuModified(base + window_outside_offset, + sizeof(window_value)), + "readback did not honor the clamped half-open 512 KiB window"); + cache.ReadMemory(base + window_outside_offset, sizeof(window_value)); + + Libs::LibKernel::Memory::WriteBacking(base + first_offset, &first_stale, + sizeof(first_stale)); + Libs::LibKernel::Memory::WriteBacking(base + second_offset, &second_stale, + sizeof(second_stale)); + MarkGpuWrite(base + first_offset, sizeof(first_value)); + MarkGpuWrite(base + second_offset, sizeof(second_value)); + cache.FillBuffer(base + first_offset, sizeof(first_value), first_value, + false); + cache.FillBuffer(base + second_offset, sizeof(second_value), second_value, + false); + for (uint32_t tick = 0; tick < 160; tick++) { cache.RunGarbageCollector(); } @@ -3605,16 +3673,24 @@ public: sizeof(partial_unmap_survivor_value), partial_unmap_survivor_value, false); resources.UnmapMemory(base + 0x8000, 0x4000); - Require(name, "partial-invalidation ownership", - !resources.IsMapped(base + 0x8000, 0x4000) && - cache.IsRegionRegistered(base + 0x8000, 0x8000) && - cache.IsRegionCpuModified(base + 0x8000, 0x4000) && - !cache.HasGpuDirtyBytes(base + partial_unmap_offset, - sizeof(partial_unmap_value)) && - cache.HasGpuDirtyBytes( - base + partial_unmap_survivor_offset, - sizeof(partial_unmap_survivor_value)), - "partial invalidation retired its owner or cleared disjoint GPU bytes"); + uint32_t partial_unmap_survivor_backing = 0; + std::memcpy(&partial_unmap_survivor_backing, + memory + partial_unmap_survivor_offset, + sizeof(partial_unmap_survivor_backing)); + Require( + name, "partial-invalidation ownership", + !resources.IsMapped(base + 0x8000, 0x4000) && + cache.IsRegionRegistered(base + 0x8000, 0x8000) && + cache.IsRegionCpuModified(base + 0x8000, 0x4000) && + !cache.IsRegionCpuModified( + base + partial_unmap_survivor_offset, + sizeof(partial_unmap_survivor_value)) && + !cache.HasGpuDirtyBytes(base + partial_unmap_offset, + sizeof(partial_unmap_value)) && + !cache.HasGpuDirtyBytes(base + partial_unmap_survivor_offset, + sizeof(partial_unmap_survivor_value)) && + partial_unmap_survivor_backing == partial_unmap_survivor_value, + "widened partial invalidation mishandled ownership or backing"); resources.MapMemory(base + 0x8000, 0x4000); auto survivor = cache.ObtainBuffer(base + partial_unmap_survivor_offset,