Fixes for robust buffer with variable pointer tests

This commit fixes several issues with robust buffer access with variable
pointers tests:

* The variable pointer tests were checking support for the
  variablePointersStorageBuffer feature but they weren't actually
  activating the feature when creating the robust buffer access device.

* Support checks were being done at instance creation time instead of
  using the proper checkSupport method.

* In-bound checks for 64-bit format accesses were only checking 4 out of
  every 8 bytes in the buffer.

* The verification routines were not taking into account 64-bit
  nonatomic accesses could be legally split into two 32-bit accesses
  that could be checked independently.

* The possibility of satisfying a (0,0,0,x) pattern in output data was
  not being checked correctly for 64-bit formats, always using 32-bit
  vectors and values.

Affected tests:
dEQP-VK.robustness.buffer_access.*
dEQP-VK.robustness.vertex_access.*

Components: Vulkan
VK-GL-CTS issue: 2908

Change-Id: I85aeb085cb31d63b70acec08551c53fcc7d2be3c

diff --git a/external/vulkancts/modules/vulkan/robustness/vktRobustBufferAccessWithVariablePointersTests.cpp b/external/vulkancts/modules/vulkan/robustness/vktRobustBufferAccessWithVariablePointersTests.cpp
index 52ff86d..291ba37 100644 (file)
--- a/external/vulkancts/modules/vulkan/robustness/vktRobustBufferAccessWithVariablePointersTests.cpp
+++ b/external/vulkancts/modules/vulkan/robustness/vktRobustBufferAccessWithVariablePointersTests.cpp
@@ -63,29 +63,17 @@ using namespace vk;
 namespace
 {
 
-// A function for getting information on variable pointer features supported through physical device
-vk::VkPhysicalDeviceVariablePointersFeatures querySupportedVariablePointersFeatures (const Context& context)
+// Creates a custom device with robust buffer access and variable pointer features.
+Move<VkDevice> createRobustBufferAccessVariablePointersDevice (Context& context)
 {
-       VkPhysicalDeviceVariablePointersFeatures extensionFeatures =
-       {
-               VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_VARIABLE_POINTER_FEATURES_KHR,        // sType
-               DE_NULL,                                                                                                                        // pNext
-               false,                                                                                                                          // variablePointersStorageBuffer
-               false,                                                                                                                          // variablePointers
-       };
+       auto pointerFeatures = context.getVariablePointersFeatures();
 
-       VkPhysicalDeviceFeatures2       features;
-       deMemset(&features, 0, sizeof(features));
-       features.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2;
-       features.pNext = &extensionFeatures;
-
-       // Call the getter only if supported. Otherwise above "zero" defaults are used
-       if (context.isInstanceFunctionalitySupported("VK_KHR_get_physical_device_properties2"))
-       {
-               context.getInstanceInterface().getPhysicalDeviceFeatures2(context.getPhysicalDevice(), &features);
-       }
+       VkPhysicalDeviceFeatures2 features2 = initVulkanStructure();
+       features2.features = context.getDeviceFeatures();
+       features2.features.robustBufferAccess = VK_TRUE;
+       features2.pNext = &pointerFeatures;
 
-       return extensionFeatures;
+       return createRobustBufferAccessDevice(context, &features2);
 }
 
 // A supplementary structures that can hold information about buffer size
@@ -201,6 +189,8 @@ public:
        {
        }
 
+       void                                            checkSupport (Context &context) const override;
+
 protected:
        const VkShaderStageFlags        m_shaderStage;
        const ShaderType                        m_shaderType;
@@ -224,6 +214,13 @@ RobustAccessWithPointersTest::RobustAccessWithPointersTest(tcu::TestContext&               te
        DE_ASSERT(m_shaderStage == VK_SHADER_STAGE_VERTEX_BIT || m_shaderStage == VK_SHADER_STAGE_FRAGMENT_BIT || m_shaderStage == VK_SHADER_STAGE_COMPUTE_BIT);
 }
 
+void RobustAccessWithPointersTest::checkSupport (Context &context) const
+{
+       const auto& pointerFeatures = context.getVariablePointersFeatures();
+       if (!pointerFeatures.variablePointersStorageBuffer)
+               TCU_THROW(NotSupportedError, "VariablePointersStorageBuffer SPIR-V capability not supported");
+}
+
 // A subclass for testing reading with variable pointers
 class RobustReadTest : public RobustAccessWithPointersTest
 {
@@ -309,7 +306,7 @@ public:
 
        virtual tcu::TestStatus         iterate                                         (void);
 
-       virtual bool                            verifyResult                            (void);
+       virtual bool                            verifyResult                            (bool splitAccess = false);
 
 private:
        bool                                            isExpectedValueFromInBuffer     (VkDeviceSize           offsetInBytes,
@@ -1286,13 +1283,7 @@ RobustReadTest::RobustReadTest (tcu::TestContext&                testContext,
 
 TestInstance* RobustReadTest::createInstance (Context& context) const
 {
-       VkPhysicalDeviceVariablePointersFeatures pointerFeatures = querySupportedVariablePointersFeatures(context);
-
-       if (pointerFeatures.variablePointersStorageBuffer != DE_TRUE)
-               return new NotSupportedInstance(context, std::string("VariablePointersStorageBuffer support is required for this test."));
-
-       // We need a device with enabled robust buffer access feature (it is disabled in default device)
-       Move<VkDevice>  device = createRobustBufferAccessDevice(context);
+       auto device = createRobustBufferAccessVariablePointersDevice(context);
        return new ReadInstance(context, device, m_shaderType, m_shaderStage, m_bufferFormat, m_readAccessRange, m_accessOutOfBackingMemory);
 }
 
@@ -1326,12 +1317,7 @@ RobustWriteTest::RobustWriteTest (tcu::TestContext&              testContext,
 
 TestInstance* RobustWriteTest::createInstance (Context& context) const
 {
-       VkPhysicalDeviceVariablePointersFeatures pointerFeatures = querySupportedVariablePointersFeatures(context);
-       if (pointerFeatures.variablePointersStorageBuffer != DE_TRUE)
-               return new NotSupportedInstance(context, std::string("VariablePointersStorageBuffer support is required for this test."));
-
-       // We need a device with enabled robust buffer access feature (it is disabled in default device)
-       Move<VkDevice>  device = createRobustBufferAccessDevice(context);
+       auto device = createRobustBufferAccessVariablePointersDevice(context);
        return new WriteInstance(context, device, m_shaderType, m_shaderStage, m_bufferFormat, m_writeAccessRange, m_accessOutOfBackingMemory);
 }
 
@@ -1519,25 +1505,29 @@ AccessInstance::AccessInstance (Context&                        context,
 
 // Verifies if the buffer has the value initialized by BufferAccessInstance::populateReadBuffer at a given offset.
 bool AccessInstance::isExpectedValueFromInBuffer (VkDeviceSize offsetInBytes,
-                                                                                                 const void*           valuePtr,
+                                                                                                 const void*   valuePtr,
                                                                                                  VkDeviceSize  valueSize)
 {
        DE_ASSERT(offsetInBytes % 4 == 0);
        DE_ASSERT(offsetInBytes < m_inBufferAccess.allocSize);
+       DE_ASSERT(valueSize == 4ull || valueSize == 8ull);
 
        const deUint32 valueIndex = deUint32(offsetInBytes / 4) + 2;
 
        if (isUintFormat(m_bufferFormat))
        {
-               return !deMemCmp(valuePtr, &valueIndex, (size_t)valueSize);
+               const deUint32 expectedValues[2] = { valueIndex, valueIndex + 1u };
+               return !deMemCmp(valuePtr, &expectedValues, (size_t)valueSize);
        }
        else if (isIntFormat(m_bufferFormat))
        {
-               const deInt32 value = -deInt32(valueIndex);
-               return !deMemCmp(valuePtr, &value, (size_t)valueSize);
+               const deInt32 value                             = -deInt32(valueIndex);
+               const deInt32 expectedValues[2] = { value, value - 1 };
+               return !deMemCmp(valuePtr, &expectedValues, (size_t)valueSize);
        }
        else if (isFloatFormat(m_bufferFormat))
        {
+               DE_ASSERT(valueSize == 4ull);
                const float value = float(valueIndex);
                return !deMemCmp(valuePtr, &value, (size_t)valueSize);
        }
@@ -1602,7 +1592,7 @@ tcu::TestStatus AccessInstance::iterate (void)
                return tcu::TestStatus::fail("Invalid value(s) found");
 }
 
-bool AccessInstance::verifyResult (void)
+bool AccessInstance::verifyResult (bool splitAccess)
 {
        std::ostringstream      logMsg;
        tcu::TestLog&           log                                     = m_context.getTestContext().getLog();
@@ -1613,7 +1603,8 @@ bool AccessInstance::verifyResult (void)
        deUint32                        valueNdx                        = 0;
        const VkDeviceSize      maxAccessRange          = isReadAccess ? m_inBufferAccess.maxAccessRange : m_outBufferAccess.maxAccessRange;
        const bool                      isR64                           = (m_bufferFormat == VK_FORMAT_R64_UINT || m_bufferFormat == VK_FORMAT_R64_SINT);
-       const deUint32          elementSize                     = isR64 ? 8 : 4;
+       const deUint32          unsplitElementSize      = (isR64 ? 8u : 4u);
+       const deUint32          elementSize                     = ((isR64 && !splitAccess) ? 8u : 4u);
 
        for (VkDeviceSize offsetInBytes = 0; offsetInBytes < m_outBufferAccess.allocSize; offsetInBytes += elementSize)
        {
@@ -1652,15 +1643,15 @@ bool AccessInstance::verifyResult (void)
                                switch (m_shaderType)
                                {
                                        case SHADER_TYPE_SCALAR_COPY:
-                                               operandSize             = elementSize; // Size of scalar
+                                               operandSize             = unsplitElementSize; // Size of scalar
                                                break;
 
                                        case SHADER_TYPE_VECTOR_COPY:
-                                               operandSize             = elementSize * 4; // Size of vec4
+                                               operandSize             = unsplitElementSize * 4; // Size of vec4
                                                break;
 
                                        case SHADER_TYPE_MATRIX_COPY:
-                                               operandSize             = elementSize * 16; // Size of mat4
+                                               operandSize             = unsplitElementSize * 16; // Size of mat4
                                                break;
 
                                        default:
@@ -1724,7 +1715,7 @@ bool AccessInstance::verifyResult (void)
                                        }
                                }
 
-                               if (!isValidValue)
+                               if (!isValidValue && !splitAccess)
                                {
                                        // Check if we are satisfying the [0, 0, 0, x] pattern, where x may be either 0 or 1,
                                        // or the maximum representable positive integer value (if the format is integer-based).
@@ -1732,12 +1723,12 @@ bool AccessInstance::verifyResult (void)
                                        const bool      canMatchVec4Pattern     = (isReadAccess
                                                                                                        && !isValuePartiallyOutOfBounds
                                                                                                        && (m_shaderType == SHADER_TYPE_VECTOR_COPY)
-                                                                                                       && (offsetInBytes / 4 + 1) % 4 == 0);
+                                                                                                       && (offsetInBytes / elementSize + 1) % 4 == 0);
                                        bool            matchesVec4Pattern      = false;
 
                                        if (canMatchVec4Pattern)
                                        {
-                                               matchesVec4Pattern      = verifyOutOfBoundsVec4(static_cast<const deUint32*>(static_cast<const void*>(outValuePtr)) - 3, m_bufferFormat);
+                                               matchesVec4Pattern = verifyOutOfBoundsVec4(outValuePtr - 3u * elementSize, m_bufferFormat);
                                        }
 
                                        if (!canMatchVec4Pattern || !matchesVec4Pattern)
@@ -1764,7 +1755,7 @@ bool AccessInstance::verifyResult (void)
                        {
                                if (isReadAccess)
                                {
-                                       if (!isExpectedValueFromInBuffer(offsetInBytes, outValuePtr, 4))
+                                       if (!isExpectedValueFromInBuffer(offsetInBytes, outValuePtr, elementSize))
                                        {
                                                logMsg << ", Failed: unexpected value";
                                                allOk = false;
@@ -1773,7 +1764,7 @@ bool AccessInstance::verifyResult (void)
                                else
                                {
                                        // Out of bounds writes may change values within the bounds.
-                                       if (!isValueWithinBufferOrZero(inDataPtr, m_inBufferAccess.accessRange, outValuePtr, 4))
+                                       if (!isValueWithinBufferOrZero(inDataPtr, m_inBufferAccess.accessRange, outValuePtr, elementSize))
                                        {
                                                logMsg << ", Failed: unexpected value";
                                                allOk = false;
@@ -1785,6 +1776,12 @@ bool AccessInstance::verifyResult (void)
 
        log << tcu::TestLog::Message << logMsg.str() << tcu::TestLog::EndMessage;
 
+       if (!allOk && unsplitElementSize > 4u && !splitAccess)
+       {
+               // "Non-atomic accesses to storage buffers that are a multiple of 32 bits may be decomposed into 32-bit accesses that are individually bounds-checked."
+               return verifyResult(true/*splitAccess*/);
+       }
+
        return allOk;
 }
 

diff --git a/external/vulkancts/modules/vulkan/robustness/vktRobustnessUtil.cpp b/external/vulkancts/modules/vulkan/robustness/vktRobustnessUtil.cpp
index 0923247..40840a4 100644 (file)
--- a/external/vulkancts/modules/vulkan/robustness/vktRobustnessUtil.cpp
+++ b/external/vulkancts/modules/vulkan/robustness/vktRobustnessUtil.cpp
@@ -151,25 +151,32 @@ bool isValueWithinBufferOrZero (const void* buffer, VkDeviceSize bufferSize, con
        return isValueWithinBuffer(buffer, bufferSize, valuePtr, valueSizeInBytes) || isValueZero(valuePtr, valueSizeInBytes);
 }
 
+template<typename T>
+bool verifyVec4IntegerValues (const void* vecPtr)
+{
+       const T Tzero   = T{0};
+       const T Tone    = T{1};
+       const T Tmax    = std::numeric_limits<T>::max();
+
+       T values[4];
+       deMemcpy(values, vecPtr, 4*sizeof(T));
+       return (values[0] == Tzero && values[1] == Tzero && values[2] == Tzero &&
+                   (values[3] == Tzero || values[3] == Tone || values[3] == Tmax));
+}
+
 bool verifyOutOfBoundsVec4 (const void* vecPtr, VkFormat bufferFormat)
 {
        if (isUintFormat(bufferFormat))
        {
-               const deUint32* data = (deUint32*)vecPtr;
-
-               return data[0] == 0u
-                       && data[1] == 0u
-                       && data[2] == 0u
-                       && (data[3] == 0u || data[3] == 1u || data[3] == std::numeric_limits<deUint32>::max());
+               if (bufferFormat == VK_FORMAT_R64_UINT)
+                       return verifyVec4IntegerValues<deUint64>(vecPtr);
+               return verifyVec4IntegerValues<deUint32>(vecPtr);
        }
        else if (isIntFormat(bufferFormat))
        {
-               const deInt32* data = (deInt32*)vecPtr;
-
-               return data[0] == 0
-                       && data[1] == 0
-                       && data[2] == 0
-                       && (data[3] == 0 || data[3] == 1 || data[3] == std::numeric_limits<deInt32>::max());
+               if (bufferFormat == VK_FORMAT_R64_SINT)
+                       return verifyVec4IntegerValues<deInt64>(vecPtr);
+               return verifyVec4IntegerValues<deInt32>(vecPtr);
        }
        else if (isFloatFormat(bufferFormat))
        {