Patchwork Scalar vector binary operation

login
register
mail settings
Submitter Artem Shinkarov
Date Nov. 8, 2010, 5:48 p.m.
Message ID <AANLkTi=Aivzx=XVig+dQ+tEqJtrFGhVD0VCkAmp4nLw-@mail.gmail.com>
Download mbox | patch
Permalink /patch/70437/
State New
Headers show

Comments

Artem Shinkarov - Nov. 8, 2010, 5:48 p.m.
Thanks for the comments.

default_conversion is switched off for the vector/scalar case.
bitwise operations are added.
new testcase is created.

2010-11-01  Artjoms Sinkarovs <artyom.shinakroff@gmail.com>

     /gcc
     * c-typeck.c (expr_fits_type_p): New function. Check if expression
     fits into the type considering constants.
     (scalar_to_vector): New function. Try scalar to vector conversion.
     (build_binary_op): Adjust.
     * doc/extend.texi: Description of scalar to vector expansion.

     /gcc/testsuite
     * gcc.c-torture/execute/scal-to-vec1.c: New test.
     * gcc.c-torture/execute/scal-to-vec2.c: New test.
     * gcc.dg/scal-to-vec1.c: New test.

bootstrapped and tested on x86_64_unknown-linux

OK?

On Sat, Nov 6, 2010 at 5:44 PM, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Wed, 3 Nov 2010, Artem Shinkarov wrote:
>
>> @@ -9387,6 +9387,104 @@ push_cleanup (tree decl, tree cleanup, b
>>    TREE_OPERAND (stmt, 0) = list;
>>    STATEMENT_LIST_STMT_EXPR (list) = stmt_expr;
>>  }
>> +
>> +/* Return true if expression EXPR can be converted to the
>> +   vectortype TYPE preserving its value.  */
>
> "vector type" (two words)
>
>> +static bool
>> +expr_fits_type_p (tree expr, tree type)
>> +{
>
> I don't see anything here to handle removing widening conversions from
> expr.  Say you have a variable of type short - do you expect to be able to
> do a binary operation between that variable and a vector of shorts?
> Because the variable will have been converted to int.  I don't see any
> testcases in this area either.
>
> If you do want to allow that, rather than making this function smarter and
> so making the language definition depend on how smart the compiler is
> about seeing what conversions are safe I think it might be better to avoid
> having build_binary_op calling default_conversion on the scalar operand in
> a mixed vector/scalar operation.
>
>> +/* Convert scalar to vector for the range of operations.
>> +   Function can return:
>> +        -2  --  Error ocured
>
> "occurred"
>
>> +        -1  --  Nothing happened
>> +         0  --  First argument must be expanded
>> +         1  --  Second argument must be expanded  */
>
> Please define an enum giving symbolic names to the possible return values,
> rather than using magic constants.  */
>
>> +              error_at (loc, "Conversion of scalar to vector "
>> +                             "involves truncation");
>
> Diagnostics should not start with capital letters.  Likewise for all the
> other diagnostics in this function.
>
> The patch does not seem to include any testcases for bitwise operations
> (&^|).  If they are meant to work with mixed scalar/vector operations
> (which would be my expectation), then testcases are needed unless already
> in the testsuite; if not, testcases are still needed and the documentation
> needs to make clear they are not allowed.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
>
Joseph S. Myers - Nov. 12, 2010, 9:22 p.m.
On Mon, 8 Nov 2010, Artem Shinkarov wrote:

> +/* Possibe cases of scalar_to_vector conversion.  */
> +enum stv_conv {
> +  stv_error,
> +  stv_nothing,
> +  stv_firstarg,
> +  stv_secondarg
> +};

The description of the semantics of the individual enum values should go 
here, not on the function scalar_to_vector.

> @@ -2156,7 +2164,7 @@ build_component_ref (location_t loc, tre
>        /* Chain the COMPONENT_REFs if necessary down to the FIELD.
>  	 This might be better solved in future the way the C++ front
>  	 end does it - by giving the anonymous entities each a
> -	 separate name and type, and then have build_component_ref
> +	 /edseparate name and type, and then have build_component_ref
>  	 recursively call itself.  We can't do that here.  */
>        do
>  	{

This change seems bogus.

> +/* Return true if expression EXPR can be converted to the
> +   vector type TYPE preserving its value.  */

I don't see anything here that checks whether integers can safely be 
converted to floating-point types (thus, any short value can be converted 
to float, but some int values cannot, for example).

Such logic exists in c-common.c:conversion_warning.  It would be good to 
share as much code as possible with that function.

> +  else if (TREE_CODE (expr) == REAL_CST)
> +    {
> +      REAL_VALUE_TYPE c, c1;
> +      c = TREE_REAL_CST (expr);
> +      real_convert (&c1, mode, &c);
> +      return real_identical (&c, &c1);

conversion_warning uses exact_real_truncate....

Patch

Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 166433)
+++ gcc/doc/extend.texi	(working copy)
@@ -6333,18 +6333,25 @@  In C it is possible to use shifting oper
 integer-type vectors. The operation is defined as following: @code{@{a0,
 a1, @dots{}, an@} >> @{b0, b1, @dots{}, bn@} == @{a0 >> b0, a1 >> b1,
 @dots{}, an >> bn@}}@. Vector operands must have the same number of
-elements.  Additionally second operands can be a scalar integer in which
-case the scalar is converted to the type used by the vector operand (with
-possible truncation) and each element of this new vector is the scalar's
-value.
+elements. 
+
+For the convenience in C it is allowed to use a binary vector operation
+where one operand is a scalar. In that case the compiler will transform
+the scalar operand into a vector where each element is the scalar from
+the operation. The transformation will happen only if the scalar could be
+safely converted to the vector-element type.
 Consider the following code.
 
 @smallexample
 typedef int v4si __attribute__ ((vector_size (16)));
 
-v4si a, b;
+v4si a, b, c;
+long l;
+
+a = b + 1;    /* a = b + @{1,1,1,1@}; */
+a = 2 * b;    /* a = @{2,2,2,2@} * b; */
 
-b = a >> 1;     /* b = a >> @{1,1,1,1@}; */
+a = l + a;    /* Error, cannot convert long to int. */
 @end smallexample
 
 In C vectors can be subscripted as if the vector were an array with
Index: gcc/testsuite/gcc.c-torture/execute/scal-to-vec1.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/scal-to-vec1.c	(revision 0)
+++ gcc/testsuite/gcc.c-torture/execute/scal-to-vec1.c	(revision 0)
@@ -0,0 +1,85 @@ 
+#define vector(elcount, type)  \
+__attribute__((vector_size((elcount)*sizeof(type)))) type
+
+#define vidx(type, vec, idx) (*((type *) &(vec) + idx))
+
+#define operl(a, b, op) (a op b)
+#define operr(a, b, op) (b op a)
+
+#define check(type, count, vec0, vec1, num, op, lr) \
+do {\
+    int __i; \
+    for (__i = 0; __i < count; __i++) {\
+        if (vidx (type, vec1, __i) != oper##lr (num, vidx (type, vec0, __i), op)) \
+            __builtin_abort (); \
+    }\
+} while (0)
+
+#define veccompare(type, count, v0, v1) \
+do {\
+    int __i; \
+    for (__i = 0; __i < count; __i++) { \
+        if (vidx (type, v0, __i) != vidx (type, v1, __i)) \
+            __builtin_abort (); \
+    } \
+} while (0)
+
+
+int main (int argc, char *argv[]) {
+#define fvec_2 (vector(4, float)){2., 2., 2., 2.}
+#define dvec_2 (vector(2, double)){2., 2.}
+
+
+    vector(8, short) v0 = {argc, 1,2,3,4,5,6,7};
+    vector(8, short) v1;
+
+    vector(4, float) f0 = {1., 2., 3., 4.};
+    vector(4, float) f1, f2;
+
+    vector(2, double) d0 = {1., 2.};
+    vector(2, double) d1, d2;
+
+
+
+    v1 = 2 + v0;   check (short, 8, v0, v1, 2, +, l);
+    v1 = 2 - v0;   check (short, 8, v0, v1, 2, -, l);
+    v1 = 2 * v0;   check (short, 8, v0, v1, 2, *, l);
+    v1 = 2 / v0;   check (short, 8, v0, v1, 2, /, l);
+    v1 = 2 % v0;   check (short, 8, v0, v1, 2, %, l);
+    v1 = 2 ^ v0;   check (short, 8, v0, v1, 2, ^, l);
+    v1 = 2 & v0;   check (short, 8, v0, v1, 2, &, l);
+    v1 = 2 | v0;   check (short, 8, v0, v1, 2, |, l);
+    v1 = 2 << v0;   check (short, 8, v0, v1, 2, <<, l);
+    v1 = 2 >> v0;   check (short, 8, v0, v1, 2, >>, l);
+
+    v1 = v0 + 2;   check (short, 8, v0, v1, 2, +, r);
+    v1 = v0 - 2;   check (short, 8, v0, v1, 2, -, r);
+    v1 = v0 * 2;   check (short, 8, v0, v1, 2, *, r);
+    v1 = v0 / 2;   check (short, 8, v0, v1, 2, /, r);
+    v1 = v0 % 2;   check (short, 8, v0, v1, 2, %, r);
+    v1 = v0 ^ 2;   check (short, 8, v0, v1, 2, ^, r);
+    v1 = v0 & 2;   check (short, 8, v0, v1, 2, &, r);
+    v1 = v0 | 2;   check (short, 8, v0, v1, 2, |, r);
+
+    f1 = 2. + f0;  f2 = fvec_2 + f0; veccompare (float, 4, f1, f2);
+    f1 = 2. - f0;  f2 = fvec_2 - f0; veccompare (float, 4, f1, f2);
+    f1 = 2. * f0;  f2 = fvec_2 * f0; veccompare (float, 4, f1, f2);
+    f1 = 2. / f0;  f2 = fvec_2 / f0; veccompare (float, 4, f1, f2);
+
+    f1 = f0 + 2.;  f2 = f0 + fvec_2; veccompare (float, 4, f1, f2);
+    f1 = f0 - 2.;  f2 = f0 - fvec_2; veccompare (float, 4, f1, f2);
+    f1 = f0 * 2.;  f2 = f0 * fvec_2; veccompare (float, 4, f1, f2);
+    f1 = f0 / 2.;  f2 = f0 / fvec_2; veccompare (float, 4, f1, f2);
+
+    d1 = 2. + d0;  d2 = dvec_2 + d0; veccompare (double, 2, d1, d2);
+    d1 = 2. - d0;  d2 = dvec_2 - d0; veccompare (double, 2, d1, d2);
+    d1 = 2. * d0;  d2 = dvec_2 * d0; veccompare (double, 2, d1, d2);
+    d1 = 2. / d0;  d2 = dvec_2 / d0; veccompare (double, 2, d1, d2);
+
+    d1 = d0 + 2.;  d2 = d0 + dvec_2; veccompare (double, 2, d1, d2);
+    d1 = d0 - 2.;  d2 = d0 - dvec_2; veccompare (double, 2, d1, d2);
+    d1 = d0 * 2.;  d2 = d0 * dvec_2; veccompare (double, 2, d1, d2);
+    d1 = d0 / 2.;  d2 = d0 / dvec_2; veccompare (double, 2, d1, d2);
+
+    return 0;
+}
Index: gcc/testsuite/gcc.c-torture/execute/scal-to-vec2.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/scal-to-vec2.c	(revision 0)
+++ gcc/testsuite/gcc.c-torture/execute/scal-to-vec2.c	(revision 0)
@@ -0,0 +1,62 @@ 
+#define vector(elcount, type)  \
+__attribute__((vector_size((elcount)*sizeof(type)))) type
+
+#define vidx(type, vec, idx) (*((type *) &(vec) + idx))
+
+#define operl(a, b, op) (a op b)
+#define operr(a, b, op) (b op a)
+
+#define check(type, count, vec0, vec1, num, op, lr) \
+do {\
+    int __i; \
+    for (__i = 0; __i < count; __i++) {\
+        if (vidx (type, vec1, __i) != oper##lr (num, vidx (type, vec0, __i), op)) \
+            __builtin_abort (); \
+    }\
+} while (0)
+
+#define veccompare(type, count, v0, v1) \
+do {\
+    int __i; \
+    for (__i = 0; __i < count; __i++) { \
+        if (vidx (type, v0, __i) != vidx (type, v1, __i)) \
+            __builtin_abort (); \
+    } \
+} while (0)
+
+
+long __attribute__ ((noinline)) vlng () {   return (long)42; }
+int  __attribute__ ((noinline)) vint () {   return (int) 43; }
+short __attribute__ ((noinline)) vsrt () {   return (short)42; }
+char __attribute__ ((noinline)) vchr () {    return (char)42; }
+
+
+int main (int argc, char *argv[]) {
+    vector(16, char) c0 = {argc, 1,2,3,4,5,6,7, argc, 1,2,3,4,5,6,7};
+    vector(16, char) c1;
+    
+    vector(8, short) s0 = {argc, 1,2,3,4,5,6,7};
+    vector(8, short) s1;
+
+    vector(4, int) i0 = {argc, 1, 2, 3};
+    vector(4, int) i1;
+
+    vector(2, long) l0 = {argc, 1};
+    vector(2, long) l1;
+
+    c1 = vchr() + c0; check (char, 16, c0, c1, vchr(), +, l);
+    
+    s1 = vsrt() + s0; check (short, 8, s0, s1, vsrt(), +, l);
+    s1 = vchr() + s0; check (short, 8, s0, s1, vchr(), +, l);
+
+    i1 = vint() * i0; check (int, 4, i0, i1, vint(), *, l);
+    i1 = vsrt() * i0; check (int, 4, i0, i1, vsrt(), *, l);
+    i1 = vchr() * i0; check (int, 4, i0, i1, vchr(), *, l);
+
+    l1 = vlng() * l0; check (long, 2, l0, l1, vlng(), *, l);
+    l1 = vint() * l0; check (long, 2, l0, l1, vint(), *, l);
+    l1 = vsrt() * l0; check (long, 2, l0, l1, vsrt(), *, l);
+    l1 = vchr() * l0; check (long, 2, l0, l1, vchr(), *, l);
+
+    return 0;
+}
Index: gcc/testsuite/gcc.dg/scal-to-vec1.c
===================================================================
--- gcc/testsuite/gcc.dg/scal-to-vec1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/scal-to-vec1.c	(revision 0)
@@ -0,0 +1,39 @@ 
+/* { dg-do compile } */
+#define vector(elcount, type)  \
+__attribute__((vector_size((elcount)*sizeof(type)))) type
+
+#define vidx(type, vec, idx) (*((type *) &(vec) + idx))
+
+
+extern float sfl;
+extern int   sint;
+
+int main (int argc, char *argv[]) {
+    vector(8, short) v0 = {argc, 1,2,3,4,5,6,7};
+    vector(8, short) v1;
+
+    vector(4, float) f0 = {1., 2., 3., 4.};
+    vector(4, float) f1, f2;
+
+    vector(4, int) i0 = {1,2,3,4};
+    vector(4, int) i1, i2;
+
+    
+    int     i = 12;
+    double  d = 3.;
+
+    v1 = i + v0;        /* { dg-error "conversion of scalar to vector" } */
+    v1 = 99999 + v0;    /* { dg-error "conversion of scalar to vector" } */
+
+    f1 = d + f0;        /* { dg-error "truncating floating point" } */
+    f1 = 1.3 + f0;      /* { dg-error "truncating floating point" } */
+
+    /* convert.c should take care of this.  */
+    i1 = sfl + i0;      /* { dg-error "can't convert value to a vector" } */
+    i1 = 1.5 + i0;      /* { dg-error "can't convert value to a vector" } */
+    v1 = d + v0;        /* { dg-error "can't convert value to a vector" } */
+    f1 = sint + f0;     /* { dg-error "can't convert between vector values of different size" } */
+
+
+    return 0;
+}
Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c	(revision 166433)
+++ gcc/c-typeck.c	(working copy)
@@ -51,6 +51,14 @@  enum impl_conv {
   ic_return
 };
 
+/* Possibe cases of scalar_to_vector conversion.  */
+enum stv_conv {
+  stv_error,
+  stv_nothing,
+  stv_firstarg,
+  stv_secondarg
+};
+
 /* Whether we are building a boolean conversion inside
    convert_for_assignment, or some other late binary operation.  If
    build_binary_op is called (from code shared with C++) in this case,
@@ -2156,7 +2164,7 @@  build_component_ref (location_t loc, tre
       /* Chain the COMPONENT_REFs if necessary down to the FIELD.
 	 This might be better solved in future the way the C++ front
 	 end does it - by giving the anonymous entities each a
-	 separate name and type, and then have build_component_ref
+	 /edseparate name and type, and then have build_component_ref
 	 recursively call itself.  We can't do that here.  */
       do
 	{
@@ -9387,6 +9395,112 @@  push_cleanup (tree decl, tree cleanup, b
   TREE_OPERAND (stmt, 0) = list;
   STATEMENT_LIST_STMT_EXPR (list) = stmt_expr;
 }
+
+/* Return true if expression EXPR can be converted to the
+   vector type TYPE preserving its value.  */
+static bool
+expr_fits_type_p (tree expr, tree type)
+{
+  enum machine_mode mode = TYPE_MODE (TREE_TYPE (type));
+  if (TYPE_MODE (TREE_TYPE (expr)) == mode)
+    return true;
+
+  if (TREE_CODE (expr) == INTEGER_CST)
+    return int_fits_type_p (expr, TREE_TYPE (type));
+  else if (TREE_CODE (TREE_TYPE (expr)) == INTEGER_TYPE)
+    return !tree_int_cst_lt 
+                (TYPE_SIZE (TREE_TYPE (type)),
+                 TYPE_SIZE (TREE_TYPE (expr)));
+  else if (TREE_CODE (expr) == REAL_CST)
+    {
+      REAL_VALUE_TYPE c, c1;
+      c = TREE_REAL_CST (expr);
+      real_convert (&c1, mode, &c);
+      return real_identical (&c, &c1);
+    }
+  else if (TREE_CODE (TREE_TYPE (expr)) == REAL_TYPE)
+    return (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (expr))) 
+            <= GET_MODE_SIZE (mode));
+  return false;
+}
+
+/* Convert scalar to vector for the range of operations.  
+   Function can return:
+        stv_error      --  Error occured
+        stv_nothing    --  Nothing happened
+        stv_firstarg   --  First argument must be expanded
+        stv_secondarg  --  Second argument must be expanded  */
+static enum stv_conv
+scalar_to_vector (location_t loc, enum tree_code code, tree op0, tree op1)
+{
+  tree type0 = TREE_TYPE (op0);
+  tree type1 = TREE_TYPE (op1);
+  bool integer_only_op = false;
+  enum stv_conv ret = stv_firstarg;
+  
+  switch (code)
+    {
+      case RSHIFT_EXPR:
+      case LSHIFT_EXPR:
+        if (TREE_CODE (type0) == INTEGER_TYPE)
+          if (!expr_fits_type_p (op0, type1))
+            {
+              error_at (loc, "conversion of scalar to vector "
+                             "involves truncation");
+              return stv_error;
+            }
+          return stv_firstarg;
+
+      case BIT_IOR_EXPR:
+      case BIT_XOR_EXPR:
+      case BIT_AND_EXPR:
+        integer_only_op = true;
+	/* ... fall through ...  */
+      
+      case PLUS_EXPR:
+      case MINUS_EXPR:
+      case MULT_EXPR:
+      case TRUNC_DIV_EXPR:
+      case TRUNC_MOD_EXPR:
+      case RDIV_EXPR:
+        if (TREE_CODE (type0) == VECTOR_TYPE)
+          {
+            tree tmp;
+            ret = stv_secondarg;
+            /* Swap TYPE0 with TYPE1 and OP0 with OP1  */
+            tmp = type0; type0 = type1; type1 = tmp;
+            tmp = op0; op0 = op1; op1 = tmp;
+          }
+
+        if (TREE_CODE (type0) == INTEGER_TYPE
+            && TREE_CODE (TREE_TYPE (type1)) == INTEGER_TYPE) 
+          {
+            if (!expr_fits_type_p (op0, type1))
+              {
+                error_at (loc, "conversion of scalar to vector "
+                               "involves truncation");
+                return stv_error;
+              }
+            return ret;
+          }
+        else if (!integer_only_op
+                 && TREE_CODE (type0) == REAL_TYPE
+                 && SCALAR_FLOAT_TYPE_P (TREE_TYPE (type1)))
+          {
+            if (!expr_fits_type_p (op0, type1))
+              {
+                error_at (loc, "truncating floating point constant to "
+                               "vector precision does not preserve value");
+                return stv_error;
+              }
+            return ret;
+          }
+      default:
+        break;
+    }
+ 
+  return stv_nothing;
+}
 
 /* Build a binary-operation expression without default conversions.
    CODE is the kind of expression to build.
@@ -9498,7 +9612,10 @@  build_binary_op (location_t location, en
   else
     int_const = int_const_or_overflow = false;
 
-  if (convert_p)
+  /* Do not apply default conversion in mixed vector/scalar expression.  */
+  if (convert_p 
+      && !((TREE_CODE (TREE_TYPE (op0)) == VECTOR_TYPE) 
+           != (TREE_CODE (TREE_TYPE (op1)) == VECTOR_TYPE)))
     {
       op0 = default_conversion (op0);
       op1 = default_conversion (op1);
@@ -9570,6 +9687,41 @@  build_binary_op (location_t location, en
 
   objc_ok = objc_compare_types (type0, type1, -3, NULL_TREE);
 
+  /* In case when one of the operands of the binary operation is
+     a vector and another is a scalar -- convert scalar to vector.  */
+  if ((code0 == VECTOR_TYPE) != (code1 == VECTOR_TYPE))
+    {
+      enum stv_conv convert_flag = scalar_to_vector (location, code, op0, op1);
+       
+      switch (convert_flag)
+        {
+          case stv_error:
+            return error_mark_node;
+          case stv_firstarg:
+            {
+              tree sc = save_expr (op0);
+              sc = convert (TREE_TYPE (type1), sc);
+              op0 = build_vector_from_val (type1, sc);
+              orig_type0 = type0 = TREE_TYPE (op0);
+              code0 = TREE_CODE (type0);
+              converted = 1;
+              break;
+            }
+          case stv_secondarg:
+            {
+              tree sc = save_expr (op1);
+              sc = convert (TREE_TYPE (type0), sc);
+              op1 = build_vector_from_val (type0, sc);
+              orig_type1 = type1 = TREE_TYPE (op1);
+              code1 = TREE_CODE (type1);
+              converted = 1;
+              break;
+            }
+          default:
+            break;
+        }
+    }
+
   switch (code)
     {
     case PLUS_EXPR: