[PATCH] D53764: [OpenCL] Enable address spaces for references in C++

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 1 09:34:31 PDT 2018


rjmccall added inline comments.


================
Comment at: lib/AST/Expr.cpp:1609
   case CK_AddressSpaceConversion:
-    assert(getType()->isPointerType() || getType()->isBlockPointerType());
-    assert(getSubExpr()->getType()->isPointerType() ||
-           getSubExpr()->getType()->isBlockPointerType());
-    assert(getType()->getPointeeType().getAddressSpace() !=
-           getSubExpr()->getType()->getPointeeType().getAddressSpace());
-    LLVM_FALLTHROUGH;
+    assert(/*If pointer type then addr spaces for pointees must differ*/
+           (((getType()->isPointerType() &&
----------------
rjmccall wrote:
> Anastasia wrote:
> > I don't like this assert now. Would adding extra variable be cleaner here?
> Yeah, this assertion doesn't make any sense like this.  It should be checking whether the cast is a gl-value and, if so, requiring the subexpression to also be a gl-value and then asserting the difference between the type.  But you can certainly do an address-space conversion on l-values that just happen to be of pointer or block-pointer type.
No, if this is a gl-value cast, the assertion must ignore whether there's a pointee type, or it will be messed up on gl-values of pointer types.

That is, if I have a gl-value of type `char * __private`, I should be able to do an address-space promotion to get a gl-value of type `char * __generic`.  It's okay that the pointers are into the same address space here — in fact, it's more than okay, it's necessary.


https://reviews.llvm.org/D53764





More information about the cfe-commits mailing list