From 0f25191c03f324762437615d961bad978b9d5e15 Mon Sep 17 00:00:00 2001 From: Ricardo Garcia Date: Thu, 26 Nov 2020 17:35:07 +0100 Subject: [PATCH] Fix buffer offsets in push descriptor tests This commit fixes push descriptor tests that were updating storage buffer descriptors using offset values that may not have been multiples of minStorageBufferOffsetAlignment. In addition, it fixes image layout problems with some of these tests that were causing validation errors. Finally, it introduces the de::gcd and de::lcm functions to calculate the greatest common divisor and least common multiple of two numbers. These are useful when calculating alignments that need to meet several base alignment requirements. Affected tests: dEQP-VK.pipeline.push_descriptor.* Components: Framework, Vulkan VK-GL-CTS issue: 2661 Change-Id: I36bcaf6114994ec5b42bcd5acc11340f079823dd --- .../pipeline/vktPipelinePushDescriptorTests.cpp | 88 ++++++++++++++-------- framework/delibs/decpp/deDefs.hpp | 30 ++++++++ 2 files changed, 85 insertions(+), 33 deletions(-) diff --git a/external/vulkancts/modules/vulkan/pipeline/vktPipelinePushDescriptorTests.cpp b/external/vulkancts/modules/vulkan/pipeline/vktPipelinePushDescriptorTests.cpp index 9b7d554..bfeb035 100644 --- a/external/vulkancts/modules/vulkan/pipeline/vktPipelinePushDescriptorTests.cpp +++ b/external/vulkancts/modules/vulkan/pipeline/vktPipelinePushDescriptorTests.cpp @@ -67,6 +67,8 @@ typedef de::SharedPtr > VkRenderPassSp; typedef de::SharedPtr > VkFramebufferSp; typedef de::SharedPtr > VkPipelineSp; +constexpr VkDeviceSize kSizeofVec4 = static_cast(sizeof(tcu::Vec4)); + struct TestParams { VkDescriptorType descriptorType; @@ -74,6 +76,12 @@ struct TestParams deUint32 numCalls; // Number of draw or dispatch calls }; +VkDeviceSize calcItemSize (const InstanceInterface& vki, VkPhysicalDevice physicalDevice) +{ + const auto minAlignment = getPhysicalDeviceProperties(vki, physicalDevice).limits.minStorageBufferOffsetAlignment; + return de::lcm(de::max(VkDeviceSize{1}, minAlignment), kSizeofVec4); +} + void checkAllSupported (const Extensions& supportedExtensions, const vector& requiredExtensions) { for (auto& requiredExtName : requiredExtensions) @@ -399,7 +407,7 @@ void PushDescriptorBufferGraphicsTestInstance::init (void) VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO, // VkStructureType sType; DE_NULL, // const void* pNext; 0u, // VkBufferCreateFlags flags - 16u, // VkDeviceSize size; + kSizeofVec4, // VkDeviceSize size; usageFlags, // VkBufferUsageFlags usage; VK_SHARING_MODE_EXCLUSIVE, // VkSharingMode sharingMode; 1u, // deUint32 queueFamilyCount; @@ -410,7 +418,7 @@ void PushDescriptorBufferGraphicsTestInstance::init (void) m_bufferAllocs.push_back(AllocationSp(m_allocator.allocate(getBufferMemoryRequirements(m_vkd, *m_device, **m_buffers[bufIdx]), MemoryRequirement::HostVisible).release())); VK_CHECK(m_vkd.bindBufferMemory(*m_device, **m_buffers[bufIdx], m_bufferAllocs[bufIdx]->getMemory(), m_bufferAllocs[bufIdx]->getOffset())); - deMemcpy(m_bufferAllocs[bufIdx]->getHostPtr(), &defaultTestColors[bufIdx], 16u); + deMemcpy(m_bufferAllocs[bufIdx]->getHostPtr(), &defaultTestColors[bufIdx], static_cast(kSizeofVec4)); flushAlloc(m_vkd, *m_device, *m_bufferAllocs[bufIdx]); } } @@ -524,7 +532,7 @@ void PushDescriptorBufferGraphicsTestInstance::init (void) { **m_buffers[quadNdx], // VkBuffer buffer; 0u, // VkDeviceSize offset; - 16u // VkDeviceSize range; + kSizeofVec4, // VkDeviceSize range; }; VkWriteDescriptorSet writeDescriptorSet = @@ -693,6 +701,7 @@ private: const Unique m_device; const DeviceDriver m_vkd; const VkQueue m_queue; + const VkDeviceSize m_itemSize; SimpleAllocator m_allocator; Move m_computeShaderModule; vector m_buffers; @@ -720,6 +729,7 @@ PushDescriptorBufferComputeTestInstance::PushDescriptorBufferComputeTestInstance , m_device (createDeviceWithPushDescriptor(context, m_vkp, m_instance, m_vki, m_physicalDevice, m_deviceExtensions, m_queueFamilyIndex)) , m_vkd (m_vkp, m_instance, *m_device) , m_queue (getDeviceQueue(m_vkd, *m_device, m_queueFamilyIndex, 0u)) + , m_itemSize (calcItemSize(m_vki, m_physicalDevice)) , m_allocator (m_vkd, *m_device, getPhysicalDeviceMemoryProperties(m_vki, m_physicalDevice)) { } @@ -800,7 +810,7 @@ void PushDescriptorBufferComputeTestInstance::init (void) VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO, // VkStructureType sType; DE_NULL, // const void* pNext; 0u, // VkBufferCreateFlags flags - 16u, // VkDeviceSize size; + kSizeofVec4, // VkDeviceSize size; usageFlags, // VkBufferUsageFlags usage; VK_SHARING_MODE_EXCLUSIVE, // VkSharingMode sharingMode; 1u, // deUint32 queueFamilyCount; @@ -811,7 +821,7 @@ void PushDescriptorBufferComputeTestInstance::init (void) m_bufferAllocs.push_back(AllocationSp(m_allocator.allocate(getBufferMemoryRequirements(m_vkd, *m_device, **m_buffers[bufIdx]), MemoryRequirement::HostVisible).release())); VK_CHECK(m_vkd.bindBufferMemory(*m_device, **m_buffers[bufIdx], m_bufferAllocs[bufIdx]->getMemory(), m_bufferAllocs[bufIdx]->getOffset())); - deMemcpy(m_bufferAllocs[bufIdx]->getHostPtr(), &m_testColors[bufIdx], 16u); + deMemcpy(m_bufferAllocs[bufIdx]->getHostPtr(), &m_testColors[bufIdx], static_cast(kSizeofVec4)); flushAlloc(m_vkd, *m_device, *m_bufferAllocs[bufIdx]); } } @@ -823,7 +833,7 @@ void PushDescriptorBufferComputeTestInstance::init (void) VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO, // VkStructureType sType; DE_NULL, // const void* pNext; 0u, // VkBufferCreateFlags flags - 16u * m_params.numCalls, // VkDeviceSize size; + m_itemSize * m_params.numCalls, // VkDeviceSize size; VK_BUFFER_USAGE_STORAGE_BUFFER_BIT, // VkBufferUsageFlags usage; VK_SHARING_MODE_EXCLUSIVE, // VkSharingMode sharingMode; 1u, // deUint32 queueFamilyCount; @@ -884,14 +894,14 @@ void PushDescriptorBufferComputeTestInstance::init (void) { **m_buffers[dispatchNdx], // VkBuffer buffer; 0u, // VkDeviceSize offset; - 16u // VkDeviceSize range; + kSizeofVec4, // VkDeviceSize range; }; VkDescriptorBufferInfo descriptorBufferInfoOutput = { - *m_outputBuffer, // VkBuffer buffer; - 16u * dispatchNdx, // VkDeviceSize offset; - 16u // VkDeviceSize range; + *m_outputBuffer, // VkBuffer buffer; + m_itemSize * dispatchNdx, // VkDeviceSize offset; + kSizeofVec4, // VkDeviceSize range; }; VkWriteDescriptorSet writeDescriptorSets[] = @@ -948,10 +958,13 @@ tcu::TestStatus PushDescriptorBufferComputeTestInstance::verifyOutput (void) invalidateAlloc(m_vkd, *m_device, *m_outputBufferAlloc); // Verify result - if (deMemCmp((void*)&m_testColors[0], m_outputBufferAlloc->getHostPtr(), (size_t)(16u * m_params.numCalls))) + auto bufferPtr = reinterpret_cast(m_outputBufferAlloc->getHostPtr()); + for (deUint32 i = 0; i < m_params.numCalls; ++i) { - return tcu::TestStatus::fail("Output mismatch"); + if (deMemCmp(&m_testColors[i], bufferPtr + (i * m_itemSize), static_cast(kSizeofVec4)) != 0) + TCU_FAIL("Output mismatch at output item " + de::toString(i)); } + return tcu::TestStatus::pass("Output matches expected values"); } @@ -2750,7 +2763,7 @@ void PushDescriptorTexelBufferGraphicsTestInstance::init (void) VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO, // VkStructureType sType; DE_NULL, // const void* pNext; 0u, // VkBufferCreateFlags flags - 16u, // VkDeviceSize size; + kSizeofVec4, // VkDeviceSize size; usageFlags, // VkBufferUsageFlags usage; VK_SHARING_MODE_EXCLUSIVE, // VkSharingMode sharingMode; 1u, // deUint32 queueFamilyCount; @@ -2761,7 +2774,7 @@ void PushDescriptorTexelBufferGraphicsTestInstance::init (void) m_bufferAllocs.push_back(AllocationSp(m_allocator.allocate(getBufferMemoryRequirements(m_vkd, *m_device, **m_buffers[bufIdx]), MemoryRequirement::HostVisible).release())); VK_CHECK(m_vkd.bindBufferMemory(*m_device, **m_buffers[bufIdx], m_bufferAllocs[bufIdx]->getMemory(), m_bufferAllocs[bufIdx]->getOffset())); - deMemcpy(m_bufferAllocs[bufIdx]->getHostPtr(), &defaultTestColors[bufIdx], 16u); + deMemcpy(m_bufferAllocs[bufIdx]->getHostPtr(), &defaultTestColors[bufIdx], static_cast(kSizeofVec4)); flushAlloc(m_vkd, *m_device, *m_bufferAllocs[bufIdx]); } @@ -3131,6 +3144,7 @@ private: const Unique m_device; const DeviceDriver m_vkd; const VkQueue m_queue; + const VkDeviceSize m_itemSize; SimpleAllocator m_allocator; vector m_buffers; vector m_bufferAllocs; @@ -3159,6 +3173,7 @@ PushDescriptorTexelBufferComputeTestInstance::PushDescriptorTexelBufferComputeTe , m_device (createDeviceWithPushDescriptor(context, m_vkp, m_instance, m_vki, m_physicalDevice, m_deviceExtensions, m_queueFamilyIndex)) , m_vkd (m_vkp, m_instance, *m_device) , m_queue (getDeviceQueue(m_vkd, *m_device, m_queueFamilyIndex, 0u)) + , m_itemSize (calcItemSize(m_vki, m_physicalDevice)) , m_allocator (m_vkd, *m_device, getPhysicalDeviceMemoryProperties(m_vki, m_physicalDevice)) , m_bufferFormat (VK_FORMAT_R32G32B32A32_SFLOAT) { @@ -3176,7 +3191,7 @@ void PushDescriptorTexelBufferComputeTestInstance::init (void) VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO, // VkStructureType sType; DE_NULL, // const void* pNext; 0u, // VkBufferCreateFlags flags - 16u, // VkDeviceSize size; + kSizeofVec4, // VkDeviceSize size; usageFlags, // VkBufferUsageFlags usage; VK_SHARING_MODE_EXCLUSIVE, // VkSharingMode sharingMode; 1u, // deUint32 queueFamilyCount; @@ -3187,7 +3202,7 @@ void PushDescriptorTexelBufferComputeTestInstance::init (void) m_bufferAllocs.push_back(AllocationSp(m_allocator.allocate(getBufferMemoryRequirements(m_vkd, *m_device, **m_buffers[bufIdx]), MemoryRequirement::HostVisible).release())); VK_CHECK(m_vkd.bindBufferMemory(*m_device, **m_buffers[bufIdx], m_bufferAllocs[bufIdx]->getMemory(), m_bufferAllocs[bufIdx]->getOffset())); - deMemcpy(m_bufferAllocs[bufIdx]->getHostPtr(), &defaultTestColors[bufIdx], 16u); + deMemcpy(m_bufferAllocs[bufIdx]->getHostPtr(), &defaultTestColors[bufIdx], static_cast(kSizeofVec4)); flushAlloc(m_vkd, *m_device, *m_bufferAllocs[bufIdx]); } @@ -3257,12 +3272,14 @@ void PushDescriptorTexelBufferComputeTestInstance::init (void) // Create output buffer { + DE_ASSERT(m_params.numCalls <= 2u); + const VkBufferCreateInfo bufferCreateInfo = { VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO, // VkStructureType sType; DE_NULL, // const void* pNext; 0u, // VkBufferCreateFlags flags - 32u, // VkDeviceSize size; + m_itemSize * m_params.numCalls, // VkDeviceSize size; VK_BUFFER_USAGE_STORAGE_BUFFER_BIT, // VkBufferUsageFlags usage; VK_SHARING_MODE_EXCLUSIVE, // VkSharingMode sharingMode; 1u, // deUint32 queueFamilyCount; @@ -3338,9 +3355,9 @@ void PushDescriptorTexelBufferComputeTestInstance::init (void) const VkDescriptorBufferInfo descriptorBufferInfoOutput = { - *m_outputBuffer, // VkBuffer buffer; - 16u * dispatchNdx, // VkDeviceSize offset; - 16u // VkDeviceSize range; + *m_outputBuffer, // VkBuffer buffer; + m_itemSize * dispatchNdx, // VkDeviceSize offset; + kSizeofVec4, // VkDeviceSize range; }; // Write output buffer descriptor set @@ -3383,21 +3400,26 @@ tcu::TestStatus PushDescriptorTexelBufferComputeTestInstance::iterate (void) tcu::TestStatus PushDescriptorTexelBufferComputeTestInstance::verifyOutput (void) { - const float ref[8] = { 1.0f, 0.0f, 0.0f, 1.0f, 0.0f, 1.0f, 0.0f, 1.0f }; + const tcu::Vec4 ref[2] = { { 1.0f, 0.0f, 0.0f, 1.0f }, { 0.0f, 1.0f, 0.0f, 1.0f } }; invalidateAlloc(m_vkd, *m_device, *m_outputBufferAlloc); // Verify result - if (deMemCmp((void*)ref, m_outputBufferAlloc->getHostPtr(), (size_t)(16u * m_params.numCalls))) - { - const float* ptr = (float*)m_outputBufferAlloc->getHostPtr(); - std::string debugMsg = "Output buffer contents:\n"; + DE_ASSERT(m_params.numCalls <= 2u); - for (deUint32 i = 0; i < m_params.numCalls * 4; i++) - debugMsg += de::toString(ptr[i]) + " vs " + de::toString(ref[i]) + "\n"; + auto bufferPtr = reinterpret_cast(m_outputBufferAlloc->getHostPtr()); + for (deUint32 i = 0; i < m_params.numCalls; ++i) + { + tcu::Vec4 bufferColor; + deMemcpy(&bufferColor, bufferPtr + (i * m_itemSize), static_cast(kSizeofVec4)); - m_context.getTestContext().getLog() << tcu::TestLog::Message << debugMsg << tcu::TestLog::EndMessage; - return tcu::TestStatus::fail("Output mismatch"); + if (bufferColor != ref[i]) + { + std::ostringstream msg; + msg << "Output mismatch at item " << i << ": expected " << ref[i] << " but found " << bufferColor; + TCU_FAIL(msg.str()); + } } + return tcu::TestStatus::pass("Output matches expected values"); } @@ -3730,7 +3752,7 @@ void PushDescriptorInputAttachmentGraphicsTestInstance::init (void) const VkImageLayout initialLayouts[] = { - VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL, + VK_IMAGE_LAYOUT_UNDEFINED, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL }; @@ -3985,9 +4007,9 @@ void PushDescriptorInputAttachmentGraphicsTestInstance::init (void) VkDescriptorImageInfo descriptorImageInfo = { - 0, // VkSampler sampler; - **m_inputImageViews[quadNdx], // VkImageView imageView; - VK_IMAGE_LAYOUT_GENERAL // VkImageLayout imageLayout; + 0, // VkSampler sampler; + **m_inputImageViews[quadNdx], // VkImageView imageView; + VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL // VkImageLayout imageLayout; }; VkWriteDescriptorSet writeDescriptorSet = diff --git a/framework/delibs/decpp/deDefs.hpp b/framework/delibs/decpp/deDefs.hpp index 895b489..62704f0 100644 --- a/framework/delibs/decpp/deDefs.hpp +++ b/framework/delibs/decpp/deDefs.hpp @@ -29,6 +29,8 @@ # error "C++ is required" #endif +#include + namespace de { @@ -68,6 +70,34 @@ template inline T roundUp (T x, T y) { DE_ASSERT(y != T(0)); cons //! Round x down to a multiple of y. template inline T roundDown (T x, T y) { DE_ASSERT(y != T(0)); return (x / y) * y; } +//! Find the greatest common divisor of x and y. +template +T gcd (T x, T y) +{ + DE_ASSERT(std::is_integral::value && std::is_unsigned::value); + + // Euclidean algorithm. + while (y != T{0}) + { + T mod = x % y; + x = y; + y = mod; + } + + return x; +} + +//! Find the least common multiple of x and y. +template +T lcm (T x, T y) +{ + DE_ASSERT(std::is_integral::value && std::is_unsigned::value); + + T prod = x * y; + DE_ASSERT(x == 0 || prod / x == y); // Check overflow just in case. + return (prod) / gcd(x, y); +} + //! Helper for DE_CHECK() macros. void throwRuntimeError (const char* message, const char* expr, const char* file, int line); -- 2.7.4