[OpenCL] Add const, volatile and pointer handling for builtin functions

[svenvh]

Summary

Volatile, const and pointer types were previously available, but not
working. This patch adds handling for OpenCL builtin functions.

Some atomic functions are added to show examples of use of
the PointerType and the VolatileType.

This depends on:
-First, splitting opencl-c.h file: https://reviews.llvm.org/D63256/new/
-Second, Adding generic types: https://reviews.llvm.org/D63434

[Pierre]

Add context lines.

[Anastasia]

clang/lib/Sema/OpenCLBuiltins.td

294 ↗

(On Diff #205397)

This patch also seems to be adding atomics?

[Pierre]

Add ConstType and VolatileType in a comment

[Anastasia]

clang/lib/Sema/OpenCLBuiltins.td

64 ↗

(On Diff #206702)

We seem to have PointerType twice now...

99 ↗

(On Diff #206702)

Why changing this comment?

112 ↗

(On Diff #206702)

I would suggest
CVQualType<Type _Ty, bit IsConst, bit IsVolatile>
just to avoid code duplication.

Btw don't you need to set QualType too?

245 ↗

(On Diff #206702)

Do you need this here?

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

23 ↗

(On Diff #206702)

Can we test const too?

If you remove volatile does clang give diagnostic?

[Pierre]

Typo and correct the order of fields in the Type class (.td file)

clang/lib/Sema/OpenCLBuiltins.td

99 ↗

(On Diff #206702)

I wanted to be consistent with the colon, I've put back the "float*"

112 ↗

(On Diff #206702)

I would suggest
CVQualType<Type _Ty, bit IsConst, bit IsVolatile>
just to avoid code duplication.

I thought creating a class for these specific cases would be clearer than having a bitfield.
Also, there are not a lot of functions using const/ volatile types, creating a class for these specific cases avoids to have fields to fill with blank values. This can be avoided with default initialisation though (and not setting these fields).

Btw don't you need to set QualType too?

The name is set by giving arguments to the parent class line 101. All this let statements are here to be sure we are taking the values of the type _Ty given as argument, and not setting the fields to the default values of the Type class.

245 ↗

(On Diff #206702)

This needs to be after the definition of all the types (char_t, int_t, etc), and before the definitions of the GenericType instances (and before the definition of the builtin functions)

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

23 ↗

(On Diff #206702)

Can we test const too?

I can add a test, but for this I need to find a builtin function using a const qualifier and add it in this patch aswell (cf the atomic functions added in the .td file). I'm ok to add some more functions if you agree.

If you remove volatile does clang give diagnostic?

Indeed, I though I remembered there was a warning emitted when the volatile/ const qualifier was not present. Warnings are only emitted in one case (see the table below).

argument given has \ function expects	scalar no qualifier	scalar const/volatile arg
scalar no qualifier	OK	No warning
scalar const/volatile	No Warning	OK

and

argument given has \ function expects	pointer no qualifier	pointer to const/volatile arg
pointer to const/volatile	OK	No warning
pointer to const/volatile	Warning	OK

So no diagnostic would be emitted in this test, whether we give a const/ volatile or not to atom_add.

[Pierre]

Rebase from the patch adding GenTypes.

[Pierre]

Reorder fields in the Type structure in the .td file

[svenvh]

• Factor out .td def renames into separate patch (already pushed).
• Add definitions for some atomic and asynchronous builtins to make use of the new functionality.
• Fix formatting.

[svenvh]

Rebased onto latest master.

[Anastasia]

LGTM! Thanks!

clang/lib/Sema/OpenCLBuiltins.td

91 ↗

(On Diff #215887)

shouldn't this be always false?

94 ↗

(On Diff #215887)

this should be none

107 ↗

(On Diff #215887)

this should be none

335 ↗

(On Diff #215887)

At least the message in the commit should reflect that we are also adding new BIFs.

[svenvh]

clang/lib/Sema/OpenCLBuiltins.td

91 ↗

(On Diff #215887)

In theory yes, but I think I'd rather propagate the value instead of silently dropping it.

94 ↗

(On Diff #215887)

As above, I feel it's safer to propagate the value instead of silently dropping it.

cfe/trunk/lib/Sema/OpenCLBuiltins.td

class QualType<string _Name, bit _IsAbstract=0> {
   // E.g. Name="IntTy" for the int type, and "getIntPtrType()" for an intptr_t
   string Name = _Name;
   // Some QualTypes in this file represent an abstract type for which there is
   // no corresponding AST QualType, e.g. a GenType or an `image2d_t` type
   // without access qualifiers.
   bit IsAbstract = _IsAbstract;
 }
 
-// Helper class to store type access qualifiers (volatile, const, ...).
-class Qualifier<string _QualName> {
-  string QualName = _QualName;
-}
-
 // List of integers.
 class IntList<string _Name, list<int> _List> {
   string Name = _Name;
   list<int> List = _List;
 }
 
 //===----------------------------------------------------------------------===//
 //                      OpenCL C classes for types
 //===----------------------------------------------------------------------===//
 // OpenCL C basic data types (int, float, image2d_t, ...).
 // Its Child classes can represent concrete types (e.g.: VectorType) or
 // custom types (e.g.: GenType).
 // Instances of these child classes should be used in Builtin function
 // arguments.  See the definition of the "read_imagef" function as example.
 class Type<string _Name, QualType _QTName> {
   // Name of the Type.
   string Name = _Name;
   // QualType associated with this type.
   QualType QTName = _QTName;
   // Size of the vector (if applicable).
   int VecWidth = 1;
   // Is a pointer.
   bit IsPointer = 0;
-  // List of qualifiers associated with the type (volatile, ...)
-  list<Qualifier> QualList = [];
+  // "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 = "clang::LangAS::Default";
+  string AddrSpace = DefaultAS.Name;
 }
 
-// OpenCL vector types (e.g. int2, int3, int16, float8, ...)
+// OpenCL vector types (e.g. int2, int3, int16, float8, ...).
 class VectorType<Type _Ty, int _VecWidth> : Type<_Ty.Name, _Ty.QTName> {
-  int VecWidth = _VecWidth;
+  let VecWidth = _VecWidth;
+  // Inherited fields
+  let IsPointer = _Ty.IsPointer;
+  let IsConst = _Ty.IsConst;
+  let IsVolatile = _Ty.IsVolatile;
+  let AccessQualifier = _Ty.AccessQualifier;
+  let AddrSpace = _Ty.AddrSpace;
 }
 
 // OpenCL pointer types (e.g. int*, float*, ...).
-class PointerType<Type _Ty, AddressSpace _AS = GlobalAS> :
+class PointerType<Type _Ty, AddressSpace _AS = DefaultAS> :
                                       Type<_Ty.Name, _Ty.QTName> {
-  bit IsPointer = 1;
-  string AddrSpace = _AS.Name;
+  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;
+}
+
+// OpenCL const types (e.g. const int).
+class ConstType<Type _Ty> : Type<_Ty.Name, _Ty.QTName> {
+  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;
+}
+
+// OpenCL volatile types (e.g. volatile int).
+class VolatileType<Type _Ty> : Type<_Ty.Name, _Ty.QTName> {
+  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;
 }
 
 // OpenCL image types (e.g. image2d_t, ...)
 class ImageType<Type _Ty, QualType _QTName, string _AccessQualifier> :
                                               Type<_Ty.Name, _QTName> {
   let AccessQualifier = _AccessQualifier;
 }
 

 // specification.
 
 // Vector width lists.
 def VecAndScalar: IntList<"VecAndScalar", [1, 2, 3, 4, 8, 16]>;
 def VecNoScalar : IntList<"VecNoScalar", [2, 3, 4, 8, 16]>;
 def Vec1        : IntList<"Vec1", [1]>;
 
 // Type lists.
+def TLAll   : TypeList<"TLAll", [Char, UChar, Short, UShort, Int, UInt, Long, ULong, Float, Double, Half]>;
 def TLFloat : TypeList<"TLFloat", [Float, Double, Half]>;
 
 def TLAllInts : TypeList<"TLAllInts", [Char, UChar, Short, UShort, Int, UInt, Long, ULong]>;
 
 // GenType definitions for multiple base types (e.g. all floating point types,
 // or all integer types).
+// All types
+def AGenTypeN              : GenericType<"AGenTypeN", TLAll, VecAndScalar>;
+def AGenTypeNNoScalar      : GenericType<"AGenTypeNNoScalar", TLAll, VecNoScalar>;
 // All integer
 def AIGenType1             : GenericType<"AIGenType1", TLAllInts, Vec1>;
 def AIGenTypeN             : GenericType<"AIGenTypeN", TLAllInts, VecAndScalar>;
 def AIGenTypeNNoScalar     : GenericType<"AIGenTypeNNoScalar", TLAllInts, VecNoScalar>;
 // Float
 def FGenTypeN              : GenericType<"FGenTypeN", TLFloat, VecAndScalar>;
 
 // GenType definitions for every single base type (e.g. fp32 only).
foreach sat = ["", "_sat"] in {
                         [VectorType<RType, v>,
                          VectorType<IType, v>]>;
         }
       }
     }
   }
 }
 
+//--------------------------------------------------------------------
+// OpenCL v1.1 s6.11.10, v1.2 s6.12.10, v2.0 s6.13.10: Async Copies from Global to Local Memory, Local to Global Memory, and Prefetch
+// OpenCL Extension v2.0 s5.1.7 and s6.1.7: Async Copies from Global to Local Memory, Local to Global Memory, and Prefetch
+// --- Table 18 ---
+foreach name = ["async_work_group_copy"] in {
+  def : Builtin<name, [Event, PointerType<AGenTypeN, LocalAS>, PointerType<ConstType<AGenTypeN>, GlobalAS>, Size, Event]>;
+  def : Builtin<name, [Event, PointerType<AGenTypeN, GlobalAS>, PointerType<ConstType<AGenTypeN>, LocalAS>, Size, Event]>;
+}
+foreach name = ["async_work_group_strided_copy"] in {
+  def : Builtin<name, [Event, PointerType<AGenTypeN, LocalAS>, PointerType<ConstType<AGenTypeN>, GlobalAS>, Size, Size, Event]>;
+  def : Builtin<name, [Event, PointerType<AGenTypeN, GlobalAS>, PointerType<ConstType<AGenTypeN>, LocalAS>, Size, Size, Event]>;
+}
+foreach name = ["wait_group_events"] in {
+  def : Builtin<name, [Void, Int, PointerType<Event, GenericAS>]>;
+}
+foreach name = ["prefetch"] in {
+  def : Builtin<name, [Void, PointerType<ConstType<AGenTypeN>, GlobalAS>, Size]>;
+}
+
+//--------------------------------------------------------------------
+// OpenCL v2.0 s6.13.11 - Atomics Functions.
+// Functions that use memory_order and cl_mem_fence_flags enums are not
+// declared here as the TableGen backend does not handle enums.
+
+// OpenCL v1.0 s9.5, s9.6, s9.7 - Atomic Functions for 32-bit integers.
+// --- Table 9.1 ---
+foreach Type = [Int, UInt] in {
+  foreach name = ["atom_add", "atom_sub", "atom_xchg"] in {
+    def : Builtin<name, [Type, PointerType<VolatileType<Type>, GlobalAS>, Type]>;
+  }
+  foreach name = ["atom_inc", "atom_dec"] in {
+    def : Builtin<name, [Type, PointerType<VolatileType<Type>, GlobalAS>]>;
+  }
+  foreach name = ["atom_cmpxchg"] in {
+    def : Builtin<name, [Type, PointerType<VolatileType<Type>, GlobalAS>, Type, Type]>;
+  }
+}
+
 // OpenCL v1.2 s6.12.1: Work-Item Functions
 def get_work_dim : Builtin<"get_work_dim", [UInt]>;
 foreach name = ["get_global_size", "get_global_id", "get_local_size",
                 "get_local_id", "get_num_groups", "get_group_id",
                 "get_global_offset"] in {
   def : Builtin<name, [Size, UInt]>;
 }
 

cfe/trunk/test/SemaOpenCL/fdeclare-opencl-builtins.cl

 typedef float float4 __attribute__((ext_vector_type(4)));
 typedef int int2 __attribute__((ext_vector_type(2)));
 typedef int int4 __attribute__((ext_vector_type(4)));
 typedef long long2 __attribute__((ext_vector_type(2)));
 typedef unsigned int uint;
 typedef __SIZE_TYPE__ size_t;
 #endif
 
+kernel void test_pointers(volatile global void *global_p, global const int4 *a) {
+  int i;
+  unsigned int ui;
+
+  prefetch(a, 2);
+
+  atom_add((volatile __global int *)global_p, i);
+  atom_cmpxchg((volatile __global unsigned int *)global_p, ui, ui);
+}
+
 kernel void basic_conversion() {
   double d;
   float f;
   char2 c2;
   long2 l2;
   float4 f4;
   int4 i4;
 

cfe/trunk/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

void BuiltinNameEmitter::EmitDeclarations() {
   OS << GenTypeEnums;
   OS << "};\n";
 
   // Structure definitions.
   OS << R"(
 
 // Represents a return type or argument type.
 struct OpenCLTypeStruct {
-  // A type (e.g. float, int, ...)
+  // A type (e.g. float, int, ...).
   const OpenCLTypeID ID;
   // Vector size (if applicable; 0 for scalars and generic types).
   const unsigned VectorWidth;
+  // 0 if the type is not a pointer.
+  const bool IsPointer;
+  // 0 if the type is not const.
+  const bool IsConst;
+  // 0 if the type is not volatile.
+  const bool IsVolatile;
+  // Address space of the pointer (if applicable).
+  const LangAS AS;
 };
 
 // One overload of an OpenCL builtin function.
 struct OpenCLBuiltinStruct {
   // Index of the signature in the OpenCLTypeStruct table.
   const unsigned SigTableIndex;
   // Entries between index SigTableIndex and (SigTableIndex + NumTypes - 1) in
   // the SignatureTable represent the complete signature.  The first type at
void BuiltinNameEmitter::GetOverloads() {
   }
 }
 
 void BuiltinNameEmitter::EmitTypeTable() {
   OS << "static const OpenCLTypeStruct TypeTable[] = {\n";
   for (const auto &T : TypeMap) {
     OS << "  // " << T.second << "\n";
     OS << "  {OCLT_" << T.first->getValueAsString("Name") << ", "
-       << T.first->getValueAsInt("VecWidth") << "},\n";
+       << T.first->getValueAsInt("VecWidth") << ", "
+       << T.first->getValueAsBit("IsPointer") << ", "
+       << T.first->getValueAsBit("IsConst") << ", "
+       << T.first->getValueAsBit("IsVolatile") << ", "
+       << T.first->getValueAsString("AddrSpace") << "},\n";
   }
   OS << "};\n\n";
 }
 
 void BuiltinNameEmitter::EmitSignatureTable() {
   // Store a type (e.g. int, float, int2, ...). The type is stored as an index
   // of a struct OpenCLType table. Multiple entries following each other form a
   // signature.
)";
   // Assign the right attributes to the types (e.g. vector size).
   OS << R"(
   // Set vector size for non-generic vector types.
   if (Ty.VectorWidth > 1) {
     for (unsigned Index = 0; Index < QT.size(); Index++) {
       QT[Index] = Context.getExtVectorType(QT[Index], Ty.VectorWidth);
     }
   }
+
+  if (Ty.IsVolatile != 0) {
+    for (unsigned Index = 0; Index < QT.size(); Index++) {
+      QT[Index] = Context.getVolatileType(QT[Index]);
+    }
+  }
+
+  if (Ty.IsConst != 0) {
+    for (unsigned Index = 0; Index < QT.size(); Index++) {
+      QT[Index] = Context.getConstType(QT[Index]);
+    }
+  }
+
+  // Transform the type to a pointer as the last step, if necessary.
+  // Builtin functions only have pointers on [const|volatile], no
+  // [const|volatile] pointers, so this is ok to do it as a last step.
+  if (Ty.IsPointer != 0) {
+    for (unsigned Index = 0; Index < QT.size(); Index++) {
+      QT[Index] = Context.getAddrSpaceQualType(QT[Index], Ty.AS);
+      QT[Index] = Context.getPointerType(QT[Index]);
+    }
+  }
 )";
 
   // End of the "OCL2Qual" function.
   OS << "\n} // OCL2Qual\n";
 }
 
 namespace clang {
 
 void EmitClangOpenCLBuiltins(RecordKeeper &Records, raw_ostream &OS) {
   BuiltinNameEmitter NameChecker(Records, OS);
   NameChecker.Emit();
 }
 
 } // end namespace clang