[PATCH] D17412: PR19957: [OpenCL] incorrectly accepts implicit address space conversion with ternary operator

[Yaxun Liu via cfe-commits]

yaxunl marked 2 inline comments as done.

================
Comment at: lib/Sema/SemaExpr.cpp:6194-6203
@@ +6193,12 @@
+
+      incompatTy = S.Context.getPointerType(
+          S.Context.getAddrSpaceQualType(S.Context.VoidTy, ResultAddrSpace));
+      LHS = S.ImpCastExprToType(LHS.get(), incompatTy,
+                                (lhQual.getAddressSpace() != ResultAddrSpace)
+                                    ? CK_AddressSpaceConversion
+                                    : CK_BitCast);
+      RHS = S.ImpCastExprToType(RHS.get(), incompatTy,
+                                (rhQual.getAddressSpace() != ResultAddrSpace)
+                                    ? CK_AddressSpaceConversion
+                                    : CK_BitCast);
+    } else {
----------------
pxli168 wrote:
> I am quite confused by these codes. It seems in some situations you need both BitCast and AddressSpaceConversion.
> It seems the logic here is too complex. Maybe you can try to simplify it
> 
if the addr space is different, CK_AddressSpaceConversion is used, which corresponds to addrspacecast in LLVM (it is OK if pointee base types are different here, addrspacecast covers that, no need for an extra bitcast).

I've tried to simplify the logic. Any suggestion how to further simplify the logic?


================
Comment at: lib/Sema/SemaExpr.cpp:6220-6232
@@ -6186,7 +6219,15 @@
     ResultTy = S.Context.getBlockPointerType(ResultTy);
-  else
+  else {
+    auto ResultAddrSpace = ResultTy.getQualifiers().getAddressSpace();
+    LHSCastKind = lhQual.getAddressSpace() == ResultAddrSpace
+                      ? CK_BitCast
+                      : CK_AddressSpaceConversion;
+    RHSCastKind = rhQual.getAddressSpace() == ResultAddrSpace
+                      ? CK_BitCast
+                      : CK_AddressSpaceConversion;
     ResultTy = S.Context.getPointerType(ResultTy);
+  }
 
-  LHS = S.ImpCastExprToType(LHS.get(), ResultTy, CK_BitCast);
-  RHS = S.ImpCastExprToType(RHS.get(), ResultTy, CK_BitCast);
+  LHS = S.ImpCastExprToType(LHS.get(), ResultTy, LHSCastKind);
+  RHS = S.ImpCastExprToType(RHS.get(), ResultTy, RHSCastKind);
   return ResultTy;
----------------
pxli168 wrote:
> Why change about block pointer?
> Block can not be used with ternary selection operator (?:) in OpenCL.
This change is for non-block pointer. 

For cases 

global int* g;
generic int* p;
0?g:p;

Anastasia suggested to change mergeTypes to return type generic int*, then an implicit cast needs to be inserted for g, which is handled here.



Repository:
  rL LLVM

http://reviews.llvm.org/D17412