Skip to content

Commit 94596e1

Browse files
[SPIRV] Allow sampled type to be half for universal (#7252)
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 --------- Co-authored-by: Cassandra Beckley <[email protected]>
1 parent b646ad3 commit 94596e1

File tree

6 files changed

+58
-22
lines changed

6 files changed

+58
-22
lines changed

tools/clang/include/clang/SPIRV/FeatureManager.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,9 @@ class FeatureManager {
132132
/// Returns false otherwise.
133133
bool isTargetEnvVulkan1p3OrAbove();
134134

135+
/// Return true if the target environment is a Vulkan environment.
136+
bool isTargetEnvVulkan();
137+
135138
/// Returns the spv_target_env matching the input string if possible.
136139
/// This functions matches the spv_target_env with the command-line version
137140
/// of the name ('vulkan1.1', not 'Vulkan 1.1').

tools/clang/lib/SPIRV/FeatureManager.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,5 +405,23 @@ bool FeatureManager::isTargetEnvVulkan1p3OrAbove() {
405405
return targetEnv >= SPV_ENV_VULKAN_1_3;
406406
}
407407

408+
bool FeatureManager::isTargetEnvVulkan() {
409+
// This assert ensure that this list will be updated, if necessary, when
410+
// a new target environment is added.
411+
static_assert(SPV_ENV_VULKAN_1_4 + 1 == SPV_ENV_MAX);
412+
413+
switch (targetEnv) {
414+
case SPV_ENV_VULKAN_1_0:
415+
case SPV_ENV_VULKAN_1_1:
416+
case SPV_ENV_VULKAN_1_2:
417+
case SPV_ENV_VULKAN_1_1_SPIRV_1_4:
418+
case SPV_ENV_VULKAN_1_3:
419+
case SPV_ENV_VULKAN_1_4:
420+
return true;
421+
default:
422+
return false;
423+
}
424+
}
425+
408426
} // end namespace spirv
409427
} // end namespace clang

tools/clang/lib/SPIRV/LowerTypeVisitor.cpp

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -834,26 +834,6 @@ LowerTypeVisitor::lowerResourceType(QualType type, SpirvLayoutRule rule,
834834

835835
// TODO: avoid string comparison once hlsl::IsHLSLResouceType() does that.
836836

837-
// Vulkan does not yet support true 16-bit float texture objexts.
838-
if (name == "Buffer" || name == "RWBuffer" || name == "Texture1D" ||
839-
name == "Texture2D" || name == "Texture3D" || name == "TextureCube" ||
840-
name == "Texture1DArray" || name == "Texture2DArray" ||
841-
name == "Texture2DMS" || name == "Texture2DMSArray" ||
842-
name == "TextureCubeArray" || name == "RWTexture1D" ||
843-
name == "RWTexture2D" || name == "RWTexture3D" ||
844-
name == "RWTexture1DArray" || name == "RWTexture2DArray") {
845-
const auto sampledType = hlsl::GetHLSLResourceResultType(type);
846-
const auto loweredType =
847-
lowerType(getElementType(astContext, sampledType), rule,
848-
/*isRowMajor*/ llvm::None, srcLoc);
849-
if (const auto *floatType = dyn_cast<FloatType>(loweredType)) {
850-
if (floatType->getBitwidth() == 16) {
851-
emitError("16-bit texture types not yet supported with -spirv", srcLoc);
852-
return nullptr;
853-
}
854-
}
855-
}
856-
857837
{ // Texture types
858838
spv::Dim dim = {};
859839
bool isArray = {};

tools/clang/lib/SPIRV/SpirvEmitter.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1880,6 +1880,19 @@ void SpirvEmitter::doVarDecl(const VarDecl *decl) {
18801880
}
18811881
}
18821882

1883+
if (featureManager.isTargetEnvVulkan() &&
1884+
(isTexture(decl->getType()) || isRWTexture(decl->getType()) ||
1885+
isBuffer(decl->getType()) || isRWBuffer(decl->getType()))) {
1886+
const auto sampledType = hlsl::GetHLSLResourceResultType(decl->getType());
1887+
if (isFloatOrVecMatOfFloatType(sampledType) &&
1888+
isOrContains16BitType(sampledType, spirvOptions.enable16BitTypes)) {
1889+
emitError("The sampled type for textures cannot be a floating point type "
1890+
"smaller than 32-bits when targeting a Vulkan environment.",
1891+
loc);
1892+
return;
1893+
}
1894+
}
1895+
18831896
if (decl->hasAttr<VKConstantIdAttr>()) {
18841897
// This is a VarDecl for specialization constant.
18851898
createSpecConstant(decl);
Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
1-
// RUN: not %dxc -T ps_6_6 -E main -fcgl %s -spirv -enable-16bit-types 2>&1 | FileCheck %s
1+
// RUN: not %dxc -T ps_6_6 -E main -fcgl %s -spirv -enable-16bit-types 2>&1 | FileCheck %s --check-prefix=VK
2+
// 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
23

3-
// CHECK: error: 16-bit texture types not yet supported with -spirv
4+
// When targeting Vulkan, A 16-bit floating pointer buffer is not valid.
5+
// VK: error: The sampled type for textures cannot be a floating point type smaller than 32-bits when targeting a Vulkan environment.
6+
7+
// When not targeting Vulkan, we should generate the 16-bit floating pointer buffer.
8+
// UNIVERSAL: %half = OpTypeFloat 16
9+
// UNIVERSAL: %type_buffer_image = OpTypeImage %half Buffer 2 0 0 1 Unknown
10+
// UNIVERSAL: %_ptr_UniformConstant_type_buffer_image = OpTypePointer UniformConstant %type_buffer_image
11+
// UNIVERSAL: %MyBuffer = OpVariable %_ptr_UniformConstant_type_buffer_image UniformConstant
412
Buffer<half> MyBuffer;
513

614
void main(): SV_Target { }
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// RUN: not %dxc -T ps_6_6 -E main -fcgl %s -spirv -enable-16bit-types 2>&1 | FileCheck %s --check-prefix=VK
2+
// 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
3+
4+
// When targeting Vulkan, A 16-bit floating pointer buffer is not valid.
5+
// VK: error: The sampled type for textures cannot be a floating point type smaller than 32-bits when targeting a Vulkan environment.
6+
7+
// When not targeting Vulkan, we should generate the 16-bit floating pointer buffer.
8+
// UNIVERSAL: %half = OpTypeFloat 16
9+
// UNIVERSAL: %type_buffer_image = OpTypeImage %half Buffer 2 0 0 1 Unknown
10+
// UNIVERSAL: %_ptr_UniformConstant_type_buffer_image = OpTypePointer UniformConstant %type_buffer_image
11+
// UNIVERSAL: %MyBuffer = OpVariable %_ptr_UniformConstant_type_buffer_image UniformConstant
12+
Buffer<half4> MyBuffer;
13+
14+
void main(): SV_Target { }

0 commit comments

Comments
 (0)