Commit bfbdd89d by Chris Forbes

Fix various descriptor handling bugs

- Use a custom descriptor structure for everything, so we have control over alignment - Fix descriptor pool size to include the proper number of headers - Remove a level of indirection in buffer descriptor handling - Correctly handle VK_WHOLE_SIZE for buffer bound ranges - Handle robustness + dynamic offset interaction where the dynamic offset places the bound range partially outside the buffer. - Sanity checks from Ben's original 'seatbelts' patch Bug: b/123244275 Test: dEQP-VK.binding_model.* Change-Id: I18cc40805aac7256a7d57a21af003f21f630b2e6 Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/30168 Presubmit-Ready: Chris Forbes <chrisforbes@google.com> Reviewed-by: 's avatarBen Clayton <bclayton@google.com> Reviewed-by: 's avatarNicolas Capens <nicolascapens@google.com> Tested-by: 's avatarChris Forbes <chrisforbes@google.com> Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
parent 793050e8
...@@ -1334,20 +1334,23 @@ namespace sw ...@@ -1334,20 +1334,23 @@ namespace sw
auto setLayout = routine->pipelineLayout->getDescriptorSetLayout(d.DescriptorSet); auto setLayout = routine->pipelineLayout->getDescriptorSetLayout(d.DescriptorSet);
int bindingOffset = static_cast<int>(setLayout->getBindingOffset(d.Binding, arrayIndex)); int bindingOffset = static_cast<int>(setLayout->getBindingOffset(d.Binding, arrayIndex));
Pointer<Byte> bufferInfo = set.base + bindingOffset; // VkDescriptorBufferInfo* Pointer<Byte> descriptor = set.base + bindingOffset; // BufferDescriptor*
Pointer<Byte> buffer = *Pointer<Pointer<Byte>>(bufferInfo + OFFSET(VkDescriptorBufferInfo, buffer)); // vk::Buffer* Pointer<Byte> data = *Pointer<Pointer<Byte>>(descriptor + OFFSET(vk::BufferDescriptor, ptr)); // void*
Pointer<Byte> data = *Pointer<Pointer<Byte>>(buffer + vk::Buffer::DataOffset); // void* Int size = *Pointer<Int>(descriptor + OFFSET(vk::BufferDescriptor, sizeInBytes));
Int offset = *Pointer<Int>(bufferInfo + OFFSET(VkDescriptorBufferInfo, offset));
Int size = *Pointer<Int>(buffer + vk::Buffer::DataSize); // void*
if (setLayout->isBindingDynamic(d.Binding)) if (setLayout->isBindingDynamic(d.Binding))
{ {
uint32_t dynamicBindingIndex = uint32_t dynamicBindingIndex =
routine->pipelineLayout->getDynamicOffsetBase(d.DescriptorSet) + routine->pipelineLayout->getDynamicOffsetBase(d.DescriptorSet) +
setLayout->getDynamicDescriptorOffset(d.Binding) + setLayout->getDynamicDescriptorOffset(d.Binding) +
arrayIndex; arrayIndex;
offset += routine->descriptorDynamicOffsets[dynamicBindingIndex]; Int offset = routine->descriptorDynamicOffsets[dynamicBindingIndex];
Int robustnessSize = *Pointer<Int>(descriptor + OFFSET(vk::BufferDescriptor, robustnessSize));
return SIMD::Pointer(data + offset, Min(size, robustnessSize - offset));
}
else
{
return SIMD::Pointer(data, size);
} }
return SIMD::Pointer(data + offset, size - offset);
} }
default: default:
......
...@@ -21,9 +21,6 @@ ...@@ -21,9 +21,6 @@
namespace vk namespace vk
{ {
const int Buffer::DataOffset = static_cast<int>(offsetof(Buffer, memory));
const int Buffer::DataSize = static_cast<int>(offsetof(Buffer, size));
Buffer::Buffer(const VkBufferCreateInfo* pCreateInfo, void* mem) : Buffer::Buffer(const VkBufferCreateInfo* pCreateInfo, void* mem) :
flags(pCreateInfo->flags), size(pCreateInfo->size), usage(pCreateInfo->usage), flags(pCreateInfo->flags), size(pCreateInfo->size), usage(pCreateInfo->usage),
sharingMode(pCreateInfo->sharingMode) sharingMode(pCreateInfo->sharingMode)
......
...@@ -40,11 +40,6 @@ public: ...@@ -40,11 +40,6 @@ public:
inline VkDeviceSize getSize() const { return size; } inline VkDeviceSize getSize() const { return size; }
uint8_t* end() const; uint8_t* end() const;
// DataOffset is the offset in bytes from the Buffer to the pointer to the
// buffer's data memory.
static const int DataOffset;
static const int DataSize;
private: private:
void* memory = nullptr; void* memory = nullptr;
VkBufferCreateFlags flags = 0; VkBufferCreateFlags flags = 0;
......
...@@ -36,13 +36,12 @@ void DescriptorPool::destroy(const VkAllocationCallbacks* pAllocator) ...@@ -36,13 +36,12 @@ void DescriptorPool::destroy(const VkAllocationCallbacks* pAllocator)
size_t DescriptorPool::ComputeRequiredAllocationSize(const VkDescriptorPoolCreateInfo* pCreateInfo) size_t DescriptorPool::ComputeRequiredAllocationSize(const VkDescriptorPoolCreateInfo* pCreateInfo)
{ {
size_t size = 0; size_t size = pCreateInfo->maxSets * sizeof(DescriptorSetHeader);
for(uint32_t i = 0; i < pCreateInfo->poolSizeCount; i++) for(uint32_t i = 0; i < pCreateInfo->poolSizeCount; i++)
{ {
size += pCreateInfo->pPoolSizes[i].descriptorCount * size += pCreateInfo->pPoolSizes[i].descriptorCount *
(sizeof(DescriptorSetHeader) + DescriptorSetLayout::GetDescriptorSize(pCreateInfo->pPoolSizes[i].type);
DescriptorSetLayout::GetDescriptorSize(pCreateInfo->pPoolSizes[i].type));
} }
return size; return size;
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "VkDescriptorSet.hpp" #include "VkDescriptorSet.hpp"
#include "VkSampler.hpp" #include "VkSampler.hpp"
#include "VkImageView.hpp" #include "VkImageView.hpp"
#include "VkBuffer.hpp"
#include "VkBufferView.hpp" #include "VkBufferView.hpp"
#include "System/Types.hpp" #include "System/Types.hpp"
...@@ -65,6 +66,7 @@ DescriptorSetLayout::DescriptorSetLayout(const VkDescriptorSetLayoutCreateInfo* ...@@ -65,6 +66,7 @@ DescriptorSetLayout::DescriptorSetLayout(const VkDescriptorSetLayoutCreateInfo*
bindingOffsets[i] = offset; bindingOffsets[i] = offset;
offset += bindings[i].descriptorCount * GetDescriptorSize(bindings[i].descriptorType); offset += bindings[i].descriptorCount * GetDescriptorSize(bindings[i].descriptorType);
} }
ASSERT_MSG(offset == getDescriptorSetDataSize(), "offset: %d, size: %d", int(offset), int(getDescriptorSetDataSize()));
} }
void DescriptorSetLayout::destroy(const VkAllocationCallbacks* pAllocator) void DescriptorSetLayout::destroy(const VkAllocationCallbacks* pAllocator)
...@@ -89,44 +91,32 @@ size_t DescriptorSetLayout::ComputeRequiredAllocationSize(const VkDescriptorSetL ...@@ -89,44 +91,32 @@ size_t DescriptorSetLayout::ComputeRequiredAllocationSize(const VkDescriptorSetL
size_t DescriptorSetLayout::GetDescriptorSize(VkDescriptorType type) size_t DescriptorSetLayout::GetDescriptorSize(VkDescriptorType type)
{ {
size_t size = 0;
switch(type) switch(type)
{ {
case VK_DESCRIPTOR_TYPE_SAMPLER: case VK_DESCRIPTOR_TYPE_SAMPLER:
case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER: case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER:
case VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE: case VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE:
size = sizeof(SampledImageDescriptor); case VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER:
break; return sizeof(SampledImageDescriptor);
case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE: case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE:
case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER: case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER:
size = sizeof(StorageImageDescriptor);
break;
case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT: case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT:
size = sizeof(VkDescriptorImageInfo); return sizeof(StorageImageDescriptor);
break;
case VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER:
size = sizeof(VkBufferView);
break;
case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER: case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER:
case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER: case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER:
case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC: case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC:
case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC: case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC:
size = sizeof(VkDescriptorBufferInfo); return sizeof(BufferDescriptor);
break;
default: default:
UNIMPLEMENTED("Unsupported Descriptor Type"); UNIMPLEMENTED("Unsupported Descriptor Type");
return 0; return 0;
} }
// Aligning each descriptor to 16 bytes allows for more efficient vector accesses in the shaders.
return sw::align<16>(size); // TODO(b/123244275): Eliminate by using a custom alignas(16) struct for each desctriptor.
} }
size_t DescriptorSetLayout::getDescriptorSetAllocationSize() const size_t DescriptorSetLayout::getDescriptorSetAllocationSize() const
{ {
// vk::DescriptorSet has a layout member field. // vk::DescriptorSet has a layout member field.
return sizeof(vk::DescriptorSetHeader) + getDescriptorSetDataSize(); return sw::align<alignof(DescriptorSet)>(OFFSET(DescriptorSet, data) + getDescriptorSetDataSize());
} }
size_t DescriptorSetLayout::getDescriptorSetDataSize() const size_t DescriptorSetLayout::getDescriptorSetDataSize() const
...@@ -258,8 +248,11 @@ void SampledImageDescriptor::updateSampler(const vk::Sampler *sampler) ...@@ -258,8 +248,11 @@ void SampledImageDescriptor::updateSampler(const vk::Sampler *sampler)
{ {
this->sampler = sampler; this->sampler = sampler;
texture.minLod = sw::clamp(sampler->minLod, 0.0f, (float)(sw::MAX_TEXTURE_LOD)); if (sampler)
texture.maxLod = sw::clamp(sampler->maxLod, 0.0f, (float)(sw::MAX_TEXTURE_LOD)); {
texture.minLod = sw::clamp(sampler->minLod, 0.0f, (float) (sw::MAX_TEXTURE_LOD));
texture.maxLod = sw::clamp(sampler->maxLod, 0.0f, (float) (sw::MAX_TEXTURE_LOD));
}
} }
void DescriptorSetLayout::WriteDescriptorSet(DescriptorSet *dstSet, VkDescriptorUpdateTemplateEntry const &entry, char const *src) void DescriptorSetLayout::WriteDescriptorSet(DescriptorSet *dstSet, VkDescriptorUpdateTemplateEntry const &entry, char const *src)
...@@ -274,7 +267,27 @@ void DescriptorSetLayout::WriteDescriptorSet(DescriptorSet *dstSet, VkDescriptor ...@@ -274,7 +267,27 @@ void DescriptorSetLayout::WriteDescriptorSet(DescriptorSet *dstSet, VkDescriptor
ASSERT(reinterpret_cast<intptr_t>(memToWrite) % 16 == 0); // Each descriptor must be 16-byte aligned. ASSERT(reinterpret_cast<intptr_t>(memToWrite) % 16 == 0); // Each descriptor must be 16-byte aligned.
if(entry.descriptorType == VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER) if (entry.descriptorType == VK_DESCRIPTOR_TYPE_SAMPLER)
{
SampledImageDescriptor *imageSampler = reinterpret_cast<SampledImageDescriptor*>(memToWrite);
for(uint32_t i = 0; i < entry.descriptorCount; i++)
{
auto update = reinterpret_cast<VkDescriptorImageInfo const *>(src + entry.offset + entry.stride * i);
// "All consecutive bindings updated via a single VkWriteDescriptorSet structure, except those with a
// descriptorCount of zero, must all either use immutable samplers or must all not use immutable samplers."
if (!binding.pImmutableSamplers)
{
imageSampler[i].updateSampler(vk::Cast(update->sampler));
}
}
}
else if (entry.descriptorType == VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER)
{
UNIMPLEMENTED("VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER");
}
else if (entry.descriptorType == VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER ||
entry.descriptorType == VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE)
{ {
SampledImageDescriptor *imageSampler = reinterpret_cast<SampledImageDescriptor*>(memToWrite); SampledImageDescriptor *imageSampler = reinterpret_cast<SampledImageDescriptor*>(memToWrite);
...@@ -486,16 +499,20 @@ void DescriptorSetLayout::WriteDescriptorSet(DescriptorSet *dstSet, VkDescriptor ...@@ -486,16 +499,20 @@ void DescriptorSetLayout::WriteDescriptorSet(DescriptorSet *dstSet, VkDescriptor
descriptor[i].sizeInBytes = bufferView->getRangeInBytes(); descriptor[i].sizeInBytes = bufferView->getRangeInBytes();
} }
} }
else else if (entry.descriptorType == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER ||
entry.descriptorType == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC ||
entry.descriptorType == VK_DESCRIPTOR_TYPE_STORAGE_BUFFER ||
entry.descriptorType == VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC)
{ {
// If the dstBinding has fewer than descriptorCount array elements remaining auto descriptor = reinterpret_cast<BufferDescriptor *>(memToWrite);
// starting from dstArrayElement, then the remainder will be used to update for (uint32_t i = 0; i < entry.descriptorCount; i++)
// the subsequent binding - dstBinding+1 starting at array element zero. If {
// a binding has a descriptorCount of zero, it is skipped. This behavior auto update = reinterpret_cast<VkDescriptorBufferInfo const *>(src + entry.offset + entry.stride * i);
// applies recursively, with the update affecting consecutive bindings as auto buffer = Cast(update->buffer);
// needed to update all descriptorCount descriptors. descriptor[i].ptr = buffer->getOffsetPointer(update->offset);
for (auto i = 0u; i < entry.descriptorCount; i++) descriptor[i].sizeInBytes = (update->range == VK_WHOLE_SIZE) ? buffer->getSize() - update->offset : update->range;
memcpy(memToWrite + typeSize * i, src + entry.offset + entry.stride * i, typeSize); descriptor[i].robustnessSize = buffer->getSize() - update->offset;
}
} }
} }
...@@ -514,7 +531,7 @@ void DescriptorSetLayout::WriteDescriptorSet(const VkWriteDescriptorSet& writeDe ...@@ -514,7 +531,7 @@ void DescriptorSetLayout::WriteDescriptorSet(const VkWriteDescriptorSet& writeDe
case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER: case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER:
case VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER: case VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER:
ptr = writeDescriptorSet.pTexelBufferView; ptr = writeDescriptorSet.pTexelBufferView;
e.stride = sizeof(*VkWriteDescriptorSet::pTexelBufferView); e.stride = sizeof(VkBufferView);
break; break;
case VK_DESCRIPTOR_TYPE_SAMPLER: case VK_DESCRIPTOR_TYPE_SAMPLER:
......
...@@ -48,6 +48,13 @@ struct alignas(16) StorageImageDescriptor ...@@ -48,6 +48,13 @@ struct alignas(16) StorageImageDescriptor
int sizeInBytes; int sizeInBytes;
}; };
struct alignas(16) BufferDescriptor
{
void *ptr;
int sizeInBytes; // intended size of the bound region -- slides along with dynamic offsets
int robustnessSize; // total accessible size from static offset -- does not move with dynamic offset
};
class DescriptorSetLayout : public Object<DescriptorSetLayout, VkDescriptorSetLayout> class DescriptorSetLayout : public Object<DescriptorSetLayout, VkDescriptorSetLayout>
{ {
public: public:
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment