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

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

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

23

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

I think this is being tested in line 178.

214

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

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

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

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

[yaxunl]

lib/Sema/SemaExpr.cpp

6194–6203

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

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

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

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

7624

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

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

6194–6203

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

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

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

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

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

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

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

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

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

6220–6232

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

6222–6227

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

6231

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

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

lib/Sema/SemaExpr.cpp

6168

Good idea.

6222–6227

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

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

Will add comments.

6220–6232

line 6218 handles the blocks

6222–6227

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

6231

Sorry what is abode code?

test/CodeGenOpenCL/address-spaces-conversions.cl

1

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

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

As -> AS

6186

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

6190

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

6229

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

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

24

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

arg_gen -> var_gen
arg_glob -> var_glob

262

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

276

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

[yaxunl]

lib/AST/ASTContext.cpp

7613

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

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

Why removing line here?

6192

add OpenCL

[yaxunl]

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

[yaxunl]

test/CodeGenOpenCL/address-spaces-conversions.cl

1

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?

include/clang/Basic/DiagnosticSemaKinds.td

   "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|"

lib/AST/ASTContext.cpp

     LHS = LHS.getUnqualifiedType();
     RHS = RHS.getUnqualifiedType();
   }
   
   QualType LHSCan = getCanonicalType(LHS),
            RHSCan = getCanonicalType(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 &&
+        LHS.getUnqualifiedType() == RHS.getUnqualifiedType() &&
+        LQuals.getCVRQualifiers() == RQuals.getCVRQualifiers()) {
+      if (LQuals.isAddressSpaceSupersetOf(RQuals))
+        return LHS;
+      if (RQuals.isAddressSpaceSupersetOf(LQuals))
+        return RHS;
+      return QualType();
+    }
     // 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
     // okay if the other type has no GC qualifier but is an Objective
     // C object pointer (i.e. implicitly strong by default).  We fix
     // this by pretending that the unqualified type was actually
     // qualified __strong.

lib/Sema/SemaExpr.cpp

     case AR_NotYetIntroduced: {
       // With nopartial, the compiler will emit delayed error just like how
       // "deprecated, unavailable" are handled.
       AvailabilityAttr *AA = D->getAttr<AvailabilityAttr>();
       if (AA && AA->getNopartial() &&
           S.getCurContextAvailability() != AR_NotYetIntroduced)
         S.EmitAvailabilityWarning(Sema::AD_NotYetIntroduced,
                                   D, Message, Loc, UnknownObjCClass, ObjCPDecl,
                                   ObjCPropertyAccess);
 
       // Don't do this for enums, they can't be redeclared.
       if (isa<EnumConstantDecl>(D) || isa<EnumDecl>(D))
         break;
- 
       bool Warn = !AA->isInherited();
       // Objective-C method declarations in categories are not modelled as
       // redeclarations, so manually look for a redeclaration in a category
       // if necessary.
       if (Warn && HasRedeclarationWithoutAvailabilityInCategory(D))
         Warn = false;
       // In general, D will point to the most recent redeclaration. However,
       // for `@class A;` decls, this isn't true -- manually go through the

   unsigned MergedCVRQual = lhQual.getCVRQualifiers() | rhQual.getCVRQualifiers();
   lhQual.removeCVRQualifiers();
   rhQual.removeCVRQualifiers();
 
   lhptee = S.Context.getQualifiedType(lhptee.getUnqualifiedType(), lhQual);
   rhptee = S.Context.getQualifiedType(rhptee.getUnqualifiedType(), rhQual);
 
   QualType CompositeTy = S.Context.mergeTypes(lhptee, rhptee);
 
   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);
-    LHS = S.ImpCastExprToType(LHS.get(), incompatTy, CK_BitCast);
-    RHS = S.ImpCastExprToType(RHS.get(), incompatTy, CK_BitCast);
+    QualType incompatTy;
+    if (S.getLangOpts().OpenCL) {
+      // OpenCL v1.1 s6.5 - 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.
+      // OpenCL v2.0 s6.5.6 - Clause 6.5.15 Conditional operator, add another
+      // constraint paragraph: If the second and third operands are pointers
+      // into different address spaces, the address spaces must overlap.
+      unsigned ResultAddrSpace;
+      if (lhQual.isAddressSpaceSupersetOf(rhQual)) {
+        ResultAddrSpace = lhQual.getAddressSpace();
+      } else if (rhQual.isAddressSpaceSupersetOf(lhQual)) {
+        ResultAddrSpace = rhQual.getAddressSpace();
+      } else {
+        S.Diag(Loc,
+               diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
+            << LHSTy << RHSTy << 2 << LHS.get()->getSourceRange()
+            << RHS.get()->getSourceRange();
+        return QualType();
+      }
+
+      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 {
+      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 {
+    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;
 }
 
 /// \brief Return the resulting type when the operands are both block pointers.
 static QualType checkConditionalBlockPointerCompatibility(Sema &S,
                                                           ExprResult &LHS,
                                                           ExprResult &RHS,
                                                           SourceLocation Loc) {

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
 
 // test that we generate address space casts everywhere we need conversions of
 // pointers to different address spaces
 
 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)*
   arg_gen = &var_priv; // implicit cast with obtaining adr, private -> generic
   // CHECK: %{{[0-9]+}} = addrspacecast i32* %var_priv to i32 addrspace(4)*
   arg_glob = (global int *)arg_gen; // explicit cast
   // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(4)* %{{[0-9]+}} to i32 addrspace(1)*
   global int *var_glob =
       (global int *)arg_glob; // explicit cast in the same address space
   // CHECK-NOT: %{{[0-9]+}} = addrspacecast i32 addrspace(1)* %{{[0-9]+}} to i32 addrspace(1)*
   var_priv = arg_gen - arg_glob; // arithmetic operation
   // CHECK: %{{.*}} = ptrtoint i32 addrspace(4)* %{{.*}} to i64
   // CHECK: %{{.*}} = ptrtoint i32 addrspace(1)* %{{.*}} to i64
   var_priv = arg_gen > arg_glob; // comparison
   // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(1)* %{{[0-9]+}} to i32 addrspace(4)*
+  arg_gen = arg_glob ? arg_gen : arg_glob; // conditional operator
+  // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(1)* %{{[0-9]+}} to i32 addrspace(4)*
 }

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

 // expected-error-re@-2{{arithmetic operation with operands of type  ('__{{global|constant}} int *' and 'int *') which are pointers to non-overlapping address spaces}}
 #endif
 
   b = var_sub - arg_gen;
 #ifdef CONSTANT
 // expected-error@-2{{arithmetic operation with operands of type  ('__constant int *' and '__generic int *') which are pointers to non-overlapping address spaces}}
 #endif
 
+  AS int *var_cond;
+  arg_gen = 0 ? var_cond : arg_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
+
+  arg_gen = 0 ? var_cond : arg_loc;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__local int *') which are pointers to non-overlapping address spaces}}
+#elif defined(GLOBAL)
+// expected-error@-4{{conditional operator with the second and third operands of type  ('__global int *' and '__local int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  var_cond = 0 ? var_cond : arg_const;
+#ifdef GENERIC
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__generic int *' and '__constant int *') which are pointers to non-overlapping address spaces}}
+#elif defined(GLOBAL)
+// expected-error@-4{{conditional operator with the second and third operands of type  ('__global int *' and '__constant int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  arg_gen = 0 ? var_cond : arg_priv;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and 'int *') which are pointers to non-overlapping address spaces}}
+#elif defined(GLOBAL)
+// expected-error@-4{{conditional operator with the second and third operands of type  ('__global int *' and 'int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  arg_gen = 0 ? var_cond : arg_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
+
   f_glob(var_sub);
 #ifndef GLOBAL
 // expected-error-re@-2{{passing '__{{constant|generic}} int *' to parameter of type '__global int *' changes address space of pointer}}
 #endif
 
   f_loc(var_sub); // expected-error-re{{passing '__{{global|constant|generic}} int *' to parameter of type '__local int *' changes address space of pointer}}
 
   f_const(var_sub);
 #ifndef CONSTANT
 // expected-error-re@-2{{passing '__{{global|generic}} int *' to parameter of type '__constant int *' changes address space of pointer}}
 #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
 }