r297468 - [OpenCL] Fix type compatibility check and generic AS mangling.

Anastasia Stulova via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 13 03:39:38 PDT 2017


Hi Galina,

Thanks for reporting it! I believe the issue is fixed in r297530.

Cheers,
Anastasia

From: Galina Kistanova [mailto:gkistanova at gmail.com]
Sent: 10 March 2017 20:29
To: Anastasia Stulova
Cc: cfe-commits
Subject: Re: r297468 - [OpenCL] Fix type compatibility check and generic AS mangling.

Hello Anastasia,

Added test fails on one of our new builders:

Failing Tests (1):
    Clang :: SemaOpenCL/overload_addrspace_resolution.cl<http://overload_addrspace_resolution.cl>

http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/23/steps/test-check-all/logs/stdio

Please have a look at it?

Thanks

Galina

On Fri, Mar 10, 2017 at 7:23 AM, Anastasia Stulova via cfe-commits <cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>> wrote:
Author: stulova
Date: Fri Mar 10 09:23:07 2017
New Revision: 297468

URL: http://llvm.org/viewvc/llvm-project?rev=297468&view=rev
Log:
[OpenCL] Fix type compatibility check and generic AS mangling.

1. Reimplemented conditional operator so that it checks
compatibility of unqualified pointees of the 2nd and
the 3rd operands (C99, OpenCL v2.0 6.5.15).

Define QualTypes compatibility for OpenCL as following:

   - corresponding types are compatible (C99 6.7.3)
   - CVR-qualifiers are equal (C99 6.7.3)
   - address spaces are equal (implementation defined)

2. Added generic address space to Itanium mangling.

Review: D30037

Patch by Dmitry Borisenkov!


Added:
    cfe/trunk/test/SemaOpenCL/overload_addrspace_resolution.cl<http://overload_addrspace_resolution.cl>
