[patch] Improvement to implicit scalar-vector conversions

Stephen Canon scanon at apple.com
Thu Apr 3 03:42:28 PDT 2014


Committed as r205521 after addressing John's notes and explicitly exempting CL from this change.

– Steve

On Apr 2, 2014, at 4:43 PM, John McCall <rjmccall at apple.com> wrote:

> On Mar 26, 2014, at 5:34 PM, Stephen Canon <scanon at apple.com> wrote:
>> ExtVectors currently support basic operations with scalar data (which is interpreted as an implicit splat).  However, this support has some serious issues.  Most critically, at present the type of the result depends on operand order:
> 
> I think the language design here makes a lot of sense, especially dropping the Open CL restriction.  Style nits:
> 
> +  if (isa<VectorType>(Target))
> +    Target = cast<VectorType>(Target)->getElementType().getTypePtr();
> 
> The idiomatic way to do this is
>  if (auto VecTy = dyn_cast<VectorType>(Target))
>    Target = VecTy->getElementType().getTypePtr();
> 
>   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;
> +        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;
> +        scalarCast = CK_FloatingCast;
> 
> You seem to be randomly 4-indenting here.
> 
> +  // If there's an ext-vector type and a scalar, try to promote (and
> +  // only promote) and splat the scalar to the vector type.
> 
> This comment is no longer accurate.
> 
> +static void splats(int i, long l, __uint128_t t, float f, double d) {
> +    short8 vs = 0;
> +    int4 vi = i;
> +    ulong2 vl = (unsigned long)l;
> +    float2 vf = f;
> +    double2 vd = d;
> 
> We try to be consistent about 2-indenting even in test cases.  Obviously that’s not always true going back, but in new code...
> 
> Otherwise looks good.
> 
> John.





More information about the cfe-commits mailing list