fix(agc): patch the memory offsets of misaligned guest buffers in the shader data buffer

A guest buffer view off minStorageBufferOffsetAlignment is bound from the
aligned offset below it, and the difference goes into the shader's memory
byte offsets (3dec6ced), but only when those live in the push constants:
a shader whose shader data is a buffer failed with "a guest buffer off the
storage buffer offset alignment in a shader whose shader data is a buffer
is not implemented".

The adjustment now goes into the shader data buffer's byte at
memoryOffsetDword * 4 + element, and a template's data refresh writes the
dispatch's words with the same patches applied.
This commit is contained in:
Dean Galvin 2026-10-01 16:09:16 +13:00
parent a67b644ff6
commit 81d6f94ceb
3 changed files with 79 additions and 4 deletions

View File

@ -242,7 +242,15 @@ private:
std::vector<std::uint32_t> dataWords;
std::uint32_t adjustment = 0;
std::int32_t pushByte = -1;
std::int64_t dataAllocation = -1;
std::uint32_t dataByte = 0;
};
struct DataPatch {
std::size_t allocation;
std::uint32_t byte;
std::uint32_t adjustment;
};
void writeDataWords(VkCommandBuffer commands, std::size_t allocation, std::span<const std::uint32_t> words) const;
struct Binding {
VkDescriptorSetLayoutBinding layout;
@ -406,6 +414,7 @@ private:
std::uint64_t dataWordsHash = 14695981039346656037ull;
void rehashDataWords();
std::vector<std::pair<std::uint32_t, std::uint32_t>> pushPatches;
std::vector<DataPatch> dataPatches;
BuildTiming timing;
// Build state carried from stage A to stage B: the bindings in plan order, the image bindings
// still to look up (index into `bindings`; the DescriptorBinding lives in the compiled shader),

View File

@ -816,6 +816,8 @@ void ShaderResources::buildPrepare(std::span<const CompiledShader> shaders, cons
Require(shader.program != nullptr, "missing compiled shader");
const VkShaderStageFlags flags = VulkanStage(shader.stage);
std::uint64_t stageDescriptors = 0;
std::vector<std::size_t> offsetsInData;
std::int64_t shaderData = -1;
for (const auto& binding : shader.program->bindings) {
Require(binding.descriptorSet == 0, "unexpected descriptor set: every shader resource must use descriptor set zero");
Require(occupied.insert(binding.binding).second, "duplicate shader binding");
@ -847,6 +849,9 @@ void ShaderResources::buildPrepare(std::span<const CompiledShader> shaders, cons
const auto position = shader.program->memoryOffsetDword * 4u + element;
Require(position < push.size(), "guest buffer offset lies outside the shader's push constants");
allocations[index].pushByte = static_cast<std::int32_t>(shader.pushConstantOffset + position);
} else {
allocations[index].dataByte = shader.program->memoryOffsetDword * 4u + element;
offsetsInData.push_back(index);
}
item.allocations.push_back(index);
}
@ -857,9 +862,11 @@ void ShaderResources::buildPrepare(std::span<const CompiledShader> shaders, cons
Require(binding.count == 1, "shader data and flattened SRT descriptors must not be arrays");
Require(!binding.guestDescriptor.empty(), "empty shader data descriptor");
item.allocations.push_back(addDataBuffer(binding.guestDescriptor));
if (binding.role == ShaderRecompiler::DescriptorRole::ShaderData) shaderData = static_cast<std::int64_t>(item.allocations.back());
}
bindings.push_back(std::move(item));
}
for (const auto index : offsetsInData) allocations[index].dataAllocation = shaderData;
}
Require(storageBuffers <= context.limits.maxDescriptorSetStorageBuffers, "pipeline descriptors exceed device limits");
timing.bindingsMs = phase(BuildPhase::Bindings);
@ -988,8 +995,15 @@ void ShaderResources::buildComplete() {
}
for (const auto& allocation : allocations) {
if (allocation.adjustment == 0) continue;
Require(allocation.pushByte >= 0, "a guest buffer off the storage buffer offset alignment in a shader whose shader data is a buffer is not implemented");
pushPatches.emplace_back(static_cast<std::uint32_t>(allocation.pushByte), allocation.adjustment);
if (allocation.pushByte >= 0) {
pushPatches.emplace_back(static_cast<std::uint32_t>(allocation.pushByte), allocation.adjustment);
continue;
}
Require(allocation.dataAllocation >= 0, "a guest buffer off the storage buffer offset alignment in a shader without shader data is not implemented");
auto& data = allocations[static_cast<std::size_t>(allocation.dataAllocation)];
Require(data.buffer != nullptr && allocation.dataByte < data.size, "guest buffer offset lies outside the shader's data buffer");
data.buffer->Bytes()[allocation.dataByte] = static_cast<std::byte>(allocation.adjustment);
dataPatches.push_back({static_cast<std::size_t>(allocation.dataAllocation), allocation.dataByte, allocation.adjustment});
}
timing.descriptorsMs += phase(BuildPhase::Descriptors);
noteReusable();
@ -2188,7 +2202,7 @@ bool ShaderResources::RefreshData(VkCommandBuffer commands, const CompiledShader
Require(allocation.buffer != nullptr && !allocation.guest && allocation.size == size && size <= MaxRefreshBytes, "template data buffer cannot take the dispatch's words");
if (allocation.dataWords.size() == binding.guestDescriptor.size() && std::equal(allocation.dataWords.begin(), allocation.dataWords.end(), binding.guestDescriptor.begin())) continue;
if (recorder != nullptr && timing == Recorder::NoTiming) timing = recorder->BeginGpuTiming(Recorder::CommandClass::TemplateDataRefresh);
context.Resolved(&DeviceFunctions::cmdUpdateBuffer, "vkCmdUpdateBuffer")(commands, allocation.buffer->Handle(), 0, size, binding.guestDescriptor.data());
writeDataWords(commands, bindings[index].allocations.front(), binding.guestDescriptor);
allocation.dataWords.assign(binding.guestDescriptor.begin(), binding.guestDescriptor.end());
refreshedBytes += size;
recorded = true;
@ -2198,6 +2212,21 @@ bool ShaderResources::RefreshData(VkCommandBuffer commands, const CompiledShader
return recorded;
}
void ShaderResources::writeDataWords(VkCommandBuffer commands, std::size_t allocation, std::span<const std::uint32_t> words) const {
const auto& buffer = *allocations[allocation].buffer;
const auto size = words.size() * sizeof(std::uint32_t);
if (std::none_of(dataPatches.begin(), dataPatches.end(), [&](const DataPatch& patch) { return patch.allocation == allocation; })) {
context.Resolved(&DeviceFunctions::cmdUpdateBuffer, "vkCmdUpdateBuffer")(commands, buffer.Handle(), 0, size, words.data());
return;
}
std::vector<std::uint32_t> patched(words.begin(), words.end());
auto* bytes = reinterpret_cast<std::byte*>(patched.data());
for (const auto& patch : dataPatches) {
if (patch.allocation == allocation && patch.byte < size) bytes[patch.byte] = static_cast<std::byte>(patch.adjustment);
}
context.Resolved(&DeviceFunctions::cmdUpdateBuffer, "vkCmdUpdateBuffer")(commands, buffer.Handle(), 0, size, patched.data());
}
void ShaderResources::PrecollectSurfaces() const {
for (const auto& range : describedRanges) GuestMemory::CollectWrites(range.address, static_cast<std::size_t>(range.bytes));
}

View File

@ -566,6 +566,12 @@ VKAPI_ATTR void VKAPI_CALL mockCmdDispatch(VkCommandBuffer, std::uint32_t x, std
mock.lastDispatchGroups = {x, y, z};
}
VKAPI_ATTR void VKAPI_CALL mockCmdUpdateBuffer(VkCommandBuffer, VkBuffer buffer, VkDeviceSize offset, VkDeviceSize size, const void* data) {
auto& memory = mock.memories.at(mock.bufferMemory.at(buffer));
Require(offset + size <= memory.size(), "a buffer update exceeds its buffer");
std::memcpy(memory.data() + offset, data, static_cast<std::size_t>(size));
}
PFN_vkVoidFunction VKAPI_CALL mockProc(VkDevice, const char* name) {
static const std::map<std::string_view, PFN_vkVoidFunction> table{
{"vkGetBufferDeviceAddressKHR", reinterpret_cast<PFN_vkVoidFunction>(mockGetBufferDeviceAddress)},
@ -592,7 +598,8 @@ PFN_vkVoidFunction VKAPI_CALL mockProc(VkDevice, const char* name) {
{"vkDestroyPipeline", reinterpret_cast<PFN_vkVoidFunction>(mockDestroyPipeline)},
{"vkCmdBindPipeline", reinterpret_cast<PFN_vkVoidFunction>(mockCmdBindPipeline)},
{"vkCmdPushConstants", reinterpret_cast<PFN_vkVoidFunction>(mockCmdPushConstants)},
{"vkCmdDispatch", reinterpret_cast<PFN_vkVoidFunction>(mockCmdDispatch)}
{"vkCmdDispatch", reinterpret_cast<PFN_vkVoidFunction>(mockCmdDispatch)},
{"vkCmdUpdateBuffer", reinterpret_cast<PFN_vkVoidFunction>(mockCmdUpdateBuffer)}
};
const auto it = table.find(name);
return it == table.end() ? nullptr : it->second;
@ -849,6 +856,35 @@ void resourceTests() {
}
}
void misalignedShaderDataTests() {
mock = MockVulkan{};
auto context = mockContext();
context.limits.minStorageBufferOffsetAlignment = 16;
const auto commands = reinterpret_cast<VkCommandBuffer>(std::uintptr_t{1});
alignas(64) std::array<std::uint32_t, 8> guest{1, 2, 3, 4, 5, 6, 7, 8};
{
ShaderRecompiler::RecompileResult compute;
compute.bindings.push_back(makeBinding(Role::GuestBuffers, 0, 2, join(vsharp(guest.data(), 32), vsharp(guest.data() + 1, 8))));
compute.bindings.push_back(makeBinding(Role::ShaderData, 1, 1, {0x11, 0x22, 0}));
compute.memoryOffsetDword = 2;
auto live = compute;
live.bindings[1].guestDescriptor = {0x33, 0x44, 0};
const AgcDriver::Graphics::CompiledShader shader{ShaderRecompiler::ShaderStage::Compute, &compute, 0};
const AgcDriver::Graphics::CompiledShader liveShader{ShaderRecompiler::ShaderStage::Compute, &live, 0};
AgcDriver::Graphics::ShaderResources resources(context, shader);
const auto& views = findWrite(0);
Require(views.buffers.size() == 2 && views.buffers[0].range == 32 && views.buffers[1].range == 12 && views.buffers[1].offset == views.buffers[0].offset, "the misaligned view is not bound from the aligned offset below it");
const auto data = findWrite(1).buffers.at(0).buffer;
const std::array<std::uint32_t, 3> patched{0x11, 0x22, 0x400};
Require(sameBytes(bufferBytes(data), patched.data(), sizeof(patched)), "the shader data buffer does not hold the misaligned view's offset");
Require(resources.RefreshData(commands, liveShader), "a refresh with different words recorded nothing");
const std::array<std::uint32_t, 3> refreshed{0x33, 0x44, 0x400};
Require(sameBytes(bufferBytes(data), refreshed.data(), sizeof(refreshed)), "a data refresh dropped the misaligned view's offset");
Require(!resources.DataWordsDiffer(liveShader) && resources.DataWordsHash() == AgcDriver::Graphics::ShaderResources::DataWordsHash(liveShader), "the refreshed template's words are not the dispatch's");
}
Require(mock.live == 0, "misaligned shader data resources leaked Vulkan objects");
}
struct ModuleShape {
bool fragment = false;
bool push = false;
@ -1322,6 +1358,7 @@ int main() {
InitialContextTests();
pushConstantTests();
resourceTests();
misalignedShaderDataTests();
debugBranchTests();
validationTests();
rectListTests();