[PATCH] Error on illegal OpenCL kernel argument types

[Matt Arsenault]

================
Comment at: lib/Sema/SemaDecl.cpp:5919
@@ +5918,3 @@
+  // OpenCL v1.2 s6.9.k:
+  // Arguments to kernel functions in a program cannot be declared with the
+  // built-in scalar types bool, half, size_t, ptrdiff_t, intptr_t, and
----------------
Richard Smith wrote:
> I assume this part of the OpenCL spec is defective and it means "Parameters" not "Arguments" here?
I just copied the text out of the spec

================
Comment at: lib/Sema/SemaDecl.cpp:5927-5937
@@ +5926,13 @@
+
+    if (const TypedefType *Typedef = dyn_cast<TypedefType>(PT)) {
+      const IdentifierInfo *Identifier = Typedef->getDecl()->getIdentifier();
+      StringRef Name = Identifier->getName();
+
+      if (Name == "size_t" ||
+          Name == "ptrdiff_t" ||
+          Name == "intptr_t" ||
+          Name == "uintptr_t") {
+        return false;
+      }
+    }
+  }
----------------
Richard Smith wrote:
> OpenCL 6.1.1 seems to imply that size_t, ptrdiff_t, intptr_t and uintptr_t are distinct built-in types in OpenCL, rather than being typedefs for some standard integer type, and it's the underlying types that we should be checking for here, not the typedefs. "typedef size_t my_size_t;" should also be prohibited here (as should, presumably, a typedef for __typeof(sizeof(0))).
> 
> However, the OpenCL spec is unclear, so I'm somewhat guessing... :(
I wasn't really sure what to do about these. I didn't see an equivalent in the builtin type enums I found. It sort of depends on the library implementation. Apple and libclc at least implicitly include a header before the program which has these typedefs in them.

For example, the Apple header defines them as
typedef __typeof__(((int*)0)-((int*)0)) ptrdiff_t;
typedef __SIZE_TYPE__ size_t;
typedef __SIZE_TYPE__ uintptr_t;
typedef __PTRDIFF_TYPE__ intptr_t;
typedef size_t       event_t;

I don't actually see them defined in libclc, but size_t is used so I have no idea where that is coming from.

================
Comment at: lib/Sema/SemaDecl.cpp:5989-5990
@@ +5988,4 @@
+    const RecordDecl *RD = RT->getDecl();
+    for (RecordDecl::field_iterator I = RD->field_begin(),
+           E = RD->field_end(); I != E; ++I) {
+      const FieldDecl *FD = *I;
----------------
Richard Smith wrote:
> Have you considered caching the result of this check? Or at least, storing a set of the types for which you've performed the check -- we don't really need to diagnose the same type more than once.
The type itself isn't the problem, it's only when used as a kernel argument. I don't think only diagnosing it on the first parameter found and then not on a second use as another argument would make much sense.


http://llvm-reviews.chandlerc.com/D1052