Merge pull request #2992 from CnRJay/fix/agc-sparse-heap-cut

fix(agc): copy a sparse descriptor heap only up to its last backed byte
This commit is contained in:
Adrià 2026-10-11 09:24:32 +02:00 committed by GitHub
commit fbcdf1a804
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 98 additions and 6 deletions

View File

@ -249,7 +249,7 @@ public:
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<std::size_t>(region.end - region.begin);
for (const auto& region : regions) if (region.buffer != nullptr) bytes += static_cast<std::size_t>((region.bufferEnd != 0 ? region.bufferEnd : region.end) - region.begin);
return bytes;
}
// Every uploaded region as [begin, end) when all of them are served by host imports, in place or
@ -283,6 +283,8 @@ private:
// memory; the rest reads as zeros and is never stored.
bool sparse = false;
std::vector<std::pair<std::uint64_t, std::uint64_t>> backed {};
std::uint64_t lastView = 0;
std::uint64_t bufferEnd = 0;
// Set when the region is served by an image mirror: nothing is copied; writable mirrors are
// written back by comparing with the mirror's shadow. Kept alive here for recorded work.
std::shared_ptr<ImageMirror> mirror {};

View File

@ -2125,6 +2125,7 @@ void GuestBufferMemory::addDescriptorRegion(std::uint64_t address, std::size_t b
Region region{begin, end, true, {}, nullptr};
region.atomic = atomic;
region.swept = swept;
region.lastView = begin;
auto committed = GuestMemory::DescribeCommitted(begin, static_cast<std::size_t>(end - begin));
if (!committed.whole) {
// A GPU heap bound whole while the guest commits its pages on demand, or a descriptor left
@ -2448,6 +2449,7 @@ void GuestBufferMemory::UploadPrepare(bool addressable) {
previous.hostBacked = true;
}
mergeBacked(previous, region);
previous.lastView = std::max({previous.lastView, region.lastView, region.begin});
if (region.end > previous.end) previous.direct = nullptr;
previous.atomic = previous.atomic || region.atomic;
previous.swept = previous.swept || region.swept;
@ -2774,8 +2776,17 @@ void GuestBufferMemory::copyRegion(Region& region, bool addressable) {
}
}
const auto allocateStart = std::chrono::steady_clock::now();
if (region.sparse && region.buffer == nullptr && (region.hostBacked || region.writable)) {
const auto backedEnd = region.backed.empty() ? region.begin : region.backed.back().second;
const auto viewEnd = std::max(region.lastView, region.begin) + 4u;
constexpr std::uint64_t granule = 256;
const auto extent = std::max(backedEnd, viewEnd) - region.begin;
const auto end = region.begin + std::min<std::uint64_t>((extent + granule - 1) / granule * granule, bytes);
region.bufferEnd = end < region.end ? end : 0;
}
const auto allocated = region.bufferEnd != 0 ? region.bufferEnd - region.begin : bytes;
// A buffer made by UploadPrepare for a GPU copy that fell back here (its import went) is kept.
if (region.buffer == nullptr) region.buffer = std::make_shared<Buffer>(context, static_cast<std::size_t>(bytes), usage);
if (region.buffer == nullptr) region.buffer = std::make_shared<Buffer>(context, static_cast<std::size_t>(allocated), usage);
const auto readStart = std::chrono::steady_clock::now();
if (profile) allocateUs.fetch_add(microsecondsSince(allocateStart), std::memory_order_relaxed);
// Named for the [hooksync] attribution: the reads below go through the flush hook.
@ -2789,11 +2800,18 @@ void GuestBufferMemory::copyRegion(Region& region, bool addressable) {
if (written) region.uploaded.assign(region.buffer->Bytes().begin(), region.buffer->Bytes().end());
} else {
// `uploaded` holds the backed pieces back to back.
const auto target = region.buffer->Bytes().first(static_cast<std::size_t>(allocated));
const bool zeroGaps = region.bufferEnd != 0;
std::size_t cursor = 0;
for (const auto& [first, last] : region.backed) {
const auto piece = region.buffer->Bytes().subspan(static_cast<std::size_t>(first - region.begin), static_cast<std::size_t>(last - first));
const auto at = static_cast<std::size_t>(first - region.begin);
if (zeroGaps && at > cursor) std::memset(target.data() + cursor, 0, at - cursor);
const auto piece = target.subspan(at, static_cast<std::size_t>(last - first));
GuestMemory::Read(first, piece);
if (written) region.uploaded.insert(region.uploaded.end(), piece.begin(), piece.end());
cursor = at + piece.size();
}
if (zeroGaps && cursor < target.size()) std::memset(target.data() + cursor, 0, target.size() - cursor);
}
}
else std::memcpy(region.buffer->Bytes().data(), region.snapshot.data(), region.snapshot.size());
@ -2983,8 +3001,13 @@ VkDescriptorBufferInfo GuestBufferMemory::Descriptor(std::uint64_t address, std:
Require(context.limits.minStorageBufferOffsetAlignment != 0, "no storage buffer offset alignment");
Require(base % 4 == 0, "a guest buffer view in a GPU owner that does not start at a DWORD boundary is not implemented");
adjustment = static_cast<std::uint32_t>(offset % std::max<VkDeviceSize>(context.limits.minStorageBufferOffsetAlignment, 4));
const auto range = ViewBytes(bytes, adjustment);
const auto end = region.direct != nullptr ? region.direct->base + region.direct->bytes : region.end;
auto range = ViewBytes(bytes, adjustment);
const bool cut = region.direct == nullptr && region.mirror == nullptr && region.bufferEnd != 0;
const auto end = region.direct != nullptr ? region.direct->base + region.direct->bytes : cut ? region.bufferEnd : region.end;
if (cut) {
Require(address - adjustment < end, "guest buffer view starts past its copy");
range = std::min<std::uint64_t>(range, end - (address - adjustment));
}
if (address - adjustment + range > end) Require(false, describe("rounded up to its view range exceeds"));
Require(range <= context.limits.maxStorageBufferRange, "guest buffer view exceeds descriptor range limit");
const auto handle = region.direct != nullptr ? region.direct->buffer : region.mirror != nullptr ? region.mirror->buffer->Handle() : region.buffer->Handle();
@ -3000,7 +3023,8 @@ ShaderRecompiler::BdaAbi::Range GuestBufferMemory::addressRange(const Region& re
const auto address = region.direct != nullptr ? region.direct->address + (region.begin - region.direct->base) : region.mirror != nullptr ? region.mirror->buffer->DeviceAddress() + (region.begin - region.mirror->base) : region.buffer->DeviceAddress();
Require(region.end - region.begin <= std::numeric_limits<std::uint64_t>::max() - address, "GPU address range overflow");
const auto permissions = ShaderRecompiler::BdaAbi::Read | (region.direct != nullptr && region.writable ? ShaderRecompiler::BdaAbi::Write : 0u);
return {region.begin, region.end, address, permissions, 0};
const bool cut = region.direct == nullptr && region.mirror == nullptr && region.bufferEnd != 0;
return {region.begin, cut ? region.bufferEnd : region.end, address, permissions, 0};
}
std::vector<ShaderRecompiler::BdaAbi::Range> GuestBufferMemory::AddressRanges() const {

View File

@ -649,6 +649,71 @@ void stridedOverhangTests(const Context& context, const BdaTestAccess& access) {
::operator delete(block, std::align_val_t{allocation});
}
std::byte* reserveSparse(std::size_t reserved, std::size_t committed) {
#ifdef _WIN32
auto* block = static_cast<std::byte*>(VirtualAlloc(nullptr, reserved, MEM_RESERVE, PAGE_NOACCESS));
Require(block != nullptr && VirtualAlloc(block, committed, MEM_COMMIT, PAGE_READWRITE) != nullptr, "cannot reserve the sparse heap");
#else
void* mapped = mmap(nullptr, reserved, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
Require(mapped != MAP_FAILED && mprotect(mapped, committed, PROT_READ | PROT_WRITE) == 0, "cannot reserve the sparse heap");
auto* block = static_cast<std::byte*>(mapped);
#endif
return block;
}
void releaseSparse(std::byte* block, std::size_t reserved) {
#ifdef _WIN32
static_cast<void>(reserved);
VirtualFree(block, 0, MEM_RELEASE);
#else
munmap(block, reserved);
#endif
}
void sparseHeapTests(const Context& context, const BdaTestAccess& access) {
constexpr std::size_t reserved = 64u << 20u;
constexpr std::size_t committed = 65536;
auto* block = reserveSparse(reserved, committed);
std::memset(block, 0x5a, committed);
const auto base = reinterpret_cast<std::uintptr_t>(block);
access.limitMemory(4u << 20u);
{
GuestBufferMemory memory(context);
memory.AddWritable(base, reserved);
memory.Upload(false);
std::uint32_t adjustment = 0;
const auto view = memory.Descriptor(base, reserved, adjustment);
Require(adjustment == 0 && view.range == committed, "a sparse heap view is not cut at its last committed byte");
Require(memory.CopiedBytes() == committed, "a cut sparse heap is charged at its reserved size");
const auto bytes = access.bytes(view.buffer);
Require(bytes.size() < reserved && bytes[view.offset + committed - 1] == std::byte{0x5a}, "a sparse heap was not copied up to its last committed byte");
bytes[view.offset + 8] = std::byte{0x77};
memory.WriteBack();
Require(block[8] == std::byte{0x77}, "a store to a cut sparse heap was not written back");
}
{
constexpr std::size_t late = 1u << 20u;
GuestBufferMemory memory(context);
memory.AddWritable(base, reserved);
memory.AddReadable(base + late, 16);
memory.Upload(true);
std::uint32_t adjustment = 0;
const auto view = memory.Descriptor(base + late, 16, adjustment);
Require(view.range == 16 + adjustment, "a view in the uncommitted part of a sparse heap is not bound");
const auto bytes = access.bytes(view.buffer);
const auto whole = memory.Descriptor(base, reserved, adjustment);
Require(whole.buffer == view.buffer && bytes.size() >= whole.offset + late + 16, "a view in the uncommitted part of a sparse heap has another owner");
Require(bytes[whole.offset + committed - 1] == std::byte{0x5a} && std::all_of(bytes.begin() + static_cast<std::ptrdiff_t>(whole.offset + committed), bytes.begin() + static_cast<std::ptrdiff_t>(whole.offset + late + 16), [](std::byte byte) { return byte == std::byte{}; }), "the uncommitted part of a cut sparse heap does not read as zeros");
Require(whole.range == late + 256, "a sparse heap copy does not end past its last view's start");
Require(memory.CopiedBytes() == late + 256, "a cut sparse heap charge does not include its last view");
const auto ranges = memory.AddressRanges();
Require(ranges.size() == 1 && ranges[0].begin == base && ranges[0].end == base + late + 256, "the BDA range of a cut sparse heap is not clamped to its copy");
memory.WriteBack();
}
access.limitMemory(std::nullopt);
releaseSparse(block, reserved);
}
void RunBdaResourceTests(const Context& context, const BdaTestAccess& access) {
alignas(64) std::array<std::uint32_t, 16> guest{};
guest[0] = 123;
@ -842,6 +907,7 @@ void RunBdaResourceTests(const Context& context, const BdaTestAccess& access) {
importedFreshTests(context);
gpuMappingTests(context);
stridedOverhangTests(context, access);
sparseHeapTests(context, access);
Require(AddressCopyOverflow({{0x1000, 0x3000, 0x2000, "uncommitted pages"}}, 0x2000).empty(), "copies within the limit were refused");
const auto copies = AddressCopyOverflow({{0x1000, 0x2000, 0x1000, "not mirrored"}, {0x10000, 0x30000, 0x18000, "uncommitted pages"}}, 0x2000);
Require(!copies.empty() && copies.find("0x10000+0x20000 (0.1 MiB committed, uncommitted pages)") < copies.find("0x1000+0x1000"), "the copy limit does not name the largest copy first");