Patchwork Scalar vector binary operation

login
register
mail settings
Submitter Artem Shinkarov
Date Nov. 3, 2010, 2:17 p.m.
Message ID <AANLkTikPoL+wSw2BrUxzQw7h_NenRT0ATyQL6SdxVoJU@mail.gmail.com>
Download mbox | patch
Permalink /patch/70007/
State New
Headers show

Comments

Artem Shinkarov - Nov. 3, 2010, 2:17 p.m.
ChangeLog:

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.dg/scal-to-vec1.c: New test.

bootstrapped and tested on x86_64_unknown-linux


On Wed, Nov 3, 2010 at 1:25 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Mon, Nov 1, 2010 at 5:16 PM, Artem Shinkarov
> <artyom.shinkaroff@gmail.com> wrote:
>> This patch allows binary operations: "vector <op> scalar" and "scalar
>> <op> vector" in which case scalar is converted into the vector type,
>> if the scalar type fits in the vectors element type.
>>
>> ChangeLog:
>>
>> 2010-11-01  Artjoms Sinkarovs <artyom.shinakroff@gmail.com>
>>
>>       /gcc
>>       * tree.c (build_vector_from_val): Build vector from scalar
>>       * tree.h (build_vector_from_val): New declaration
>
> Those are no longer part of this patch.
>
>>       * 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.dg/scal-to-vec1.c: New test.
>>
>> bootstrapped and tested on x86_64_unknown-linux
>
> +/* Check whether expression EXPR can be converted to the
> +   vectortype TYPE, considering the case when EXPR is a constant.  */
> +static bool
> +expr_fits_type_p (tree expr, tree type)
>
> "Return true if expression ..."
>
> also documents the return value.  I think the comment should be adjusted
> to match the intent of the function, namely
>
> "... can be converted to TYPE preserving its value."
>
> Passing the element type as TYPE seems to be enough as well.
>
> The rationale is that OpenCL does not permit truncations to happen,
> but people will write things like  float_vector + 2.0 (instead of 2.0f),
> so we allow value-preserving truncations for QOI reasons.
>
> +#define SWAP(x, y) do { __typeof (x) __tmp = x; x = y; y = __tmp; } while (0)
>
> we don't usually define such thing but expand it everywhere needed.
>
> +            {
> +              tree sc = save_expr(op0);
>
> space after save_expr, likewise below.
>
> Otherwise looks ok (but I've been looking at the patches before).  I can't
> approve it though.
>
> Thanks,
> Richard.
>
Artem Shinkarov - Nov. 4, 2010, 8:57 p.m.
Patch pinging.

On Wed, Nov 3, 2010 at 2:17 PM, Artem Shinkarov
<artyom.shinkaroff@gmail.com> wrote:
> ChangeLog:
>
> 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.dg/scal-to-vec1.c: New test.
>
> bootstrapped and tested on x86_64_unknown-linux
>
>
> On Wed, Nov 3, 2010 at 1:25 PM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Mon, Nov 1, 2010 at 5:16 PM, Artem Shinkarov
>> <artyom.shinkaroff@gmail.com> wrote:
>>> This patch allows binary operations: "vector <op> scalar" and "scalar
>>> <op> vector" in which case scalar is converted into the vector type,
>>> if the scalar type fits in the vectors element type.
>>>
>>> ChangeLog:
>>>
>>> 2010-11-01  Artjoms Sinkarovs <artyom.shinakroff@gmail.com>
>>>
>>>       /gcc
>>>       * tree.c (build_vector_from_val): Build vector from scalar
>>>       * tree.h (build_vector_from_val): New declaration
>>
>> Those are no longer part of this patch.
>>
>>>       * 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.dg/scal-to-vec1.c: New test.
>>>
>>> bootstrapped and tested on x86_64_unknown-linux
>>
>> +/* Check whether expression EXPR can be converted to the
>> +   vectortype TYPE, considering the case when EXPR is a constant.  */
>> +static bool
>> +expr_fits_type_p (tree expr, tree type)
>>
>> "Return true if expression ..."
>>
>> also documents the return value.  I think the comment should be adjusted
>> to match the intent of the function, namely
>>
>> "... can be converted to TYPE preserving its value."
>>
>> Passing the element type as TYPE seems to be enough as well.
>>
>> The rationale is that OpenCL does not permit truncations to happen,
>> but people will write things like  float_vector + 2.0 (instead of 2.0f),
>> so we allow value-preserving truncations for QOI reasons.
>>
>> +#define SWAP(x, y) do { __typeof (x) __tmp = x; x = y; y = __tmp; } while (0)
>>
>> we don't usually define such thing but expand it everywhere needed.
>>
>> +            {
>> +              tree sc = save_expr(op0);
>>
>> space after save_expr, likewise below.
>>
>> Otherwise looks ok (but I've been looking at the patches before).  I can't
>> approve it though.
>>
>> Thanks,
>> Richard.
>>
>
Joseph S. Myers - Nov. 6, 2010, 5:44 p.m.
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.

Patch

Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 166126)
+++ gcc/doc/extend.texi	(working copy)
@@ -6319,18 +6319,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,79 @@ 
+#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 = 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.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 166126)
+++ gcc/c-typeck.c	(working copy)
@@ -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.  */
+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:
+        -2  --  Error ocured
+        -1  --  Nothing happened
+         0  --  First argument must be expanded
+         1  --  Second argument must be expanded  */
+static int
+scalar_to_vector (location_t loc, enum tree_code code, tree op0, tree op1)
+{
+  tree type0 = TREE_TYPE (op0);
+  tree type1 = TREE_TYPE (op1);
+  int ret = 0;
+  
+  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 -2;
+            }
+          return 0;
+      
+      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)
+          {
+            ret = 1;
+            tree tmp;
+            /* 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 -2;
+              }
+           return ret;
+          }
+        else if (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 -2;
+              }
+           return ret;
+          }
+      default:
+        break;
+    }
+ 
+  return -1;
+}
 
 /* Build a binary-operation expression without default conversions.
    CODE is the kind of expression to build.
@@ -9570,6 +9668,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))
+    {
+      int convert_flag = scalar_to_vector (location, code, op0, op1);
+       
+      switch (convert_flag)
+        {
+          case -2:
+            return error_mark_node;
+          case  0:
+            {
+              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  1:
+            {
+              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: