diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp index 4153efa14..24584da02 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp @@ -516,7 +516,7 @@ struct VulkanDevice::State { // Whether a recorded dispatch's or draw's copied write-back (listed until it ran) stores into // the range on the CPU; under GpuMutex. bool CopiedWriterOverlaps(std::uint64_t address, std::size_t bytes) const { - const auto overlaps = [&](const auto& writer) { return writer->WritesOverlap(address, bytes); }; + const auto overlaps = [&](const auto& writer) { return writer->CopiedWritesOverlap(address, bytes); }; return std::any_of(copiedWriters->begin(), copiedWriters->end(), overlaps) || std::any_of(Graphics::DrawCopiedWriters()->begin(), Graphics::DrawCopiedWriters()->end(), overlaps); } @@ -1957,8 +1957,7 @@ bool VulkanDevice::FillBuffer(std::uint64_t address, std::size_t bytes, std::spa if (recorder.PendingWriteOverlaps(address, bytes)) { // Recorded draws with copied writes keep their own registry (Graphics::DrawCopiedWriters): // their CPU write-back would land after a fill recorded on the GPU, so they count too. - bool sync = alwaysSync || std::any_of(state->copiedWriters->begin(), state->copiedWriters->end(), [&](const auto& writer) { return writer->WritesOverlap(address, bytes); }) - || std::any_of(Graphics::DrawCopiedWriters()->begin(), Graphics::DrawCopiedWriters()->end(), [&](const auto& writer) { return writer->WritesOverlap(address, bytes); }); + bool sync = alwaysSync || state->CopiedWriterOverlaps(address, bytes); if (!sync && Graphics::Recorder::PendingCompletionLabels() != 0) { // A label the CPU will store after a completion must not be overwritten by the fill: // only a label inside the filled range matters, whatever the range's size. @@ -2161,8 +2160,7 @@ VulkanDevice::CopyOutcome VulkanDevice::CopyBuffer(std::uint64_t destination, st // A store a batch's completion makes on the CPU over the range (a copied buffer's write-back, // a completion-deferred label): it lands only when that batch is reaped, whatever its fence. const auto completionStoreOverlaps = [&](std::uint64_t address) { - if (alwaysSync || std::any_of(state->copiedWriters->begin(), state->copiedWriters->end(), [&](const auto& writer) { return writer->WritesOverlap(address, bytes); }) - || std::any_of(Graphics::DrawCopiedWriters()->begin(), Graphics::DrawCopiedWriters()->end(), [&](const auto& writer) { return writer->WritesOverlap(address, bytes); })) return true; + if (alwaysSync || state->CopiedWriterOverlaps(address, bytes)) return true; return Graphics::Recorder::PendingCompletionLabels() != 0 && recorder.PendingLabelIn(address, bytes); }; // The CPU decision (see the header): pure queries, in the order that makes the first failing @@ -3847,8 +3845,7 @@ void VulkanDevice::decideIndirect(RecordedDispatch& record, IndirectOutcome& out recorder.Sync(); } else if (record.shader->program->workgroupMemoryDwords != 0) { outcome.cpuReason = 8; - } else if (labelPending() || std::any_of(state->copiedWriters->begin(), state->copiedWriters->end(), [&](const auto& writer) { return writer->WritesOverlap(arguments, 12); }) - || std::any_of(Graphics::DrawCopiedWriters()->begin(), Graphics::DrawCopiedWriters()->end(), [&](const auto& writer) { return writer->WritesOverlap(arguments, 12); })) { + } else if (labelPending() || state->CopiedWriterOverlaps(arguments, 12)) { outcome.cpuReason = 2; } else if ((record.argumentImport = Graphics::HostImportFor(context, arguments, 12)) == nullptr) { // Valid until the next refreshImports, which only runs under GuestMemory::GpuMutex (held diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp index a0e1dc4ae..adac3c826 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp @@ -238,6 +238,7 @@ public: struct AddressSpace; void WriteBack(); bool WritesOverlap(std::uint64_t address, std::size_t bytes) const; + bool CopiedWritesOverlap(std::uint64_t address, std::size_t bytes) const; // Guest ranges the shader may write through descriptors, as [begin, end). const std::vector>& Writes() const { return writes; } // Whether any written range lives in a copied buffer, so a write-back must run once the GPU is done. diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp index 7b29f6df0..81c0a2a80 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp @@ -166,6 +166,7 @@ public: bool UsesAddressTable() const { return usesBda; } std::size_t CopiedBytes() const { return guestMemory.CopiedBytes(); } bool WritesOverlap(std::uint64_t address, std::size_t bytes) const { return guestMemory.WritesOverlap(address, bytes); } + bool CopiedWritesOverlap(std::uint64_t address, std::size_t bytes) const { return guestMemory.CopiedWritesOverlap(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; // The recorder's hazard tracker inputs (Recorder::NoteAccess): the written elements' guest diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp index 3cb6a005c..428976353 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp @@ -2037,7 +2037,7 @@ void Draw(const Context& context, const State& state, const Pm4::DrawParameters& return IndirectDrawPath::PendingImage; } if (recorder != nullptr && recorder->PendingLabelIn(address, bytes)) return IndirectDrawPath::PendingLabelOrCopy; - const auto overlaps = [&](const auto& writer) { return writer->WritesOverlap(address, bytes); }; + const auto overlaps = [&](const auto& writer) { return writer->CopiedWritesOverlap(address, bytes); }; if (context.copiedWriters != nullptr && std::any_of(context.copiedWriters->begin(), context.copiedWriters->end(), overlaps)) return IndirectDrawPath::PendingLabelOrCopy; if (std::any_of(DrawCopiedWriters()->begin(), DrawCopiedWriters()->end(), overlaps)) return IndirectDrawPath::PendingLabelOrCopy; if ((import = HostImportFor(context, address, bytes)) == nullptr) return IndirectDrawPath::NotImported; diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp index 385cb69a8..c87f85021 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp @@ -3461,6 +3461,21 @@ bool GuestBufferMemory::HasCopiedWrites() const { return false; } +bool GuestBufferMemory::CopiedWritesOverlap(std::uint64_t address, std::size_t bytes) const { + if (bytes == 0) return false; + for (const auto& [begin, end] : writes) { + if (!(address < end && begin < address + bytes)) continue; + const auto* found = owner(begin); + if (found == nullptr) continue; + const auto& region = *found; + if (region.direct != nullptr) continue; + if (region.gpuCopy && (region.copiedBack || region.deviceLocal)) continue; + if (region.mirror != nullptr && !region.mirror->writable) continue; + return true; + } + return false; +} + void GuestBufferMemory::MarkDirectWrites() const { for (const auto& [begin, end] : writes) { const auto* found = owner(begin); diff --git a/core/libs/prx/libSceAgcDriver/tests/Graphics.cpp b/core/libs/prx/libSceAgcDriver/tests/Graphics.cpp index f3a3abe45..7bfbc5a0d 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Graphics.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Graphics.cpp @@ -20,6 +20,7 @@ #include "CacheKey.hpp" #include "Optimization/ResourceProgram.hpp" #include "BdaAbi.hpp" +#include "prx/libc/include/GuestAllocations.hpp" #include "prx/libc/include/general/VabiMacros.hpp" #include #include @@ -3711,6 +3712,99 @@ void barycentricEmulationTests() { expectFailure([&] { static_cast(BuildBarycentricGeometryShader(vertex, fragment, target, limits)); }, "not exported by the vertex shader"); } +void copiedWriterIndirectOverlapTests() { + using namespace ShaderRecompiler; + using namespace AgcDriver::Graphics; + + constexpr std::size_t half = 65536; + + void* directBlock = ::operator new(half, std::align_val_t{half}); + std::memset(directBlock, 0xaa, half); + const auto directAddress = reinterpret_cast(directBlock); + + void* copiedBlock = ::operator new(half, std::align_val_t{half}); + std::memset(copiedBlock, 0xbb, half); + const auto copiedAddress = reinterpret_cast(copiedBlock); + + { + auto* mutation = GuestAllocations::GuestAllocationsBegin_nid_postfix(); + GuestAllocations::GuestAllocationsAdd_nid_postfix(mutation, directBlock, half, true, true, true); + GuestAllocations::GuestAllocationsEnd_nid_postfix(mutation); + } + + mock = MockVulkan{}; + auto context = mockContext(); + context.hostImportAlignment = half; + context.limits.minStorageBufferOffsetAlignment = 16; + context.limits.maxStorageBufferRange = 1u << 27; + + { + GuestBufferMemory memory(context); + memory.AcquireRegistered(); + Require(HostImportCovers(context, directAddress, half), "direct registered range must be covered by host import"); + Require(!HostImportCovers(context, copiedAddress, half), "copied unregistered range must NOT be covered by host import"); + + memory.AddWritable(copiedAddress, 1024); + memory.AddWritable(directAddress, 1024); + memory.Upload(false); + + Require(memory.HasCopiedWrites(), "writer with one copied region must report HasCopiedWrites() == true"); + + Require(memory.WritesOverlap(directAddress, 16), "WritesOverlap must return true for direct written region"); + Require(memory.WritesOverlap(copiedAddress, 16), "WritesOverlap must return true for copied written region"); + Require(!memory.WritesOverlap(directAddress + 2048, 16), "WritesOverlap must return false for unwritten range"); + + Require(!memory.CopiedWritesOverlap(directAddress, 16), "CopiedWritesOverlap must return false for direct written region"); + Require(memory.CopiedWritesOverlap(copiedAddress, 16), "CopiedWritesOverlap must return true for copied written region"); + Require(!memory.CopiedWritesOverlap(directAddress + 2048, 16), "CopiedWritesOverlap must return false for unwritten range"); + + memory.WriteBack(); + } + + { + RecompileResult shader; + shader.runtimeAbiVersion = RuntimeAbi::Version; + auto copiedBinding = makeBinding(Role::GuestBuffers, 0, 1, vsharp(copiedBlock, 1024)); + copiedBinding.bufferWritten = {true}; + auto directBinding = makeBinding(Role::GuestBuffers, 1, 1, vsharp(directBlock, 1024)); + directBinding.bufferWritten = {true}; + shader.bindings = {copiedBinding, directBinding}; + CompiledShader compiled{ShaderStage::Compute, &shader, 0}; + auto writer = std::make_shared(context, compiled); + + Require(writer->HasCopiedWrites(), "ShaderResources with copied region must report HasCopiedWrites() == true"); + Require(writer->WritesOverlap(directAddress, 16), "ShaderResources must report WritesOverlap for direct region"); + Require(!writer->CopiedWritesOverlap(directAddress, 16), "ShaderResources must NOT report CopiedWritesOverlap for direct region"); + Require(writer->CopiedWritesOverlap(copiedAddress, 16), "ShaderResources must report CopiedWritesOverlap for copied region"); + + auto copiedWriters = DrawCopiedWriters(); + copiedWriters->push_back(writer); + + const auto overlapsMain = [&](const auto& w) { return w->WritesOverlap(directAddress, 16); }; + const auto overlapsPR = [&](const auto& w) { return w->CopiedWritesOverlap(directAddress, 16); }; + + const bool mainOverlaps = std::any_of(copiedWriters->begin(), copiedWriters->end(), overlapsMain); + const bool prOverlaps = std::any_of(copiedWriters->begin(), copiedWriters->end(), overlapsPR); + + Require(mainOverlaps, "main's check (WritesOverlap) must flag an overlap over direct arguments"); + Require(!prOverlaps, "PR's check (CopiedWritesOverlap) must disambiguate and NOT flag an overlap over direct arguments"); + + const auto copiedOverlapsPR = [&](const auto& w) { return w->CopiedWritesOverlap(copiedAddress, 16); }; + Require(std::any_of(copiedWriters->begin(), copiedWriters->end(), copiedOverlapsPR), "PR's check must flag an overlap over copied arguments"); + + copiedWriters->erase(std::remove(copiedWriters->begin(), copiedWriters->end(), writer), copiedWriters->end()); + writer->WriteBack(); + } + + { + auto* mutation = GuestAllocations::GuestAllocationsBegin_nid_postfix(); + GuestAllocations::GuestAllocationsRemove_nid_postfix(mutation, directBlock); + GuestAllocations::GuestAllocationsEnd_nid_postfix(mutation); + } + ::operator delete(directBlock, std::align_val_t{half}); + ::operator delete(copiedBlock, std::align_val_t{half}); +} + void rectListTests() { using namespace ShaderRecompiler; using namespace AgcDriver::Graphics; @@ -5335,6 +5429,7 @@ int main(int argc, char** argv) { rectListTests(); floatControlsModeTests(); barycentricEmulationTests(); + copiedWriterIndirectOverlapTests(); mock = MockVulkan{}; auto bdaContext = mockContext(); bdaContext.bufferDeviceAddress = true;