From db8f804bc311657c87e99140c95a7d3d4734c48d Mon Sep 17 00:00:00 2001 From: Xinghua Cao Date: Mon, 8 Jun 2020 12:18:21 +0000 Subject: [PATCH] Reland "Check FP16 support on vulkan backend" This reverts commit 0357eed7de15e901d14f6149e91bfe03d86958de and reland commit bdc05c3d5fef780382c3fd5e5ebcb14bd929819a. The Vulkan-Loader has a bug where if the instance is created with Vulkan 1.1 and not the promoted extensions, it will skip emulation and if the ICD doesn't support Vulkan 1.1 nor the extensions. Enable the promoted extensions, even when creating a Vulkan 1.1 instance. Original change's description: > Check FP16 support on vulkan backend > > This patch check FP16 support on vulkan backend, and introduces > the shader_float16 extension. > > BUG=dawn:426 > TEST=dawn_end2end_tests > > Change-Id: Ie09568a416ce9eb2c11afeede3e7da520550d5fb > Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/21901 > Commit-Queue: Xinghua Cao > Reviewed-by: Corentin Wallez > Reviewed-by: Jiawei Shao > Reviewed-by: Kai Ninomiya > Reviewed-by: Austin Eng Bug: chromium:1087896, dawn:426 Change-Id: I2c4465fb2fe957966b44d3e5840112219481c639 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/22781 Commit-Queue: Corentin Wallez Reviewed-by: Corentin Wallez --- dawn.json | 1 + src/dawn_native/Extensions.cpp | 5 + src/dawn_native/Extensions.h | 1 + src/dawn_native/vulkan/AdapterVk.cpp | 7 ++ src/dawn_native/vulkan/BackendVk.cpp | 35 ++++--- src/dawn_native/vulkan/DeviceVk.cpp | 19 ++++ src/dawn_native/vulkan/VulkanInfo.cpp | 34 +++++++ src/dawn_native/vulkan/VulkanInfo.h | 6 ++ src/tests/BUILD.gn | 1 + src/tests/DawnTest.cpp | 1 + src/tests/DawnTest.h | 9 ++ src/tests/end2end/ShaderFloat16Tests.cpp | 111 +++++++++++++++++++++++ 12 files changed, 217 insertions(+), 13 deletions(-) create mode 100644 src/tests/end2end/ShaderFloat16Tests.cpp diff --git a/dawn.json b/dawn.json index d398a4e74e..a9f278bdb4 100644 --- a/dawn.json +++ b/dawn.json @@ -651,6 +651,7 @@ "extensible": false, "members": [ {"name": "texture compression BC", "type": "bool", "default": "false"}, + {"name": "shader float16", "type": "bool", "default": "false"}, {"name": "pipeline statistics query", "type": "bool", "default": "false"}, {"name": "timestamp query", "type": "bool", "default": "false"} ] diff --git a/src/dawn_native/Extensions.cpp b/src/dawn_native/Extensions.cpp index ee3e460f26..d356616558 100644 --- a/src/dawn_native/Extensions.cpp +++ b/src/dawn_native/Extensions.cpp @@ -35,6 +35,11 @@ namespace dawn_native { {"texture_compression_bc", "Support Block Compressed (BC) texture formats", "https://bugs.chromium.org/p/dawn/issues/detail?id=42"}, &WGPUDeviceProperties::textureCompressionBC}, + {Extension::ShaderFloat16, + {"shader_float16", + "Support 16bit float arithmetic and declarations in uniform and storage buffers", + "https://bugs.chromium.org/p/dawn/issues/detail?id=426"}, + &WGPUDeviceProperties::shaderFloat16}, {Extension::PipelineStatisticsQuery, {"pipeline_statistics_query", "Support Pipeline Statistics Query", "https://bugs.chromium.org/p/dawn/issues/detail?id=434"}, diff --git a/src/dawn_native/Extensions.h b/src/dawn_native/Extensions.h index d27116364d..ba32ee153e 100644 --- a/src/dawn_native/Extensions.h +++ b/src/dawn_native/Extensions.h @@ -25,6 +25,7 @@ namespace dawn_native { enum class Extension { TextureCompressionBC, + ShaderFloat16, PipelineStatisticsQuery, TimestampQuery, diff --git a/src/dawn_native/vulkan/AdapterVk.cpp b/src/dawn_native/vulkan/AdapterVk.cpp index 032e3e5d24..c220a8e937 100644 --- a/src/dawn_native/vulkan/AdapterVk.cpp +++ b/src/dawn_native/vulkan/AdapterVk.cpp @@ -74,6 +74,13 @@ namespace dawn_native { namespace vulkan { mSupportedExtensions.EnableExtension(Extension::TextureCompressionBC); } + if (mDeviceInfo.shaderFloat16Int8 && + mDeviceInfo.shaderFloat16Int8Features.shaderFloat16 == VK_TRUE && + mDeviceInfo._16BitStorage && + mDeviceInfo._16BitStorageFeatures.uniformAndStorageBuffer16BitAccess == VK_TRUE) { + mSupportedExtensions.EnableExtension(Extension::ShaderFloat16); + } + if (mDeviceInfo.features.pipelineStatisticsQuery == VK_TRUE) { mSupportedExtensions.EnableExtension(Extension::PipelineStatisticsQuery); } diff --git a/src/dawn_native/vulkan/BackendVk.cpp b/src/dawn_native/vulkan/BackendVk.cpp index fe9eba6dbc..f664cd6404 100644 --- a/src/dawn_native/vulkan/BackendVk.cpp +++ b/src/dawn_native/vulkan/BackendVk.cpp @@ -253,19 +253,28 @@ namespace dawn_native { namespace vulkan { usedKnobs.getPhysicalDeviceProperties2 = true; usedKnobs.externalMemoryCapabilities = true; usedKnobs.externalSemaphoreCapabilities = true; - } else { - if (mGlobalInfo.externalMemoryCapabilities) { - extensionsToRequest.push_back(kExtensionNameKhrExternalMemoryCapabilities); - usedKnobs.externalMemoryCapabilities = true; - } - if (mGlobalInfo.externalSemaphoreCapabilities) { - extensionsToRequest.push_back(kExtensionNameKhrExternalSemaphoreCapabilities); - usedKnobs.externalSemaphoreCapabilities = true; - } - if (mGlobalInfo.getPhysicalDeviceProperties2) { - extensionsToRequest.push_back(kExtensionNameKhrGetPhysicalDeviceProperties2); - usedKnobs.getPhysicalDeviceProperties2 = true; - } + } + + // The Vulkan-Loader has emulation of VkPhysicalDevices functions such as + // vkGetPhysicalDeviceProperties2 when the ICD doesn't support the extension. However the + // loader has a bug where if the instance is created with Vulkan 1.1 and not the promoted + // extensions, it will skip emulation and if the ICD doesn't support Vulkan 1.1 nor the + // extensions, we will crash on nullptr function pointer when the loader tries to call the + // ICD's vkGetPhysicalDeviceProperties2. See + // https://github.com/KhronosGroup/Vulkan-Loader/issues/412. We work around this by + // specifying we want to enable the promoted extensions, even when we create a Vulkan 1.1 + // instance. + if (mGlobalInfo.externalMemoryCapabilities) { + extensionsToRequest.push_back(kExtensionNameKhrExternalMemoryCapabilities); + usedKnobs.externalMemoryCapabilities = true; + } + if (mGlobalInfo.externalSemaphoreCapabilities) { + extensionsToRequest.push_back(kExtensionNameKhrExternalSemaphoreCapabilities); + usedKnobs.externalSemaphoreCapabilities = true; + } + if (mGlobalInfo.getPhysicalDeviceProperties2) { + extensionsToRequest.push_back(kExtensionNameKhrGetPhysicalDeviceProperties2); + usedKnobs.getPhysicalDeviceProperties2 = true; } VkApplicationInfo appInfo; diff --git a/src/dawn_native/vulkan/DeviceVk.cpp b/src/dawn_native/vulkan/DeviceVk.cpp index 469ef23c85..9c56c5efdb 100644 --- a/src/dawn_native/vulkan/DeviceVk.cpp +++ b/src/dawn_native/vulkan/DeviceVk.cpp @@ -328,6 +328,25 @@ namespace dawn_native { namespace vulkan { usedKnobs.features.textureCompressionBC = VK_TRUE; } + if (IsExtensionEnabled(Extension::ShaderFloat16)) { + const VulkanDeviceInfo& deviceInfo = ToBackend(GetAdapter())->GetDeviceInfo(); + ASSERT(deviceInfo.shaderFloat16Int8 && + deviceInfo.shaderFloat16Int8Features.shaderFloat16 == VK_TRUE && + deviceInfo._16BitStorage && + deviceInfo._16BitStorageFeatures.uniformAndStorageBuffer16BitAccess == VK_TRUE); + + usedKnobs.shaderFloat16Int8 = true; + usedKnobs.shaderFloat16Int8Features.shaderFloat16 = VK_TRUE; + extensionsToRequest.push_back(kExtensionNameKhrShaderFloat16Int8); + + usedKnobs._16BitStorage = true; + usedKnobs._16BitStorageFeatures.uniformAndStorageBuffer16BitAccess = VK_TRUE; + // VK_KHR_16bit_storage is promoted to Vulkan 1.1. + if (deviceInfo.properties.apiVersion < VK_MAKE_VERSION(1, 1, 0)) { + extensionsToRequest.push_back(kExtensionNameKhr16BitStorage); + } + } + // Find a universal queue family { // Note that GRAPHICS and COMPUTE imply TRANSFER so we don't need to check for it. diff --git a/src/dawn_native/vulkan/VulkanInfo.cpp b/src/dawn_native/vulkan/VulkanInfo.cpp index 2a3226245b..b2d99289af 100644 --- a/src/dawn_native/vulkan/VulkanInfo.cpp +++ b/src/dawn_native/vulkan/VulkanInfo.cpp @@ -79,6 +79,8 @@ namespace dawn_native { namespace vulkan { const char kExtensionNameKhrXlibSurface[] = "VK_KHR_xlib_surface"; const char kExtensionNameFuchsiaImagePipeSurface[] = "VK_FUCHSIA_imagepipe_surface"; const char kExtensionNameKhrMaintenance1[] = "VK_KHR_maintenance1"; + const char kExtensionNameKhrShaderFloat16Int8[] = "VK_KHR_shader_float16_int8"; + const char kExtensionNameKhr16BitStorage[] = "VK_KHR_16bit_storage"; ResultOrError GatherGlobalInfo(const Backend& backend) { VulkanGlobalInfo info = {}; @@ -221,6 +223,7 @@ namespace dawn_native { namespace vulkan { ResultOrError GatherDeviceInfo(const Adapter& adapter) { VulkanDeviceInfo info = {}; VkPhysicalDevice physicalDevice = adapter.GetPhysicalDevice(); + const VulkanGlobalInfo& globalInfo = adapter.GetBackend()->GetGlobalInfo(); const VulkanFunctions& vkFunctions = adapter.GetBackend()->GetFunctions(); // Gather general info about the device @@ -311,6 +314,23 @@ namespace dawn_native { namespace vulkan { if (IsExtensionName(extension, kExtensionNameKhrMaintenance1)) { info.maintenance1 = true; } + if (IsExtensionName(extension, kExtensionNameKhrShaderFloat16Int8) && + globalInfo.getPhysicalDeviceProperties2) { + info.shaderFloat16Int8 = true; + info.shaderFloat16Int8Features.sType = + VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SHADER_FLOAT16_INT8_FEATURES_KHR; + + VkPhysicalDeviceFeatures2KHR physicalDeviceFeatures2 = {}; + physicalDeviceFeatures2.sType = + VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2_KHR; + physicalDeviceFeatures2.pNext = &info.shaderFloat16Int8Features; + vkFunctions.GetPhysicalDeviceFeatures2(physicalDevice, + &physicalDeviceFeatures2); + } + if (IsExtensionName(extension, kExtensionNameKhr16BitStorage) && + globalInfo.getPhysicalDeviceProperties2) { + info._16BitStorage = true; + } } } @@ -319,6 +339,20 @@ namespace dawn_native { namespace vulkan { info.maintenance1 = true; } + // VK_KHR_16bit_storage is promoted to Vulkan 1.1, so gather information if either is + // present, and mark the extension as available. + if (info._16BitStorage || info.properties.apiVersion >= VK_MAKE_VERSION(1, 1, 0)) { + ASSERT(globalInfo.getPhysicalDeviceProperties2); + info._16BitStorage = true; + info._16BitStorageFeatures.sType = + VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_16BIT_STORAGE_FEATURES; + + VkPhysicalDeviceFeatures2 physicalDeviceFeatures2 = {}; + physicalDeviceFeatures2.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2; + physicalDeviceFeatures2.pNext = &info._16BitStorageFeatures; + vkFunctions.GetPhysicalDeviceFeatures2(physicalDevice, &physicalDeviceFeatures2); + } + // TODO(cwallez@chromium.org): gather info about formats return std::move(info); diff --git a/src/dawn_native/vulkan/VulkanInfo.h b/src/dawn_native/vulkan/VulkanInfo.h index 354d9b38c9..81ef0549da 100644 --- a/src/dawn_native/vulkan/VulkanInfo.h +++ b/src/dawn_native/vulkan/VulkanInfo.h @@ -52,6 +52,8 @@ namespace dawn_native { namespace vulkan { extern const char kExtensionNameKhrXlibSurface[]; extern const char kExtensionNameFuchsiaImagePipeSurface[]; extern const char kExtensionNameKhrMaintenance1[]; + extern const char kExtensionNameKhrShaderFloat16Int8[]; + extern const char kExtensionNameKhr16BitStorage[]; // Global information - gathered before the instance is created struct VulkanGlobalKnobs { @@ -85,6 +87,8 @@ namespace dawn_native { namespace vulkan { // Device information - gathered before the device is created. struct VulkanDeviceKnobs { VkPhysicalDeviceFeatures features; + VkPhysicalDeviceShaderFloat16Int8FeaturesKHR shaderFloat16Int8Features; + VkPhysicalDevice16BitStorageFeaturesKHR _16BitStorageFeatures; // Extensions, promoted extensions are set to true if their core version is supported. bool debugMarker = false; @@ -98,6 +102,8 @@ namespace dawn_native { namespace vulkan { bool externalSemaphoreZirconHandle = false; bool swapchain = false; bool maintenance1 = false; + bool shaderFloat16Int8 = false; + bool _16BitStorage = false; }; struct VulkanDeviceInfo : VulkanDeviceKnobs { diff --git a/src/tests/BUILD.gn b/src/tests/BUILD.gn index 9b9907e411..b55bde4702 100644 --- a/src/tests/BUILD.gn +++ b/src/tests/BUILD.gn @@ -287,6 +287,7 @@ source_set("dawn_end2end_tests_sources") { "end2end/RenderPassTests.cpp", "end2end/SamplerTests.cpp", "end2end/ScissorTests.cpp", + "end2end/ShaderFloat16Tests.cpp", "end2end/StorageTextureTests.cpp", "end2end/SubresourceOutputAttachmentTests.cpp", "end2end/TextureFormatTests.cpp", diff --git a/src/tests/DawnTest.cpp b/src/tests/DawnTest.cpp index 21ddcaa5e6..09848de150 100644 --- a/src/tests/DawnTest.cpp +++ b/src/tests/DawnTest.cpp @@ -1144,6 +1144,7 @@ namespace detail { } template class ExpectEq; + template class ExpectEq; template class ExpectEq; template class ExpectEq; template class ExpectEq; diff --git a/src/tests/DawnTest.h b/src/tests/DawnTest.h index f80197fab3..3fd3f20122 100644 --- a/src/tests/DawnTest.h +++ b/src/tests/DawnTest.h @@ -30,6 +30,14 @@ // until the end of the test. Also expectations use a copy to a MapRead buffer to get the data // so resources should have the CopySrc allowed usage bit if you want to add expectations on // them. +#define EXPECT_BUFFER_U16_EQ(expected, buffer, offset) \ + AddBufferExpectation(__FILE__, __LINE__, buffer, offset, sizeof(uint16_t), \ + new ::detail::ExpectEq(expected)) + +#define EXPECT_BUFFER_U16_RANGE_EQ(expected, buffer, offset, count) \ + AddBufferExpectation(__FILE__, __LINE__, buffer, offset, sizeof(uint16_t) * count, \ + new ::detail::ExpectEq(expected, count)) + #define EXPECT_BUFFER_U32_EQ(expected, buffer, offset) \ AddBufferExpectation(__FILE__, __LINE__, buffer, offset, sizeof(uint32_t), \ new ::detail::ExpectEq(expected)) @@ -418,6 +426,7 @@ namespace detail { std::vector mExpected; }; extern template class ExpectEq; + extern template class ExpectEq; extern template class ExpectEq; extern template class ExpectEq; extern template class ExpectEq; diff --git a/src/tests/end2end/ShaderFloat16Tests.cpp b/src/tests/end2end/ShaderFloat16Tests.cpp new file mode 100644 index 0000000000..57d94beae2 --- /dev/null +++ b/src/tests/end2end/ShaderFloat16Tests.cpp @@ -0,0 +1,111 @@ +// Copyright 2020 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "common/Math.h" +#include "tests/DawnTest.h" + +#include "utils/WGPUHelpers.h" + +class ShaderFloat16Tests : public DawnTest { + protected: + std::vector GetRequiredExtensions() override { + mIsShaderFloat16Supported = SupportsExtensions({"shader_float16"}); + if (!mIsShaderFloat16Supported) { + return {}; + } + + return {"shader_float16"}; + } + + bool IsShaderFloat16Supported() const { + return mIsShaderFloat16Supported; + } + + bool mIsShaderFloat16Supported = false; +}; + +// Test basic 16bit float arithmetic and 16bit storage features. +TEST_P(ShaderFloat16Tests, Basic16BitFloatFeaturesTest) { + DAWN_SKIP_TEST_IF(!IsShaderFloat16Supported()); + + uint16_t uniformData[] = {Float32ToFloat16(1.23), Float32ToFloat16(0.0)}; // 0.0 is a padding. + wgpu::Buffer uniformBuffer = utils::CreateBufferFromData( + device, &uniformData, sizeof(uniformData), wgpu::BufferUsage::Uniform); + + uint16_t bufferInData[] = {Float32ToFloat16(2.34), Float32ToFloat16(0.0)}; // 0.0 is a padding. + wgpu::Buffer bufferIn = utils::CreateBufferFromData(device, &bufferInData, sizeof(bufferInData), + wgpu::BufferUsage::Storage); + + // TODO(xinghua.cao@intel.com): the zero for padding is required now. No need to + // createBufferFromData once buffer lazy-zero-init is done. + uint16_t bufferOutData[] = {Float32ToFloat16(0.0), Float32ToFloat16(0.0)}; + wgpu::Buffer bufferOut = + utils::CreateBufferFromData(device, &bufferOutData, sizeof(bufferOutData), + wgpu::BufferUsage::Storage | wgpu::BufferUsage::CopySrc); + + wgpu::ShaderModule module = + utils::CreateShaderModule(device, utils::SingleShaderStage::Compute, R"( + #version 450 + + #extension GL_AMD_gpu_shader_half_float : require + + struct S { + float16_t f; + float16_t padding; + }; + layout(std140, set = 0, binding = 0) uniform uniformBuf { + S c; + }; + + layout(std140, set = 0, binding = 1) readonly buffer bufA { + S a; + } ; + + layout(std140, set = 0, binding = 2) buffer bufB { + S b; + } ; + + void main() { + b.f = a.f + c.f; + } + + )"); + + wgpu::ComputePipelineDescriptor csDesc; + csDesc.computeStage.module = module; + csDesc.computeStage.entryPoint = "main"; + wgpu::ComputePipeline pipeline = device.CreateComputePipeline(&csDesc); + + wgpu::BindGroup bindGroup = utils::MakeBindGroup(device, pipeline.GetBindGroupLayout(0), + { + {0, uniformBuffer, 0, sizeof(uniformData)}, + {1, bufferIn, 0, sizeof(bufferInData)}, + {2, bufferOut, 0, sizeof(bufferOutData)}, + }); + + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + wgpu::ComputePassEncoder pass = encoder.BeginComputePass(); + pass.SetPipeline(pipeline); + pass.SetBindGroup(0, bindGroup); + pass.Dispatch(1); + pass.EndPass(); + wgpu::CommandBuffer commands = encoder.Finish(); + queue.Submit(1, &commands); + + uint16_t expected[] = {Float32ToFloat16(3.57), Float32ToFloat16(0.0)}; // 0.0 is a padding. + + EXPECT_BUFFER_U16_RANGE_EQ(expected, bufferOut, 0, 2); +} + +DAWN_INSTANTIATE_TEST(ShaderFloat16Tests, VulkanBackend()); \ No newline at end of file