Fix buffer offsets in push descriptor tests
authorRicardo Garcia <rgarcia@igalia.com>
Thu, 26 Nov 2020 16:35:07 +0000 (17:35 +0100)
committerAlexander Galazin <Alexander.Galazin@arm.com>
Mon, 7 Dec 2020 14:43:06 +0000 (09:43 -0500)
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
(cherry picked from commit 0f25191c03f324762437615d961bad978b9d5e15)

external/vulkancts/modules/vulkan/pipeline/vktPipelinePushDescriptorTests.cpp
framework/delibs/decpp/deDefs.hpp

index 9b7d554..bfeb035 100644 (file)
@@ -67,6 +67,8 @@ typedef de::SharedPtr<Unique<VkRenderPass> >  VkRenderPassSp;
 typedef de::SharedPtr<Unique<VkFramebuffer> >  VkFramebufferSp;
 typedef de::SharedPtr<Unique<VkPipeline> >             VkPipelineSp;
 
+constexpr VkDeviceSize kSizeofVec4 = static_cast<VkDeviceSize>(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<string>& 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<size_t>(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<VkDevice>          m_device;
        const DeviceDriver                      m_vkd;
        const VkQueue                           m_queue;
+       const VkDeviceSize                      m_itemSize;
        SimpleAllocator                         m_allocator;
        Move<VkShaderModule>            m_computeShaderModule;
        vector<VkBufferSp>                      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<size_t>(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<const char*>(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<size_t>(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<size_t>(kSizeofVec4));
                flushAlloc(m_vkd, *m_device, *m_bufferAllocs[bufIdx]);
        }
 
@@ -3131,6 +3144,7 @@ private:
        const Unique<VkDevice>          m_device;
        const DeviceDriver                      m_vkd;
        const VkQueue                           m_queue;
+       const VkDeviceSize                      m_itemSize;
        SimpleAllocator                         m_allocator;
        vector<VkBufferSp>                      m_buffers;
        vector<AllocationSp>            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<size_t>(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<const char*>(m_outputBufferAlloc->getHostPtr());
+       for (deUint32 i = 0; i < m_params.numCalls; ++i)
+       {
+               tcu::Vec4 bufferColor;
+               deMemcpy(&bufferColor, bufferPtr + (i * m_itemSize), static_cast<size_t>(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      =
index 895b489..62704f0 100644 (file)
@@ -29,6 +29,8 @@
 #      error "C++ is required"
 #endif
 
+#include <type_traits>
+
 namespace de
 {
 
@@ -68,6 +70,34 @@ template<typename T> inline T                roundUp                 (T x, T y)      { DE_ASSERT(y != T(0)); cons
 //! Round x down to a multiple of y.
 template<typename T> 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<typename T>
+T gcd (T x, T y)
+{
+       DE_ASSERT(std::is_integral<T>::value && std::is_unsigned<T>::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<typename T>
+T lcm (T x, T y)
+{
+       DE_ASSERT(std::is_integral<T>::value && std::is_unsigned<T>::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);