From 00fafd437999f68d46d6b0b5ad6bd51717b0f5f8 Mon Sep 17 00:00:00 2001 From: MathiasDevelopes <25400788+MathiasDevelopes@users.noreply.github.com> Date: Fri, 9 Oct 2026 16:27:24 +0200 Subject: [PATCH 1/4] fix(agc): count copied guest buffers and draw snapshots in the batch byte budget The recorder bounds the bytes a batch keeps alive (KeptBytesBudget) and waits for older batches past twice that. A recorded dispatch or draw kept its ShaderResources, and a draw its snapshot buffers, with a size of 0, so the host-visible copies of guest buffers they own never counted. A title that binds a 124 MiB writable guest buffer in every draw then held about ninety copies, 11 GB, until the GPU caught up, and aborted with VK_ERROR_OUT_OF_HOST_MEMORY from vkAllocateMemory. GuestBufferMemory reports the bytes of its copied regions, and the dispatch and draw keeps and the draw snapshots pass their bytes to Recorder::Keep, so the existing bound applies to them. Co-Authored-By: Claude Sonnet 5.5 --- core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp | 2 +- .../libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp | 5 +++++ .../prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp | 1 + core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp | 2 +- .../prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp | 4 +++- core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp | 2 ++ 6 files changed, 13 insertions(+), 3 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp index aa1135187..ed683ed90 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp @@ -3485,7 +3485,7 @@ void VulkanDevice::recordDispatch(RecordedDispatch& record) { const auto commands = recorder.Commands(&covered); recordStep(PhaseRecordCommands); recorder.Keep(record.objects); - recorder.Keep(record.resources); + recorder.Keep(record.resources, record.resources != nullptr ? record.resources->CopiedBytes() : 0u); recordStep(PhaseRecordKeeps); if (record.dataRefresh != RecordedDispatch::DataRefresh::None) { // The template's data buffers take this dispatch's words: a transfer write the pre-dispatch diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp index 7aa407690..cea14f48c 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp @@ -243,6 +243,11 @@ public: // Whether registered allocations are pinned until write-back (address-based shaders): by this // build's own lease, or by the cached address space it holds. bool HoldsLease() const { return !lease.empty() || space != nullptr; } + std::size_t CopiedBytes() const { + std::size_t bytes = 0; + for (const auto& region : regions) if (region.buffer != nullptr) bytes += static_cast(region.end - region.begin); + return bytes; + } // Every uploaded region as [begin, end) when all of them are served by host imports, in place or // through a device-local staging copy of the import (nothing was copied through the CPU, so the // upload can serve a later identical build), else nothing. diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp index 53b5650c6..3539c7019 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp @@ -149,6 +149,7 @@ public: // Whether a written buffer was copied (its results reach guest memory by the CPU write-back). bool HasCopiedWrites() const { return guestMemory.HasCopiedWrites(); } bool HoldsLease() const { return guestMemory.HoldsLease(); } + std::size_t CopiedBytes() const { return guestMemory.CopiedBytes(); } bool WritesOverlap(std::uint64_t address, std::size_t bytes) const { return guestMemory.WritesOverlap(address, bytes); } // Whether a region the recorded work reads in place through a host import overlaps the range. bool ReadsOverlap(std::uint64_t address, std::size_t bytes) const; diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp index fb6a8680e..2af257b28 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp @@ -1432,7 +1432,7 @@ void keepRecordedDraw(Recorder& recorder, const std::shared_ptr kept->scratch = std::move(scratch); kept->targets = std::move(targets); const auto& residents = kept->targets; - recorder.Keep(kept); + recorder.Keep(kept, kept->resources != nullptr ? kept->resources->CopiedBytes() : 0u); if (checkRecords) recorder.OnComplete(std::move(checkRecords)); // Counted for the [address-sync] line: a lease released by the completion (the pin waiter // finishes the recorder up to the open batch, which holds the kept resources and gets the diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp index 8afcbb6c1..1033234fb 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp @@ -3368,7 +3368,9 @@ std::shared_ptr ShaderResources::PrepareDrawBindi writes.push_back(write); } update(context.device, static_cast(writes.size()), writes.data(), 0, nullptr); - recorder.Keep(result); + std::size_t snapshotBytes = 0; + for (const auto& snapshot : result->snapshots) snapshotBytes += snapshot.buffer->Bytes().size(); + recorder.Keep(result, snapshotBytes); return result; } diff --git a/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp b/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp index fdf13a5c5..79403d4cf 100644 --- a/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp @@ -576,7 +576,9 @@ void RunBdaResourceTests(const Context& context, const BdaTestAccess& access) { GuestBufferMemory memory(context); memory.AddWritable(address, sizeof(guest)); memory.AddWritable(address + 16, 16); + Require(memory.CopiedBytes() == 0, "a region counted as copied before its upload"); memory.Upload(true); + Require(memory.CopiedBytes() == 0 || memory.CopiedBytes() == sizeof(guest), "copied bytes do not match the one region copied"); std::uint32_t adjustment = 0; const auto first = memory.Descriptor(address, sizeof(guest), adjustment); Require(adjustment == 0, "a view at its owner's start is bound off it"); From 4940de5436a449ad6685ca25bd9b821a7b5419d1 Mon Sep 17 00:00:00 2001 From: MathiasDevelopes <25400788+MathiasDevelopes@users.noreply.github.com> Date: Sat, 10 Oct 2026 09:44:30 +0000 Subject: [PATCH 2/4] fix(agc): count memory shared by a batch's keeps once and test the three keep sites A ShaderResources from the resource caches is the same object for every draw or dispatch that uses it, and Recorder::ReusableDrawSnapshot hands the next draw the same snapshot buffer, so passing their bytes with each Keep counted one allocation once per use and submitted the batch before it held the budget. Recorder::KeepBytes counts an owner's bytes once per batch, and the dispatch, draw and snapshot keeps use it. The recorded kept bytes test runs a dispatch and a draw that copy a guest buffer and two draws that snapshot one on the device, and fails when any of the three sites stops counting or a reused snapshot counts twice. The BdaResources check pins the copied region it sets up. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01HZHkb8kUwnHAuBBbHywvJf --- .../Execution/src/VulkanDevice.cpp | 3 +- .../Graphics/include/Recorder.hpp | 10 + .../prx/libSceAgcDriver/Graphics/src/Draw.cpp | 3 +- .../libSceAgcDriver/Graphics/src/Recorder.cpp | 6 + .../Graphics/src/ShaderResources.cpp | 6 +- .../libSceAgcDriver/tests/BdaResources.cpp | 4 +- .../tests/RecorderKeptBytes.cpp | 23 ++ .../tests/execution/RecordedKeptBytes.cpp | 319 ++++++++++++++++++ 8 files changed, 368 insertions(+), 6 deletions(-) create mode 100644 core/libs/prx/libSceAgcDriver/tests/execution/RecordedKeptBytes.cpp diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp index ed683ed90..f2a14508f 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp @@ -3485,7 +3485,8 @@ void VulkanDevice::recordDispatch(RecordedDispatch& record) { const auto commands = recorder.Commands(&covered); recordStep(PhaseRecordCommands); recorder.Keep(record.objects); - recorder.Keep(record.resources, record.resources != nullptr ? record.resources->CopiedBytes() : 0u); + recorder.Keep(record.resources); + if (record.resources != nullptr) recorder.KeepBytes(record.resources.get(), record.resources->CopiedBytes()); recordStep(PhaseRecordKeeps); if (record.dataRefresh != RecordedDispatch::DataRefresh::None) { // The template's data buffers take this dispatch's words: a transfer write the pre-dispatch diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp index d8888787d..490d2479e 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -85,6 +86,13 @@ public: // Whether recorded work still has completion actions (write-backs the CPU must see) to run. bool HasCompletions() const; void Keep(std::shared_ptr object, std::size_t bytes = 0); + // Counts `bytes` held by `owner` in the open batch's kept bytes once per batch: memory several + // keeps of one batch share (a cached ShaderResources kept by every draw or dispatch using it, a + // reused draw snapshot) counts once. The caller keeps `owner` alive in the same batch, so its + // address names one object for the batch's lifetime. + void KeepBytes(const void* owner, std::size_t bytes); + // The kept bytes of the open batch (0 with none open), for tests. + std::size_t OpenKeptBytes() const { return open != nullptr ? open->keptBytes : 0; } static constexpr std::size_t KeptBytesBudget = std::size_t{512} << 20u; void BoundKeptBytes(); std::size_t InFlightKeptBytes() const { return inFlightKeptBytes; } @@ -511,6 +519,8 @@ private: VkFence fence = VK_NULL_HANDLE; std::vector> kept; std::size_t keptBytes = 0; + // The owners KeepBytes counted in this batch. + std::unordered_set keptOwners; std::vector> completions; std::vector> writes; std::vector writeNotes; diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp index 2af257b28..6b808d864 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp @@ -1432,7 +1432,8 @@ void keepRecordedDraw(Recorder& recorder, const std::shared_ptr kept->scratch = std::move(scratch); kept->targets = std::move(targets); const auto& residents = kept->targets; - recorder.Keep(kept, kept->resources != nullptr ? kept->resources->CopiedBytes() : 0u); + recorder.Keep(kept); + if (resources != nullptr) recorder.KeepBytes(resources.get(), resources->CopiedBytes()); if (checkRecords) recorder.OnComplete(std::move(checkRecords)); // Counted for the [address-sync] line: a lease released by the completion (the pin waiter // finishes the recorder up to the open batch, which holds the kept resources and gets the diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp index 5297bd42f..fea9b921a 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp @@ -2452,6 +2452,12 @@ void Recorder::Keep(std::shared_ptr object, std::size_t bytes) { open->keptBytes += bytes; } +void Recorder::KeepBytes(const void* owner, std::size_t bytes) { + if (owner == nullptr || bytes == 0) return; + ensureOpen(); + if (open->keptOwners.insert(owner).second) open->keptBytes += bytes; +} + void Recorder::BoundKeptBytes() { if (!GuestMemory::GpuMutex().HeldByThisThread()) return; if (open != nullptr && open->keptBytes >= KeptBytesBudget) Submit(); diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp index 1033234fb..f5b41011e 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp @@ -3368,9 +3368,9 @@ std::shared_ptr ShaderResources::PrepareDrawBindi writes.push_back(write); } update(context.device, static_cast(writes.size()), writes.data(), 0, nullptr); - std::size_t snapshotBytes = 0; - for (const auto& snapshot : result->snapshots) snapshotBytes += snapshot.buffer->Bytes().size(); - recorder.Keep(result, snapshotBytes); + recorder.Keep(result); + // A reused snapshot (Recorder::ReusableDrawSnapshot) is one buffer however many draws bind it. + for (const auto& snapshot : result->snapshots) recorder.KeepBytes(snapshot.buffer.get(), snapshot.buffer->Bytes().size()); return result; } diff --git a/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp b/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp index 79403d4cf..2ad77ce62 100644 --- a/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp @@ -578,7 +578,9 @@ void RunBdaResourceTests(const Context& context, const BdaTestAccess& access) { memory.AddWritable(address + 16, 16); Require(memory.CopiedBytes() == 0, "a region counted as copied before its upload"); memory.Upload(true); - Require(memory.CopiedBytes() == 0 || memory.CopiedBytes() == sizeof(guest), "copied bytes do not match the one region copied"); + // `guest` is stack memory no allocation registers, so no host import serves it: its two views merge + // into one copied region. + Require(memory.CopiedBytes() == sizeof(guest), "copied bytes " + std::to_string(memory.CopiedBytes()) + " do not match the one region copied"); std::uint32_t adjustment = 0; const auto first = memory.Descriptor(address, sizeof(guest), adjustment); Require(adjustment == 0, "a view at its owner's start is bound off it"); diff --git a/core/libs/prx/libSceAgcDriver/tests/RecorderKeptBytes.cpp b/core/libs/prx/libSceAgcDriver/tests/RecorderKeptBytes.cpp index a8d342563..4997d6be4 100644 --- a/core/libs/prx/libSceAgcDriver/tests/RecorderKeptBytes.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/RecorderKeptBytes.cpp @@ -194,6 +194,28 @@ void CountFollowsEveryPath() { Expect(recorder.Reap() && recorder.InFlightKeptBytes() == 0, "a reaped batch still counts " + std::to_string(recorder.InFlightKeptBytes()) + " kept bytes in flight"); } +void SharedOwnerCountsOncePerBatch() { + mock = MockDevice{}; + std::lock_guard gpu(AgcDriver::GuestMemory::GpuMutex()); + Recorder recorder(mockContext()); + auto shared = std::make_shared(0); + for (int keep = 0; keep < 3; ++keep) { + recorder.Keep(shared); + recorder.KeepBytes(shared.get(), Budget / 2); + recorder.BoundKeptBytes(); + } + Expect(recorder.Recording() && recorder.OpenKeptBytes() == Budget / 2 && mock.submits == 0, "three keeps of one owner's half budget counted " + std::to_string(recorder.OpenKeptBytes()) + " kept bytes, not half the budget once"); + auto other = std::make_shared(1); + recorder.Keep(other); + recorder.KeepBytes(other.get(), Budget / 2); + recorder.BoundKeptBytes(); + Expect(!recorder.Recording() && recorder.Submissions() == 1 && recorder.InFlightKeptBytes() == Budget, "a second owner's half budget did not complete the batch's budget"); + recorder.Keep(shared); + recorder.KeepBytes(shared.get(), Budget / 2); + Expect(recorder.OpenKeptBytes() == Budget / 2, "the next batch did not count an owner the submitted batch counted"); + recorder.Sync(); +} + } int main() { @@ -202,6 +224,7 @@ int main() { InFlightWithinTwiceTheBudget(); SyncedWithinTwiceTheBudget(); CountFollowsEveryPath(); + SharedOwnerCountsOncePerBatch(); } catch (const std::exception& error) { std::fprintf(stderr, "%s\n", error.what()); return 1; diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/RecordedKeptBytes.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/RecordedKeptBytes.cpp new file mode 100644 index 000000000..b82cc95b2 --- /dev/null +++ b/core/libs/prx/libSceAgcDriver/tests/execution/RecordedKeptBytes.cpp @@ -0,0 +1,319 @@ +#include "prx/libSceAgcDriver/Execution/include/GuestMemory.hpp" +#include "prx/libSceAgcDriver/Execution/include/VulkanDevice.hpp" +#include "prx/libSceAgcDriver/Graphics/include/ColorTargetLayout.hpp" +#include "prx/libSceAgcDriver/Graphics/include/Draw.hpp" +#include "prx/libSceAgcDriver/Graphics/include/Recorder.hpp" +#include "prx/libSceAgcDriver/Graphics/include/Texture.hpp" +#include "prx/libc/include/GuestAllocations.hpp" +#include "prx/libc/include/GuestArena.hpp" +#include "prx/libc/include/GuestWriteWatch.hpp" +#include "Recompiler.hpp" +#include "VulkanTestDevice.hpp" +#ifdef _WIN32 +#ifndef NOMINMAX +#define NOMINMAX +#endif +#include +#else +#include +#endif +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +// The bytes a recorded dispatch or draw holds alive reach the recorder's batch budget +// (Recorder::KeptBytesBudget): the copies of guest buffers its ShaderResources own (kept by +// VulkanDevice::recordDispatch and Draw.cpp's keepRecordedDraw) and a draw's snapshots of buffers it +// reads in place (ShaderResources::PrepareDrawBindings). A snapshot reused by the next draw of the +// batch is one buffer and counts once. + +namespace { + +using AgcDriver::Graphics::ColorTargetLayout; +using AgcDriver::Graphics::ColorTileMode; +using AgcDriver::Graphics::Recorder; +using AgcDriver::Graphics::Require; +using ShaderRecompiler::ShaderStage; + +constexpr std::uint32_t Threads = 32; +constexpr std::size_t BufferBytes = 65536; +constexpr std::uint32_t Width = 256; +constexpr std::uint32_t Height = 128; + +// v1 = v0 << 3; v[2:3] = 0; buffer_store_dwordx2 v[2:3], v1, s[0:3] offen +alignas(256) constexpr std::array StoreCode{ + 0x34020083, 0x7e040280, 0x7e060280, 0xe0741000, 0x80000201, 0xbf810000, +}; + +// buffer_load_format_xyzw v[0:3], v5, s[0:3] idxen; exp pos0 v[0:3] done; s_endpgm +alignas(256) constexpr std::array VertexCode{ + 0xe0382000, 0x80000005, 0xbf8c3f70, 0xf80008cf, 0x03020100, 0xbf810000, +}; + +// v7 = 1.0; exp mrt0 v7 v7 v7 v7 done vm; s_endpgm +alignas(256) constexpr std::array PixelCode{ + 0x7e0e02f2, 0xf800180f, 0x07070707, 0xbf810000, +}; + +std::uint64_t AddressOf(const void* data) { + return static_cast(reinterpret_cast(data)); +} + +std::array RawBuffer(const void* data, std::uint32_t bytes) { + const auto address = AddressOf(data); + return {static_cast(address), static_cast((address >> 32u) & 0xffffu), bytes, 0x31016facu}; +} + +std::array VertexBuffer(const void* data, std::uint32_t stride, std::uint32_t count) { + const auto address = AddressOf(data); + return {static_cast(address), static_cast((address >> 32u) & 0xffffu) | (stride << 16u), count, 0x01016facu}; +} + +// Guest memory, registered (served in place by a host import where the device has them, and +// write-watched where the host can watch it, as draw snapshots need to be reused) or plain (every +// use copies it). +class GuestBuffer { +public: + explicit GuestBuffer(bool registered, std::size_t bytes = BufferBytes) : registered(registered), bytes(bytes), watched(registered && AgcDriver::GuestMemory::WriteWatched()) { +#ifdef _WIN32 + if (watched) { + block = static_cast(GuestArena::GuestArenaAllocate_nid_postfix(bytes, 65536)); + if (block != nullptr) GuestArena::GuestArenaCommit_nid_postfix(block, bytes, PAGE_READWRITE, bytes); + } else { + block = static_cast(VirtualAlloc(nullptr, bytes, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE)); + } +#else + if (watched) { + constexpr std::uintptr_t alignment = 65536; + void* mapped = mmap(nullptr, bytes + alignment, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + Require(mapped != MAP_FAILED, "recorded kept bytes: cannot map a guest buffer"); + const auto begin = reinterpret_cast(mapped); + const auto aligned = (begin + alignment - 1) & ~(alignment - 1); + if (aligned != begin) munmap(mapped, aligned - begin); + if (aligned + bytes != begin + bytes + alignment) munmap(reinterpret_cast(aligned + bytes), begin + alignment - aligned); + block = reinterpret_cast(aligned); + GuestWriteWatch::GuestWriteWatchRegister_nid_postfix(block, bytes); + } else { + block = static_cast(std::aligned_alloc(65536, bytes)); + } +#endif + Require(block != nullptr, "recorded kept bytes: cannot allocate a guest buffer"); + std::memset(block, 0, bytes); + if (registered) GuestAllocations::Mutation().Add(block, bytes, true, true); + } + + ~GuestBuffer() { + if (registered) { + AgcDriver::Graphics::StorageTexture::FlushPending(AddressOf(block), bytes, nullptr, "test release"); + GuestAllocations::Mutation().Remove(block); + } +#ifdef _WIN32 + if (watched) { + GuestArena::GuestArenaReset_nid_postfix(block, bytes); + GuestArena::GuestArenaRelease_nid_postfix(block, bytes); + } else { + VirtualFree(block, 0, MEM_RELEASE); + } +#else + if (watched) { + munmap(block, bytes); + GuestWriteWatch::GuestWriteWatchUnregister_nid_postfix(block, bytes); + } else { + std::free(block); + } +#endif + } + + GuestBuffer(const GuestBuffer&) = delete; + GuestBuffer& operator=(const GuestBuffer&) = delete; + + std::uint8_t* Data() { return block; } + +private: + bool registered; + std::size_t bytes; + bool watched; + std::uint8_t* block = nullptr; +}; + +Recorder& ActiveRecorder() { + auto* recorder = Recorder::Active(); + Require(recorder != nullptr, "recorded kept bytes: no active recorder"); + return *recorder; +} + +std::string Bytes(std::size_t bytes) { + return std::to_string(bytes) + " bytes"; +} + +void Dispatch(AgcDriver::VulkanDevice& device, std::uint8_t* target) { + std::vector userData(16, 0u); + const auto descriptor = RawBuffer(target, static_cast(BufferBytes)); + std::copy(descriptor.begin(), descriptor.end(), userData.begin()); + const std::span code(StoreCode); + const std::array memory{{{reinterpret_cast(code.data()), std::as_bytes(code)}}}; + const ShaderRecompiler::ShaderComputeStageInfo compute{{Threads, 1, 1}, 0u, {false, false, false}, false, 1}; + ShaderRecompiler::RecompileRequest request{ + {ShaderStage::Compute, reinterpret_cast(code.data()), code, 0, {}}, + {32, 0, userData, compute, std::nullopt, std::nullopt, memory}, + device.Target(), + {0, 0, 0, 128} + }; + request.useCache = false; + const auto result = ShaderRecompiler::Recompile(request); + device.Dispatch(result, 1, 1, 1, {}, reinterpret_cast(code.data())); +} + +const ColorTargetLayout TargetLayout(Width, Height, ColorTileMode::RenderTarget, 4); + +constexpr std::array, 3> Triangle{{ + {-1.0f, -1.0f, 0.5f, 1.0f}, {3.0f, -1.0f, 0.5f, 1.0f}, {-1.0f, 3.0f, 0.5f, 1.0f} +}}; + +// Vertex records for Draw: the triangle, then zeros up to BufferBytes. +void FillVertices(GuestBuffer& vertices) { + std::memcpy(vertices.Data(), Triangle.data(), sizeof(Triangle)); +} + +// A full-target triangle into a resident (tiled) color target, so the draw is recorded, whose +// vertex stage reads its positions from `vertices` (BufferBytes of records, so the bound range is +// the whole buffer). +void Draw(AgcDriver::VulkanDevice& device, GuestBuffer& pixels, GuestBuffer& vertices) { + const auto target = device.Target(); + std::vector vertexUserData(4, 0u); + const auto vertexBuffer = VertexBuffer(vertices.Data(), 16u, static_cast(BufferBytes / 16u)); + std::copy(vertexBuffer.begin(), vertexBuffer.end(), vertexUserData.begin()); + const std::array vertexMemory{{{reinterpret_cast(VertexCode.data()), std::as_bytes(std::span(VertexCode))}}}; + ShaderRecompiler::RecompileRequest vertex{ + {ShaderStage::Vertex, reinterpret_cast(VertexCode.data()), VertexCode, 0, {}}, + {64, 0, vertexUserData, std::nullopt, std::nullopt, ShaderRecompiler::ShaderVertexStageInfo{}, vertexMemory}, + target, + {0, 0, 0, 64} + }; + vertex.useCache = false; + const auto vertexResult = ShaderRecompiler::Recompile(vertex); + const auto vertexPush = static_cast(vertexResult.pushConstants.size()); + + ShaderRecompiler::ShaderPixelStageInfo pixel{}; + pixel.inputAddr = ShaderRecompiler::PixelInputBit(ShaderRecompiler::PixelInput::PositionX) | ShaderRecompiler::PixelInputBit(ShaderRecompiler::PixelInput::PositionY); + pixel.posX = true; + pixel.posY = true; + pixel.targetOutputMode[0] = 9; + pixel.targetExportMapping.fill(0xe4u); + std::vector pixelUserData(4, 0u); + const std::array pixelMemory{{{reinterpret_cast(PixelCode.data()), std::as_bytes(std::span(PixelCode))}}}; + ShaderRecompiler::RecompileRequest fragment{ + {ShaderStage::Fragment, reinterpret_cast(PixelCode.data()), PixelCode, 0, {}}, + {64, 0, pixelUserData, std::nullopt, pixel, std::nullopt, pixelMemory}, + target, + {0, 0, vertexPush, 128 - vertexPush} + }; + fragment.useCache = false; + const auto pixelResult = ShaderRecompiler::Recompile(fragment); + const std::array shaders{{ + {ShaderStage::Vertex, &vertexResult, 0}, + {ShaderStage::Fragment, &pixelResult, vertexPush} + }}; + + AgcDriver::Graphics::State state{}; + state.stages = {AgcDriver::Graphics::ShaderPath::Vertex, 0u, 64u, 64u, std::nullopt, std::nullopt}; + state.color = {AddressOf(pixels.Data()), {Width, Height}, VK_FORMAT_R8G8B8A8_UNORM, TargetLayout.Bytes(), 0xe4u, ColorTileMode::RenderTarget}; + state.colors = {state.color}; + state.hasColorTarget = true; + state.renderExtent = {Width, Height}; + state.topology = VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST; + state.viewport = {0, static_cast(Height), static_cast(Width), -static_cast(Height), 0, 1}; + state.negativeOneToOne = false; + state.scissor = {{0, 0}, {Width, Height}}; + state.cullMode = VK_CULL_MODE_NONE; + state.frontFace = VK_FRONT_FACE_COUNTER_CLOCKWISE; + state.blend.colorWriteMask = 15; + state.blends = {state.blend}; + state.blendConstants = {}; + const AgcDriver::Pm4::DrawParameters draw{0, static_cast(Triangle.size()), 0, 1, 0, false}; + device.Draw(state, draw, shaders); +} + +// VulkanDevice::recordDispatch: the dispatch's copy of the buffer it writes. +void CopiedDispatch(AgcDriver::VulkanDevice& device) { + GuestBuffer target(false); + device.WaitIdle(); + Dispatch(device, target.Data()); + const auto& recorder = ActiveRecorder(); + Require(recorder.Recording(), "recorded kept bytes: the dispatch was not left in the open batch"); + const auto kept = recorder.OpenKeptBytes(); + Require(kept >= BufferBytes, "a dispatch writing a copied " + Bytes(BufferBytes) + " buffer left " + Bytes(kept) + " in its batch's kept bytes"); + device.WaitIdle(); +} + +// The target's first draw makes it resident (its upload counts in that batch's kept bytes), so the +// draws measured after it keep only their own bytes. +void ResidentTarget(AgcDriver::VulkanDevice& device, GuestBuffer& pixels) { + GuestBuffer vertices(false); + FillVertices(vertices); + Draw(device, pixels, vertices); + device.WaitIdle(); +} + +// keepRecordedDraw: the draw's copy of the buffer it reads. +void CopiedDraw(AgcDriver::VulkanDevice& device, GuestBuffer& pixels) { + GuestBuffer vertices(false); + FillVertices(vertices); + device.WaitIdle(); + Draw(device, pixels, vertices); + const auto& recorder = ActiveRecorder(); + Require(recorder.Recording(), "recorded kept bytes: the draw was not left in the open batch"); + const auto kept = recorder.OpenKeptBytes(); + Require(kept >= BufferBytes, "a draw reading a copied " + Bytes(BufferBytes) + " buffer left " + Bytes(kept) + " in its batch's kept bytes"); + device.WaitIdle(); +} + +// PrepareDrawBindings: the draw's snapshot of the buffer it reads in place, counted once for a +// second draw of the batch that reuses it. +bool SnapshotDraws(AgcDriver::VulkanDevice& device, GuestBuffer& pixels) { + GuestBuffer vertices(true); + FillVertices(vertices); + device.WaitIdle(); + Draw(device, pixels, vertices); + auto& recorder = ActiveRecorder(); + Require(recorder.Recording(), "recorded kept bytes: the snapshot draw was not left in the open batch"); + if (recorder.ReusableDrawSnapshot(AddressOf(vertices.Data()), BufferBytes) == nullptr) { + std::puts("the draw took no reusable snapshot (no host import, or the buffer is not write-watched): draw snapshots are not tested"); + device.WaitIdle(); + return false; + } + const auto first = recorder.OpenKeptBytes(); + Require(first >= BufferBytes, "a draw snapshotting a " + Bytes(BufferBytes) + " buffer left " + Bytes(first) + " in its batch's kept bytes"); + Draw(device, pixels, vertices); + Require(recorder.Recording(), "recorded kept bytes: the second snapshot draw was not left in the open batch"); + const auto second = recorder.OpenKeptBytes(); + Require(second == first, "a second draw reusing the batch's snapshot raised its kept bytes from " + Bytes(first) + " to " + Bytes(second)); + device.WaitIdle(); + return true; +} + +} + +int main() { + try { + const auto device = OpenVulkanTestDevice(); + if (!device) return VulkanTestSkipped; + CopiedDispatch(*device); + GuestBuffer pixels(true, TargetLayout.Bytes()); + ResidentTarget(*device, pixels); + CopiedDraw(*device, pixels); + const bool snapshots = SnapshotDraws(*device, pixels); + std::printf("recorded kept bytes tests passed%s\n", snapshots ? "" : " (draw snapshots not tested)"); + return 0; + } catch (const std::exception& error) { + std::cerr << error.what() << '\n'; + return 1; + } +} From 078b6d886fab994a5756fec08bf286a38ff4fd9f Mon Sep 17 00:00:00 2001 From: MathiasDevelopes <25400788+MathiasDevelopes@users.noreply.github.com> Date: Sat, 10 Oct 2026 09:52:47 +0000 Subject: [PATCH 3/4] style(agc): drop the comments the conventions check rejects Comments are reserved for technical debt (docs/dev/TechnicalDebt.md), so the ones the previous commit added to the recorder, the snapshot keep and the tests go. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01HZHkb8kUwnHAuBBbHywvJf --- .../Graphics/include/Recorder.hpp | 6 ----- .../Graphics/src/ShaderResources.cpp | 1 - .../libSceAgcDriver/tests/BdaResources.cpp | 2 -- .../tests/execution/RecordedKeptBytes.cpp | 22 ------------------- 4 files changed, 31 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp index 490d2479e..e59a93738 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp @@ -86,12 +86,7 @@ public: // Whether recorded work still has completion actions (write-backs the CPU must see) to run. bool HasCompletions() const; void Keep(std::shared_ptr object, std::size_t bytes = 0); - // Counts `bytes` held by `owner` in the open batch's kept bytes once per batch: memory several - // keeps of one batch share (a cached ShaderResources kept by every draw or dispatch using it, a - // reused draw snapshot) counts once. The caller keeps `owner` alive in the same batch, so its - // address names one object for the batch's lifetime. void KeepBytes(const void* owner, std::size_t bytes); - // The kept bytes of the open batch (0 with none open), for tests. std::size_t OpenKeptBytes() const { return open != nullptr ? open->keptBytes : 0; } static constexpr std::size_t KeptBytesBudget = std::size_t{512} << 20u; void BoundKeptBytes(); @@ -519,7 +514,6 @@ private: VkFence fence = VK_NULL_HANDLE; std::vector> kept; std::size_t keptBytes = 0; - // The owners KeepBytes counted in this batch. std::unordered_set keptOwners; std::vector> completions; std::vector> writes; diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp index f5b41011e..159c31a31 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp @@ -3369,7 +3369,6 @@ std::shared_ptr ShaderResources::PrepareDrawBindi } update(context.device, static_cast(writes.size()), writes.data(), 0, nullptr); recorder.Keep(result); - // A reused snapshot (Recorder::ReusableDrawSnapshot) is one buffer however many draws bind it. for (const auto& snapshot : result->snapshots) recorder.KeepBytes(snapshot.buffer.get(), snapshot.buffer->Bytes().size()); return result; } diff --git a/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp b/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp index 2ad77ce62..037991ce5 100644 --- a/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp @@ -578,8 +578,6 @@ void RunBdaResourceTests(const Context& context, const BdaTestAccess& access) { memory.AddWritable(address + 16, 16); Require(memory.CopiedBytes() == 0, "a region counted as copied before its upload"); memory.Upload(true); - // `guest` is stack memory no allocation registers, so no host import serves it: its two views merge - // into one copied region. Require(memory.CopiedBytes() == sizeof(guest), "copied bytes " + std::to_string(memory.CopiedBytes()) + " do not match the one region copied"); std::uint32_t adjustment = 0; const auto first = memory.Descriptor(address, sizeof(guest), adjustment); diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/RecordedKeptBytes.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/RecordedKeptBytes.cpp index b82cc95b2..d2f66285a 100644 --- a/core/libs/prx/libSceAgcDriver/tests/execution/RecordedKeptBytes.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/execution/RecordedKeptBytes.cpp @@ -28,12 +28,6 @@ #include #include -// The bytes a recorded dispatch or draw holds alive reach the recorder's batch budget -// (Recorder::KeptBytesBudget): the copies of guest buffers its ShaderResources own (kept by -// VulkanDevice::recordDispatch and Draw.cpp's keepRecordedDraw) and a draw's snapshots of buffers it -// reads in place (ShaderResources::PrepareDrawBindings). A snapshot reused by the next draw of the -// batch is one buffer and counts once. - namespace { using AgcDriver::Graphics::ColorTargetLayout; @@ -47,17 +41,14 @@ constexpr std::size_t BufferBytes = 65536; constexpr std::uint32_t Width = 256; constexpr std::uint32_t Height = 128; -// v1 = v0 << 3; v[2:3] = 0; buffer_store_dwordx2 v[2:3], v1, s[0:3] offen alignas(256) constexpr std::array StoreCode{ 0x34020083, 0x7e040280, 0x7e060280, 0xe0741000, 0x80000201, 0xbf810000, }; -// buffer_load_format_xyzw v[0:3], v5, s[0:3] idxen; exp pos0 v[0:3] done; s_endpgm alignas(256) constexpr std::array VertexCode{ 0xe0382000, 0x80000005, 0xbf8c3f70, 0xf80008cf, 0x03020100, 0xbf810000, }; -// v7 = 1.0; exp mrt0 v7 v7 v7 v7 done vm; s_endpgm alignas(256) constexpr std::array PixelCode{ 0x7e0e02f2, 0xf800180f, 0x07070707, 0xbf810000, }; @@ -76,9 +67,6 @@ std::array VertexBuffer(const void* data, std::uint32_t stride return {static_cast(address), static_cast((address >> 32u) & 0xffffu) | (stride << 16u), count, 0x01016facu}; } -// Guest memory, registered (served in place by a host import where the device has them, and -// write-watched where the host can watch it, as draw snapshots need to be reused) or plain (every -// use copies it). class GuestBuffer { public: explicit GuestBuffer(bool registered, std::size_t bytes = BufferBytes) : registered(registered), bytes(bytes), watched(registered && AgcDriver::GuestMemory::WriteWatched()) { @@ -177,14 +165,10 @@ constexpr std::array, 3> Triangle{{ {-1.0f, -1.0f, 0.5f, 1.0f}, {3.0f, -1.0f, 0.5f, 1.0f}, {-1.0f, 3.0f, 0.5f, 1.0f} }}; -// Vertex records for Draw: the triangle, then zeros up to BufferBytes. void FillVertices(GuestBuffer& vertices) { std::memcpy(vertices.Data(), Triangle.data(), sizeof(Triangle)); } -// A full-target triangle into a resident (tiled) color target, so the draw is recorded, whose -// vertex stage reads its positions from `vertices` (BufferBytes of records, so the bound range is -// the whole buffer). void Draw(AgcDriver::VulkanDevice& device, GuestBuffer& pixels, GuestBuffer& vertices) { const auto target = device.Target(); std::vector vertexUserData(4, 0u); @@ -241,7 +225,6 @@ void Draw(AgcDriver::VulkanDevice& device, GuestBuffer& pixels, GuestBuffer& ver device.Draw(state, draw, shaders); } -// VulkanDevice::recordDispatch: the dispatch's copy of the buffer it writes. void CopiedDispatch(AgcDriver::VulkanDevice& device) { GuestBuffer target(false); device.WaitIdle(); @@ -253,8 +236,6 @@ void CopiedDispatch(AgcDriver::VulkanDevice& device) { device.WaitIdle(); } -// The target's first draw makes it resident (its upload counts in that batch's kept bytes), so the -// draws measured after it keep only their own bytes. void ResidentTarget(AgcDriver::VulkanDevice& device, GuestBuffer& pixels) { GuestBuffer vertices(false); FillVertices(vertices); @@ -262,7 +243,6 @@ void ResidentTarget(AgcDriver::VulkanDevice& device, GuestBuffer& pixels) { device.WaitIdle(); } -// keepRecordedDraw: the draw's copy of the buffer it reads. void CopiedDraw(AgcDriver::VulkanDevice& device, GuestBuffer& pixels) { GuestBuffer vertices(false); FillVertices(vertices); @@ -275,8 +255,6 @@ void CopiedDraw(AgcDriver::VulkanDevice& device, GuestBuffer& pixels) { device.WaitIdle(); } -// PrepareDrawBindings: the draw's snapshot of the buffer it reads in place, counted once for a -// second draw of the batch that reuses it. bool SnapshotDraws(AgcDriver::VulkanDevice& device, GuestBuffer& pixels) { GuestBuffer vertices(true); FillVertices(vertices); From d731f5034e102049461f87b4a8d8ce4f679e6fb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Franch?= Date: Sun, 11 Oct 2026 00:33:12 +0200 Subject: [PATCH 4/4] test(agc): pass the gpu argument to GuestAllocations::Mutation().Add Co-authored-by: orishamir Co-Authored-By: Claude --- .../prx/libSceAgcDriver/tests/execution/RecordedKeptBytes.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/RecordedKeptBytes.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/RecordedKeptBytes.cpp index d2f66285a..11b98c82d 100644 --- a/core/libs/prx/libSceAgcDriver/tests/execution/RecordedKeptBytes.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/execution/RecordedKeptBytes.cpp @@ -94,7 +94,7 @@ public: #endif Require(block != nullptr, "recorded kept bytes: cannot allocate a guest buffer"); std::memset(block, 0, bytes); - if (registered) GuestAllocations::Mutation().Add(block, bytes, true, true); + if (registered) GuestAllocations::Mutation().Add(block, bytes, true, true, true); } ~GuestBuffer() {