[cfe-commits] [PATCH] Error checking for OpenCL kernel return type and pointer args.

[Eli Friedman]

(Resending; I forgot to CC cfe-commits.)

On Fri, Aug 24, 2012 at 6:06 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Wed, Aug 22, 2012 at 1:14 PM, Tanya Lattner <lattner at apple.com> 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