diff --git a/core/libs/prx/libSceAgcDriver/CMakeLists.txt b/core/libs/prx/libSceAgcDriver/CMakeLists.txt index 98d405ca532..723b39c175d 100644 --- a/core/libs/prx/libSceAgcDriver/CMakeLists.txt +++ b/core/libs/prx/libSceAgcDriver/CMakeLists.txt @@ -1017,6 +1017,8 @@ if(BUILD_TESTING) set_tests_properties(agc_copied_direct_ordering PROPERTIES ENVIRONMENT_MODIFICATION "PATH=path_list_prepend:$") endif() + add_test(NAME agc_recorder_read_list COMMAND agc_driver_recorder_tests --read-list-only) + set_tests_properties(agc_recorder_read_list PROPERTIES SKIP_RETURN_CODE 77 TIMEOUT 30) add_test(NAME agc_host_import_unmap COMMAND agc_driver_recorder_tests --host-import-unmap-only) set_tests_properties(agc_host_import_unmap PROPERTIES SKIP_RETURN_CODE 77) add_test(NAME agc_parked_import_unmap COMMAND agc_driver_recorder_tests --parked-import-unmap-only) @@ -1056,7 +1058,7 @@ if(BUILD_TESTING) add_test(NAME agc_metadata_pass COMMAND agc_driver_recorder_tests --metadata-pass-only) set_tests_properties(agc_metadata_pass PROPERTIES SKIP_RETURN_CODE 77) if(WIN32) - set_tests_properties(agc_host_import_unmap agc_parked_import_unmap agc_host_import_retirement agc_sparse_run_import agc_sparse_run_import_copied agc_unit_shadow agc_descriptor_pool_recycling agc_draw_snapshot_cap agc_multisample_guest_access agc_draw_inputs_in_place agc_resident_present agc_slice_dirty agc_queued_write_groups agc_metadata_pass PROPERTIES ENVIRONMENT_MODIFICATION + set_tests_properties(agc_recorder_read_list agc_host_import_unmap agc_parked_import_unmap agc_host_import_retirement agc_sparse_run_import agc_sparse_run_import_copied agc_unit_shadow agc_descriptor_pool_recycling agc_draw_snapshot_cap agc_multisample_guest_access agc_draw_inputs_in_place agc_resident_present agc_slice_dirty agc_queued_write_groups agc_metadata_pass PROPERTIES ENVIRONMENT_MODIFICATION "PATH=path_list_prepend:$") add_test(NAME agc_host_import_second_mapping COMMAND agc_driver_recorder_tests --host-import-second-mapping-only) set_tests_properties(agc_host_import_second_mapping PROPERTIES SKIP_RETURN_CODE 77 ENVIRONMENT_MODIFICATION diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp index c710acbb620..e9b639cbeb9 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp @@ -276,6 +276,11 @@ class GuestBufferMemory { // the GPU copies out of an import (gpuCopy) notes its read itself when the copy is recorded. // For the recorder's read tracking (ShaderResources::MarkGpuWrites); nothing once committed. std::vector> InPlaceReads() const; + struct SplitReads { + std::shared_ptr>> space; + std::vector> own; + }; + std::optional SplitInPlaceReads() const; std::vector> DeviceReads() const; private: diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp index 134703542e7..02d25125f52 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp @@ -135,6 +135,9 @@ class Recorder { static bool ReadTracking(); void NotePendingRead(std::uint64_t address, std::size_t bytes, ReadKind kind); void NotePendingReads(std::span> ranges, ReadKind kind); + using ReadList = std::vector>; + void NotePendingReadList(std::shared_ptr ranges, ReadKind kind); + static bool ValidReadList(const ReadList& ranges); // Whether an unexecuted batch (open, or in flight with its fence unsignaled) reads the range in // place. A hit in an in-flight batch whose fence signaled meanwhile is a miss (one status // query per hit). `ignoreSignaled` false: every in-flight batch counts, whatever its fence. @@ -533,6 +536,12 @@ class Recorder { ReadKind kind; }; std::vector reads; + struct ReadListNote { + std::shared_ptr ranges; + std::size_t position; + ReadKind kind; + }; + std::vector readLists; // Submission number (1-based): identifies a batch after its allocation may have been reused. std::uint64_t serial = 0; std::uint64_t traceId = 0; @@ -738,8 +747,8 @@ class Recorder { void finish(std::unique_ptr batch, bool wait, int source, bool releaseNow = false); void release(Batch& batch) noexcept; static bool overlaps(const Batch& batch, std::uint64_t address, std::uint64_t end); - // The first noted read of `batch` overlapping [address, end), or null. - static const Batch::Read* readOverlap(const Batch& batch, std::uint64_t address, std::uint64_t end); + static std::optional readOverlap(const Batch& batch, std::uint64_t address, std::uint64_t end); + static std::vector> notedReads(const Batch& batch); bool signaled(const Batch& batch) const; // Rebuilds the lock-free snapshot of pending writes from open, inFlight and finishing. void publishPendingWrites(std::span> additions = {}) const; diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp index ad5b1483083..578128f2fe8 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp @@ -110,6 +110,7 @@ struct GuestBufferMemory::AddressSpace { std::vector copied; // The BDA table entries of `base`, in its order. std::vector ranges; + std::shared_ptr>> directReads; mutable std::mutex tableMutex; mutable std::map>, std::pair>>> writeTables; }; @@ -2616,6 +2617,13 @@ void GuestBufferMemory::AcquireRegistered() { regions = std::move(extras); lease.clear(); planSpaceMirrors(*built); + { + auto reads = std::make_shared(); + for (const auto& region : built->base) { + if (region.direct != nullptr) reads->emplace_back(region.begin, region.end); + } + if (Recorder::ValidReadList(*reads)) built->directReads = std::move(reads); + } space = std::move(built); spaces.droppedByWaiter.store(false, std::memory_order_relaxed); spaces.current.store(space); @@ -4118,6 +4126,15 @@ std::vector> GuestBufferMemory::InPlaceR return result; } +std::optional GuestBufferMemory::SplitInPlaceReads() const { + if (!uploaded || committed || space == nullptr || space->directReads == nullptr) return std::nullopt; + SplitReads split{space->directReads, {}}; + for (const auto& region : regions) { + if (region.direct != nullptr) split.own.emplace_back(region.begin, region.end); + } + return split; +} + std::vector> GuestBufferMemory::DeviceReads() const { std::vector> result; if (!uploaded || committed) return result; diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp index f4a993d8da9..e329a0d4499 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp @@ -2863,11 +2863,60 @@ void Recorder::NotePendingReads(std::span ranges, ReadKind kind) { + if (ranges == nullptr || ranges->empty() || !ReadTrackingEnabled()) return; + if (CaptureTrace::Enabled() || FlipReadCheck()) { + NotePendingReads(*ranges, kind); + return; + } + ensureOpen(); + readsNoted.fetch_add(ranges->size(), std::memory_order_relaxed); + for (const auto& note : open->readLists) { + if (note.ranges == ranges && note.kind == kind) return; + } + open->readLists.push_back({std::move(ranges), open->reads.size(), kind}); +} + +std::optional Recorder::readOverlap(const Batch& batch, std::uint64_t address, std::uint64_t end) { + const auto listOverlaps = [&](const Batch::ReadListNote& note) { + const auto& list = *note.ranges; + const auto found = std::partition_point(list.begin(), list.end(), [&](const auto& range) { return range.second <= address; }); + return found != list.end() && found->first < end; + }; + std::size_t next = 0; + for (std::size_t index = 0; index < batch.reads.size(); ++index) { + for (; next < batch.readLists.size() && batch.readLists[next].position == index; ++next) { + if (listOverlaps(batch.readLists[next])) return batch.readLists[next].kind; + } + const auto& read = batch.reads[index]; + if (address < read.end && read.begin < end) return read.kind; + } + for (; next < batch.readLists.size(); ++next) { + if (listOverlaps(batch.readLists[next])) return batch.readLists[next].kind; + } + return std::nullopt; +} + +std::vector> Recorder::notedReads(const Batch& batch) { + std::vector> result; + std::size_t next = 0; + const auto expand = [&](std::size_t position) { + for (; next < batch.readLists.size() && batch.readLists[next].position == position; ++next) result.insert(result.end(), batch.readLists[next].ranges->begin(), batch.readLists[next].ranges->end()); + }; + for (std::size_t index = 0; index < batch.reads.size(); ++index) { + expand(index); + result.emplace_back(batch.reads[index].begin, batch.reads[index].end); + } + expand(batch.reads.size()); + return result; } bool Recorder::PendingReadOverlaps(std::uint64_t address, std::size_t bytes, bool ignoreSignaled) const { @@ -2875,20 +2924,20 @@ bool Recorder::PendingReadOverlaps(std::uint64_t address, std::size_t bytes, boo readQueries.fetch_add(1, std::memory_order_relaxed); const auto end = address + bytes; if (open != nullptr) { - if (const auto* read = readOverlap(*open, address, end)) { - readHits[static_cast(read->kind)].fetch_add(1, std::memory_order_relaxed); + if (const auto kind = readOverlap(*open, address, end)) { + readHits[static_cast(*kind)].fetch_add(1, std::memory_order_relaxed); return true; } } // Newest first: the batch most likely still running decides without a status query per batch. for (auto it = inFlight.rbegin(); it != inFlight.rend(); ++it) { - const auto* read = readOverlap(**it, address, end); - if (read == nullptr) continue; + const auto kind = readOverlap(**it, address, end); + if (!kind.has_value()) continue; if (ignoreSignaled && signaled(**it)) { readStaleIgnored.fetch_add(1, std::memory_order_relaxed); continue; } - readHits[static_cast(read->kind)].fetch_add(1, std::memory_order_relaxed); + readHits[static_cast(*kind)].fetch_add(1, std::memory_order_relaxed); return true; } return false; @@ -2898,10 +2947,10 @@ std::optional Recorder::DescribePendingRead(std::uint if (bytes == 0) return std::nullopt; const auto end = address + bytes; if (open != nullptr) { - if (const auto* read = readOverlap(*open, address, end)) return PendingReadInfo{submissions + 1, open->queue, read->kind, true, false}; + if (const auto kind = readOverlap(*open, address, end)) return PendingReadInfo{submissions + 1, open->queue, *kind, true, false}; } for (auto it = inFlight.rbegin(); it != inFlight.rend(); ++it) { - if (const auto* read = readOverlap(**it, address, end)) return PendingReadInfo{(*it)->serial, (*it)->queue, read->kind, false, signaled(**it)}; + if (const auto kind = readOverlap(**it, address, end)) return PendingReadInfo{(*it)->serial, (*it)->queue, *kind, false, signaled(**it)}; } return std::nullopt; } @@ -3209,7 +3258,7 @@ void Recorder::Submit() { if (FlipReadCheck()) { // A CPU write into a noted read after this collect (and so possibly before the GPU read // it) fails UnchangedSince at the presenter's check (VulkanDevice RetirePresents). - for (const auto& read : open->reads) open->readGeneration = std::max(open->readGeneration, GuestMemory::CollectWrites(read.begin, read.end - read.begin)); + for (const auto& [begin, end] : notedReads(*open)) open->readGeneration = std::max(open->readGeneration, GuestMemory::CollectWrites(begin, end - begin)); } auto batch = std::move(open); Check(function(endCommandBuffer, "vkEndCommandBuffer")(batch->commands), "vkEndCommandBuffer recorder"); @@ -3708,7 +3757,7 @@ void Recorder::finish(std::unique_ptr batch, bool wait, int source, bool entry.readGeneration = batch->readGeneration; entry.reads.clear(); if (FlipReadCheck()) { - for (const auto& read : batch->reads) entry.reads.emplace_back(read.begin, read.end); + entry.reads = notedReads(*batch); } } // Completions store GPU results to guest memory; a failing one is reported, the rest still run. diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp index 89cd8882b38..7aa9caa54b8 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp @@ -3860,7 +3860,13 @@ void ShaderResources::MarkGpuWrites(Recorder& recorder) { // The ranges this use reads in place through their host imports (read-only and written elements // alike, and an address-based build's whole leased heaps), before the writes: a CPU store into // one of them (the copy HLE) must not land before the recorded work read it. - recorder.NotePendingReads(guestMemory.InPlaceReads(), guestMemory.HoldsLease() ? Recorder::ReadKind::AddressBased : Recorder::ReadKind::DispatchElement); + const auto kind = guestMemory.HoldsLease() ? Recorder::ReadKind::AddressBased : Recorder::ReadKind::DispatchElement; + if (auto split = guestMemory.SplitInPlaceReads()) { + recorder.NotePendingReadList(std::move(split->space), kind); + recorder.NotePendingReads(split->own, kind); + } else { + recorder.NotePendingReads(guestMemory.InPlaceReads(), kind); + } if (SkipWriteBack()) return; for (std::size_t index = 0; index < storageTextures.size(); ++index) { if (!storageWritten[index]) continue; diff --git a/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp b/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp index 1cda156b51e..ab8b8f31259 100644 --- a/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp @@ -1,4 +1,5 @@ #include "BdaTests.hpp" +#include "prx/libSceAgcDriver/Graphics/include/Recorder.hpp" #include "prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp" #include "prx/libc/include/GuestAllocations.hpp" #include "prx/libc/include/GuestHeap.hpp" @@ -19,6 +20,7 @@ #include #include #include +#include #include #include @@ -264,6 +266,62 @@ void spaceImportRunTests(const Context& context) { ::operator delete(block, std::align_val_t{piece}); } +void spaceReadListTests(const Context& context) { + if (!AddressSpaceCounters().enabled) return; + constexpr std::size_t piece = 65536; + void* block = ::operator new(3 * piece, std::align_val_t{piece}); + std::memset(block, 0x5c, 3 * piece); + auto importing = context; + importing.hostImportAlignment = piece; + auto* guest = static_cast(block); + const auto change = [&](std::uint8_t* range, bool add) { + auto* mutation = GuestAllocations::GuestAllocationsBegin_nid_postfix(); + if (add) GuestAllocations::GuestAllocationsAdd_nid_postfix(mutation, range, piece, true, true, true); + else GuestAllocations::GuestAllocationsRemove_nid_postfix(mutation, range); + GuestAllocations::GuestAllocationsEnd_nid_postfix(mutation); + }; + const auto build = [&](const char* step) { + GuestBufferMemory leased(importing); + leased.AcquireRegistered(); + leased.Upload(true); + const auto split = leased.SplitInPlaceReads(); + Require(split.has_value() && split->space != nullptr, std::string("an address-based build that took the space whole has no split reads: ") + step); + auto joined = *split->space; + joined.insert(joined.end(), split->own.begin(), split->own.end()); + Require(joined == leased.InPlaceReads(), std::string("the split reads differ from InPlaceReads: ") + step); + Require(Recorder::ValidReadList(*split->space), std::string("the space's read list is not sorted and disjoint: ") + step); + leased.WriteBack(); + Require(!leased.SplitInPlaceReads().has_value(), "a committed build still offers split reads"); + return split->space; + }; + const auto lists = [&](const std::shared_ptr& list, const std::uint8_t* range) { + const auto begin = reinterpret_cast(range); + return std::any_of(list->begin(), list->end(), [&](const auto& read) { return read.first <= begin && begin + piece <= read.second; }); + }; + change(guest, true); + change(guest + piece, true); + const auto first = build("first build"); + Require(lists(first, guest) && lists(first, guest + piece) && !lists(first, guest + 2 * piece), "the space's read list does not hold exactly the registered pieces"); + const auto again = build("unchanged registry"); + Require(again == first, "a build in an unchanged registry did not share the space's read list"); + change(guest + 2 * piece, true); + const auto added = build("range added"); + Require(added != first && lists(added, guest + 2 * piece), "a registered range is missing from the read list after the registry changed"); + change(guest + piece, false); + const auto removed = build("range removed"); + Require(removed != added && !lists(removed, guest + piece) && lists(removed, guest), "a removed range is still in the read list"); + { + GuestBufferMemory plain(importing); + plain.AddReadable(reinterpret_cast(guest), 256); + plain.Upload(false); + Require(!plain.SplitInPlaceReads().has_value(), "a build without the address space offers split reads"); + plain.WriteBack(); + } + change(guest, false); + change(guest + 2 * piece, false); + ::operator delete(block, std::align_val_t{piece}); +} + void importWindowTests(const Context& context) { constexpr std::size_t piece = 65536; constexpr std::size_t window = 2 * piece; @@ -1607,6 +1665,7 @@ void RunBdaResourceTests(const Context& context, const BdaTestAccess& access) { failedImportRemapTests(context, access); spaceImportRunTests(context); spaceWritableWritesTests(context); + spaceReadListTests(context); importWindowTests(context); spaceReadAcrossImportsTests(context, access); spaceReadAcrossImportsWrittenTests(context); diff --git a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp index 6d59db6e2a2..2af4ef723ec 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp @@ -731,6 +731,80 @@ void pendingWriteSnapshotBenchmark(Recorder& recorder) { recorder.Sync(); } +void readListTests(const Device& device, Recorder& recorder) { + using Ranges = Recorder::ReadList; + Require(Recorder::ValidReadList({{0x1000, 0x2000}, {0x2000, 0x2100}}) && Recorder::ValidReadList({}), "a sorted disjoint list is refused"); + Require(!Recorder::ValidReadList({{0x2000, 0x2100}, {0x1000, 0x2000}}) && !Recorder::ValidReadList({{0x1000, 0x2001}, {0x2000, 0x2100}}) && !Recorder::ValidReadList({{0x1000, 0x1000}}), "an unsorted, overlapping or empty list is accepted"); + std::uint64_t seed = 0x9e3779b97f4a7c15ull; + const auto next = [&](std::uint64_t bound) { + seed = seed * 6364136223846793005ull + 1442695040888963407ull; + return (seed >> 33u) % bound; + }; + const auto makeList = [&](std::uint64_t base, std::size_t count) { + auto list = std::make_shared(); + auto at = base; + for (std::size_t index = 0; index < count; ++index) { + at += next(3) * 0x100; + const auto bytes = (next(16) + 1) * 0x10; + list->emplace_back(at, at + bytes); + at += bytes; + } + return list; + }; + const auto first = makeList(0x100000, 300); + const auto second = makeList(0x100000, 120); + const Ranges before{{0x100400, 0x100410}, {0x200000, 0x200100}}; + const Ranges between{{0x100800, 0x100900}, {0x300000, 0x300040}}; + const Ranges after{{0x100000, 0x180000}}; + const auto note = [&](bool listed) { + recorder.NotePendingReads(before, Kind::DispatchElement); + if (listed) recorder.NotePendingReadList(first, Kind::AddressBased); + else recorder.NotePendingReads(*first, Kind::AddressBased); + recorder.NotePendingReads(between, Kind::GpuCopy); + if (listed) { + recorder.NotePendingReadList(second, Kind::Indirect); + recorder.NotePendingReadList(first, Kind::AddressBased); + } else { + recorder.NotePendingReads(*second, Kind::Indirect); + } + recorder.NotePendingReads(after, Kind::CopySource); + }; + std::vector> queries; + for (const auto* list : {first.get(), second.get()}) { + for (const auto& [begin, end] : *list) { + for (const auto at : {begin - 1, begin, end - 1, end}) queries.emplace_back(at, 1 + next(0x80)); + } + } + for (int query = 0; query < 2000; ++query) queries.emplace_back(0xff000 + next(0x300000), 1 + next(0x2000)); + const auto answers = [&] { + std::vector result; + for (const auto& [address, bytes] : queries) { + const auto described = recorder.DescribePendingRead(address, bytes); + result.push_back(recorder.PendingReadOverlaps(address, bytes) ? 1 : 0); + result.push_back(described.has_value() ? 10 + static_cast(described->kind) : -1); + } + recorder.Submit(); + for (const auto& [address, bytes] : queries) result.push_back(recorder.PendingReadOverlaps(address, bytes, false) ? 1 : 0); + recorder.Sync(); + Require(recorder.Idle() && !recorder.PendingReadOverlaps(0x100000, 0x200000, false), "listed reads outlived their batch"); + return result; + }; + const auto notedBefore = Recorder::ReadCounts().noted; + note(false); + const auto plain = answers(); + const auto notedPlain = Recorder::ReadCounts().noted - notedBefore; + note(true); + const auto listed = answers(); + const auto notedListed = Recorder::ReadCounts().noted - notedBefore - notedPlain; + Require(plain == listed, "reads noted by reference answer a query differently from the same ranges noted one by one"); + Require(notedListed == notedPlain + first->size(), "a list's ranges are not counted as noted"); + for (const auto kind : {Kind::DispatchElement, Kind::AddressBased, Kind::CopySource}) Require(std::count(plain.begin(), plain.end(), 10 + static_cast(kind)) != 0, "the queries miss a reader kind: the comparison proves too little"); + recorder.NotePendingReadList(std::make_shared(), Kind::AddressBased); + recorder.NotePendingReadList(nullptr, Kind::AddressBased); + Require(!recorder.Recording(), "an empty list opened a batch"); + static_cast(device); +} + void writeSettledTests(const Device& device, Recorder& recorder) { Require(recorder.PendingWriteSettled(0x50000, 0x100), "an unwritten range is not settled"); recorder.NotePendingWrite(0x50000, 0x100); @@ -7034,6 +7108,11 @@ int main(int argc, char** argv) { std::cout << "written buffer order tests passed\n"; return 0; } + if (argc == 2 && std::string_view(argv[1]) == "--read-list-only") { + readListTests(device, recorder); + std::cout << "Read list tests passed\n"; + return 0; + } if (argc == 2 && std::string_view(argv[1]) == "--host-import-unmap-only") { gpu.unlock(); hostImportUnmapTests(device, recorder); @@ -7118,6 +7197,7 @@ int main(int argc, char** argv) { } #endif readTrackingTests(device, recorder); + readListTests(device, recorder); writeSettledTests(device, recorder); completionCountTests(device, recorder); afterRecordedWorkTests(device, recorder);