Hi Tanya, I haven't seen this committed, so let me suggest slightly shorter error messages:
+def err_expected_kernel_void_return_type : Error< + "a function using the __kernel qualifier must return type void">; "kernel function must return void" +def err_kernel_arg_with_private_address_space : Error< + "kernel pointer arguments must have a global, local, or constant address space qualifier">; "kernel argument cannot point to private address space" Thanks, Anton. > Date: Fri, 24 Aug 2012 18:08:08 -0700 > From: Eli Friedman <[email protected]> > To: Tanya Lattner <[email protected]> > Cc: llvm cfe <[email protected]> > Subject: Re: [cfe-commits] [PATCH] Error checking for OpenCL kernel > return type and pointer args. > Message-ID: > <[email protected] > om> > Content-Type: text/plain; charset=ISO-8859-1 > > (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
