From 82dcbc98302432bdd288b15aba8097ba9bb37eb4 Mon Sep 17 00:00:00 2001 From: Steven Perron Date: Mon, 24 Mar 2025 12:09:42 -0400 Subject: [PATCH 1/3] [SPIRV] Allow sampled type to be half for universal We have a check that the sample type for an image cannot be a 16-bit float. This is true for Vulkan, but not true for general spir-v. We modify this check to only apply when the target env is vulkan. Wew also move the check to spirvemitter where the error handling is better. In its current location, the compiler continue to run with an unexpected nullptr. Fixes #6987 Fixes #6989 --- .../include/clang/SPIRV/FeatureManager.h | 3 +++ tools/clang/lib/SPIRV/FeatureManager.cpp | 14 +++++++++++++ tools/clang/lib/SPIRV/LowerTypeVisitor.cpp | 20 ------------------- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 13 ++++++++++++ .../test/CodeGenSPIRV/type.buffer.half.hlsl | 12 +++++++++-- .../test/CodeGenSPIRV/type.buffer.half4.hlsl | 14 +++++++++++++ 6 files changed, 54 insertions(+), 22 deletions(-) create mode 100644 tools/clang/test/CodeGenSPIRV/type.buffer.half4.hlsl diff --git a/tools/clang/include/clang/SPIRV/FeatureManager.h b/tools/clang/include/clang/SPIRV/FeatureManager.h index 32ee187091..6003d00583 100644 --- a/tools/clang/include/clang/SPIRV/FeatureManager.h +++ b/tools/clang/include/clang/SPIRV/FeatureManager.h @@ -132,6 +132,9 @@ class FeatureManager { /// Returns false otherwise. bool isTargetEnvVulkan1p3OrAbove(); + /// Return true if the target environmnet is a Vulkan environment. + bool isTargetEnvVulkan(); + /// Returns the spv_target_env matching the input string if possible. /// This functions matches the spv_target_env with the command-line version /// of the name ('vulkan1.1', not 'Vulkan 1.1'). diff --git a/tools/clang/lib/SPIRV/FeatureManager.cpp b/tools/clang/lib/SPIRV/FeatureManager.cpp index 2512984a4c..0c8ec38a6a 100644 --- a/tools/clang/lib/SPIRV/FeatureManager.cpp +++ b/tools/clang/lib/SPIRV/FeatureManager.cpp @@ -405,5 +405,19 @@ bool FeatureManager::isTargetEnvVulkan1p3OrAbove() { return targetEnv >= SPV_ENV_VULKAN_1_3; } +bool FeatureManager::isTargetEnvVulkan() { + switch (targetEnv) { + case SPV_ENV_VULKAN_1_0: + case SPV_ENV_VULKAN_1_1: + case SPV_ENV_VULKAN_1_2: + case SPV_ENV_VULKAN_1_1_SPIRV_1_4: + case SPV_ENV_VULKAN_1_3: + case SPV_ENV_VULKAN_1_4: + return true; + default: + return false; + } +} + } // end namespace spirv } // end namespace clang diff --git a/tools/clang/lib/SPIRV/LowerTypeVisitor.cpp b/tools/clang/lib/SPIRV/LowerTypeVisitor.cpp index 24cce9d89e..a5bc4a4aa8 100644 --- a/tools/clang/lib/SPIRV/LowerTypeVisitor.cpp +++ b/tools/clang/lib/SPIRV/LowerTypeVisitor.cpp @@ -834,26 +834,6 @@ LowerTypeVisitor::lowerResourceType(QualType type, SpirvLayoutRule rule, // TODO: avoid string comparison once hlsl::IsHLSLResouceType() does that. - // Vulkan does not yet support true 16-bit float texture objexts. - if (name == "Buffer" || name == "RWBuffer" || name == "Texture1D" || - name == "Texture2D" || name == "Texture3D" || name == "TextureCube" || - name == "Texture1DArray" || name == "Texture2DArray" || - name == "Texture2DMS" || name == "Texture2DMSArray" || - name == "TextureCubeArray" || name == "RWTexture1D" || - name == "RWTexture2D" || name == "RWTexture3D" || - name == "RWTexture1DArray" || name == "RWTexture2DArray") { - const auto sampledType = hlsl::GetHLSLResourceResultType(type); - const auto loweredType = - lowerType(getElementType(astContext, sampledType), rule, - /*isRowMajor*/ llvm::None, srcLoc); - if (const auto *floatType = dyn_cast(loweredType)) { - if (floatType->getBitwidth() == 16) { - emitError("16-bit texture types not yet supported with -spirv", srcLoc); - return nullptr; - } - } - } - { // Texture types spv::Dim dim = {}; bool isArray = {}; diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 557768f59a..e1124999ec 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -1880,6 +1880,19 @@ void SpirvEmitter::doVarDecl(const VarDecl *decl) { } } + if (featureManager.isTargetEnvVulkan() && + (isTexture(decl->getType()) || isRWTexture(decl->getType()) || + isBuffer(decl->getType()) || isRWBuffer(decl->getType()))) { + const auto sampledType = hlsl::GetHLSLResourceResultType(decl->getType()); + if (isFloatOrVecMatOfFloatType(sampledType) && + isOrContains16BitType(sampledType, spirvOptions.enable16BitTypes)) { + emitError("The sampled type for textures cannot be a floating point type " + "smaller than 32-bits when targeting a Vulkan environment.", + loc); + return; + } + } + if (decl->hasAttr()) { // This is a VarDecl for specialization constant. createSpecConstant(decl); diff --git a/tools/clang/test/CodeGenSPIRV/type.buffer.half.hlsl b/tools/clang/test/CodeGenSPIRV/type.buffer.half.hlsl index e5954abae5..99d365b5e2 100644 --- a/tools/clang/test/CodeGenSPIRV/type.buffer.half.hlsl +++ b/tools/clang/test/CodeGenSPIRV/type.buffer.half.hlsl @@ -1,6 +1,14 @@ -// RUN: not %dxc -T ps_6_6 -E main -fcgl %s -spirv -enable-16bit-types 2>&1 | FileCheck %s +// RUN: not %dxc -T ps_6_6 -E main -fcgl %s -spirv -enable-16bit-types 2>&1 | FileCheck %s --check-prefix=VK +// RUN: %dxc -T ps_6_6 -E main -fcgl %s -spirv -fspv-target-env=universal1.5 -enable-16bit-types 2>&1 | FileCheck %s --check-prefix=UNIVERSAL -// CHECK: error: 16-bit texture types not yet supported with -spirv +// When targeting Vulkan, A 16-bit floating pointer buffer is not valid. +// VK: error: The sampled type for textures cannot be a floating point type smaller than 32-bits when targeting a Vulkan environment. + +// When not targeting Vulkan, we should generate the 16-bit floating pointer buffer. +// UNIVERSAL: %half = OpTypeFloat 16 +// UNIVERSAL: %type_buffer_image = OpTypeImage %half Buffer 2 0 0 1 Unknown +// UNIVERSAL: %_ptr_UniformConstant_type_buffer_image = OpTypePointer UniformConstant %type_buffer_image +// UNIVERSAL: %MyBuffer = OpVariable %_ptr_UniformConstant_type_buffer_image UniformConstant Buffer MyBuffer; void main(): SV_Target { } diff --git a/tools/clang/test/CodeGenSPIRV/type.buffer.half4.hlsl b/tools/clang/test/CodeGenSPIRV/type.buffer.half4.hlsl new file mode 100644 index 0000000000..f29af69c1c --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/type.buffer.half4.hlsl @@ -0,0 +1,14 @@ +// RUN: not %dxc -T ps_6_6 -E main -fcgl %s -spirv -enable-16bit-types 2>&1 | FileCheck %s --check-prefix=VK +// RUN: %dxc -T ps_6_6 -E main -fcgl %s -spirv -fspv-target-env=universal1.5 -enable-16bit-types 2>&1 | FileCheck %s --check-prefix=UNIVERSAL + +// When targeting Vulkan, A 16-bit floating pointer buffer is not valid. +// VK: error: The sampled type for textures cannot be a floating point type smaller than 32-bits when targeting a Vulkan environment. + +// When not targeting Vulkan, we should generate the 16-bit floating pointer buffer. +// UNIVERSAL: %half = OpTypeFloat 16 +// UNIVERSAL: %type_buffer_image = OpTypeImage %half Buffer 2 0 0 1 Unknown +// UNIVERSAL: %_ptr_UniformConstant_type_buffer_image = OpTypePointer UniformConstant %type_buffer_image +// UNIVERSAL: %MyBuffer = OpVariable %_ptr_UniformConstant_type_buffer_image UniformConstant +Buffer MyBuffer; + +void main(): SV_Target { } From beda68622531b7111fbc308119f978d80bc5fb32 Mon Sep 17 00:00:00 2001 From: Steven Perron Date: Mon, 24 Mar 2025 14:26:58 -0400 Subject: [PATCH 2/3] Update tools/clang/include/clang/SPIRV/FeatureManager.h Co-authored-by: Cassandra Beckley --- tools/clang/include/clang/SPIRV/FeatureManager.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/clang/include/clang/SPIRV/FeatureManager.h b/tools/clang/include/clang/SPIRV/FeatureManager.h index 6003d00583..841708d8d5 100644 --- a/tools/clang/include/clang/SPIRV/FeatureManager.h +++ b/tools/clang/include/clang/SPIRV/FeatureManager.h @@ -132,7 +132,7 @@ class FeatureManager { /// Returns false otherwise. bool isTargetEnvVulkan1p3OrAbove(); - /// Return true if the target environmnet is a Vulkan environment. + /// Return true if the target environment is a Vulkan environment. bool isTargetEnvVulkan(); /// Returns the spv_target_env matching the input string if possible. From 849b55fe70992beb829aa447ebd9139d0748424c Mon Sep 17 00:00:00 2001 From: Steven Perron Date: Mon, 24 Mar 2025 14:45:00 -0400 Subject: [PATCH 3/3] Add static assert to know when a new target env is added. --- tools/clang/lib/SPIRV/FeatureManager.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/clang/lib/SPIRV/FeatureManager.cpp b/tools/clang/lib/SPIRV/FeatureManager.cpp index 0c8ec38a6a..c459f7af0f 100644 --- a/tools/clang/lib/SPIRV/FeatureManager.cpp +++ b/tools/clang/lib/SPIRV/FeatureManager.cpp @@ -406,6 +406,10 @@ bool FeatureManager::isTargetEnvVulkan1p3OrAbove() { } bool FeatureManager::isTargetEnvVulkan() { + // This assert ensure that this list will be updated, if necessary, when + // a new target environment is added. + static_assert(SPV_ENV_VULKAN_1_4 + 1 == SPV_ENV_MAX); + switch (targetEnv) { case SPV_ENV_VULKAN_1_0: case SPV_ENV_VULKAN_1_1: