[Patch] OpenCL kernel-arg-info

Pekka Jääskeläinen pekka.jaaskelainen at tut.fi
Mon Feb 18 14:11:11 PST 2013


Hi,

+/// \brief Defines emumerations to bet used when generating metadata for

"to be"

+/// OpenCL -kernel-arg-info flag. These enumerations represent the
+/// order in the OpenCL spec 1.2 5.7.3 definition for clKernelArgInfo API

"clGetKernelArgInfo"

+        typeQuals |= 1;

A magic number.

+      if(ty.isRestrictQualified())
+        typeQuals |= CLKAITQ_restrict;
+      if(pointeeTy.isConstQualified())
+        typeQuals |= CLKAITQ_const;
+      if(pointeeTy.isVolatileQualified())
+        typeQuals |= CLKAITQ_volatile;

Still some white space nitpicking here (should have a space after 'if').

Otherwise LGTM (I'm not a Clang expert though).


On 02/18/2013 02:00 PM, Benyei, Guy wrote:
> Pekka,
> Thanks for the prompt review.
>
> Attached an updated patch. Added a header file with the kKernelArgInfo specific enumerations.
>
> Please review.
>
> Thanks
>      Guy
>
> -----Original Message-----
> From: Pekka Jääskeläinen [mailto:pekka.jaaskelainen at tut.fi]
> Sent: Sunday, February 17, 2013 18:23
> To: Benyei, Guy
> Cc: cfe-commits at cs.uiuc.edu
> Subject: Re: [Patch] OpenCL kernel-arg-info
>
> Hi,
>
> You use "magic numbers" for the different address space ids.
> Similarly the type qualifier bit masks could use some named consts, IMO.
>
> These are from the SPIR specs, but still, using some sort of named constant/enum from a header file would clean it up.
> There could be a header for the SPIR-specific constants?
>
> The consumers for this metadata (at least the OpenCL
> clGetKernelArgInfo() implementations) need to refer to these numbers too.
>
> +      if(ty.isRestrictQualified())        typeQuals |= 2;
> +      if(pointeeTy.isConstQualified())    typeQuals |= 1;
> +      if(pointeeTy.isVolatileQualified()) typeQuals |= 4;
>
> Some white space issues.
>
> On 02/17/2013 04:49 PM, Benyei, Guy wrote:
>> Please review.
>
> --
> --Pekka
>
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.




More information about the cfe-commits mailing list