Logic operators ! && || for vectors

Hello,

this adds support for vector !, && and ||. In the long run, I think it 
would be good to be able to use TRUTH_*_EXPR with vectors, but that's 
probably a lot of work.

It currently restricts && and || to vector-vector operations. I'd like to 
also support mixed scalar-vector later, but it is a bit more complicated. 
With vectors, && evaluates both sides. For scal && vec, we have the choice 
of making it short-circuit: cond_expr((bool)scal, vec!=0, {0}) or do a 
vector and. For vec && scal, it seems clear we have to evaluate both 
operands, but then we can also make it a cond_expr instead of a 
BIT_AND_EXPR (technically, I think I can achieve that with save_expr and a 
compound_expr, I don't know if there is a better way to add statements).

The missing save_expr before build_vector_from_val are a bug I introduced 
when I adapted the code from the C front-end. This bit (and the 
vector-scalar-2.c testcase that goes with it) should probably be 
backported to 4.8.

The code we generate for these examples is not very good, but that's a 
different issue.

Bootstrap+testsuite on x86_64-linux-gnu.

2013-04-12  Marc Glisse  <marc.glisse@inria.fr>

gcc/cp/
 	* typeck.c (cp_build_binary_op): Call save_expr before
 	build_vector_from_val.
 	<TRUTH_ANDIF_EXPR, TRUTH_ORIF_EXPR, TRUTH_AND_EXPR,
 	TRUTH_OR_EXPR>: Handle vectors.
 	(cp_build_unary_op) <TRUTH_NOT_EXPR>: Likewise.

gcc/c-family/
 	* c-common.c (warn_logical_operator): Give up for vectors.

gcc/testsuite/
 	* c-c++-common/vector-scalar-2.c: New testcase.
 	* g++.dg/ext/vector22.C: Likewise.
 	* g++.dg/ext/vector23.C: Likewise.
 	* g++.dg/ext/vector9.C: Adapt.
 	* g++.dg/other/error23.C: Adapt.

On Fri, Apr 12, 2013 at 11:03 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> Hello,
>
> this adds support for vector !, && and ||. In the long run, I think it would
> be good to be able to use TRUTH_*_EXPR with vectors, but that's probably a
> lot of work.
>
> It currently restricts && and || to vector-vector operations. I'd like to
> also support mixed scalar-vector later, but it is a bit more complicated.
> With vectors, && evaluates both sides.

Exactly for this reason I think supporting logical operations is ill-defined
for vectors.  The way we define "boolean" vectors allows to simply
use bit operations here - so what issue do you want to address?  The
automatic X-to-bool decay?  Thus, avoid the need to write v1 != 0 | v2 != 0?

Thanks,
Richard.

> For scal && vec, we have the choice
> of making it short-circuit: cond_expr((bool)scal, vec!=0, {0}) or do a
> vector and. For vec && scal, it seems clear we have to evaluate both
> operands, but then we can also make it a cond_expr instead of a BIT_AND_EXPR
> (technically, I think I can achieve that with save_expr and a compound_expr,
> I don't know if there is a better way to add statements).
>
> The missing save_expr before build_vector_from_val are a bug I introduced
> when I adapted the code from the C front-end. This bit (and the
> vector-scalar-2.c testcase that goes with it) should probably be backported
> to 4.8.
>
> The code we generate for these examples is not very good, but that's a
> different issue.
>
> Bootstrap+testsuite on x86_64-linux-gnu.
>
> 2013-04-12  Marc Glisse  <marc.glisse@inria.fr>
>
> gcc/cp/
>         * typeck.c (cp_build_binary_op): Call save_expr before
>         build_vector_from_val.
>         <TRUTH_ANDIF_EXPR, TRUTH_ORIF_EXPR, TRUTH_AND_EXPR,
>         TRUTH_OR_EXPR>: Handle vectors.
>         (cp_build_unary_op) <TRUTH_NOT_EXPR>: Likewise.
>
> gcc/c-family/
>         * c-common.c (warn_logical_operator): Give up for vectors.
>
> gcc/testsuite/
>         * c-c++-common/vector-scalar-2.c: New testcase.
>         * g++.dg/ext/vector22.C: Likewise.
>         * g++.dg/ext/vector23.C: Likewise.
>         * g++.dg/ext/vector9.C: Adapt.
>         * g++.dg/other/error23.C: Adapt.
>
> --
> Marc Glisse
> Index: gcc/c-family/c-common.c
> ===================================================================
> --- gcc/c-family/c-common.c     (revision 197846)
> +++ gcc/c-family/c-common.c     (working copy)
> @@ -1624,20 +1624,24 @@ warn_logical_operator (location_t locati
>    if (CONSTANT_CLASS_P (op_left) || CONSTANT_CLASS_P (op_right))
>      return;
>
>    /* This warning only makes sense with logical operands.  */
>    if (!(truth_value_p (TREE_CODE (op_left))
>         || INTEGRAL_TYPE_P (TREE_TYPE (op_left)))
>        || !(truth_value_p (TREE_CODE (op_right))
>            || INTEGRAL_TYPE_P (TREE_TYPE (op_right))))
>      return;
>
> +  /* The range computations only work with scalars.  */
> +  if (VECTOR_TYPE_P (TREE_TYPE (op_left))
> +      || VECTOR_TYPE_P (TREE_TYPE (op_right)))
> +    return;
>
>    /* We first test whether either side separately is trivially true
>       (with OR) or trivially false (with AND).  If so, do not warn.
>       This is a common idiom for testing ranges of data types in
>       portable code.  */
>    lhs = make_range (op_left, &in0_p, &low0, &high0, &strict_overflow_p);
>    if (!lhs)
>      return;
>    if (TREE_CODE (lhs) == C_MAYBE_CONST_EXPR)
>      lhs = C_MAYBE_CONST_EXPR_EXPR (lhs);
> Index: gcc/cp/typeck.c
> ===================================================================
> --- gcc/cp/typeck.c     (revision 197846)
> +++ gcc/cp/typeck.c     (working copy)
> @@ -3970,30 +3970,30 @@ cp_build_binary_op (location_t location,
>      {
>        enum stv_conv convert_flag = scalar_to_vector (location, code, op0,
> op1,
>                                                      complain & tf_error);
>
>        switch (convert_flag)
>          {
>            case stv_error:
>              return error_mark_node;
>            case stv_firstarg:
>              {
> -              op0 = convert (TREE_TYPE (type1), op0);
> +              op0 = save_expr (convert (TREE_TYPE (type1), op0));
>                op0 = build_vector_from_val (type1, op0);
>                type0 = TREE_TYPE (op0);
>                code0 = TREE_CODE (type0);
>                converted = 1;
>                break;
>              }
>            case stv_secondarg:
>              {
> -              op1 = convert (TREE_TYPE (type0), op1);
> +              op1 = save_expr (convert (TREE_TYPE (type0), op1));
>                op1 = build_vector_from_val (type0, op1);
>                type1 = TREE_TYPE (op1);
>                code1 = TREE_CODE (type1);
>                converted = 1;
>                break;
>              }
>            default:
>              break;
>          }
>      }
> @@ -4111,20 +4111,43 @@ cp_build_binary_op (location_t location,
>                      || (TREE_CODE (op1) == INTEGER_CST
>                          && ! integer_all_onesp (op1)));
>           common = 1;
>         }
>        break;
>
>      case TRUTH_ANDIF_EXPR:
>      case TRUTH_ORIF_EXPR:
>      case TRUTH_AND_EXPR:
>      case TRUTH_OR_EXPR:
> +      if (VECTOR_TYPE_P (type0) != VECTOR_TYPE_P (type1))
> +       {
> +         if (complain & tf_error)
> +            sorry ("%<&&%> or %<||%> with a vector and a scalar");
> +         return error_mark_node;
> +       }
> +      if (VECTOR_TYPE_P (type0) && VECTOR_TYPE_P (type1))
> +       {
> +         if (!COMPARISON_CLASS_P (op0))
> +           op0 = cp_build_binary_op (EXPR_LOCATION (op0), NE_EXPR, op0,
> +                                     build_zero_cst (type0), complain);
> +         if (!COMPARISON_CLASS_P (op1))
> +           op1 = cp_build_binary_op (EXPR_LOCATION (op1), NE_EXPR, op1,
> +                                     build_zero_cst (type1), complain);
> +
> +         if (code == TRUTH_ANDIF_EXPR || code == TRUTH_AND_EXPR)
> +           code = BIT_AND_EXPR;
> +         else /* if (code == TRUTH_ORIF_EXPR || code == TRUTH_OR_EXPR) */
> +           code = BIT_IOR_EXPR;
> +
> +         return cp_build_binary_op (location, code, op0, op1, complain);
> +       }
> +
>        result_type = boolean_type_node;
>        break;
>
>        /* Shift operations: result has same type as first operand;
>          always convert second operand to int.
>          Also set SHORT_SHIFT if shifting rightward.  */
>
>      case RSHIFT_EXPR:
>        if (code0 == VECTOR_TYPE && code1 == INTEGER_TYPE
>            && TREE_CODE (TREE_TYPE (type0)) == INTEGER_TYPE)
> @@ -5476,20 +5499,23 @@ cp_build_unary_op (enum tree_code code,
>         errstring = _("wrong type argument to conjugation");
>        else if (!noconvert)
>         {
>           arg = cp_default_conversion (arg, complain);
>           if (arg == error_mark_node)
>             return error_mark_node;
>         }
>        break;
>
>      case TRUTH_NOT_EXPR:
> +      if (VECTOR_INTEGER_TYPE_P (TREE_TYPE (arg)))
> +       return cp_build_binary_op (input_location, EQ_EXPR, arg,
> +                                  build_zero_cst (TREE_TYPE (arg)),
> complain);
>        arg = perform_implicit_conversion (boolean_type_node, arg,
>                                          complain);
>        val = invert_truthvalue_loc (input_location, arg);
>        if (arg != error_mark_node)
>         return val;
>        errstring = _("in argument to unary !");
>        break;
>
>      case NOP_EXPR:
>        break;
> Index: gcc/testsuite/g++.dg/ext/vector22.C
> ===================================================================
> --- gcc/testsuite/g++.dg/ext/vector22.C (revision 0)
> +++ gcc/testsuite/g++.dg/ext/vector22.C (revision 0)
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +
> +typedef int veci __attribute__ ((vector_size (4 * sizeof (int))));
> +typedef float vecf __attribute__ ((vector_size (4 * sizeof (float))));
> +
> +void f (veci *a, veci *b, int c)
> +{
> +  *a = !*a || *b < ++c;
> +}
> +void g (vecf *a, vecf *b)
> +{
> +  *a = (*a < 1 && !(*b > 2)) ? *a + *b : 3;
> +}
>
> Property changes on: gcc/testsuite/g++.dg/ext/vector22.C
> ___________________________________________________________________
> Added: svn:eol-style
>    + native
> Added: svn:keywords
>    + Author Date Id Revision URL
>
> Index: gcc/testsuite/g++.dg/ext/vector23.C
> ===================================================================
> --- gcc/testsuite/g++.dg/ext/vector23.C (revision 0)
> +++ gcc/testsuite/g++.dg/ext/vector23.C (revision 0)
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +
> +typedef int veci __attribute__ ((vector_size (4 * sizeof (int))));
> +typedef float vecf __attribute__ ((vector_size (4 * sizeof (float))));
> +
> +void f (veci *a, veci *b, int c)
> +{
> +  *a = *a || c; /* { dg-message "a vector and a scalar" } */
> +}
>
> Property changes on: gcc/testsuite/g++.dg/ext/vector23.C
> ___________________________________________________________________
> Added: svn:keywords
>    + Author Date Id Revision URL
> Added: svn:eol-style
>    + native
>
> Index: gcc/testsuite/g++.dg/ext/vector9.C
> ===================================================================
> --- gcc/testsuite/g++.dg/ext/vector9.C  (revision 197846)
> +++ gcc/testsuite/g++.dg/ext/vector9.C  (working copy)
> @@ -1,10 +1,10 @@
>  // PR c++/34891
>
>  typedef float v4f __attribute__((vector_size(8)));
>  typedef int   v4i __attribute__((vector_size(8)));
>
>  void foo()
>  {
>    v4f v;
> -  !(v4i)v; // { dg-error "v4i|argument" }
> +  !(v4i)v;
>  }
> Index: gcc/testsuite/g++.dg/other/error23.C
> ===================================================================
> --- gcc/testsuite/g++.dg/other/error23.C        (revision 197846)
> +++ gcc/testsuite/g++.dg/other/error23.C        (working copy)
> @@ -1,5 +1,5 @@
>  // PR c++/34918
>  // { dg-do compile }
>
>  int v __attribute ((vector_size (8)));
> -bool b = !(v - v);     // { dg-error "could not convert .\\(__vector.2.
> int\\)\\{0, 0\\}. from .__vector.2. int. to .bool.|in argument to unary" }
> +bool b = !(v - v);     // { dg-error "not convert .__vector.2. int. to
> .bool. in initialization" }
> Index: gcc/testsuite/c-c++-common/vector-scalar-2.c
> ===================================================================
> --- gcc/testsuite/c-c++-common/vector-scalar-2.c        (revision 0)
> +++ gcc/testsuite/c-c++-common/vector-scalar-2.c        (revision 0)
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fdump-tree-gimple" } */
> +
> +typedef int veci __attribute__ ((vector_size (4 * sizeof (int))));
> +
> +int c;
> +
> +void f (veci *a)
> +{
> +  *a = *a + ++c;
> +}
> +
> +/* { dg-final { scan-tree-dump-times " \\\+ 1" 1 "gimple" } } */
> +/* { dg-final { cleanup-tree-dump "gimple" } } */
>
> Property changes on: gcc/testsuite/c-c++-common/vector-scalar-2.c
> ___________________________________________________________________
> Added: svn:eol-style
>    + native
> Added: svn:keywords
>    + Author Date Id Revision URL
>
>

On Fri, 12 Apr 2013, Richard Biener wrote:

> On Fri, Apr 12, 2013 at 11:03 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Hello,
>>
>> this adds support for vector !, && and ||. In the long run, I think it would
>> be good to be able to use TRUTH_*_EXPR with vectors, but that's probably a
>> lot of work.
>>
>> It currently restricts && and || to vector-vector operations. I'd like to
>> also support mixed scalar-vector later, but it is a bit more complicated.
>> With vectors, && evaluates both sides.
>
> Exactly for this reason I think supporting logical operations is ill-defined
> for vectors.

First, as a remark, in C++, && and || can be overloaded by the user, and 
in that case they are not short-circuit, so doing this for vectors 
wouldn't really be anything new. OpenCL also defines && and || for 
vectors.

> The way we define "boolean" vectors allows to simply
> use bit operations here - so what issue do you want to address?  The
> automatic X-to-bool decay?  Thus, avoid the need to write v1 != 0 | v2 != 0?

My goal would be for people to share the code between the scalar and 
vector versions, that code written for scalars would start just working 
with vectors. Since people use !, && and || for scalars and not ~, & and 
|, that means supporting those.

(there would still always be traps, like the fact that the equivalent of 
"true" for vectors is -1, or that && is not short-circuit, but most code 
would be fine)

Ping http://gcc.gnu.org/ml/gcc-patches/2013-04/msg00783.html

Even if we decide not to implement logic operators in the front-end, we 
still need the fix for the wrong code (the 2 save_expr in 
cp_build_binary_op, is that part of the patch ok with the 
vector-scalar-2.c testcase? and for 4.8?) and to avoid ICEing on __m128i 
f(__m128d a,__m128d b){return a<b&&b<a;} as we currently do in 
warn_logical_operator.

On Fri, 12 Apr 2013, Marc Glisse wrote:

> Hello,
>
> this adds support for vector !, && and ||. In the long run, I think it would 
> be good to be able to use TRUTH_*_EXPR with vectors, but that's probably a 
> lot of work.
>
> It currently restricts && and || to vector-vector operations. I'd like to 
> also support mixed scalar-vector later, but it is a bit more complicated. 
> With vectors, && evaluates both sides. For scal && vec, we have the choice of 
> making it short-circuit: cond_expr((bool)scal, vec!=0, {0}) or do a vector 
> and. For vec && scal, it seems clear we have to evaluate both operands, but 
> then we can also make it a cond_expr instead of a BIT_AND_EXPR (technically, 
> I think I can achieve that with save_expr and a compound_expr, I don't know 
> if there is a better way to add statements).
>
> The missing save_expr before build_vector_from_val are a bug I introduced 
> when I adapted the code from the C front-end. This bit (and the 
> vector-scalar-2.c testcase that goes with it) should probably be backported 
> to 4.8.
>
> The code we generate for these examples is not very good, but that's a 
> different issue.
>
> Bootstrap+testsuite on x86_64-linux-gnu.
>
> 2013-04-12  Marc Glisse  <marc.glisse@inria.fr>
>
> gcc/cp/
> 	* typeck.c (cp_build_binary_op): Call save_expr before
> 	build_vector_from_val.
> 	<TRUTH_ANDIF_EXPR, TRUTH_ORIF_EXPR, TRUTH_AND_EXPR,
> 	TRUTH_OR_EXPR>: Handle vectors.
> 	(cp_build_unary_op) <TRUTH_NOT_EXPR>: Likewise.
>
> gcc/c-family/
> 	* c-common.c (warn_logical_operator): Give up for vectors.
>
> gcc/testsuite/
> 	* c-c++-common/vector-scalar-2.c: New testcase.
> 	* g++.dg/ext/vector22.C: Likewise.
> 	* g++.dg/ext/vector23.C: Likewise.
> 	* g++.dg/ext/vector9.C: Adapt.
> 	* g++.dg/other/error23.C: Adapt.
>
>

On Fri, Apr 26, 2013 at 2:23 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> Ping http://gcc.gnu.org/ml/gcc-patches/2013-04/msg00783.html
>
> Even if we decide not to implement logic operators in the front-end, we
> still need the fix for the wrong code (the 2 save_expr in
> cp_build_binary_op, is that part of the patch ok with the vector-scalar-2.c
> testcase? and for 4.8?) and to avoid ICEing on __m128i f(__m128d a,__m128d
> b){return a<b&&b<a;} as we currently do in warn_logical_operator.

That part looks ok, but I'd rather have Jason approve it.  Maybe post it
separately?

Thanks,
Richard.

> On Fri, 12 Apr 2013, Marc Glisse wrote:
>
>> Hello,
>>
>> this adds support for vector !, && and ||. In the long run, I think it
>> would be good to be able to use TRUTH_*_EXPR with vectors, but that's
>> probably a lot of work.
>>
>> It currently restricts && and || to vector-vector operations. I'd like to
>> also support mixed scalar-vector later, but it is a bit more complicated.
>> With vectors, && evaluates both sides. For scal && vec, we have the choice
>> of making it short-circuit: cond_expr((bool)scal, vec!=0, {0}) or do a
>> vector and. For vec && scal, it seems clear we have to evaluate both
>> operands, but then we can also make it a cond_expr instead of a BIT_AND_EXPR
>> (technically, I think I can achieve that with save_expr and a compound_expr,
>> I don't know if there is a better way to add statements).
>>
>> The missing save_expr before build_vector_from_val are a bug I introduced
>> when I adapted the code from the C front-end. This bit (and the
>> vector-scalar-2.c testcase that goes with it) should probably be backported
>> to 4.8.
>>
>> The code we generate for these examples is not very good, but that's a
>> different issue.
>>
>> Bootstrap+testsuite on x86_64-linux-gnu.
>>
>> 2013-04-12  Marc Glisse  <marc.glisse@inria.fr>
>>
>> gcc/cp/
>>         * typeck.c (cp_build_binary_op): Call save_expr before
>>         build_vector_from_val.
>>         <TRUTH_ANDIF_EXPR, TRUTH_ORIF_EXPR, TRUTH_AND_EXPR,
>>         TRUTH_OR_EXPR>: Handle vectors.
>>         (cp_build_unary_op) <TRUTH_NOT_EXPR>: Likewise.
>>
>> gcc/c-family/
>>         * c-common.c (warn_logical_operator): Give up for vectors.
>>
>> gcc/testsuite/
>>         * c-c++-common/vector-scalar-2.c: New testcase.
>>         * g++.dg/ext/vector22.C: Likewise.
>>         * g++.dg/ext/vector23.C: Likewise.
>>         * g++.dg/ext/vector9.C: Adapt.
>>         * g++.dg/other/error23.C: Adapt.
>>
>>
>
> --
> Marc Glisse