[OpenCL] Do not add builtins with unavailable types

[svenvh]

Summary

Add functionality to assign extensions to types in OpenCLBuiltins.td and use that information to filter candidates that should not be exposed if a type is not available.

Most of the changes affect the addition of scalar base types into the QT vector in the generated OCL2Qual function (that is, the switch statement as described in step 1 in the OCL2Qual top comment). For types that are associated with an extension, the QualType is now added only if the corresponding extension macro is defined.

The old code was as follows for the FGenTypeN GenericType, which represents all floating point types for all vector sizes.

case OCLT_FGenTypeN:
  QT.append({Context.FloatTy, Context.DoubleTy, Context.HalfTy, Context.FloatTy, Context.DoubleTy, Context.HalfTy, Context.FloatTy, ...
  GenTypeNumTypes = 3;
  GenVectorSizes = ListVecAndScalar;
  break;

With this patch, the generated code becomes:

case OCLT_FGenTypeN: {
  SmallVector<QualType, 3> TypeList;
  TypeList.push_back(Context.FloatTy);
  if (S.getPreprocessor().isMacroDefined("cl_khr_fp64")) {
    TypeList.push_back(Context.DoubleTy);
  }
  if (S.getPreprocessor().isMacroDefined("cl_khr_fp16")) {
    TypeList.push_back(Context.HalfTy);
  }
  GenTypeNumTypes = TypeList.size();
  QT.reserve(18);
  for (unsigned I = 0; I < 6; I++) {
    QT.append(TypeList);
  }
  GenVectorSizes = ListVecAndScalar;
  break;
}

[Anastasia]

clang/lib/Sema/OpenCLBuiltins.td

54

I am trying to understand why would we need a special abstraction for the type? Would it not be easier if we just guard the BIFs by the extensions that allow the use of the type?

We would need to separate the definitions of course but this can be more helpful in order to understand what overloads are available conditionally?

[svenvh]

clang/lib/Sema/OpenCLBuiltins.td

54

Yes, it is possible to achieve the same with the current FunctionExtension class.

However, that would require a lot of duplication in the Builtin descriptions, as for example many math builtins have overloads for half and double that will have to be conditionalized. The purpose of TypeExtension is precisely to avoid separating and duplicating the definitions. For example, acos is currently more or less defined as:

def FGenTypeN : GenericType<"FGenTypeN", TypeList<[Float, Double, Half]>, VecAndScalar>;
def : Builtin<"acos", [FGenTypeN, FGenTypeN], Attr.Const>;

with double and half conditionalization conveniently handled in a single place through a TypeExtension.

If we would only use FunctionExtensions, the definition would become more like the following:

def FGenTypeN : GenericType<"FGenTypeN", TypeList<[Float]>, VecAndScalar>;
def : Builtin<"acos", [FGenTypeN, FGenTypeN], Attr.Const>;

let Extension = Fp64 in {
  def DGenTypeN : GenericType<"DGenTypeN", TypeList<[Double]>, VecAndScalar>;
  def : Builtin<"acos", [DGenTypeN, DGenTypeN], Attr.Const>;
}

let Extension = Fp16 in {
  def HGenTypeN : GenericType<"HGenTypeN", TypeList<[Half]>, VecAndScalar>;
  def : Builtin<"acos", [HGenTypeN, HGenTypeN], Attr.Const>;
}

I personally don't think there is value in adding these explicit guards for every conditional builtin, as the duplication makes the definitions harder to maintain. In addition, I expect it would also increase the size of the generated tables, as the GenericTypes have to be split up (though I have not measured this).

[Anastasia]

clang/lib/Sema/OpenCLBuiltins.td

54

Ok I see. Yeah it looks quite a lot of duplication indeed for some types. However, this patch provides extra flexibility and doesn't enforce to use the new approach evrywhere so I think it is very reasonable.

Could you just provide a bit more documentation and especially the difference to the other approach here?

Do you think it is work extending the online documentation for that too?

clang/lib/Sema/SemaLookup.cpp

762

This deserved the comment.

[svenvh]

Adding some more comments / explanation.

[svenvh]

clang/lib/Sema/OpenCLBuiltins.td

54

Do you think it is work extending the online documentation for that too?

Yes we should, but it is perhaps worth waiting until we have clarified the design wrt feature optionality.

[Anastasia]

LGTM! Thanks!

clang/lib/Sema/OpenCLBuiltins.td

class AbstractExtension<string _Ext> {
   // One or more OpenCL extensions, space separated.  Each extension must be
   // a valid extension name for the opencl extension pragma.
   string ExtName = _Ext;
 }
 
 // Extension associated to a builtin function.
 class FunctionExtension<string _Ext> : AbstractExtension<_Ext>;
 
+// Extension associated to a type.  This enables implicit conditionalization of
+// builtin function overloads containing a type that depends on an extension.
+// During overload resolution, when a builtin function overload contains a type
+// with a TypeExtension, those overloads are skipped when the extension is
+// disabled.
+class TypeExtension<string _Ext> : AbstractExtension<_Ext>;
+
+// TypeExtension definitions.
+def NoTypeExt   : TypeExtension<"">;
+def Fp16TypeExt : TypeExtension<"cl_khr_fp16">;
+def Fp64TypeExt : TypeExtension<"cl_khr_fp64">;
+
 // FunctionExtension definitions.
 def FuncExtNone                          : FunctionExtension<"">;
 def FuncExtKhrSubgroups                  : FunctionExtension<"cl_khr_subgroups">;
 def FuncExtKhrSubgroupExtendedTypes      : FunctionExtension<"cl_khr_subgroup_extended_types">;
 def FuncExtKhrSubgroupNonUniformVote     : FunctionExtension<"cl_khr_subgroup_non_uniform_vote">;
 def FuncExtKhrSubgroupBallot             : FunctionExtension<"cl_khr_subgroup_ballot">;
 def FuncExtKhrSubgroupNonUniformArithmetic: FunctionExtension<"cl_khr_subgroup_non_uniform_arithmetic">;
 def FuncExtKhrSubgroupShuffle            : FunctionExtension<"cl_khr_subgroup_shuffle">;
class Type<string _Name, QualType _QTExpr> {
   // "const" qualifier.
   bit IsConst = 0;
   // "volatile" qualifier.
   bit IsVolatile = 0;
   // Access qualifier. Must be one of ("RO", "WO", "RW").
   string AccessQualifier = "";
   // Address space.
   string AddrSpace = DefaultAS.Name;
+  // Extension that needs to be enabled to expose a builtin that uses this type.
+  TypeExtension Extension = NoTypeExt;
 }
 
 // OpenCL vector types (e.g. int2, int3, int16, float8, ...).
 class VectorType<Type _Ty, int _VecWidth> : Type<_Ty.Name, _Ty.QTExpr> {
   let VecWidth = _VecWidth;
   let AccessQualifier = "";
   // Inherited fields
   let IsPointer = _Ty.IsPointer;
   let IsConst = _Ty.IsConst;
   let IsVolatile = _Ty.IsVolatile;
   let AddrSpace = _Ty.AddrSpace;
+  let Extension = _Ty.Extension;
 }
 
 // OpenCL pointer types (e.g. int*, float*, ...).
 class PointerType<Type _Ty, AddressSpace _AS = DefaultAS> :
     Type<_Ty.Name, _Ty.QTExpr> {
   let AddrSpace = _AS.Name;
   // Inherited fields
   let VecWidth = _Ty.VecWidth;
   let IsPointer = 1;
   let IsConst = _Ty.IsConst;
   let IsVolatile = _Ty.IsVolatile;
   let AccessQualifier = _Ty.AccessQualifier;
+  let Extension = _Ty.Extension;
 }
 
 // OpenCL const types (e.g. const int).
 class ConstType<Type _Ty> : Type<_Ty.Name, _Ty.QTExpr> {
   let IsConst = 1;
   // Inherited fields
   let VecWidth = _Ty.VecWidth;
   let IsPointer = _Ty.IsPointer;
   let IsVolatile = _Ty.IsVolatile;
   let AccessQualifier = _Ty.AccessQualifier;
   let AddrSpace = _Ty.AddrSpace;
+  let Extension = _Ty.Extension;
 }
 
 // OpenCL volatile types (e.g. volatile int).
 class VolatileType<Type _Ty> : Type<_Ty.Name, _Ty.QTExpr> {
   let IsVolatile = 1;
   // Inherited fields
   let VecWidth = _Ty.VecWidth;
   let IsPointer = _Ty.IsPointer;
   let IsConst = _Ty.IsConst;
   let AccessQualifier = _Ty.AccessQualifier;
   let AddrSpace = _Ty.AddrSpace;
+  let Extension = _Ty.Extension;
 }
 
 // OpenCL image types (e.g. image2d).
 class ImageType<Type _Ty, string _AccessQualifier> :
     Type<_Ty.Name, QualType<_Ty.QTExpr.TypeExpr # _AccessQualifier # "Ty", 0>> {
   let VecWidth = 0;
   let AccessQualifier = _AccessQualifier;
   // Inherited fields
   let IsPointer = _Ty.IsPointer;
   let IsConst = _Ty.IsConst;
   let IsVolatile = _Ty.IsVolatile;
   let AddrSpace = _Ty.AddrSpace;
+  let Extension = _Ty.Extension;
 }
 
 // OpenCL enum type (e.g. memory_scope).
 class EnumType<string _Name> :
     Type<_Name, QualType<"getOpenCLEnumType(S, \"" # _Name # "\")", 0>> {
 }
 
 // OpenCL typedef type (e.g. cl_mem_fence_flags).

 def UChar     : Type<"uchar",     QualType<"Context.UnsignedCharTy">>;
 def Short     : Type<"short",     QualType<"Context.ShortTy">>;
 def UShort    : Type<"ushort",    QualType<"Context.UnsignedShortTy">>;
 def Int       : Type<"int",       QualType<"Context.IntTy">>;
 def UInt      : Type<"uint",      QualType<"Context.UnsignedIntTy">>;
 def Long      : Type<"long",      QualType<"Context.LongTy">>;
 def ULong     : Type<"ulong",     QualType<"Context.UnsignedLongTy">>;
 def Float     : Type<"float",     QualType<"Context.FloatTy">>;
