From d8bc8a4e450914a0be75c4fb36aaa3da7ba037c3 Mon Sep 17 00:00:00 2001 From: catbot Date: Wed, 17 Jun 2026 17:37:39 +0000 Subject: [PATCH] perf(rt): allocate TLAS instance buffer in BAR/VRAM, not system RAM (#65) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TLAS instance buffer is rebuilt/refit every frame and is co-written by both the CPU (host-authored fields) and the GPU (the compute shader writes the transform field in place for transformOwnedByGpu elements). Allocated HOST_VISIBLE | HOST_COHERENT (system RAM), both GPU accesses crossed PCIe: the compute write of the transform, and the AS build's read of the instances. Upgrade the request to HOST_VISIBLE with a best-effort DEVICE_LOCAL preference (BAR/VRAM), keeping the single shared, persistently-mapped buffer — no staging, no copy, no extra barrier. A whole-buffer staging copy was rejected: it would clobber the GPU-written transforms in this in-place co-write design. Now the compute write and AS build stay in local VRAM; the CPU's write-only, sequential, never-read-back field writes are the ideal write-combined BAR load. Correctness: - DEVICE_LOCAL is a preference only (depends on #59): GetMemoryType falls back to plain HOST_VISIBLE (the previous behaviour) on GPUs without resizable BAR. - HOST_COHERENT is dropped from the required set — the combined type may not be coherent. The existing FlushDevice(cmd, ...) already establishes host->build visibility and gates its flush-skip on the *chosen* type's flags (#60), so a non-coherent BAR type still flushes correctly. The barrier is never removed. metadataBuffer gets the same upgrade separately (#75). Extends the TLASHighWaterMark hardware test to assert the chosen memory type matches the HOST_VISIBLE | prefer-DEVICE_LOCAL request and lands on a DEVICE_LOCAL type where one is reachable; the existing zero-validation-errors check covers the dropped HOST_COHERENT path. Co-Authored-By: Claude Opus 4.8 --- .../Crafter.Graphics-RenderingElement3D.cpp | 17 +++++- tests/TLASHighWaterMark/main.cpp | 53 +++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/implementations/Crafter.Graphics-RenderingElement3D.cpp b/implementations/Crafter.Graphics-RenderingElement3D.cpp index 5eea69c..1da2a3a 100644 --- a/implementations/Crafter.Graphics-RenderingElement3D.cpp +++ b/implementations/Crafter.Graphics-RenderingElement3D.cpp @@ -106,9 +106,24 @@ void RenderingElement3D::BuildTLAS(VkCommandBuffer cmd, std::uint32_t index, RTB // STORAGE_BUFFER_BIT is required because the application's compute // shaders bind these buffers as storage SSBOs (e.g. to write // per-instance transforms directly into the TLAS instance data). + // + // instanceBuffer prefers HOST_VISIBLE | DEVICE_LOCAL (BAR/VRAM) so the + // two per-frame GPU accesses stay on-chip instead of crossing PCIe: the + // compute shader writes the transform field in place, and the AS build + // reads the whole instance. The CPU still writes the host-authored + // fields below — those are write-only, sequential, never-read-back + // writes, the ideal write-combined BAR workload. DEVICE_LOCAL is a + // best-effort preference: GetMemoryType falls back to plain + // HOST_VISIBLE (the previous behaviour) when no combined type exists + // (no resizable BAR). HOST_COHERENT is intentionally dropped from the + // required set — the combined type may not be coherent, and the + // FlushDevice(cmd, ...) below already establishes host->build + // visibility, gating its flush-skip on the *chosen* type's flags. The + // single shared buffer is preserved: a whole-buffer copy would clobber + // the GPU-written transforms, so staging is deliberately avoided. if (tlas.instanceBuffer.buffer == VK_NULL_HANDLE || primitiveCount > tlas.instanceBuffer.size / sizeof(VkAccelerationStructureInstanceKHR)) { - tlas.instanceBuffer.Resize(VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT | VK_BUFFER_USAGE_ACCELERATION_STRUCTURE_BUILD_INPUT_READ_ONLY_BIT_KHR | VK_BUFFER_USAGE_STORAGE_BUFFER_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT, primitiveCount); + tlas.instanceBuffer.Resize(VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT | VK_BUFFER_USAGE_ACCELERATION_STRUCTURE_BUILD_INPUT_READ_ONLY_BIT_KHR | VK_BUFFER_USAGE_STORAGE_BUFFER_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, primitiveCount, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT); tlas.metadataBuffer.Resize(VK_BUFFER_USAGE_STORAGE_BUFFER_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT, primitiveCount); } } diff --git a/tests/TLASHighWaterMark/main.cpp b/tests/TLASHighWaterMark/main.cpp index d35de8a..55bd544 100644 --- a/tests/TLASHighWaterMark/main.cpp +++ b/tests/TLASHighWaterMark/main.cpp @@ -50,6 +50,14 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA // — the strongest check that feeding an oversized instance buffer to the // AS build (with tlasRangeInfo.primitiveCount < capacity) is spec-correct. // +// Issue #65: this test also guards the instance buffer's memory-type request — +// HOST_VISIBLE (required) with a best-effort DEVICE_LOCAL preference and no +// mandatory HOST_COHERENT, so the per-frame GPU accesses stay in local VRAM +// (BAR) rather than crossing PCIe. See the assertion block after the first +// build. The zero-validation-errors check below also covers the dropped +// HOST_COHERENT: the FlushDevice(cmd, ...) in BuildTLAS gates on the chosen +// type's coherency flag, so a non-coherent BAR type still flushes correctly. +// // Validation layers are required for the last check to be meaningful; the build // marks this test as needing the SDK layers. @@ -207,6 +215,51 @@ int main() { Check(RenderingElement3D::tlases[0].builtInstanceCount == 4, "builtInstanceCount == 4 after first build"); + // ── Issue #65: the instance buffer is allocated HOST_VISIBLE (required, + // mappable) with a best-effort DEVICE_LOCAL preference (BAR/VRAM) and the + // mandatory HOST_COHERENT dropped — so the per-frame compute-write of the + // transform field and the AS build's read of the instances both stay in + // local VRAM instead of crossing PCIe. The chosen type must match exactly + // what GetMemoryType resolves for this buffer's memoryTypeBits with that + // prefer-but-don't-require request: DEVICE_LOCAL when a host-visible + // device-local type is reachable (resizable BAR), plain HOST_VISIBLE + // otherwise. Asserting equality with the recomputed selection makes the + // check device-independent — it tracks the call-site request, not the + // particular GPU this runs on. ──────────────────────────────────────────── + { + auto& ib = RenderingElement3D::tlases[0].instanceBuffer; + VkMemoryRequirements req{}; + vkGetBufferMemoryRequirements(Device::device, ib.buffer, &req); + std::uint32_t expectIdx = Device::GetMemoryType( + req.memoryTypeBits, + VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, + VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT); + VkMemoryPropertyFlags expectFlags = + Device::memoryProperties.memoryTypes[expectIdx].propertyFlags; + Check((ib.memoryPropertyFlagsChosen & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT) != 0, + "instance buffer memory is HOST_VISIBLE (mappable — the required flag)"); + Check(ib.memoryPropertyFlagsChosen == expectFlags, + "instance buffer memory matches the HOST_VISIBLE | prefer-DEVICE_LOCAL request (#65)"); + + bool barReachable = false; + constexpr VkMemoryPropertyFlags kBar = + VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT; + for (std::uint32_t i = 0; i < Device::memoryProperties.memoryTypeCount; ++i) { + if ((req.memoryTypeBits & (1u << i)) + && (Device::memoryProperties.memoryTypes[i].propertyFlags & kBar) == kBar) { + barReachable = true; + break; + } + } + if (barReachable) { + Check((ib.memoryPropertyFlagsChosen & VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT) != 0, + "instance buffer landed on a DEVICE_LOCAL (BAR/VRAM) type where one is reachable"); + } else { + std::println("INFO no host-visible device-local type reachable — " + "fell back to plain HOST_VISIBLE (expected on non-BAR GPUs)"); + } + } + // ── 2. Grow to 16 (past capacity) → REALLOCATES. ──────────────────────── SetCount(pool, 16); BuildOnce();