(Resending; I forgot to CC cfe-commits.) On Fri, Aug 24, 2012 at 6:06 PM, Eli Friedman <[email protected]> wrote: > On Wed, Aug 22, 2012 at 1:14 PM, Tanya Lattner <[email protected]> wrote: >> Ping. >> >> On Aug 15, 2012, at 6:11 PM, Tanya Lattner wrote: >> >>> The attached patch add the following checks and errors: >>> 1) Checks that an OpenCL kernel returns void, otherwise throws error. >>> 2) Check that kernel arguments that are pointers are in the constant, >>> global, or local address space, otherwise throws an error. >>> >>> I have included a test case. >>> >>> Please review. > > + if ((getLangOpts().OpenCLVersion >= 120) > + && (SC == SC_Static)) { > + Diag(D.getIdentifierLoc(), diag::err_static_kernel); > + D.setInvalidType(); > + } > > Not really part of this patch, but the OpenCLVersion check appears to > be redundant. > > + const PointerType *PT = dyn_cast<PointerType>(T.getTypePtr()); > > T->getAs<PointerType>(). dyn_cast will mess up in the case of > typedefs. (A test for this case would be nice.) > > You might also want to add a test for the case where the parameter is > declared as an array; I think your patch will work as-is, but it's > worth checking. > > + if (PT > + && PT->getPointeeType().getAddressSpace() == 0) { > > Weird indentation. > > + // OpenCL v1.1 s6.8j Kernels may only have void return type. > + if (!NewFD->getResultType()->isVoidType()) { > + Diag(D.getIdentifierLoc(), > + diag::err_expected_kernel_void_return_type); > + D.setInvalidType(); > + } > > As written, this allows "const void"; not sure if that's intentional. > > Otherwise, looks fine.
-Eli _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