+let Extension = Fp64TypeExt in {
   def Double    : Type<"double",    QualType<"Context.DoubleTy">>;
+}
+let Extension = Fp16TypeExt in {
   def Half      : Type<"half",      QualType<"Context.HalfTy">>;
+}
 def Size      : Type<"size_t",    QualType<"Context.getSizeType()">>;
 def PtrDiff   : Type<"ptrdiff_t", QualType<"Context.getPointerDiffType()">>;
 def IntPtr    : Type<"intptr_t",  QualType<"Context.getIntPtrType()">>;
 def UIntPtr   : Type<"uintptr_t", QualType<"Context.getUIntPtrType()">>;
 def Void      : Type<"void",      QualType<"Context.VoidTy">>;
 
 // OpenCL v1.0/1.2/2.0 s6.1.2: Built-in Vector Data Types.
 // Built-in vector data types are created by TableGen's OpenCLBuiltinEmitter.

clang/lib/Sema/SemaLookup.cpp

 static void GetOpenCLBuiltinFctOverloads(
     ASTContext &Context, unsigned GenTypeMaxCnt,
     std::vector<QualType> &FunctionList, SmallVector<QualType, 1> &RetTypes,
     SmallVector<SmallVector<QualType, 1>, 5> &ArgTypes) {
   FunctionProtoType::ExtProtoInfo PI(
       Context.getDefaultCallingConvention(false, false, true));
   PI.Variadic = false;
 
+  // Do not attempt to create any FunctionTypes if there are no return types,
+  // which happens when a type belongs to a disabled extension.
+  if (RetTypes.size() == 0)
+    return;
+
   // Create FunctionTypes for each (gen)type.
   for (unsigned IGenType = 0; IGenType < GenTypeMaxCnt; IGenType++) {
     SmallVector<QualType, 5> ArgList;
 
     for (unsigned A = 0; A < ArgTypes.size(); A++) {
+      // Bail out if there is an argument that has no available types.
+      if (ArgTypes[A].size() == 0)
+        return;
+
       // Builtins such as "max" have an "sgentype" argument that represents
       // the corresponding scalar type of a gentype.  The number of gentypes
       // must be a multiple of the number of sgentypes.
       assert(GenTypeMaxCnt % ArgTypes[A].size() == 0 &&
              "argument type count not compatible with gentype type count");
       unsigned Idx = IGenType % ArgTypes[A].size();
       ArgList.push_back(ArgTypes[A][Idx]);
     }

clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl

 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CL -fdeclare-opencl-builtins -DNO_HEADER
 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CL -fdeclare-opencl-builtins -finclude-default-header
 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CL1.2 -fdeclare-opencl-builtins -DNO_HEADER
 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CL1.2 -fdeclare-opencl-builtins -finclude-default-header
 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CL2.0 -fdeclare-opencl-builtins -DNO_HEADER
 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CL2.0 -fdeclare-opencl-builtins -finclude-default-header
 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CLC++ -fdeclare-opencl-builtins -DNO_HEADER
 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CLC++ -fdeclare-opencl-builtins -finclude-default-header
+// RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CL2.0 -fdeclare-opencl-builtins -finclude-default-header -cl-ext=-cl_khr_fp64 -DNO_FP64
 
 // Test the -fdeclare-opencl-builtins option.  This is not a completeness
 // test, so it should not test for all builtins defined by OpenCL.  Instead
 // this test should cover different functional aspects of the TableGen builtin
 // function machinery.
 
 #pragma OPENCL EXTENSION cl_khr_fp16 : enable
 #if __OPENCL_C_VERSION__ < CL_VERSION_1_2
void test_atomic_fetch(volatile __generic atomic_int *a_int,
   uip = atomic_fetch_add(a_uintptr, ptrdiff);
 
   ip = atomic_fetch_or(a_intptr, ip);
   uip = atomic_fetch_or(a_uintptr, uip);
 }
 #endif
 
 kernel void basic_conversion() {
-  double d;
   float f;
   char2 c2;
   long2 l2;
   float4 f4;
   int4 i4;
 
+#ifdef NO_FP64
+  (void)convert_double_rtp(f);
+  // expected-error@-1{{implicit declaration of function 'convert_double_rtp' is invalid in OpenCL}}
+#else
+  double d;
   f = convert_float(d);
-  d = convert_double_rtp(f);
+#endif
   l2 = convert_long2_rtz(c2);
   i4 = convert_int4_sat(f4);
 }
 
 kernel void basic_conversion_neg() {
   int i;
   float f;
 
kernel void basic_work_item() {
 
   barrier(CLK_GLOBAL_MEM_FENCE);
 
   get_enqueued_local_size(ui);
 #if !defined(__OPENCL_CPP_VERSION__) && __OPENCL_C_VERSION__ < CL_VERSION_2_0
 // expected-error@-2{{implicit declaration of function 'get_enqueued_local_size' is invalid in OpenCL}}
 #endif
 }
+
+#ifdef NO_FP64
+void test_extension_types(char2 c2) {
+  // We should see 6 candidates for float and half types, and none for double types.
+  int i = isnan(c2);
+  // expected-error@-1{{no matching function for call to 'isnan'}}
+  // expected-note@-2 6 {{candidate function not viable: no known conversion from '__private char2' (vector of 2 'char' values) to 'float}}
+  // expected-note@-3 6 {{candidate function not viable: no known conversion from '__private char2' (vector of 2 'char' values) to 'half}}
+}
+#endif

clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

for (const auto &Image : ITE.getValue()) {
          << "          break;\n";
     }
     OS << "      }\n"
        << "      break;\n";
   }
 
   // Switch cases for generic types.
   for (const auto *GenType : Records.getAllDerivedDefinitions("GenericType")) {
-    OS << "    case OCLT_" << GenType->getValueAsString("Name") << ":\n";
-    OS << "      QT.append({";
+    OS << "    case OCLT_" << GenType->getValueAsString("Name") << ": {\n";
 
     // Build the Cartesian product of (vector sizes) x (types).  Only insert
     // the plain scalar types for now; other type information such as vector
     // size and type qualifiers will be added after the switch statement.
-    for (unsigned I = 0; I < GenType->getValueAsDef("VectorList")
-                                 ->getValueAsListOfInts("List")
-                                 .size();
-         I++) {
-      for (const auto *T :
-           GenType->getValueAsDef("TypeList")->getValueAsListOfDefs("List")) {
-        OS << T->getValueAsDef("QTExpr")->getValueAsString("TypeExpr") << ", ";
-      }
-    }
-    OS << "});\n";
-    // GenTypeNumTypes is the number of types in the GenType
-    // (e.g. float/double/half).
-    OS << "      GenTypeNumTypes = "
-       << GenType->getValueAsDef("TypeList")->getValueAsListOfDefs("List")
-              .size()
-       << ";\n";
+    std::vector<Record *> BaseTypes =
+        GenType->getValueAsDef("TypeList")->getValueAsListOfDefs("List");
+
+    // Collect all QualTypes for a single vector size into TypeList.
+    OS << "      SmallVector<QualType, " << BaseTypes.size() << "> TypeList;\n";
+    for (const auto *T : BaseTypes) {
+      StringRef Ext =
+          T->getValueAsDef("Extension")->getValueAsString("ExtName");
+      if (!Ext.empty()) {
+        OS << "      if (S.getPreprocessor().isMacroDefined(\"" << Ext
+           << "\")) {\n  ";
+      }
+      OS << "      TypeList.push_back("
+         << T->getValueAsDef("QTExpr")->getValueAsString("TypeExpr") << ");\n";
+      if (!Ext.empty()) {
+        OS << "      }\n";
+      }
+    }
+    OS << "      GenTypeNumTypes = TypeList.size();\n";
+
+    // Duplicate the TypeList for every vector size.
+    std::vector<int64_t> VectorList =
+        GenType->getValueAsDef("VectorList")->getValueAsListOfInts("List");
+    OS << "      QT.reserve(" << VectorList.size() * BaseTypes.size() << ");\n"
+       << "      for (unsigned I = 0; I < " << VectorList.size() << "; I++) {\n"
+       << "        QT.append(TypeList);\n"
+       << "      }\n";
+
     // GenVectorSizes is the list of vector sizes for this GenType.
-    // QT contains GenTypeNumTypes * #GenVectorSizes elements.
     OS << "      GenVectorSizes = List"
        << GenType->getValueAsDef("VectorList")->getValueAsString("Name")
-       << ";\n";
-    OS << "      break;\n";
+       << ";\n"
+       << "      break;\n"
+       << "    }\n";
   }
 
   // Switch cases for non generic, non image types (int, int4, float, ...).
   // Only insert the plain scalar type; vector information and type qualifiers
   // are added in step 2.
   std::vector<Record *> Types = Records.getAllDerivedDefinitions("Type");
   StringMap<bool> TypesSeen;
 
   for (const auto *T : Types) {
     // Check this is not an image type
     if (ImageTypesMap.find(T->getValueAsString("Name")) != ImageTypesMap.end())
       continue;
     // Check we have not seen this Type
     if (TypesSeen.find(T->getValueAsString("Name")) != TypesSeen.end())
       continue;
     TypesSeen.insert(std::make_pair(T->getValueAsString("Name"), true));
 
     // Check the Type does not have an "abstract" QualType
     auto QT = T->getValueAsDef("QTExpr");
     if (QT->getValueAsBit("IsAbstract") == 1)
       continue;
     // Emit the cases for non generic, non image types.
-    OS << "    case OCLT_" << T->getValueAsString("Name") << ":\n"
-       << "      QT.push_back(" << QT->getValueAsString("TypeExpr") << ");\n"
-       << "      break;\n";
+    OS << "    case OCLT_" << T->getValueAsString("Name") << ":\n";
+
+    StringRef Ext = T->getValueAsDef("Extension")->getValueAsString("ExtName");
+    // If this type depends on an extension, ensure the extension macro is
+    // defined.
+    if (!Ext.empty()) {
+      OS << "      if (S.getPreprocessor().isMacroDefined(\"" << Ext
+         << "\")) {\n  ";
+    }
+    OS << "      QT.push_back(" << QT->getValueAsString("TypeExpr") << ");\n";
+    if (!Ext.empty()) {
+      OS << "      }\n";
+    }
+    OS << "      break;\n";
   }
 
   // End of switch statement.
   OS << "  } // end of switch (Ty.ID)\n\n";
 
   // Step 2.
   // Add ExtVector types if this was a generic type, as the switch statement
   // above only populated the list with scalar types.  This completes the