Anastasia accepted this revision.
Anastasia added a comment.
This revision is now accepted and ready to land.

LGTM! Thanks a lot for fixing this old bug! Btw, do you plan to look at 
generalizing this to C++ as well?

I don't feel we need anything for mixing OpenCL with GNU style address spaces 
at this point. We can always extend this in the future (in case it turns out to 
be useful to someone!).



================
Comment at: test/SemaOpenCL/address-spaces.cl:87
+
+  // FIXME: This doesn't seem right. This should be an error, not a warning.
+  __local int * __global * __private * lll;
----------------
ebevhan wrote:
> Anastasia wrote:
> > Are we sure it has to be an error? May be we can change this wording to 
> > something like - unknown behavior that needs clarifying? Some similar text 
> > to your comment in the code.
> Well... If `__private int **` to `__generic int **` is an error but 
> `__private int ***` to `__generic int **` isn't, that seems a bit odd to me...
> 
Ok, I am still confused about this case because it's converting number of 
pointers. It's not exactly the same to me as converting address spaces of 
pointers. Since `__generic` is default addr space in OpenCL, in your example 
following 2 inner address spaces seem to match:
`__private int * __generic* __generic*` -> `__generic int * __generic*`

But in any case keeping FIXME is definitely the right thing to do here until we 
come up with some sort of rules.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58236/new/

https://reviews.llvm.org/D58236



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to