[SemaExpr] Support lax conversions in assignments with vector and scalars with same size

[bruno]

Summary

Before r266366, clang used to support constructs like:

typedef __attribute__((vector_size(8))) double float64x1_t;
typedef __attribute__((vector_size(16))) double float64x2_t;
float64x1_t vget_low_f64(float64x2_t __p0);

double y = 3.0 + vget_low_f64(v);

But it would reject:

double y = vget_low_f64(v) + 3.0;

It also always rejected assignments:

double y = vget_low_f64(v);

This patch: (a) revivies the behavior of 3.0 + vget_low_f64(v) prior to
r266366, (b) add support for vget_low_f64(v) + 3.0 and (c) add support for
assignments.

These vector semantics have never really been tied up but it seems
odd that we used to support some binop froms but do not support
assignment. If we did support scalar for the purposes of arithmetic, we
should probably be able to reinterpret as scalar for the purposes of
assignment too.

[bruno]

Ping!

[rnk]

After writing r266366, we discovered that GCC accepts none of the code in that test case, so we should consider turning -flax-vector-conversions off by default.

test/Sema/vector-cast.c

65

Why should we allow this conversion? I don't see how <2 x float> and double should be convertible. I'm not sure why we allow f2 += d above, but I think of it as "widening" the type from scalar to vector.

Would it be OK for your if we tightened our lax vector conversion checks to just allow conversion from <1 x T> to T? The test case in the summary seems like pretty reasonable code, even if GCC rejects.

[bruno]

In D21700#470103, @rnk wrote:

After writing r266366, we discovered that GCC accepts none of the code in that test case, so we should consider turning -flax-vector-conversions off by default.

Makes sense, specially given the non obvious semantics; users shouldn't rely on this behavior by default without the knowledge of what's actually going on.

test/Sema/vector-cast.c

65

I misread the testcase, <2 x float> and double shouldn't be convertible indeed. Thanks for the catch.

Allowing conversions from <1 x T> to T is enough for us, it also sounds more correct.

[bruno]

Update patch after Reid's suggestions.

[rnk]

lgtm Seems OK for now. :)

[bruno]

Thanks Reid,

Committed r274646.

lib/Sema/SemaExpr.cpp

if (LHSType->isVectorType() && RHSType->isVectorType()) {
       // 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 (isLaxVectorConversion(RHSType, LHSType)) {
         Kind = CK_BitCast;
         return IncompatibleVectors;
       }
     }
+
+    // When the RHS comes from another lax conversion (e.g. binops between
+    // scalars and vectors) the result is canonicalized as a vector. When the
+    // LHS is also a vector, the lax is allowed by the condition above. Handle
+    // the case where LHS is a scalar.
+    if (LHSType->isScalarType() && isLaxVectorConversion(RHSType, LHSType)) {
+      ExprResult *VecExpr = &RHS;
+      *VecExpr = ImpCastExprToType(VecExpr->get(), LHSType, CK_BitCast);
+      Kind = CK_BitCast;
+      return Compatible;
+    }
+
     return Incompatible;
   }
 
   // Diagnose attempts to convert between __float128 and long double where
   // such conversions currently can't be handled.
   if (unsupportedTypeConversion(*this, LHSType, RHSType))
     return Incompatible;
 

test/Sema/vector-cast.c

 
 void f2(t2 X); // expected-note{{passing argument to parameter 'X' here}}
 
 void f3(t3 Y) {
   f2(Y);  // expected-warning {{incompatible vector types passing 't3' (vector of 4 'float' values) to parameter of type 't2' (vector of 16 'char' values)}}
 }
 
 typedef float float2 __attribute__ ((vector_size (8)));
+typedef __attribute__((vector_size(8))) double float64x1_t;
+typedef __attribute__((vector_size(16))) double float64x2_t;
+float64x1_t vget_low_f64(float64x2_t __p0);
 
 void f4() {
   float2 f2;
-  double d;
+  double d, a, b, c;
+  float64x2_t v = {0.0, 1.0};
   f2 += d;
-  // We used to allow the next statement, but we've always rejected the next two
-  // statements
+  a = 3.0 + vget_low_f64(v);
+  b = vget_low_f64(v) + 3.0;
+  c = vget_low_f64(v);
+  // Compound assignments are not supported.
   // FIXME: This diagnostic is inaccurate.
   d += f2; // expected-error {{cannot convert between vector values of different size}}
-  d = f2; // expected-error {{assigning to 'double' from incompatible type 'float2'}}
-  d = d + f2; // expected-error {{assigning to 'double' from incompatible type 'float2'}}
+  c -= vget_low_f64(v); // expected-error {{cannot convert between vector values of different size}}
+  d = 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-re {{assigning to 'short_sizeof_pointer' (vector of {{[0-9]+}} 'short' values) from incompatible type 'void *'}}
   ptr = v; // expected-error {{assigning to 'void *' from incompatible type 'short_sizeof_pointer'}}
 }