[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: lib/Sema/SemaDecl.cpp:11846
   // an address space.
   if (T.getAddressSpace() != 0) {
     // OpenCL allows function arguments declared to be an array of a type
----------------
yaxunl wrote:
> Anastasia wrote:
> > yaxunl wrote:
> > > Anastasia wrote:
> > > > yaxunl wrote:
> > > > > Anastasia wrote:
> > > > > > Could we use `LangAS::Default` here too?
> > > > > done
> > > > Sorry, I wasn't clear. I think we could have:
> > > > 
> > > >   if (T.getAddressSpace() != LangAS::Default && T.getAddressSpace() != LangAS::opencl_private)
> > > > 
> > > > and then original condition. It is a bit clearer I think.
> > > No. For OpenCL, the condition is on line 11847 and 11848. An array type in other address spaces is allowed.
> > I think the initial condition was `T.getAddressSpace() != 0` i.e. if not private address space.
> > 
> > So now since we added private this should be `T.getAddressSpace() !=LangAS::opencl_private` but we can have the default case as well hence 'T.getAddressSpace() !=LangAS::opencl_private || T.getAddressSpace() !=LangAS::Default'.
> > 
> > This entire case seems to be for OpenCL anyways. So you could also move out the OpenCL check if you prefer. I am just trying to see if we can make this easier to understand.
> This is not just for OpenCL.
> 
> The condition expresses the following logic:
> 
> For non-OpenCL languages, only default addr space is allowed on function argument.
> 
> For OpenCL, for array type argument, any addr space is allowed; for non-array type argument, only private addr space is allowed.
Parameters declared with type 'foo T[]' will be canonicalized to 'foo T*' when forming the function type, so that's not actually a different rule in the end.


https://reviews.llvm.org/D35082