[PATCH] D58236: Make address space conversions a bit stricter.

Bevin Hansson via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 28 00:46:11 PST 2019


ebevhan added a comment.

In D58236#1412267 <https://reviews.llvm.org/D58236#1412267>, @Anastasia wrote:

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


That does sound like a good idea and I will probably look into it when I have more time. I'd also like to look into making an RFC for the improved address space specification support. Again, when I have time :)

In D58236#1412324 <https://reviews.llvm.org/D58236#1412324>, @efriedma wrote:

> Generally, with an explicit cast, C allows any pointer cast with a reasonable interpretation, even if the underlying operation is suspicious.  For example, you can cast an "long*" to a "int*" (as in "(int*)(long*)p") without any complaint, even though dereferencing the result is likely undefined behavior.  (C doesn't allow explicitly writing a reinterpret_cast, but that's basically the operation in question.)


That is true... The current rules are very permissive about pointer casts.

> Along those lines, in general, the normal C rules should allow casting `foo*` to `bar*` for any object types foo and bar, even if foo and bar are pointers with address spaces, like `__local int *` and `__global int *`.  I don't see anything in the OpenCL standard that would contradict this.  It looks like this patch changes that?

I vaguely recall that when I was looking into fixing this in our downstream a while back I found that OpenCL did not have any explicit provisions for nested address spaces, only direct ones.

So should we drop the extra error on nested space mismatches, since the rules should be as permissive as possible unless the spec prohibits it? Technically the patch could be changed to only fix the incorrect warning on direct address space mismatches instead.


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

https://reviews.llvm.org/D58236





More information about the cfe-commits mailing list