Browse Source

[spirv] Don't emit extension for using fp16 in GLSLstd450 instructions. (#2503)

With the exception of interpolation instructions.

New revision of SPIR-V Extended Instructions for GLSL allows fp16
except for interpolation instructions.

See: https://www.khronos.org/registry/spir-v/specs/1.0/GLSL.std.450.html

Fixes #2500.
Ehsan 5 years ago
parent
commit
6bf91e8a41

+ 4 - 26
tools/clang/lib/SPIRV/CapabilityVisitor.cpp

@@ -530,37 +530,15 @@ bool CapabilityVisitor::visit(SpirvExecutionMode *execMode) {
 
 bool CapabilityVisitor::visit(SpirvExtInst *instr) {
   // OpExtInst using the GLSL extended instruction allows only 32-bit types by
-  // default. The AMD_gpu_shader_half_float extension adds support for 16-bit
-  // floating-point component types for the following instructions described in
-  // the GLSL.std.450 extended instruction set:
-  // Acos, Acosh, Asin, Asinh, Atan2, Atanh, Atan, Cos, Cosh, Degrees, Exp,
-  // Exp2, InterpolateAtCentroid, InterpolateAtSample, InterpolateAtOffset, Log,
-  // Log2, Pow, Radians, Sin, Sinh, Tan, Tanh
+  // default for interpolation instructions. The AMD_gpu_shader_half_float
+  // extension adds support for 16-bit floating-point component types for these
+  // instructions:
+  // InterpolateAtCentroid, InterpolateAtSample, InterpolateAtOffset
   if (SpirvType::isOrContainsType<FloatType, 16>(instr->getResultType()))
     switch (instr->getInstruction()) {
-    case GLSLstd450::GLSLstd450Acos:
-    case GLSLstd450::GLSLstd450Acosh:
-    case GLSLstd450::GLSLstd450Asin:
-    case GLSLstd450::GLSLstd450Asinh:
-    case GLSLstd450::GLSLstd450Atan2:
-    case GLSLstd450::GLSLstd450Atanh:
-    case GLSLstd450::GLSLstd450Atan:
-    case GLSLstd450::GLSLstd450Cos:
-    case GLSLstd450::GLSLstd450Cosh:
-    case GLSLstd450::GLSLstd450Degrees:
-    case GLSLstd450::GLSLstd450Exp:
-    case GLSLstd450::GLSLstd450Exp2:
     case GLSLstd450::GLSLstd450InterpolateAtCentroid:
     case GLSLstd450::GLSLstd450InterpolateAtSample:
     case GLSLstd450::GLSLstd450InterpolateAtOffset:
-    case GLSLstd450::GLSLstd450Log:
-    case GLSLstd450::GLSLstd450Log2:
-    case GLSLstd450::GLSLstd450Pow:
-    case GLSLstd450::GLSLstd450Radians:
-    case GLSLstd450::GLSLstd450Sin:
-    case GLSLstd450::GLSLstd450Sinh:
-    case GLSLstd450::GLSLstd450Tan:
-    case GLSLstd450::GLSLstd450Tanh:
       addExtension(Extension::AMD_gpu_shader_half_float, "16-bit float",
                    instr->getSourceLocation());
     default:

+ 0 - 11
tools/clang/test/CodeGenSPIRV/extension.GLSLstd450-fp16.not-allowed.hlsl

@@ -1,11 +0,0 @@
-// Run: %dxc -T ps_6_2 -E main -enable-16bit-types -fspv-extension=KHR
-
-// The command line option instructs the compiler to only use core Vulkan features and KHR extensions,
-// and prevents the compiler from using vendor-specific extensions.
-
-// CHECK: 10:23: error: SPIR-V extension 'SPV_AMD_gpu_shader_half_float' required for 16-bit float but not permitted to use
-
-void main() {
-  float16_t4 a;
-  float16_t4 result = atan(a);
-}

+ 0 - 3
tools/clang/test/CodeGenSPIRV/extension.GLSLstd450-fp16.allowed.hlsl → tools/clang/test/CodeGenSPIRV/intrinsics.atan.fp16.hlsl

@@ -1,9 +1,6 @@
 // Run: %dxc -T ps_6_2 -E main -enable-16bit-types
 
-// The GLSL extended instruction set may only use 16-bit floats if AMD_gpu_shader_half_float extension is used.
-
 // CHECK: OpCapability Float16
-// CHECK: OpExtension "SPV_AMD_gpu_shader_half_float"
 // CHECK: OpExtInstImport "GLSL.std.450"
 void main() {
   float16_t4 a;

+ 5 - 13
tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp

@@ -1114,6 +1114,11 @@ TEST_F(FileTest, IntrinsicsAsin) { runFileTest("intrinsics.asin.hlsl"); }
 TEST_F(FileTest, IntrinsicsAcos) { runFileTest("intrinsics.acos.hlsl"); }
 TEST_F(FileTest, IntrinsicsAtan) { runFileTest("intrinsics.atan.hlsl"); }
 TEST_F(FileTest, IntrinsicsAtan2) { runFileTest("intrinsics.atan2.hlsl"); }
+TEST_F(FileTest, IntrinsicsAtanFp16) {
+  // Float16 capability should be emitted for usage of fp16 in the extended
+  // instruction set.
+  runFileTest("intrinsics.atan.fp16.hlsl");
+}
 
 // Unspported intrinsic functions
 TEST_F(FileTest, IntrinsicsAbort) {
@@ -2035,19 +2040,6 @@ TEST_F(FileTest, CapabilityUnique) { runFileTest("capability.unique.hlsl"); }
 // For extension uniqueness
 TEST_F(FileTest, ExtensionUnique) { runFileTest("extension.unique.hlsl"); }
 
-// For vendor-specific extensions
-TEST_F(FileTest, VendorSpecificExtensionAllowed) {
-  // The SPV_AMD_gpu_shader_half_float extension adds support for 16-bit
-  // floating-point component types for a number of instructions in the
-  // GLSL.std.450 extended instruction set.
-  runFileTest("extension.GLSLstd450-fp16.allowed.hlsl");
-}
-TEST_F(FileTest, VendorSpecificExtensionNotAllowed) {
-  // Command line options can entirely prevent the compiler from using
-  // vendor-specific extensions.
-  runFileTest("extension.GLSLstd450-fp16.not-allowed.hlsl", Expect::Failure);
-}
-
 // For RelaxedPrecision decorations
 TEST_F(FileTest, DecorationRelaxedPrecisionBasic) {
   runFileTest("decoration.relaxed-precision.basic.hlsl");