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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 6 inline comments as done.
yaxunl 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
----------------
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.


https://reviews.llvm.org/D35082