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

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 9 09:42:17 PST 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());
+    assert(/*If pointer type then addr spaces for pointees must differ*/
+           (((getType()->isPointerType() &&
rjmccall wrote:
> 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.
Thanks, that's right now.  Although please assert that the base has the same value kind; I've seen bugs before where ICEs tried to implicitly materialize their arguments, and it's really frustrating to root out.

Comment at: lib/Sema/SemaExprCXX.cpp:4285
+            ? CK_AddressSpaceConversion
+            : CK_NoOp;
If `ToType` is a reference type, the address space will be on its pointee type.


More information about the cfe-commits mailing list