Modified:
    cfe/trunk/lib/AST/ASTContext.cpp
    cfe/trunk/lib/AST/ItaniumMangle.cpp
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl<http://address-spaces-conversions-cl2.0.cl>

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=297468&r1=297467&r2=297468&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Fri Mar 10 09:23:07 2017
@@ -8066,15 +8066,6 @@ QualType ASTContext::mergeTypes(QualType
   Qualifiers LQuals = LHSCan.getLocalQualifiers();
   Qualifiers RQuals = RHSCan.getLocalQualifiers();
   if (LQuals != RQuals) {
-    if (getLangOpts().OpenCL) {
-      if (LHSCan.getUnqualifiedType() != RHSCan.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() ||
@@ -8200,6 +8191,20 @@ QualType ASTContext::mergeTypes(QualType
       LHSPointee = LHSPointee.getUnqualifiedType();
       RHSPointee = RHSPointee.getUnqualifiedType();
     }
+    if (getLangOpts().OpenCL) {
+      Qualifiers LHSPteeQual = LHSPointee.getQualifiers();
+      Qualifiers RHSPteeQual = RHSPointee.getQualifiers();
+      // Blocks can't be an expression in a ternary operator (OpenCL v2.0
+      // 6.12.5) thus the following check is asymmetric.
+      if (!LHSPteeQual.isAddressSpaceSupersetOf(RHSPteeQual))
+        return QualType();
+      LHSPteeQual.removeAddressSpace();
+      RHSPteeQual.removeAddressSpace();
+      LHSPointee =
+          QualType(LHSPointee.getTypePtr(), LHSPteeQual.getAsOpaqueValue());
+      RHSPointee =
+          QualType(RHSPointee.getTypePtr(), RHSPteeQual.getAsOpaqueValue());
+    }
     QualType ResultType = mergeTypes(LHSPointee, RHSPointee, OfBlockPointer,
                                      Unqualified);
     if (ResultType.isNull()) return QualType();

Modified: cfe/trunk/lib/AST/ItaniumMangle.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ItaniumMangle.cpp?rev=297468&r1=297467&r2=297468&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ItaniumMangle.cpp (original)
+++ cfe/trunk/lib/AST/ItaniumMangle.cpp Fri Mar 10 09:23:07 2017
@@ -2159,10 +2159,12 @@ void CXXNameMangler::mangleQualifiers(Qu
     } else {
       switch (AS) {
       default: llvm_unreachable("Not a language specific address space");
-      //  <OpenCL-addrspace> ::= "CL" [ "global" | "local" | "constant" ]
+      //  <OpenCL-addrspace> ::= "CL" [ "global" | "local" | "constant |
+      //                                "generic" ]
       case LangAS::opencl_global:   ASString = "CLglobal";   break;
       case LangAS::opencl_local:    ASString = "CLlocal";    break;
       case LangAS::opencl_constant: ASString = "CLconstant"; break;
+      case LangAS::opencl_generic:  ASString = "CLgeneric";  break;
       //  <CUDA-addrspace> ::= "CU" [ "device" | "constant" | "shared" ]
       case LangAS::cuda_device:     ASString = "CUdevice";   break;
       case LangAS::cuda_constant:   ASString = "CUconstant"; break;

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=297468&r1=297467&r2=297468&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Mar 10 09:23:07 2017
@@ -6335,92 +6335,98 @@ static QualType checkConditionalPointerC
   Qualifiers lhQual = lhptee.getQualifiers();
   Qualifiers rhQual = rhptee.getQualifiers();

+  unsigned ResultAddrSpace = 0;
+  unsigned LAddrSpace = lhQual.getAddressSpace();
+  unsigned RAddrSpace = rhQual.getAddressSpace();
+  if (S.getLangOpts().OpenCL) {
+    // OpenCL v1.1 s6.5 - Conversion between pointers to distinct address
+    // spaces is disallowed.
+    if (lhQual.isAddressSpaceSupersetOf(rhQual))
+      ResultAddrSpace = LAddrSpace;
+    else if (rhQual.isAddressSpaceSupersetOf(lhQual))
+      ResultAddrSpace = RAddrSpace;
+    else {
+      S.Diag(Loc,
+             diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
+          << LHSTy << RHSTy << 2 << LHS.get()->getSourceRange()
+          << RHS.get()->getSourceRange();
+      return QualType();
+    }
+  }
+
   unsigned MergedCVRQual = lhQual.getCVRQualifiers() | rhQual.getCVRQualifiers();
+  auto LHSCastKind = CK_BitCast, RHSCastKind = CK_BitCast;
   lhQual.removeCVRQualifiers();
   rhQual.removeCVRQualifiers();

+  // OpenCL v2.0 specification doesn't extend compatibility of type qualifiers
+  // (C99 6.7.3) for address spaces. We assume that the check should behave in
+  // the same manner as it's defined for CVR qualifiers, so for OpenCL two
+  // qual types are compatible iff
+  //  * corresponded types are compatible
+  //  * CVR qualifiers are equal
+  //  * address spaces are equal
+  // Thus for conditional operator we merge CVR and address space unqualified
+  // pointees and if there is a composite type we return a pointer to it with
+  // merged qualifiers.
+  if (S.getLangOpts().OpenCL) {
+    LHSCastKind = LAddrSpace == ResultAddrSpace
+                      ? CK_BitCast
+                      : CK_AddressSpaceConversion;
+    RHSCastKind = RAddrSpace == ResultAddrSpace
+                      ? CK_BitCast
+                      : CK_AddressSpaceConversion;
+    lhQual.removeAddressSpace();
+    rhQual.removeAddressSpace();
+  }
+
   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()) {
     // 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;
-    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);
-    }
+    incompatTy = S.Context.getPointerType(
+        S.Context.getAddrSpaceQualType(S.Context.VoidTy, ResultAddrSpace));
+    LHS = S.ImpCastExprToType(LHS.get(), incompatTy, LHSCastKind);
+    RHS = S.ImpCastExprToType(RHS.get(), incompatTy, RHSCastKind);
+    // FIXME: For OpenCL the warning emission and cast to void* leaves a room
+    // for casts between types with incompatible address space qualifiers.
+    // For the following code the compiler produces casts between global and
+    // local address spaces of the corresponded innermost pointees:
+    // local int *global *a;
+    // global int *global *b;
+    // a = (0 ? a : b); // see C99 6.5.16.1.p1.
+    S.Diag(Loc, diag::ext_typecheck_cond_incompatible_pointers)
+        << LHSTy << RHSTy << LHS.get()->getSourceRange()
+        << RHS.get()->getSourceRange();
     return incompatTy;
   }

   // The pointer types are compatible.
-  QualType ResultTy = CompositeTy.withCVRQualifiers(MergedCVRQual);
-  auto LHSCastKind = CK_BitCast, RHSCastKind = CK_BitCast;
+  // In case of OpenCL ResultTy should have the address space qualifier
+  // which is a superset of address spaces of both the 2nd and the 3rd
+  // operands of the conditional operator.
+  QualType ResultTy = [&, ResultAddrSpace]() {
+    if (S.getLangOpts().OpenCL) {
+      Qualifiers CompositeQuals = CompositeTy.getQualifiers();
+      CompositeQuals.setAddressSpace(ResultAddrSpace);
+      return S.Context
+          .getQualifiedType(CompositeTy.getUnqualifiedType(), CompositeQuals)
+          .withCVRQualifiers(MergedCVRQual);
+    } else
+      return CompositeTy.withCVRQualifiers(MergedCVRQual);
+  }();
   if (IsBlockPointer)
     ResultTy = S.Context.getBlockPointerType(ResultTy);
   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);
   }

-  // 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;
@@ -7399,7 +7405,22 @@ checkBlockPointerTypesForAssignment(Sema
   if (LQuals != RQuals)
     ConvTy = Sema::CompatiblePointerDiscardsQualifiers;

-  if (!S.Context.typesAreBlockPointerCompatible(LHSType, RHSType))
+  // FIXME: OpenCL doesn't define the exact compile time semantics for a block
+  // assignment.
+  // The current behavior is similar to C++ lambdas. A block might be
+  // assigned to a variable iff its return type and parameters are compatible
+  // (C99 6.2.7) with the corresponding return type and parameters of the LHS of
+  // an assignment. Presumably it should behave in way that a function pointer
+  // assignment does in C, so for each parameter and return type:
+  //  * CVR and address space of LHS should be a superset of CVR and address
+  //  space of RHS.
+  //  * unqualified types should be compatible.
+  if (S.getLangOpts().OpenCL) {
+    if (!S.Context.typesAreBlockPointerCompatible(
+            S.Context.getQualifiedType(LHSType.getUnqualifiedType(), LQuals),
+            S.Context.getQualifiedType(RHSType.getUnqualifiedType(), RQuals)))
+      return Sema::IncompatibleBlockPointer;
+  } else if (!S.Context.typesAreBlockPointerCompatible(LHSType, RHSType))
     return Sema::IncompatibleBlockPointer;

   return ConvTy;

Modified: cfe/trunk/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl<http://address-spaces-conversions-cl2.0.cl>
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl?rev=297468&r1=297467&r2=297468&view=diff
==============================================================================
--- cfe/trunk/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl<http://address-spaces-conversions-cl2.0.cl> (original)
+++ cfe/trunk/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl<http://address-spaces-conversions-cl2.0.cl> Fri Mar 10 09:23:07 2017
@@ -18,14 +18,20 @@

 #ifdef GENERIC
 #define AS generic
+#define AS_COMP local
+#define AS_INCOMP constant
 #endif

 #ifdef GLOBAL
 #define AS global
+#define AS_COMP global
+#define AS_INCOMP local
 #endif

 #ifdef CONSTANT
 #define AS constant
+#define AS_COMP constant
+#define AS_INCOMP global
 #endif

 void f_glob(global int *arg_glob) {}
@@ -263,12 +269,16 @@ void test_ternary() {
   var_void_gen = 0 ? var_cond : var_glob_ch;
 #ifdef CONSTANT
 // expected-error at -2{{conditional operator with the second and third operands of type  ('__constant int *' and '__global char *') which are pointers to non-overlapping address spaces}}
+#else
+// expected-warning-re at -4{{pointer type mismatch ('__{{global|generic}} int *' and '__global char *')}}
 #endif

   local char *var_loc_ch;
   var_void_gen = 0 ? var_cond : var_loc_ch;
 #ifndef GENERIC
 // expected-error-re at -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}}
+#else
+// expected-warning at -4{{pointer type mismatch ('__generic int *' and '__local char *')}}
 #endif

   constant void *var_void_const;
@@ -276,18 +286,45 @@ void test_ternary() {
   var_void_const = 0 ? var_cond : var_const_ch;
 #ifndef CONSTANT
 // expected-error-re at -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}}
+#else
+// expected-warning at -4{{pointer type mismatch ('__constant int *' and '__constant char *')}}
 #endif

   private char *var_priv_ch;
   var_void_gen = 0 ? var_cond : var_priv_ch;
 #ifndef GENERIC
 // expected-error-re at -2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and 'char *') which are pointers to non-overlapping address spaces}}
+#else
+// expected-warning at -4{{pointer type mismatch ('__generic int *' and 'char *')}}
 #endif

   generic char *var_gen_ch;
   var_void_gen = 0 ? var_cond : var_gen_ch;
 #ifdef CONSTANT
 // expected-error at -2{{conditional operator with the second and third operands of type  ('__constant int *' and '__generic char *') which are pointers to non-overlapping address spaces}}
+#else
+// expected-warning-re at -4{{pointer type mismatch ('__{{global|generic}} int *' and '__generic char *')}}
 #endif
 }

+void test_pointer_chains() {
+  AS int *AS *var_as_as_int;
+  AS int *AS_COMP *var_asc_as_int;
+  AS_INCOMP int *AS_COMP *var_asc_asn_int;
+  AS_COMP int *AS_COMP *var_asc_asc_int;
+
+  // Case 1:
+  //  * address spaces of corresponded most outer pointees overlaps, their canonical types are equal
+  //  * CVR, address spaces and canonical types of the rest of pointees are equivalent.
+  var_as_as_int = 0 ? var_as_as_int : var_asc_as_int;
+
+  // Case 2: Corresponded inner pointees has non-overlapping address spaces.
+  var_as_as_int = 0 ? var_as_as_int : var_asc_asn_int;
+// expected-warning-re at -1{{pointer type mismatch ('__{{(generic|global|constant)}} int *__{{(generic|global|constant)}} *' and '__{{(local|global|constant)}} int *__{{(constant|local|global)}} *')}}
+
+  // Case 3: Corresponded inner pointees has overlapping but not equivalent address spaces.
+#ifdef GENERIC
+  var_as_as_int = 0 ? var_as_as_int : var_asc_asc_int;
+// expected-warning-re at -1{{pointer type mismatch ('__{{(generic|global|constant)}} int *__{{(generic|global|constant)}} *' and '__{{(local|global|constant)}} int *__{{(local|global|constant)}} *')}}
+#endif
+}

Added: cfe/trunk/test/SemaOpenCL/overload_addrspace_resolution.cl<http://overload_addrspace_resolution.cl>
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/overload_addrspace_resolution.cl?rev=297468&view=auto
==============================================================================
--- cfe/trunk/test/SemaOpenCL/overload_addrspace_resolution.cl<http://overload_addrspace_resolution.cl> (added)
+++ cfe/trunk/test/SemaOpenCL/overload_addrspace_resolution.cl<http://overload_addrspace_resolution.cl> Fri Mar 10 09:23:07 2017
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -cl-std=CL2.0 -emit-llvm -o - %s | FileCheck %s
+
+void __attribute__((overloadable)) foo(global int *a, global int *b);
+void __attribute__((overloadable)) foo(generic int *a, generic int *b);
+void __attribute__((overloadable)) bar(generic int *global *a, generic int *global *b);
+void __attribute__((overloadable)) bar(generic int *generic *a, generic int *generic *b);
+
+void kernel ker() {
+  global int *a;
+  global int *b;
+  generic int *c;
+  local int *d;
+  generic int *generic *gengen;
+  generic int *local *genloc;
+  generic int *global *genglob;
+  // CHECK: call void @_Z3fooPU8CLglobaliS0_(i32* undef, i32* undef)
+  foo(a, b);
+  // CHECK: call void @_Z3fooPU9CLgenericiS0_(i32* undef, i32* undef)
+  foo(b, c);
+  // CHECK: call void @_Z3fooPU9CLgenericiS0_(i32* undef, i32* undef)
+  foo(a, d);
+
+  // CHECK: call void @_Z3barPU9CLgenericPU9CLgenericiS2_(i32** undef, i32** undef)
+  bar(gengen, genloc);
+  // CHECK: call void @_Z3barPU9CLgenericPU9CLgenericiS2_(i32** undef, i32** undef)
+  bar(gengen, genglob);
+  // CHECK: call void @_Z3barPU8CLglobalPU9CLgenericiS2_(i32** undef, i32** undef)
+  bar(genglob, genglob);
+}


_______________________________________________
cfe-commits mailing list
cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170313/4ab65fe4/attachment-0001.html>


More information about the cfe-commits mailing list