[cfe-dev] OpenCL & SPIR specific types - proposal and patch

[Tanya Lattner]

I already responded about sampler briefly, but I'm not in favor of making sampler a pointer to an opaque type. I have more comments, but I need more than a day to write them up.

-Tanya

On Oct 4, 2012, at 3:52 PM, "Ouriel, Boaz" <boaz.ouriel at intel.com> wrote:

> Hi Doug & Tanya,
> 
> Could you please have a look at this patch?
> (see the description below as sent by Guy Benyei)
> 
> Thanks in advance,
> Boaz
> 
> -----Original Message-----
> From: Anton Lokhmotov [mailto:Anton.Lokhmotov at arm.com] 
> Sent: Thursday, October 04, 2012 17:37
> To: Benyei, Guy; cfe-dev at cs.uiuc.edu
> Cc: Villmow, Micah; Ouriel, Boaz
> Subject: RE: OpenCL & SPIR specific types - proposal and patch
> 
> Hi Guy,
> 
> Basically, the patch looks good (after a quick look).  Please find our
> initial comments below.  
> 
> Look forward to seeing more code for various restrictions on these data
> types.  We can help you there if needed.
> 
> Many thanks,
> Anton.
> 
> 
> +  CanQualType Image1dTTy, Image1dArrayTTy, Image1dBufferTTy;
> +  CanQualType Image2dTTy, Image2dArrayTTy, Image3dTTy;
> 
> Please move Image3dTTy to a separate line for consistency.
> 
> 
> +inline bool Type::isImageType() const {
> +  return isImage1dT() || isImage1dArrayT() || isImage1dBufferT() ||
> +         isImage2dT() || isImage2dArrayT() ||
> +         isImage3dT();
> +}
> 
> Start the check with 3d and 2d types (this will benefit OpenCL 1.1
> compilers):
> 
>  return isImage3dT() || isImage2dT() || isImage2dArrayT() ||
>         isImage1dT() || isImage1dArrayT() || isImage1dBufferT();
> 
> 
> --- include/clang/Sema/DeclSpec.h	(revision 165097)
> +++ include/clang/Sema/DeclSpec.h	(working copy)
>   // type-qualifiers
> @@ -306,7 +314,7 @@
>   /*TSW*/unsigned TypeSpecWidth : 2;
>   /*TSC*/unsigned TypeSpecComplex : 2;
>   /*TSS*/unsigned TypeSpecSign : 2;
> -  /*TST*/unsigned TypeSpecType : 5;
> +  /*TST*/unsigned TypeSpecType : 6;
> 
> We suspect that in addition to this change you also need to change:
> 
> --- a/include/clang/Basic/Specifiers.h
> +++ b/include/clang/Basic/Specifiers.h
> @@ -72,7 +72,7 @@ namespace clang {
>   /// \brief Structure that packs information about the type specifiers
> that
>   /// were written in a particular type specifier sequence.
>   struct WrittenBuiltinSpecs {
> -    /*DeclSpec::TST*/ unsigned Type  : 5;
> +    /*DeclSpec::TST*/ unsigned Type  : 6;
>     /*DeclSpec::TSS*/ unsigned Sign  : 2;
>     /*DeclSpec::TSW*/ unsigned Width : 2;
>     bool ModeAttr : 1;
> 
> 
> +      Width = Target->getPointerWidth(0); // Currently these types are
> pointers
> +      Align = Target->getPointerAlign(0); // to opaque types
>      Width = Target->getPointerWidth(0); // Currently these types are
>      Align = Target->getPointerAlign(0); // pointers to opaque types.
> 
> 
> +    case tok::kw_image1d_t:
> +       isInvalid = DS.SetTypeSpecType(DeclSpec::TST_image1d_t, Loc, 
> +                                      PrevSpec, DiagID);
> +      break;
> Please check indentation after this code.  Sometimes there's an extra space
> after 'Loc,'.
> 
> +      isInvalid = DS.SetTypeSpecType(DeclSpec::TST_image3d_t, Loc,
> +                                      PrevSpec, DiagID);
> PrevSpec is misaligned.
> 
> 
> 
> 
> From: Benyei, Guy [mailto:guy.benyei at intel.com] 
> Sent: 03 October 2012 16:06
> To: cfe-dev at cs.uiuc.edu
> Cc: Villmow, Micah; Anton Lokhmotov; Ouriel, Boaz
> Subject: OpenCL & SPIR specific types - proposal and patch
> 
> I'd like to renew the discussion about making the OpenCL specific types
> first class citizens in Clang.
> 
> I think this change is required by the OpenCL specifications, since these
> type names are keywords of the OpenCL C language.
> This change is also needed in order to enable efficient checking of OpenCL
> restrictions on these types (OpenCL 1.2 spec, section 6.9).
> Furthermore, the proposed change will turn these types to pointers to opaque
> types, which means that it will hide the actual (vendor specific)
> implementation, so the OpenCL vendors using Clang will be able to implement
> these types in their own way.
> 
> This change would also be a basis for the implementation of SPIR generation
> by Clang. The SPIR discussion and spec can be found here:
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-September/024132.html
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-September/024178.html
> 
> Earlier discussion about the OpenCL types was started by Anton Lokhmotov:
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-May/015297.html
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-April/014741.html
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-March/014118.html
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-March/014121.html
> 
> 
> 
> ---------------------------------------------------------------------
> 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.
> <opencl_types.patch>