Vulkan: Properly handle Device extension dependencies

The checks for dependencies of Device extensions were incomplete, makes
sure the transitive dependencies of the extensions we care about are all
known so they can participate in the dependency check.

Also removes a workaround for surprising Vulkan behavior with instance
extensions getting promoted as device functions. This should be handled
correctly now as DeviceExt contains the physical device extensions as
well.

Bug: dawn:457

Change-Id: I4b79729d809c9edfedcb075a0e6aa5b4dd473ab3
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/22942
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>

src/dawn_native/vulkan/BackendVk.cpp

@@ -210,22 +210,18 @@ namespace dawn_native { namespace vulkan {
         // cases are removed.
         InstanceExtSet extensionsToRequest = mGlobalInfo.extensions;
 
-        // TODO(cwallez@chromium.org): don't request extensions that have been promoted to Vulkan
-        // 1.1. This can only happen when we correctly detect and handle VkPhysicalDevice instance
-        // extensions that are promoted to be "device" extensions in the core Vulkan. If we don't
-        // do this there is a crash because a Vulkan 1.1 loader instance will not emulate the call
-        // on a Vulkan 1.0 ICD (and call nullptr).
-        // See https://github.com/KhronosGroup/Vulkan-Loader/issues/412.
-
         if (!GetInstance()->IsBackendValidationEnabled()) {
             extensionsToRequest.Set(InstanceExt::DebugReport, false);
         }
-
         usedKnobs.extensions = extensionsToRequest;
 
         std::vector<const char*> extensionNames;
         for (uint32_t ext : IterateBitSet(extensionsToRequest.extensionBitSet)) {
-            extensionNames.push_back(GetInstanceExtInfo(static_cast<InstanceExt>(ext)).name);
+            const InstanceExtInfo& info = GetInstanceExtInfo(static_cast<InstanceExt>(ext));
+
+            if (info.versionPromoted > mGlobalInfo.apiVersion) {
+                extensionNames.push_back(info.name);
+            }
         }
 
         VkApplicationInfo appInfo;

src/dawn_native/vulkan/VulkanExtensions.cpp

@@ -136,11 +136,23 @@ namespace dawn_native { namespace vulkan {
     static constexpr size_t kDeviceExtCount = static_cast<size_t>(DeviceExt::EnumCount);
     static constexpr std::array<DeviceExtInfo, kDeviceExtCount> sDeviceExtInfos{{
         //
+        {DeviceExt::BindMemory2, "VK_KHR_bind_memory2", VulkanVersion_1_1},
         {DeviceExt::Maintenance1, "VK_KHR_maintenance1", VulkanVersion_1_1},
+        {DeviceExt::StorageBufferStorageClass, "VK_KHR_storage_buffer_storage_class",
+         VulkanVersion_1_1},
+        {DeviceExt::GetPhysicalDeviceProperties2, "VK_KHR_get_physical_device_properties2",
+         VulkanVersion_1_1},
+        {DeviceExt::GetMemoryRequirements2, "VK_KHR_get_memory_requirements2", VulkanVersion_1_1},
+        {DeviceExt::ExternalMemoryCapabilities, "VK_KHR_external_memory_capabilities",
+         VulkanVersion_1_1},
+        {DeviceExt::ExternalSemaphoreCapabilities, "VK_KHR_external_semaphore_capabilities",
+         VulkanVersion_1_1},
         {DeviceExt::ExternalMemory, "VK_KHR_external_memory", VulkanVersion_1_1},
         {DeviceExt::ExternalSemaphore, "VK_KHR_external_semaphore", VulkanVersion_1_1},
         {DeviceExt::_16BitStorage, "VK_KHR_16bit_storage", VulkanVersion_1_1},
+        {DeviceExt::SamplerYCbCrConversion, "VK_KHR_sampler_ycbcr_conversion", VulkanVersion_1_1},
 
+        {DeviceExt::ImageFormatList, "VK_KHR_image_format_list", VulkanVersion_1_2},
         {DeviceExt::ShaderFloat16Int8, "VK_KHR_shader_float16_int8", VulkanVersion_1_2},
 
         {DeviceExt::ExternalMemoryFD, "VK_KHR_external_memory_fd", NeverPromoted},
@@ -195,6 +207,32 @@ namespace dawn_native { namespace vulkan {
 
             bool hasDependencies = false;
             switch (ext) {
+                // Happy extensions don't need anybody else!
+                case DeviceExt::BindMemory2:
+                case DeviceExt::GetMemoryRequirements2:
+                case DeviceExt::Maintenance1:
+                case DeviceExt::ImageFormatList:
+                case DeviceExt::StorageBufferStorageClass:
+                    hasDependencies = true;
+                    break;
+
+                // Physical device extensions technically don't require the instance to support
+                // them but VulkanFunctions only loads the function pointers if the instance
+                // advertises the extension. So if we didn't have this check, we'd risk a calling
+                // a nullptr.
+                case DeviceExt::GetPhysicalDeviceProperties2:
+                    hasDependencies = instanceExts.Has(InstanceExt::GetPhysicalDeviceProperties2);
+                    break;
+                case DeviceExt::ExternalMemoryCapabilities:
+                    hasDependencies = instanceExts.Has(InstanceExt::ExternalMemoryCapabilities) &&
+                                      HasDep(DeviceExt::GetPhysicalDeviceProperties2);
+                    break;
+                case DeviceExt::ExternalSemaphoreCapabilities:
+                    hasDependencies =
+                        instanceExts.Has(InstanceExt::ExternalSemaphoreCapabilities) &&
+                        HasDep(DeviceExt::GetPhysicalDeviceProperties2);
+                    break;
+
                 case DeviceExt::DebugMarker:
                     // TODO(cwallez@chromium.org): VK_KHR_debug_report is deprecated, switch to
                     // using VK_KHR_debug_utils instead.
@@ -202,51 +240,33 @@ namespace dawn_native { namespace vulkan {
                     break;
 
                 case DeviceExt::ImageDrmFormatModifier:
-                    // TODO(cwallez@chromium.org): ImageDrmFormatModifier actually requires:
+                    hasDependencies = HasDep(DeviceExt::BindMemory2) &&
-                    //  - VK_KHR_bind_memory2
+                                      HasDep(DeviceExt::GetPhysicalDeviceProperties2) &&
-                    //  - VK_KHR_image_format_list
+                                      HasDep(DeviceExt::ImageFormatList) &&
-                    //  - VK_KHR_sampler_ycbcr_conversion
+                                      HasDep(DeviceExt::SamplerYCbCrConversion);
-                    //
-                    // Also switch to using DeviceExt::GetPhysicalDeviceProperties2 when we decouple
-                    // Instance / Device physical device extensions.
-                    hasDependencies = instanceExts.Has(InstanceExt::GetPhysicalDeviceProperties2);
                     break;
 
                 case DeviceExt::Swapchain:
                     hasDependencies = instanceExts.Has(InstanceExt::Surface);
                     break;
 
-                case DeviceExt::Maintenance1:
+                case DeviceExt::SamplerYCbCrConversion:
-                    hasDependencies = true;
+                    hasDependencies = HasDep(DeviceExt::Maintenance1) &&
+                                      HasDep(DeviceExt::BindMemory2) &&
+                                      HasDep(DeviceExt::GetMemoryRequirements2) &&
+                                      HasDep(DeviceExt::GetPhysicalDeviceProperties2);
                     break;
 
                 case DeviceExt::ShaderFloat16Int8:
-                    // TODO(cwallez@chromium.org): switch to using
+                    hasDependencies = HasDep(DeviceExt::GetPhysicalDeviceProperties2);
-                    // DeviceExt::GetPhysicalDeviceProperties2 when we decouple Instance / Device
-                    // physical device extensions.
-                    hasDependencies = instanceExts.Has(InstanceExt::GetPhysicalDeviceProperties2);
                     break;
 
                 case DeviceExt::ExternalMemory:
-                    // TODO(cwallez@chromium.org): switch to using
+                    hasDependencies = HasDep(DeviceExt::ExternalMemoryCapabilities);
-                    // DeviceExt::ExternalMemoryCapabilities when we decouple Instance / Device
-                    // physical device extensions.
-                    hasDependencies = instanceExts.Has(InstanceExt::ExternalMemoryCapabilities);
                     break;
 
                 case DeviceExt::ExternalSemaphore:
-                    // TODO(cwallez@chromium.org): switch to using
+                    hasDependencies = HasDep(DeviceExt::ExternalSemaphoreCapabilities);
-                    // DeviceExt::ExternalSemaphoreCapabilities when we decouple Instance / Device
-                    // physical device extensions.
-                    hasDependencies = instanceExts.Has(InstanceExt::ExternalSemaphoreCapabilities);
-                    break;
-
-                case DeviceExt::_16BitStorage:
-                    // TODO(cwallez@chromium.org): switch to using
-                    // DeviceExt::GetPhysicalDeviceProperties2 when we decouple Instance / Device
-                    // physical device extensions.
-                    // Also depends on VK_KHR_storage_buffer_storage_class
-                    hasDependencies = instanceExts.Has(InstanceExt::GetPhysicalDeviceProperties2);
                     break;
 
                 case DeviceExt::ExternalMemoryFD:
@@ -263,6 +283,11 @@ namespace dawn_native { namespace vulkan {
                     hasDependencies = HasDep(DeviceExt::ExternalSemaphore);
                     break;
 
+                case DeviceExt::_16BitStorage:
+                    hasDependencies = HasDep(DeviceExt::GetPhysicalDeviceProperties2) &&
+                                      HasDep(DeviceExt::StorageBufferStorageClass);
+                    break;
+
                 default:
                     UNREACHABLE();
                     break;

src/dawn_native/vulkan/VulkanExtensions.h

@@ -74,12 +74,20 @@ namespace dawn_native { namespace vulkan {
     // inside EnsureDependencies)
     enum class DeviceExt {
         // Promoted to 1.1
+        BindMemory2,
         Maintenance1,
+        StorageBufferStorageClass,
+        GetPhysicalDeviceProperties2,
+        GetMemoryRequirements2,
+        ExternalMemoryCapabilities,
+        ExternalSemaphoreCapabilities,
         ExternalMemory,
         ExternalSemaphore,
         _16BitStorage,
+        SamplerYCbCrConversion,
 
         // Promoted to 1.2
+        ImageFormatList,
         ShaderFloat16Int8,
 
         // External* extensions