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

[yaxunl]

Summary

Generates addrcast instead of bitcast for ternary operator when necessary, and diagnose ternary operator with incompatible second and third operands according to OpenCL v2.0 s6.5.6.

https://llvm.org/bugs/show_bug.cgi?id=19957

Differential Revision: http://reviews.llvm.org/D17412

[Anastasia]

I think we will also need tests for non-CL2.0 part.

Thanks!

lib/Sema/SemaExpr.cpp

6196 ↗

(On Diff #48398)

Do we actually need a bitcast?

I think we could do a better handling of address spaces here, because now you generate a cast to void* even though both pointers are of the same actual type and only differ by an address space.

Essentially if:

local int* l = ...;
generic int* g = ...;

you now generate a cast of l -> generic void* and a cast of g -> generic void*. While it would be better to cast l -> generic int* and avoid casting g completely.

It's because inside the ASTContext::mergeTypes() we have a check:

LQuals.getAddressSpace() != RQuals.getAddressSpace()

which is wrong! We should use isAddressSpaceSupersetOf() helper instead. Then we should get a non-NULL composite type for the line above:

QualType CompositeTy = S.Context.mergeTypes(lhptee, rhptee);

After what some more code would be needed in mergeTypes to merge address space qualifiers correctly and below here:

LHS = S.ImpCastExprToType(LHS.get(), ResultTy, CK_BitCast);
RHS = S.ImpCastExprToType(RHS.get(), ResultTy, CK_BitCast);

To change CK_BitCast -> CK_AddressSpaceConversion if needed.

Do you think you could try that? Because currently it's still doesn't work correctly as it generated void* casts and also gives bad error:

test.cl:2:11: error: assigning '__generic void *' to '__global int *' changes address space of pointer
arg_glob  = arg_glob ? arg_gen : arg_glob;

The error doesn't show the types right.

test/CodeGenOpenCL/address-spaces-conversions.cl

1 ↗

(On Diff #48398)

if you remove -ffake-address-space-map your test will result in ICE. Could you look into it?

23 ↗

(On Diff #48398)

I think the right thing to do is to cast to the same type in the generic AS and not to void* in generic AS.

test/SemaOpenCL/address-spaces-conversions-cl2.0.cl

209 ↗

(On Diff #48398)

I think this is being tested in line 178.

214 ↗

(On Diff #48398)

Could we test all combinations of address spaces? I think this test is generally doing that. You might just need to adjust your code.

[Anastasia]

lib/Sema/SemaExpr.cpp

6174 ↗

(On Diff #48398)

I think this fix should be done for all OpenCL versions not just OpenCL 2.0.

We can quote s6.5 of OpenCL v1.1 here as well:
"A pointer to address space A can only be assigned to a pointer to the same address space A. Casting a pointer to address space A to a pointer to address space B is illegal."

Also there is a helper function isAddressSpaceOverlapping(...) of the PointerType class that can do this check. It also takes into account OpenCL version.

[Anastasia]

This review doesn't have cfe-commits in subscribers. Could you please add?

Thanks!

[yaxunl]

Revised as Anastasia suggested.

Modified ASTContext::mergeTypes to handle types with different address space properly.
Added more sema tests.

[pxli168]

lib/Sema/SemaExpr.cpp

6194–6203 ↗

(On Diff #48967)

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

6220–6232 ↗

(On Diff #48967)

Why change about block pointer?
Block can not be used with ternary selection operator (?:) in OpenCL.

[yaxunl]

lib/Sema/SemaExpr.cpp

6194–6203 ↗

(On Diff #48967)

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?

6220–6232 ↗

(On Diff #48967)

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.

[Anastasia]

I think we are not covering all the possible cases with tests now! May be we could also create a separate cl file since it becomes quite large.

lib/AST/ASTContext.cpp

7605 ↗

(On Diff #48967)

I feel like the AS check should be lifted here instead, because here we check unqualified types and then return LHS type. In OpenCL we have to return either LHS or RHS type just like you do below if AS overlap.

7616 ↗

(On Diff #48967)

I think this should be rather done above (see comment to line 7605) w/o CVR Quals check.

7624 ↗

(On Diff #48967)

We should add !OpenCL here. Because for OpenCL this check is wrong to do.

But I am not sure whether we need to add an OpenCL check here though, as we don't seem to return any type but void for OpenCL after this statement anyways.

However, we might return 'generic void*' if AS overlap, instead of 'private void*' as we do now. Would this make more sense?

lib/Sema/SemaExpr.cpp

6182 ↗

(On Diff #48967)

if we return generic pointer type in mergeTypes, we won't need ResultAddrSpace as it will be returned in CompisiteTy.

6194–6203 ↗

(On Diff #48967)

Yes, it's a bit complicated here because we are trying to extend C rules.

In general we might have the following situations:

1. If LHS and RHS types match exactly and:

(a) AS match => use standard C rules, no bitcast or addrspacecast
(b) AS overlap => generate addrspacecast
(c) As don't overlap => give an error

2. if LHS and RHS types don't match:

(a) AS match => use standard C rules, generate bitcast
(b) AS overlap => generate addrspacecast instead of bitcast
(c) AS don't overlap => give an error

I think however we are missing testing all of the cases at the moment. Could you please add more tests!

[yaxunl]

Revised as Anastasis suggested.

Modified mergeTypes() for un-handled case.
Separate sema tests for condition operator to a new file.

[yaxunl]

lib/AST/ASTContext.cpp

7605 ↗

(On Diff #48967)

Here is still checking qualified type since 'Unqualified' is false. So this condition is for the case when the two types are the same.

7624 ↗

(On Diff #48967)

I think I need to handle all OpenCL cases above.

1. if types match, return LHS
2. if types differ, but addr spaces overlap and cvs qual match, return type with bigger addr space
3. otherwise return empty type

then we don't need check OpenCL here.

lib/Sema/SemaExpr.cpp

6182 ↗

(On Diff #48967)

This part handles the case when mergeTypes() returns an empty type. Since the returned type is empty, we still need to find the result addr space.

This happens if the addr spaces are not overlapping or the unqualified pointee types are different.

For non-OpenCL case, Clang will insert implicit cast to void* for the two operands.

For OpenCL, we check the addr spaces. If they overlap, that means the unqualified pointee types are different, e.g.

global int* a;
generic char *b;
0?a:b;

in this case, to mimic the original Clang behavior, we insert casts so that we get 0?(generic void*)a:(generic void*)b.

6194–6203 ↗

(On Diff #48967)

I think we are missing tests for 2a 2b 2c. I will add them.

[pxli168]

The logic is still to complex and I hope it can be optimized.

lib/Sema/SemaExpr.cpp

6222–6227 ↗

(On Diff #49232)

What will mergetypes return?
It seems the LHS and RHS are compatibel here, and may be they did not need bitcast?

[Anastasia]

I think it's hard to write this simpler as logic is quite complicated. But we can definitely improve understanding with comments!

Btw, regarding tests to cover case 2, could we also add test with types in which CV qualifiers differ. May be just at least one. No need to test every possible combination.

lib/AST/ASTContext.cpp

7613 ↗

(On Diff #49232)

Do you think we should check the types here? I was thinking we should do the check exactly as below apart from AS specific part.

lib/Sema/SemaExpr.cpp

6168 ↗

(On Diff #49232)

Could we instead add a comment explaining different cases with AS we can have here i.e. 1(a-c)&2(a-c)!

And may be we could refer to each case by adding comments in code below.

6175 ↗

(On Diff #49232)

Could we write it shorter? For example: Conversion between pointers to distinct address spaces is disallowed...

6220–6232 ↗

(On Diff #49232)

There are still blocks in ObjC, we shouldn't change!

6222–6227 ↗

(On Diff #49232)

I think we always need a cast here, because types are not exactly the same at this point!

6231 ↗

(On Diff #49232)

Btw, could we do the same thing with {LHS/RHS}CastKind variables in abode code. It looks cleaner I think.

test/SemaOpenCL/condition-operator-cl2.0.cl

19 ↗

(On Diff #49232)

May be we could rename it to test_ternary and also merge with test/SemaOpenCL/address-spaces-conversions-cl2.0.cl?

20 ↗

(On Diff #49232)

Too many arguments. Could we move some into local variables instead?

33 ↗

(On Diff #49232)

You can combine these two errors into 1 using regex. See examples in function test_conversion of test/SemaOpenCL/address-spaces-conversions-cl2.0.cl

57 ↗

(On Diff #49232)

Could you declare just before the line using it?

[pxli168]

lib/AST/ASTContext.cpp

7613 ↗

(On Diff #49232)

I think the mergeType function it very complex, too. It seems to check type can be merged recursively later.

lib/Sema/SemaExpr.cpp

6168 ↗

(On Diff #49232)

Good idea.

6222–6227 ↗

(On Diff #49232)

I tried to figure out what mergetypes will return and find it seems to have logic far more complex than this.

[yaxunl]

lib/AST/ASTContext.cpp

7613 ↗

(On Diff #49232)

Here if unqualified types are different or CVS qualifiers are different, the two types cannot be merged, so an empty QualType is returned.

If unqualified types and CVS qualifiers are the same, we check if address spaces are compatible and returns the merged type.

lib/Sema/SemaExpr.cpp

6168 ↗

(On Diff #49232)

Will add comments.

6220–6232 ↗

(On Diff #49232)

line 6218 handles the blocks

6222–6227 ↗

(On Diff #49232)

ImpCastExprToType handles the case when the target type matches the value, it simply does not generate the cast.

6231 ↗

(On Diff #49232)

Sorry what is abode code?

test/CodeGenOpenCL/address-spaces-conversions.cl

1 ↗

(On Diff #49232)

it seems to be separate issue. I will take a look in another patch

[yaxunl]

Added comments. Revised test.

[Anastasia]

LG, apart from small comments mentioned here.

lib/AST/ASTContext.cpp

7613 ↗

(On Diff #51460)

Here if unqualified types are different

I think this check is redundant considering that we make check of canonical types equivalence in line 7605. Also it doesn't really have anything to do with any OpenCL specific rule. Therefore I would remove this check and just merge with lines 7623 - 7624 as much as possible.

or CVS qualifiers are different, the two types cannot be merged

The same is already being checked in line 7623. Could we merge with that code?

lib/Sema/SemaExpr.cpp

6171 ↗

(On Diff #51460)

As -> AS

6186 ↗

(On Diff #51460)

Could you remove "Conditional operator, add another constraint paragraph: "

6190 ↗

(On Diff #51460)

Could you add a comment referring to the case from 1(a-c)&2(a-c) you are handling here!

6229 ↗

(On Diff #51460)

The same here - could you add a comment explaining the case from 1(a-c)&2(a-c) being handled!

test/CodeGenOpenCL/address-spaces-conversions.cl

1 ↗

(On Diff #51460)

Cool, thanks! Could you insert a link to the new review here if possible.

37 ↗

(On Diff #51460)

Could we also add case 1a and 2a to test that we don't affect standard cases.

test/SemaOpenCL/address-spaces-conversions-cl2.0.cl

231 ↗

(On Diff #51460)

arg_gen -> var_gen
arg_glob -> var_glob

262 ↗

(On Diff #51460)

I think you should change all arg to var in the name as it mean argument, but it's just a variable now.

276 ↗

(On Diff #51460)

btw, what happens if we assign into non void* var? Do we get another error?

[yaxunl]

lib/AST/ASTContext.cpp

7613 ↗

(On Diff #51460)

The check for unqualified type is not redundant.

Let's say global int and generic float gets here. If we don't check unqualified type, we will get a non-null merged type, which is not correct.

It seems to be cleaner to keep the OpenCL logic separate from line 7623.

[yaxunl]

Revised by Anastasia's comments.

Added comments to code for cases 1a-c and 2a-c.
Added codegen test for missing cases of 1a-b and 2a-b.
Rename variables in sema test.

test/SemaOpenCL/address-spaces-conversions-cl2.0.cl

276 ↗

(On Diff #51460)

No error, since void pointer can be casted to non-void pointer implicitly in C.

[Anastasia]

LGTM, please correct two small issues commented here!

lib/Sema/SemaExpr.cpp

169 ↗

(On Diff #52073)

Why removing line here?

6192 ↗

(On Diff #52073)

add OpenCL

[yaxunl]

Add back a blank line deleted by accident.
Add OpenCL to a comment.

[yaxunl]

test/CodeGenOpenCL/address-spaces-conversions.cl

1 ↗

(On Diff #51460)

I looked into the ICE when -ffake-address-space-map is not set.

when -ffake-address-space-map is set, address space 0-4 mapped to 0-4. When it is not set, it maps to values defined by target. x86 uses DefaultAddrSpaceMap which maps 0-4 all to 0. This causes assertion for testcase like

global int *a;
generic void *b = a;

A review is opened:

http://reviews.llvm.org/D18713

[Anastasia]

Cool, thanks! I think we should commit this ASAP.

@Xiuli/@Matt, do you have any more comments here?

[Matt]

In D17412#391322, @Anastasia wrote:

Cool, thanks! I think we should commit this ASAP.

@Xiuli/@Matt, do you have any more comments here?

Hi! I think that "Matt" for this one would be @arsenm :-)

[pxli168]

This comment was removed by pxli168.

[pxli168]

LGTM!
Thanks!

[Anastasia]

In D17412#391351, @Matt wrote:

In D17412#391322, @Anastasia wrote:

Cool, thanks! I think we should commit this ASAP.

@Xiuli/@Matt, do you have any more comments here?

Hi! I think that "Matt" for this one would be @arsenm :-)

Oh, sure. Thanks for pointing this out!!!

@arsenm, if you don't have any objections, could we commit this?

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def err_cond_voidptr_arc : Error <
   "in ARC mode">;
 def err_typecheck_comparison_of_distinct_pointers : Error<
   "comparison of distinct pointer types%diff{ ($ and $)|}0,1">;
 def ext_typecheck_comparison_of_distinct_pointers_nonstandard : ExtWarn<
   "comparison of distinct pointer types (%0 and %1) uses non-standard "
   "composite pointer type %2">, InGroup<CompareDistinctPointerType>;
 def err_typecheck_op_on_nonoverlapping_address_space_pointers : Error<
   "%select{comparison between %diff{ ($ and $)|}0,1"
-  "|arithmetic operation with operands of type %diff{ ($ and $)|}0,1}2"
+  "|arithmetic operation with operands of type %diff{ ($ and $)|}0,1"
+  "|conditional operator with the second and third operands of type "
+  "%diff{ ($ and $)|}0,1}2"
   " which are pointers to non-overlapping address spaces">;
 
 def err_typecheck_assign_const : Error<
   "%select{"
   "cannot assign to return value because function %1 returns a const value|"
   "cannot assign to variable %1 with const-qualified type %2|"
   "cannot assign to %select{non-|}1static data member %2 "
   "with const-qualified type %3|"

cfe/trunk/lib/AST/ASTContext.cpp

QualType ASTContext::mergeTypes(QualType LHS, QualType RHS,
   // If two types are identical, they are compatible.
   if (LHSCan == RHSCan)
     return LHS;
 
   // If the qualifiers are different, the types aren't compatible... mostly.
   Qualifiers LQuals = LHSCan.getLocalQualifiers();
   Qualifiers RQuals = RHSCan.getLocalQualifiers();
   if (LQuals != RQuals) {
+    if (getLangOpts().OpenCL) {
+      if (LHS.getUnqualifiedType() != RHS.getUnqualifiedType() ||
+          LQuals.getCVRQualifiers() != RQuals.getCVRQualifiers())
+        return QualType();
+      if (LQuals.isAddressSpaceSupersetOf(RQuals))
+        return LHS;
+      if (RQuals.isAddressSpaceSupersetOf(LQuals))
+        return RHS;
+    }
     // If any of these qualifiers are different, we have a type
     // mismatch.
     if (LQuals.getCVRQualifiers() != RQuals.getCVRQualifiers() ||
         LQuals.getAddressSpace() != RQuals.getAddressSpace() ||
         LQuals.getObjCLifetime() != RQuals.getObjCLifetime())
       return QualType();
 
     // Exactly one GC qualifier difference is allowed: __strong is

cfe/trunk/lib/Sema/SemaExpr.cpp

static QualType checkConditionalPointerCompatibility(Sema &S, ExprResult &LHS,
 
   unsigned MergedCVRQual = lhQual.getCVRQualifiers() | rhQual.getCVRQualifiers();
   lhQual.removeCVRQualifiers();
   rhQual.removeCVRQualifiers();
 
   lhptee = S.Context.getQualifiedType(lhptee.getUnqualifiedType(), lhQual);
   rhptee = S.Context.getQualifiedType(rhptee.getUnqualifiedType(), rhQual);
 
+  // For OpenCL:
+  // 1. If LHS and RHS types match exactly and:
+  //  (a) AS match => use standard C rules, no bitcast or addrspacecast
+  //  (b) AS overlap => generate addrspacecast
+  //  (c) AS don't overlap => give an error
+  // 2. if LHS and RHS types don't match:
+  //  (a) AS match => use standard C rules, generate bitcast
+  //  (b) AS overlap => generate addrspacecast instead of bitcast
+  //  (c) AS don't overlap => give an error
+
+  // For OpenCL, non-null composite type is returned only for cases 1a and 1b.
   QualType CompositeTy = S.Context.mergeTypes(lhptee, rhptee);
 
+  // OpenCL cases 1c, 2a, 2b, and 2c.
   if (CompositeTy.isNull()) {
-    S.Diag(Loc, diag::ext_typecheck_cond_incompatible_pointers)
-      << LHSTy << RHSTy << LHS.get()->getSourceRange()
-      << RHS.get()->getSourceRange();
     // In this situation, we assume void* type. No especially good
     // reason, but this is what gcc does, and we do have to pick
     // to get a consistent AST.
-    QualType incompatTy = S.Context.getPointerType(S.Context.VoidTy);
+    QualType incompatTy;
+    if (S.getLangOpts().OpenCL) {
+      // OpenCL v1.1 s6.5 - Conversion between pointers to distinct address
+      // spaces is disallowed.
+      unsigned ResultAddrSpace;
+      if (lhQual.isAddressSpaceSupersetOf(rhQual)) {
+        // Cases 2a and 2b.
+        ResultAddrSpace = lhQual.getAddressSpace();
+      } else if (rhQual.isAddressSpaceSupersetOf(lhQual)) {
+        // Cases 2a and 2b.
+        ResultAddrSpace = rhQual.getAddressSpace();
+      } else {
+        // Cases 1c and 2c.
+        S.Diag(Loc,
+               diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
+            << LHSTy << RHSTy << 2 << LHS.get()->getSourceRange()
+            << RHS.get()->getSourceRange();
+        return QualType();
+      }
+
+      // Continue handling cases 2a and 2b.
+      incompatTy = S.Context.getPointerType(
+          S.Context.getAddrSpaceQualType(S.Context.VoidTy, ResultAddrSpace));
+      LHS = S.ImpCastExprToType(LHS.get(), incompatTy,
+                                (lhQual.getAddressSpace() != ResultAddrSpace)
+                                    ? CK_AddressSpaceConversion /* 2b */
+                                    : CK_BitCast /* 2a */);
+      RHS = S.ImpCastExprToType(RHS.get(), incompatTy,
+                                (rhQual.getAddressSpace() != ResultAddrSpace)
+                                    ? CK_AddressSpaceConversion /* 2b */
+                                    : CK_BitCast /* 2a */);
+    } else {
+      S.Diag(Loc, diag::ext_typecheck_cond_incompatible_pointers)
+          << LHSTy << RHSTy << LHS.get()->getSourceRange()
+          << RHS.get()->getSourceRange();
+      incompatTy = S.Context.getPointerType(S.Context.VoidTy);
       LHS = S.ImpCastExprToType(LHS.get(), incompatTy, CK_BitCast);
       RHS = S.ImpCastExprToType(RHS.get(), incompatTy, CK_BitCast);
+    }
     return incompatTy;
   }
 
   // The pointer types are compatible.
   QualType ResultTy = CompositeTy.withCVRQualifiers(MergedCVRQual);
+  auto LHSCastKind = CK_BitCast, RHSCastKind = CK_BitCast;
   if (IsBlockPointer)
     ResultTy = S.Context.getBlockPointerType(ResultTy);
-  else
+  else {
+    // Cases 1a and 1b for OpenCL.
+    auto ResultAddrSpace = ResultTy.getQualifiers().getAddressSpace();
+    LHSCastKind = lhQual.getAddressSpace() == ResultAddrSpace
+                      ? CK_BitCast /* 1a */
+                      : CK_AddressSpaceConversion /* 1b */;
+    RHSCastKind = rhQual.getAddressSpace() == ResultAddrSpace
+                      ? CK_BitCast /* 1a */
+                      : CK_AddressSpaceConversion /* 1b */;
     ResultTy = S.Context.getPointerType(ResultTy);
+  }
 
-  LHS = S.ImpCastExprToType(LHS.get(), ResultTy, CK_BitCast);
-  RHS = S.ImpCastExprToType(RHS.get(), ResultTy, CK_BitCast);
+  // For case 1a of OpenCL, S.ImpCastExprToType will not insert bitcast
+  // if the target type does not change.
+  LHS = S.ImpCastExprToType(LHS.get(), ResultTy, LHSCastKind);
+  RHS = S.ImpCastExprToType(RHS.get(), ResultTy, RHSCastKind);
   return ResultTy;
 }
 
 /// \brief Return the resulting type when the operands are both block pointers.
 static QualType checkConditionalBlockPointerCompatibility(Sema &S,
                                                           ExprResult &LHS,
                                                           ExprResult &RHS,
                                                           SourceLocation Loc) {

cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl

 // RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 -ffake-address-space-map -cl-std=CL2.0 -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 -cl-std=CL2.0 -emit-llvm -o - | FileCheck --check-prefix=CHECK-NOFAKE %s
 // When -ffake-address-space-map is not used, all addr space mapped to 0 for x86_64.
 
 // test that we generate address space casts everywhere we need conversions of
 // pointers to different address spaces
 
+// CHECK: define void @test
 void test(global int *arg_glob, generic int *arg_gen) {
   int var_priv;
   arg_gen = arg_glob; // implicit cast global -> generic
   // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(1)* %{{[0-9]+}} to i32 addrspace(4)*
   // CHECK-NOFAKE-NOT: addrspacecast
 
   arg_gen = &var_priv; // implicit cast with obtaining adr, private -> generic
   // CHECK: %{{[0-9]+}} = addrspacecast i32* %var_priv to i32 addrspace(4)*
void test(global int *arg_glob, generic int *arg_gen) {
   // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(1)* %{{[0-9]+}} to i32 addrspace(4)*
 
   generic void *var_gen_v = arg_glob;
   // CHECK: addrspacecast
   // CHECK-NOT: bitcast
   // CHECK-NOFAKE: bitcast
   // CHECK-NOFAKE-NOT: addrspacecast
 }
+
+// Test ternary operator.
+// CHECK: define void @test_ternary
+void test_ternary(void) {
+  global int *var_glob;
+  generic int *var_gen;
+  generic int *var_gen2;
+  generic float *var_gen_f;
+  generic void *var_gen_v;
+
+  var_gen = var_gen ? var_gen : var_gen2; // operands of the same addr spaces and the same type
+  // CHECK: icmp
+  // CHECK-NOT: addrspacecast
+  // CHECK-NOT: bitcast
+  // CHECK: phi
+  // CHECK: store i32 addrspace(4)* %{{.+}}, i32 addrspace(4)** %{{.+}}
+
+  var_gen = var_gen ? var_gen : var_glob; // operands of overlapping addr spaces and the same type
+  // CHECK: icmp
+  // CHECK-NOT: bitcast
+  // CHECK: %{{.+}} = addrspacecast i32 addrspace(1)* %{{.+}} to i32 addrspace(4)*
+  // CHECK: phi
+  // CHECK: store
+  
+  var_gen_v = var_gen ? var_gen : var_gen_f; // operands of the same addr space and different types
+  // CHECK: icmp
+  // CHECK: %{{.+}} = bitcast i32 addrspace(4)* %{{.+}} to i8 addrspace(4)*
+  // CHECK: %{{.+}} = bitcast float addrspace(4)* %{{.+}} to i8 addrspace(4)*
+  // CHECK: phi
+  // CHECK: store
+
+  var_gen_v = var_gen ? var_glob : var_gen_f; // operands of overlapping addr spaces and different types
+  // CHECK: icmp
+  // CHECK: %{{.+}} = addrspacecast i32 addrspace(1)* %{{.+}} to i8 addrspace(4)*
+  // CHECK: %{{.+}} = bitcast float addrspace(4)* %{{.+}} to i8 addrspace(4)*
+  // CHECK: phi
+  // CHECK: store
+}

cfe/trunk/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl

#endif
 
   f_priv(var_sub); // expected-error-re{{passing '__{{global|constant|generic}} int *' to parameter of type 'int *' changes address space of pointer}}
 
   f_gen(var_sub);
 #ifdef CONSTANT
 // expected-error@-2{{passing '__constant int *' to parameter of type '__generic int *' changes address space of pointer}}
 #endif
 }
+
+void test_ternary() {
+  AS int *var_cond;
+  generic int *var_gen;
+  global int *var_glob;
+  var_gen = 0 ? var_cond : var_glob;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__global int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  local int *var_loc;
+  var_gen = 0 ? var_cond : var_loc;
+#ifndef GENERIC
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and '__local int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  constant int *var_const;
+  var_cond = 0 ? var_cond : var_const;
+#ifndef CONSTANT
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|generic}} int *' and '__constant int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  private int *var_priv;
+  var_gen = 0 ? var_cond : var_priv;
+#ifndef GENERIC
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and 'int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  var_gen = 0 ? var_cond : var_gen;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__generic int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  void *var_void_gen;
+  global char *var_glob_ch;
+  var_void_gen = 0 ? var_cond : var_glob_ch;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__global char *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  local char *var_loc_ch;
+  var_void_gen = 0 ? var_cond : var_loc_ch;
+#ifndef GENERIC
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and '__local char *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  constant void *var_void_const;
+  constant char *var_const_ch;
+  var_void_const = 0 ? var_cond : var_const_ch;
+#ifndef CONSTANT
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|generic}} int *' and '__constant char *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  private char *var_priv_ch;
+  var_void_gen = 0 ? var_cond : var_priv_ch;
+#ifndef GENERIC
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and 'char *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  generic char *var_gen_ch;
+  var_void_gen = 0 ? var_cond : var_gen_ch;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__generic char *') which are pointers to non-overlapping address spaces}}
+#endif
+}
+