Anastasia added inline comments.

================
Comment at: lib/Sema/SemaCast.cpp:2288
+      SrcType->isPointerType()) {
+    const PointerType *DestPtr = DestType->getAs<PointerType>();
+    if (!DestPtr->isAddressSpaceOverlapping(*SrcType->getAs<PointerType>())) {
----------------
rjmccall wrote:
> Anastasia wrote:
> > rjmccall wrote:
> > > Anastasia wrote:
> > > > Anastasia wrote:
> > > > > rjmccall wrote:
> > > > > > Please test the result of `getAs` instead of separately testing 
> > > > > > `isPointerType`.
> > > > > > 
> > > > > > Why is this check OpenCL-specific?  Address spaces are a general 
> > > > > > language feature.
> > > > > I think mainly because I am factoring out from the existing OpenCL 
> > > > > check. But it probably makes sense that the semantics of this is not 
> > > > > different in other languages. I will update it! Thanks!
> > > > After playing with this for a bit longer I discovered that I have to 
> > > > keep the OpenCL check unfortunately.
> > > > 
> > > > I found this old commit (`d4c5f84`/`r129583`) that says:
> > > >   C-style casts can add/remove/change address spaces through the 
> > > > reinterpret_cast mechanism.
> > > > That's not the same as in OpenCL because it seems for C++ you can cast 
> > > > any AS to any other AS. Therefore, no checks are needed at all. I am 
> > > > not sure if we can come up with a common function for the moment.
> > > > 
> > > > The following  tests are checking this:
> > > >    CodeGenCXX/address-space-cast.cpp
> > > >    SemaCXX/address-space-conversion.cpp
> > > > 
> > > > Do you think it would make sense to rename this method with 
> > > > OpenCL-something or keep in case may be CUDA or some other languages 
> > > > might need similar functionality...
> > > > 
> > > I think you can leave it alone for now, but please add a comment 
> > > explaining the reasoning as best you see it, and feel free to express 
> > > uncertainty about the right rule.
> > > 
> > > Don't take `QualType` by `const &`, by the way.  It's a cheap-to-copy 
> > > value type and should always be passed by value.
> > My general understand of the address spaces in C and C++ that they are 
> > intended to be more general than in OpenCL (i.e. they don't reflect any 
> > particular memory system). Perhaps, it makes sense that we have more 
> > restrictions in OpenCL or other similar languages.
> No, their primary use is to reflect underlying memory systems, like near vs. 
> far pointers on 32-on-64 targets or specialized address spaces like 
> `gs`-segment addressing on x86-64, and it doesn't make sense to allow 
> arbitrary conversions any more than it does for OpenCL.  The Embedded C spec 
> has rules about overlapping address spaces, and while it doesn't say that 
> casts between non-overlapping address spaces are ill-formed, it probably 
> ought to.  Regardless, if we've permitted arbitrary casts in the past, we'll 
> need to investigate the source compatibility issues before imposing 
> restrictions.
> 
> There have also been proposals for "superficial" address spaces that are 
> eliminated during lowering which might have their own restrictions.
Ok, I see. Even if they seem to cover variety of different memory 
architectures, it probably doesn't make sense to have arbitrary conversions 
indeed. I think heterogeneous C++ group was looking at regulating the address 
spaces. I will try to get in touch with them and investigate.


https://reviews.llvm.org/D52598



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

Reply via email to