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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D35082#890359, @Anastasia wrote:

> In https://reviews.llvm.org/D35082#890162, @rjmccall wrote:
>
> > Okay.  I think I see your point about why it would be better to have a canonical __private address space that is different from the implicit address space 0.  I assume this means that there should basically never be pointers, references, or l-values in address space 0 in OpenCL.
>
>
> If you mean 0 is `private` address space then no. There can be private AS pointers. But if you mean 0 is no address space then this doesn't exist in OpenCL. I think the right design in the first place  would have been to keep address spaces in AST just as they are in the source code and add explicit address spaces to all types in IR instead. In this case absence of any address spaces in AST would signal implicit AS to be used. This would make some Sema checks a bit more complicated though, but the design would become a lot cleaner. Unfortunately I think it would not be worth doing this big change now.


My understanding is that that is exactly what the majority of this patch is trying to achieve: making __private its own address space, separate from address space 0.  The patch is certainly introducing a new address-space enumerator for __private; if that's meant to be treated as canonically equal to address space 0, well, (1) this patch doesn't seem to add any of the APIs that I'd expect would be required to make that work, like a way to canonicalize an address space or ask if two address spaces are canonically the same, and (2) that would be a major new complexity in the representation that we wouldn't welcome anyway.

The patch is also tracking explicitness of address spaces in the non-canonical type system, but the majority of the patch is spent dealing with the consequences of splitting __private away from address space 0.

Yaxun has convinced me that it's important to represent a __private-qualified type differently from an unqualified type.(*)  It seems that you agree, but you're concerned about how that will affect your schedule.  I can't speak to your schedule; it seems to me that you and Yaxun need to figure that out privately.  If you decide not to separate them yet, then the majority of this patch needs to be put on hold.  If you do decide to separate them, it seems to me that we should probably split the change to __private out from the introduction of implicit address spaces.  Just let me know what you want to do.

(*) I know that you aren't considering OpenCL C++ yet, but often these representation/model questions are easier to answer when thinking about C++ instead of C because type differences are so much more directly important in C++.  In OpenCL C++, I assume it will be important to distinguish between <int> and <__private int> as template arguments.  That tells us straight up that __private needs to be represented differently from a lack of qualifier.  Note that the result, where certain type representations (e.g. an unqualified int*) become basically meaningless and should never be constructed by Sema, is already precedented in Clang: ObjC ARC does the same thing to unqualified ObjC pointer types, like id*.


https://reviews.llvm.org/D35082