Merge pull request #3471 from Kikoarl/perf/agc-copied-writes-overlap

perf(agc): disambiguate direct GPU writes from copied write-backs in indirect draws
This commit is contained in:
Dean 2026-10-13 17:52:29 +13:00 committed by GitHub
commit 547807422c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 117 additions and 8 deletions

View File

@ -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

View File

@ -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<std::pair<std::uint64_t, std::uint64_t>>& Writes() const { return writes; }
// Whether any written range lives in a copied buffer, so a write-back must run once the GPU is done.

View File

@ -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

View File

@ -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;

View File

@ -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);

View File

@ -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 <spirv/unified1/spirv.hpp>
#include <algorithm>
@ -3711,6 +3712,99 @@ void barycentricEmulationTests() {
expectFailure([&] { static_cast<void>(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<std::uintptr_t>(directBlock);
void* copiedBlock = ::operator new(half, std::align_val_t{half});
std::memset(copiedBlock, 0xbb, half);
const auto copiedAddress = reinterpret_cast<std::uintptr_t>(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<ShaderResources>(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;