From 73dd3b9af875dae322b91b00b021fa3582cb800c Mon Sep 17 00:00:00 2001 From: Ahmed Hesham <117350656+ahesham-arm@users.noreply.github.com> Date: Tue, 28 Jan 2025 18:08:24 +0000 Subject: [PATCH] Fix errors in `test_vulkan` (#2183) This fixes three problems in `test_vulkan`: 1. One negative test is violating the OpenCL specification. A call to `clEnqueue{Wait,Signal}SemaphoresKHR` with an invalid semaphore should return `CL_INVALID_SEMAPHORE_KHR` and not `CL_INVALID_VALUE`. > [CL_INVALID_SEMAPHORE_KHR](https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_API.html#CL_INVALID_SEMAPHORE_KHR) if any of the semaphore objects specified by sema_objects is not valid. 2. When populating the list of supported external memory handle types for Vulkan, the types are unconditionally added to the list, without checking if the device supports it or not, this fix is namely for `VULKAN_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD`. 3. If a device does not support an optional extension (that is required for a test), the test should skip, not throw an exception and fail. A test failure should be reserved for the cases where a device claims support for an extension but then fails to execute the test correctly. --------- Signed-off-by: Ahmed Hesham --- test_common/harness/errorHelpers.cpp | 1 + .../common/vulkan_wrapper/vulkan_api_list.hpp | 5 ++- .../common/vulkan_wrapper/vulkan_utility.cpp | 22 ++++++++-- .../common/vulkan_wrapper/vulkan_utility.hpp | 3 +- .../vulkan/test_vulkan_api_consistency.cpp | 40 ++++++++++--------- ...st_vulkan_api_consistency_for_1dimages.cpp | 9 +++-- ...st_vulkan_api_consistency_for_3dimages.cpp | 9 +++-- .../vulkan/test_vulkan_interop_buffer.cpp | 12 ++++-- .../vulkan/test_vulkan_interop_image.cpp | 7 +++- 9 files changed, 70 insertions(+), 38 deletions(-) diff --git a/test_common/harness/errorHelpers.cpp b/test_common/harness/errorHelpers.cpp index 64c730b3..63bd107c 100644 --- a/test_common/harness/errorHelpers.cpp +++ b/test_common/harness/errorHelpers.cpp @@ -111,6 +111,7 @@ const char *IGetErrorString(int clErrorCode) return "CL_INVALID_SYNC_POINT_WAIT_LIST_KHR"; case CL_INVALID_COMMAND_BUFFER_KHR: return "CL_INVALID_COMMAND_BUFFER_KHR"; + case CL_INVALID_SEMAPHORE_KHR: return "CL_INVALID_SEMAPHORE_KHR"; default: return "(unknown)"; } } diff --git a/test_conformance/common/vulkan_wrapper/vulkan_api_list.hpp b/test_conformance/common/vulkan_wrapper/vulkan_api_list.hpp index ab4acb43..f5e7437a 100644 --- a/test_conformance/common/vulkan_wrapper/vulkan_api_list.hpp +++ b/test_conformance/common/vulkan_wrapper/vulkan_api_list.hpp @@ -101,7 +101,8 @@ VK_FUNC_DECL(vkGetPhysicalDeviceSurfaceSupportKHR) \ VK_FUNC_DECL(vkImportSemaphoreFdKHR) \ VK_FUNC_DECL(vkGetPhysicalDeviceExternalSemaphorePropertiesKHR) \ - VK_FUNC_DECL(vkGetImageSubresourceLayout) + VK_FUNC_DECL(vkGetImageSubresourceLayout) \ + VK_FUNC_DECL(vkGetPhysicalDeviceExternalBufferProperties) #define VK_WINDOWS_FUNC_LIST \ VK_FUNC_DECL(vkGetMemoryWin32HandleKHR) \ VK_FUNC_DECL(vkGetSemaphoreWin32HandleKHR) \ @@ -202,5 +203,7 @@ #define vkGetSemaphoreWin32HandleKHR _vkGetSemaphoreWin32HandleKHR #define vkImportSemaphoreWin32HandleKHR _vkImportSemaphoreWin32HandleKHR #define vkGetImageSubresourceLayout _vkGetImageSubresourceLayout +#define vkGetPhysicalDeviceExternalBufferProperties \ + _vkGetPhysicalDeviceExternalBufferProperties #endif //_vulkan_api_list_hpp_ diff --git a/test_conformance/common/vulkan_wrapper/vulkan_utility.cpp b/test_conformance/common/vulkan_wrapper/vulkan_utility.cpp index e4796bc9..6391d36b 100644 --- a/test_conformance/common/vulkan_wrapper/vulkan_utility.cpp +++ b/test_conformance/common/vulkan_wrapper/vulkan_utility.cpp @@ -225,7 +225,8 @@ getDefaultVulkanQueueFamilyToQueueCountMap() } const std::vector -getSupportedVulkanExternalMemoryHandleTypeList() +getSupportedVulkanExternalMemoryHandleTypeList( + const VulkanPhysicalDevice &physical_device) { std::vector externalMemoryHandleTypeList; @@ -238,8 +239,23 @@ getSupportedVulkanExternalMemoryHandleTypeList() externalMemoryHandleTypeList.push_back( VULKAN_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_WIN32_KMT); #else - externalMemoryHandleTypeList.push_back( - VULKAN_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD); + VkPhysicalDeviceExternalBufferInfo buffer_info = {}; + buffer_info.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_EXTERNAL_BUFFER_INFO; + buffer_info.handleType = VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT_KHR; + + VkExternalBufferProperties buffer_properties = {}; + buffer_properties.sType = VK_STRUCTURE_TYPE_EXTERNAL_BUFFER_PROPERTIES; + + vkGetPhysicalDeviceExternalBufferProperties(physical_device, &buffer_info, + &buffer_properties); + + if (buffer_properties.externalMemoryProperties.externalMemoryFeatures + & VK_EXTERNAL_MEMORY_FEATURE_EXPORTABLE_BIT) + { + + externalMemoryHandleTypeList.push_back( + VULKAN_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD); + } #endif return externalMemoryHandleTypeList; diff --git a/test_conformance/common/vulkan_wrapper/vulkan_utility.hpp b/test_conformance/common/vulkan_wrapper/vulkan_utility.hpp index 486ad97c..85028662 100644 --- a/test_conformance/common/vulkan_wrapper/vulkan_utility.hpp +++ b/test_conformance/common/vulkan_wrapper/vulkan_utility.hpp @@ -47,7 +47,8 @@ const VulkanDescriptorSetLayoutList& getEmptyVulkanDescriptorSetLayoutList(); const VulkanQueueFamilyToQueueCountMap& getDefaultVulkanQueueFamilyToQueueCountMap(); const std::vector -getSupportedVulkanExternalMemoryHandleTypeList(); +getSupportedVulkanExternalMemoryHandleTypeList( + const VulkanPhysicalDevice& physical_device); const std::vector getSupportedVulkanExternalSemaphoreHandleTypeList(const VulkanDevice& vkDevice); std::vector diff --git a/test_conformance/vulkan/test_vulkan_api_consistency.cpp b/test_conformance/vulkan/test_vulkan_api_consistency.cpp index b27a3c74..f3ce4a79 100644 --- a/test_conformance/vulkan/test_vulkan_api_consistency.cpp +++ b/test_conformance/vulkan/test_vulkan_api_consistency.cpp @@ -61,14 +61,15 @@ struct ConsistencyExternalBufferTest : public VulkanTestBase #else if (!is_extension_available(device, "cl_khr_external_memory_opaque_fd")) { - throw std::runtime_error( - "Device does not support " - "cl_khr_external_memory_opaque_fd extension \n"); + log_info("Device does not support " + "cl_khr_external_memory_opaque_fd extension \n"); + return TEST_SKIPPED_ITSELF; } #endif VulkanExternalMemoryHandleType vkExternalMemoryHandleType = - getSupportedVulkanExternalMemoryHandleTypeList()[0]; + getSupportedVulkanExternalMemoryHandleTypeList( + vkDevice->getPhysicalDevice())[0]; VulkanBuffer vkDummyBuffer(*vkDevice, 4 * 1024, vkExternalMemoryHandleType); @@ -200,9 +201,9 @@ struct ConsistencyExternalImageTest : public VulkanTestBase #else if (!is_extension_available(device, "cl_khr_external_memory_opaque_fd")) { - test_fail( - "Device does not support cl_khr_external_memory_opaque_fd " - "extension \n"); + log_info("Device does not support cl_khr_external_memory_opaque_fd " + "extension \n"); + return TEST_SKIPPED_ITSELF; } #endif uint32_t width = 256; @@ -212,7 +213,8 @@ struct ConsistencyExternalImageTest : public VulkanTestBase cl_image_format img_format = { 0 }; VulkanExternalMemoryHandleType vkExternalMemoryHandleType = - getSupportedVulkanExternalMemoryHandleTypeList()[0]; + getSupportedVulkanExternalMemoryHandleTypeList( + vkDevice->getPhysicalDevice())[0]; VulkanImageTiling vulkanImageTiling = vkClExternalMemoryHandleTilingAssumption( @@ -355,9 +357,9 @@ struct ConsistencyExternalSemaphoreTest : public VulkanTestBase #else if (!is_extension_available(device, "cl_khr_external_memory_opaque_fd")) { - test_fail( - "Device does not support cl_khr_external_memory_opaque_fd " - "extension \n"); + log_info("Device does not support cl_khr_external_memory_opaque_fd " + "extension \n"); + return TEST_SKIPPED_ITSELF; } #endif @@ -484,18 +486,18 @@ struct ConsistencyExternalSemaphoreTest : public VulkanTestBase // Pass invalid semaphore object to wait errNum = clEnqueueWaitSemaphoresKHRptr(queue, 1, NULL, NULL, 0, NULL, NULL); - test_failure_error( - errNum, CL_INVALID_VALUE, - "clEnqueueWaitSemaphoresKHR fails with CL_INVALID_VALUE " - "when invalid semaphore object is passed"); + test_failure_error(errNum, CL_INVALID_SEMAPHORE_KHR, + "clEnqueueWaitSemaphoresKHR fails with " + "CL_INVALID_SEMAPHORE_KHR " + "when invalid semaphore object is passed"); // Pass invalid semaphore object to signal errNum = clEnqueueSignalSemaphoresKHRptr(queue, 1, NULL, NULL, 0, NULL, NULL); - test_failure_error( - errNum, CL_INVALID_VALUE, - "clEnqueueSignalSemaphoresKHR fails with CL_INVALID_VALUE" - "when invalid semaphore object is passed"); + test_failure_error(errNum, CL_INVALID_SEMAPHORE_KHR, + "clEnqueueSignalSemaphoresKHR fails with " + "CL_INVALID_SEMAPHORE_KHR" + "when invalid semaphore object is passed"); // Create two semaphore objects clVk2Clsemaphore = clCreateSemaphoreWithPropertiesKHRptr( diff --git a/test_conformance/vulkan/test_vulkan_api_consistency_for_1dimages.cpp b/test_conformance/vulkan/test_vulkan_api_consistency_for_1dimages.cpp index 799a73f0..346a3ce5 100644 --- a/test_conformance/vulkan/test_vulkan_api_consistency_for_1dimages.cpp +++ b/test_conformance/vulkan/test_vulkan_api_consistency_for_1dimages.cpp @@ -59,9 +59,9 @@ struct ConsistencyExternalImage1DTest : public VulkanTestBase #else if (!is_extension_available(device, "cl_khr_external_memory_opaque_fd")) { - throw std::runtime_error( - "Device does not support cl_khr_external_memory_opaque_fd " - "extension \n"); + log_info("Device does not support " + "cl_khr_external_memory_opaque_fd extension \n"); + return TEST_SKIPPED_ITSELF; } #endif uint32_t width = 256; @@ -70,7 +70,8 @@ struct ConsistencyExternalImage1DTest : public VulkanTestBase cl_image_format img_format = { 0 }; VulkanExternalMemoryHandleType vkExternalMemoryHandleType = - getSupportedVulkanExternalMemoryHandleTypeList()[0]; + getSupportedVulkanExternalMemoryHandleTypeList( + vkDevice->getPhysicalDevice())[0]; VulkanImageTiling vulkanImageTiling = vkClExternalMemoryHandleTilingAssumption( diff --git a/test_conformance/vulkan/test_vulkan_api_consistency_for_3dimages.cpp b/test_conformance/vulkan/test_vulkan_api_consistency_for_3dimages.cpp index b30f3747..8dd9ae9a 100644 --- a/test_conformance/vulkan/test_vulkan_api_consistency_for_3dimages.cpp +++ b/test_conformance/vulkan/test_vulkan_api_consistency_for_3dimages.cpp @@ -60,9 +60,9 @@ struct ConsistencyExternalImage3DTest : public VulkanTestBase #else if (!is_extension_available(device, "cl_khr_external_memory_opaque_fd")) { - throw std::runtime_error( - "Device does not support cl_khr_external_memory_opaque_fd " - "extension \n"); + log_info("Device does not support " + "cl_khr_external_memory_opaque_fd extension \n"); + return TEST_SKIPPED_ITSELF; } #endif uint32_t width = 256; @@ -73,7 +73,8 @@ struct ConsistencyExternalImage3DTest : public VulkanTestBase cl_image_format img_format = { 0 }; VulkanExternalMemoryHandleType vkExternalMemoryHandleType = - getSupportedVulkanExternalMemoryHandleTypeList()[0]; + getSupportedVulkanExternalMemoryHandleTypeList( + vkDevice->getPhysicalDevice())[0]; VulkanImageTiling vulkanImageTiling = vkClExternalMemoryHandleTilingAssumption( diff --git a/test_conformance/vulkan/test_vulkan_interop_buffer.cpp b/test_conformance/vulkan/test_vulkan_interop_buffer.cpp index e5f1a728..94078031 100644 --- a/test_conformance/vulkan/test_vulkan_interop_buffer.cpp +++ b/test_conformance/vulkan/test_vulkan_interop_buffer.cpp @@ -113,7 +113,8 @@ int run_test_with_two_queue( const std::vector vkExternalMemoryHandleTypeList = - getSupportedVulkanExternalMemoryHandleTypeList(); + getSupportedVulkanExternalMemoryHandleTypeList( + vkDevice.getPhysicalDevice()); VulkanSemaphore vkVk2CLSemaphore(vkDevice, vkExternalSemaphoreHandleType); VulkanSemaphore vkCl2VkSemaphore(vkDevice, vkExternalSemaphoreHandleType); std::shared_ptr fence = nullptr; @@ -447,7 +448,8 @@ int run_test_with_one_queue( const std::vector vkExternalMemoryHandleTypeList = - getSupportedVulkanExternalMemoryHandleTypeList(); + getSupportedVulkanExternalMemoryHandleTypeList( + vkDevice.getPhysicalDevice()); VulkanSemaphore vkVk2CLSemaphore(vkDevice, vkExternalSemaphoreHandleType); VulkanSemaphore vkCl2VkSemaphore(vkDevice, vkExternalSemaphoreHandleType); std::shared_ptr fence = nullptr; @@ -752,7 +754,8 @@ int run_test_with_multi_import_same_ctx( const std::vector vkExternalMemoryHandleTypeList = - getSupportedVulkanExternalMemoryHandleTypeList(); + getSupportedVulkanExternalMemoryHandleTypeList( + vkDevice.getPhysicalDevice()); VulkanSemaphore vkVk2CLSemaphore(vkDevice, vkExternalSemaphoreHandleType); VulkanSemaphore vkCl2VkSemaphore(vkDevice, vkExternalSemaphoreHandleType); std::shared_ptr fence = nullptr; @@ -1092,7 +1095,8 @@ int run_test_with_multi_import_diff_ctx( const std::vector vkExternalMemoryHandleTypeList = - getSupportedVulkanExternalMemoryHandleTypeList(); + getSupportedVulkanExternalMemoryHandleTypeList( + vkDevice.getPhysicalDevice()); VulkanSemaphore vkVk2CLSemaphore(vkDevice, vkExternalSemaphoreHandleType); VulkanSemaphore vkCl2VkSemaphore(vkDevice, vkExternalSemaphoreHandleType); std::shared_ptr fence = nullptr; diff --git a/test_conformance/vulkan/test_vulkan_interop_image.cpp b/test_conformance/vulkan/test_vulkan_interop_image.cpp index 7808ef64..d529f482 100644 --- a/test_conformance/vulkan/test_vulkan_interop_image.cpp +++ b/test_conformance/vulkan/test_vulkan_interop_image.cpp @@ -208,7 +208,9 @@ int run_test_with_two_queue( std::vector vkFormatList = getSupportedVulkanFormatList(); const std::vector vkExternalMemoryHandleTypeList = - getSupportedVulkanExternalMemoryHandleTypeList(); + getSupportedVulkanExternalMemoryHandleTypeList( + + vkDevice.getPhysicalDevice()); char magicValue = 0; VulkanBuffer vkParamsBuffer(vkDevice, sizeof(Params)); @@ -820,7 +822,8 @@ int run_test_with_one_queue( std::vector vkFormatList = getSupportedVulkanFormatList(); const std::vector vkExternalMemoryHandleTypeList = - getSupportedVulkanExternalMemoryHandleTypeList(); + getSupportedVulkanExternalMemoryHandleTypeList( + vkDevice.getPhysicalDevice()); char magicValue = 0; VulkanBuffer vkParamsBuffer(vkDevice, sizeof(Params));