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

Galina Kistanova via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 10 12:29:21 PST 2017


Hello Anastasia,

Added test fails on one of our new builders:

Failing Tests (1):
    Clang :: SemaOpenCL/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> 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
> 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
>
> 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
> 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
> (original)
> +++ cfe/trunk/test/SemaOpenCL/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
> 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 (added)
> +++ cfe/trunk/test/SemaOpenCL/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
> 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/20170310/2db0a6f1/attachment-0001.html>


More information about the cfe-commits mailing list