From: John McCall Date: Tue, 4 Feb 2014 23:58:19 +0000 (+0000) Subject: Tighten lax vector-conversion rules and enforce them consistently. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=66ad8de6288e61a9b86d36283fd154179a42b1d4;p=clang Tighten lax vector-conversion rules and enforce them consistently. When a lax conversion featured a vector and a non-vector, we were only requiring the non-vector to be a scalar type, but really it needs to be a real type (i.e. integral or real floating); it is not reasonable to allow a pointer, member pointer, or complex type here. r198474 required lax conversions to match in "data size", i.e. element size * element count, forbidding matches that happen only because a vector is rounded up to the nearest power of two in size. Unfortunately, the erroneous logic was repeated in several different places; unify them to use the new condition, so that it triggers for arbitrary conversions and not just those performed as part of binary operator checking. rdar://15931426 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@200810 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 57f6c891a0..c98b10134b 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -7441,6 +7441,8 @@ public: QualType CheckVectorLogicalOperands(ExprResult &LHS, ExprResult &RHS, SourceLocation Loc); + bool isLaxVectorConversion(QualType srcType, QualType destType); + /// type checking declaration initializers (C99 6.7.8) bool CheckForConstantInitializer(Expr *e, QualType t); diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 1ad10950b8..44e1b8c549 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -6259,8 +6259,7 @@ Sema::CheckAssignmentConstraints(QualType LHSType, ExprResult &RHS, // If we are allowing lax vector conversions, and LHS and RHS are both // vectors, the total size only needs to be the same. This is a bitcast; // no bits are changed but the result type is different. - if (getLangOpts().LaxVectorConversions && - (Context.getTypeSize(LHSType) == Context.getTypeSize(RHSType))) { + if (isLaxVectorConversion(RHSType, LHSType)) { Kind = CK_BitCast; return IncompatibleVectors; } @@ -6626,44 +6625,86 @@ QualType Sema::InvalidOperands(SourceLocation Loc, ExprResult &LHS, return QualType(); } -static bool areVectorOperandsLaxBitCastable(ASTContext &Ctx, - QualType LHSType, QualType RHSType){ - if (!Ctx.getLangOpts().LaxVectorConversions) - return false; +static bool breakDownVectorType(QualType type, uint64_t &len, + QualType &eltType) { + // Vectors are simple. + if (const VectorType *vecType = type->getAs()) { + len = vecType->getNumElements(); + eltType = vecType->getElementType(); + assert(eltType->isScalarType()); + return true; + } - if (!(LHSType->isVectorType() || LHSType->isScalarType()) || - !(RHSType->isVectorType() || RHSType->isScalarType())) - return false; + // We allow lax conversion to and from non-vector types, but only if + // they're real types (i.e. non-complex, non-pointer scalar types). + if (!type->isRealType()) return false; + + len = 1; + eltType = type; + return true; +} + +/// Is this a legal conversion between two known vector types? +bool Sema::isLaxVectorConversion(QualType srcTy, QualType destTy) { + assert(destTy->isVectorType() || srcTy->isVectorType()); - unsigned LHSSize = Ctx.getTypeSize(LHSType); - unsigned RHSSize = Ctx.getTypeSize(RHSType); - if (LHSSize != RHSSize) + if (!Context.getLangOpts().LaxVectorConversions) return false; - // For a non-power-of-2 vector ASTContext::getTypeSize returns the size - // rounded to the next power-of-2, but the LLVM IR type that we create - // is considered to have num-of-elements*width-of-element width. - // Make sure such width is the same between the types, otherwise we may end - // up with an invalid bitcast. - unsigned LHSIRSize, RHSIRSize; - if (LHSType->isVectorType()) { - const VectorType *Vec = LHSType->getAs(); - LHSIRSize = Vec->getNumElements() * - Ctx.getTypeSize(Vec->getElementType()); - } else { - LHSIRSize = LHSSize; - } - if (RHSType->isVectorType()) { - const VectorType *Vec = RHSType->getAs(); - RHSIRSize = Vec->getNumElements() * - Ctx.getTypeSize(Vec->getElementType()); + uint64_t srcLen, destLen; + QualType srcElt, destElt; + if (!breakDownVectorType(srcTy, srcLen, srcElt)) return false; + if (!breakDownVectorType(destTy, destLen, destElt)) return false; + + // ASTContext::getTypeSize will return the size rounded up to a + // power of 2, so instead of using that, we need to use the raw + // element size multiplied by the element count. + uint64_t srcEltSize = Context.getTypeSize(srcElt); + uint64_t destEltSize = Context.getTypeSize(destElt); + + return (srcLen * srcEltSize == destLen * destEltSize); +} + +/// Try to convert a value of non-vector type to a vector type by +/// promoting (and only promoting) the type to the element type of the +/// vector and then performing a vector splat. +/// +/// \param scalar - if non-null, actually perform the conversions +/// \return true if the operation fails (but without diagnosing the failure) +static bool tryVectorPromoteAndSplat(Sema &S, ExprResult *scalar, + QualType scalarTy, + QualType vectorEltTy, + QualType vectorTy) { + // The conversion to apply to the scalar before splatting it, + // if necessary. + CastKind scalarCast = CK_Invalid; + + if (vectorEltTy->isIntegralType(S.Context)) { + if (!scalarTy->isIntegralType(S.Context)) return true; + int order = S.Context.getIntegerTypeOrder(vectorEltTy, scalarTy); + if (order < 0) return true; + if (order > 0) scalarCast = CK_IntegralCast; + } else if (vectorEltTy->isRealFloatingType()) { + if (scalarTy->isRealFloatingType()) { + int order = S.Context.getFloatingTypeOrder(vectorEltTy, scalarTy); + if (order < 0) return true; + if (order > 0) scalarCast = CK_FloatingCast; + } else if (scalarTy->isIntegralType(S.Context)) { + scalarCast = CK_IntegralToFloating; + } else { + return true; + } } else { - RHSIRSize = RHSSize; + return true; } - if (LHSIRSize != RHSIRSize) - return false; - return true; + // Adjust scalar if desired. + if (scalar) { + if (scalarCast != CK_Invalid) + *scalar = S.ImpCastExprToType(scalar->take(), vectorEltTy, scalarCast); + *scalar = S.ImpCastExprToType(scalar->take(), vectorTy, CK_VectorSplat); + } + return false; } QualType Sema::CheckVectorOperands(ExprResult &LHS, ExprResult &RHS, @@ -6679,19 +6720,21 @@ QualType Sema::CheckVectorOperands(ExprResult &LHS, ExprResult &RHS, // For conversion purposes, we ignore any qualifiers. // For example, "const float" and "float" are equivalent. - QualType LHSType = - Context.getCanonicalType(LHS.get()->getType()).getUnqualifiedType(); - QualType RHSType = - Context.getCanonicalType(RHS.get()->getType()).getUnqualifiedType(); + QualType LHSType = LHS.get()->getType().getUnqualifiedType(); + QualType RHSType = RHS.get()->getType().getUnqualifiedType(); // If the vector types are identical, return. - if (LHSType == RHSType) + if (Context.hasSameType(LHSType, RHSType)) return LHSType; - // Handle the case of equivalent AltiVec and GCC vector types - if (LHSType->isVectorType() && RHSType->isVectorType() && + const VectorType *LHSVecType = LHSType->getAs(); + const VectorType *RHSVecType = RHSType->getAs(); + assert(LHSVecType || RHSVecType); + + // If we have compatible AltiVec and GCC vector types, use the AltiVec type. + if (LHSVecType && RHSVecType && Context.areCompatibleVectorTypes(LHSType, RHSType)) { - if (LHSType->isExtVectorType()) { + if (isa(LHSVecType)) { RHS = ImpCastExprToType(RHS.take(), LHSType, CK_BitCast); return LHSType; } @@ -6701,69 +6744,43 @@ QualType Sema::CheckVectorOperands(ExprResult &LHS, ExprResult &RHS, return RHSType; } - if (areVectorOperandsLaxBitCastable(Context, LHSType, RHSType)) { - // If we are allowing lax vector conversions, and LHS and RHS are both - // vectors, the total size only needs to be the same. This is a - // bitcast; no bits are changed but the result type is different. - // FIXME: Should we really be allowing this? - RHS = ImpCastExprToType(RHS.take(), LHSType, CK_BitCast); - return LHSType; + // If we're allowing lax vector conversions, only the total (data) size + // needs to be the same. + // FIXME: Should we really be allowing this? + // FIXME: We really just pick the LHS type arbitrarily? + if (isLaxVectorConversion(RHSType, LHSType)) { + QualType resultType = LHSType; + RHS = ImpCastExprToType(RHS.take(), resultType, CK_BitCast); + return resultType; } - if (!(LHSType->isVectorType() || LHSType->isScalarType()) || - !(RHSType->isVectorType() || RHSType->isScalarType())) { - Diag(Loc, diag::err_typecheck_vector_not_convertable_non_scalar) - << LHS.get()->getType() << RHS.get()->getType() - << LHS.get()->getSourceRange() << RHS.get()->getSourceRange(); - return QualType(); + // If there's an ext-vector type and a scalar, try to promote (and + // only promote) and splat the scalar to the vector type. + if (!RHSVecType && isa(LHSVecType)) { + if (!tryVectorPromoteAndSplat(*this, &RHS, RHSType, + LHSVecType->getElementType(), LHSType)) + return LHSType; + } + if (!LHSVecType && isa(RHSVecType)) { + if (!tryVectorPromoteAndSplat(*this, (IsCompAssign ? 0 : &LHS), LHSType, + RHSVecType->getElementType(), RHSType)) + return RHSType; } - // Canonicalize the ExtVector to the LHS, remember if we swapped so we can - // swap back (so that we don't reverse the inputs to a subtract, for instance. - bool swapped = false; - if (RHSType->isExtVectorType() && !IsCompAssign) { - swapped = true; - std::swap(RHS, LHS); - std::swap(RHSType, LHSType); - } - - // Handle the case of an ext vector and scalar. - if (const ExtVectorType *LV = LHSType->getAs()) { - QualType EltTy = LV->getElementType(); - if (EltTy->isIntegralType(Context) && RHSType->isIntegralType(Context)) { - int order = Context.getIntegerTypeOrder(EltTy, RHSType); - if (order > 0) - RHS = ImpCastExprToType(RHS.take(), EltTy, CK_IntegralCast); - if (order >= 0) { - RHS = ImpCastExprToType(RHS.take(), LHSType, CK_VectorSplat); - if (swapped) std::swap(RHS, LHS); - return LHSType; - } - } - if (EltTy->isRealFloatingType() && RHSType->isScalarType()) { - if (RHSType->isRealFloatingType()) { - int order = Context.getFloatingTypeOrder(EltTy, RHSType); - if (order > 0) - RHS = ImpCastExprToType(RHS.take(), EltTy, CK_FloatingCast); - if (order >= 0) { - RHS = ImpCastExprToType(RHS.take(), LHSType, CK_VectorSplat); - if (swapped) std::swap(RHS, LHS); - return LHSType; - } - } - if (RHSType->isIntegralType(Context)) { - RHS = ImpCastExprToType(RHS.take(), EltTy, CK_IntegralToFloating); - RHS = ImpCastExprToType(RHS.take(), LHSType, CK_VectorSplat); - if (swapped) std::swap(RHS, LHS); - return LHSType; - } - } + // Okay, the expression is invalid. + + // If there's a non-vector, non-real operand, diagnose that. + if ((!RHSVecType && !RHSType->isRealType()) || + (!LHSVecType && !LHSType->isRealType())) { + Diag(Loc, diag::err_typecheck_vector_not_convertable_non_scalar) + << LHSType << RHSType + << LHS.get()->getSourceRange() << RHS.get()->getSourceRange(); + return QualType(); } - // Vectors of different size or scalar and non-ext-vector are errors. - if (swapped) std::swap(RHS, LHS); + // Otherwise, use the generic diagnostic. Diag(Loc, diag::err_typecheck_vector_not_convertable) - << LHS.get()->getType() << RHS.get()->getType() + << LHSType << RHSType << LHS.get()->getSourceRange() << RHS.get()->getSourceRange(); return QualType(); } diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp index 8e52e620a7..b95e289318 100644 --- a/lib/Sema/SemaOverload.cpp +++ b/lib/Sema/SemaOverload.cpp @@ -1372,7 +1372,7 @@ bool Sema::IsNoReturnConversion(QualType FromType, QualType ToType, /// /// \param ICK Will be set to the vector conversion kind, if this is a vector /// conversion. -static bool IsVectorConversion(ASTContext &Context, QualType FromType, +static bool IsVectorConversion(Sema &S, QualType FromType, QualType ToType, ImplicitConversionKind &ICK) { // We need at least one of these types to be a vector type to have a vector // conversion. @@ -1380,7 +1380,7 @@ static bool IsVectorConversion(ASTContext &Context, QualType FromType, return false; // Identical types require no conversions. - if (Context.hasSameUnqualifiedType(FromType, ToType)) + if (S.Context.hasSameUnqualifiedType(FromType, ToType)) return false; // There are no conversions between extended vector types, only identity. @@ -1402,9 +1402,8 @@ static bool IsVectorConversion(ASTContext &Context, QualType FromType, // 2)lax vector conversions are permitted and the vector types are of the // same size if (ToType->isVectorType() && FromType->isVectorType()) { - if (Context.areCompatibleVectorTypes(FromType, ToType) || - (Context.getLangOpts().LaxVectorConversions && - (Context.getTypeSize(FromType) == Context.getTypeSize(ToType)))) { + if (S.Context.areCompatibleVectorTypes(FromType, ToType) || + S.isLaxVectorConversion(FromType, ToType)) { ICK = ICK_Vector_Conversion; return true; } @@ -1633,7 +1632,7 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType, InOverloadResolution, FromType)) { // Pointer to member conversions (4.11). SCS.Second = ICK_Pointer_Member; - } else if (IsVectorConversion(S.Context, FromType, ToType, SecondICK)) { + } else if (IsVectorConversion(S, FromType, ToType, SecondICK)) { SCS.Second = SecondICK; FromType = ToType.getUnqualifiedType(); } else if (!S.getLangOpts().CPlusPlus && diff --git a/test/Sema/ext_vector_casts.c b/test/Sema/ext_vector_casts.c index 33da0d6daa..c590932678 100644 --- a/test/Sema/ext_vector_casts.c +++ b/test/Sema/ext_vector_casts.c @@ -49,7 +49,7 @@ static void test() { ivec4 += (int4)vec4; ivec4 -= ivec4; ivec4 |= ivec4; - ivec4 += ptr; // expected-error {{can't convert between vector values of different size ('int4' and 'int *')}} + ivec4 += ptr; // expected-error {{can't convert between vector and non-scalar values ('int4' and 'int *')}} } typedef __attribute__(( ext_vector_type(2) )) float2 vecfloat2; // expected-error{{invalid vector element type 'float2'}} diff --git a/test/Sema/vector-cast.c b/test/Sema/vector-cast.c index 6a5f0eca42..bfc731e7b0 100644 --- a/test/Sema/vector-cast.c +++ b/test/Sema/vector-cast.c @@ -44,3 +44,13 @@ void f4() { f2 += d; d += f2; } + +// rdar://15931426 +// Don't permit a lax conversion to and from a pointer type. +typedef short short_sizeof_pointer __attribute__((vector_size(sizeof(void*)))); +void f5() { + short_sizeof_pointer v; + void *ptr; + v = ptr; // expected-error {{assigning to 'short_sizeof_pointer' from incompatible type 'void *'}} + ptr = v; // expected-error {{assigning to 'void *' from incompatible type 'short_sizeof_pointer'}} +} diff --git a/test/SemaCXX/vector-casts.cpp b/test/SemaCXX/vector-casts.cpp index 25c59e6427..3aa097b652 100644 --- a/test/SemaCXX/vector-casts.cpp +++ b/test/SemaCXX/vector-casts.cpp @@ -2,6 +2,7 @@ typedef int __v2si __attribute__((__vector_size__(8))); typedef short __v4hi __attribute__((__vector_size__(8))); typedef short __v8hi __attribute__((__vector_size__(16))); +typedef short __v3hi __attribute__((__ext_vector_type__(3))); struct S { }; // expected-note 2 {{candidate constructor}} @@ -46,3 +47,19 @@ struct testvec { v = v + rhs; // expected-error {{can't convert between vector and non-scalar values}} } }; + +// rdar://15931426 +// Conversions for return values. +__v4hi threeToFour(__v3hi v) { // expected-note {{not viable}} + return v; // expected-error {{cannot initialize return object}} +} +__v3hi fourToThree(__v4hi v) { // expected-note {{not viable}} + return v; // expected-error {{cannot initialize return object}} +} +// Conversions for calls. +void call3to4(__v4hi v) { + (void) threeToFour(v); // expected-error {{no matching function for call}} +} +void call4to3(__v3hi v) { + (void) fourToThree(v); // expected-error {{no matching function for call}} +}