================
Comment at: lib/Sema/SemaDecl.cpp:5960
@@ +5959,3 @@
+
+void Sema::checkIsValidOpenCLKernelArgument(Declarator &D, ParmVarDecl *Param)
{
+ QualType PT = Param->getType();
----------------
This doesn't need to be a Sema member; a static non-member function would be
preferred.
================
Comment at: lib/Sema/SemaDecl.cpp:5917
@@ -5916,1 +5916,3 @@
+static bool isValidOpenCLKernelArgumentType(QualType PT) {
+ // OpenCL v1.2 s6.9.k:
----------------
"...ParameterType" not "...ArgumentType".
================
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
----------------
I assume this part of the OpenCL spec is defective and it means "Parameters"
not "Arguments" here?
================
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;
+ }
+ }
+ }
----------------
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... :(
================
Comment at: lib/Sema/SemaDecl.cpp:5952-5958
@@ +5951,9 @@
+
+static const RecordType *getAsStructOrUnionType(QualType QT) {
+ const RecordType *RT = QT->getAsStructureType();
+ if (!RT)
+ RT = QT->getAsUnionType();
+
+ return RT;
+}
+
----------------
This is just QT->getAs<RecordType>().
================
Comment at: lib/Sema/SemaDecl.cpp:5966-5969
@@ +5965,6 @@
+ // pointer to a pointer type.
+ if (PT->isPointerType() && PT->getPointeeType()->isPointerType()) {
+ Diag(Param->getLocation(), diag::err_opencl_ptrptr_kernel_arg);
+ D.setInvalidType();
+ }
+
----------------
It would seem cleaner to fold both this pointer check and the one below into
isValidOpenCLKernelArgumentType. You can return an enum value from there to
indicate the flavor of problem, and fold together the diagnostics with a
%select. You can also pass a value into this function to indicate whether
you're checking a direct argument (where only a pointer-to-pointer is blocked)
or a field of an argument (where any pointer is blocked).
================
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;
----------------
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.
================
Comment at: lib/Sema/SemaDecl.cpp:6000-6002
@@ +5999,5 @@
+ if (!isValidOpenCLKernelArgumentType(QT)) {
+ // TODO: A more specific warning about which struct members forbid this
+ // would be useful
+ Diag(Param->getLocation(), diag::err_bad_kernel_arg_type) << PT;
+ D.setInvalidType();
----------------
Maybe reformulate the queue as a recursive walk, and produce notes on the way
back up after diagnosing a problem?
http://llvm-reviews.chandlerc.com/D1052
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
