Patchwork Fold REDUC_*_EXPR

login
register
mail settings
Submitter Jakub Jelinek
Date Oct. 10, 2012, 5:49 p.m.
Message ID <20121010174940.GA26735@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/190734/
State New
Headers show

Comments

Jakub Jelinek - Oct. 10, 2012, 5:49 p.m.
Hi!

This patch folds REDUC_*_EXPR (e.g. on pr54877.c -Ofast -mavx
testcase we end up with unfolded REDUC_PLUS_EXPR till *.optimized).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2012-10-10  Jakub Jelinek  <jakub@redhat.com>

	* fold-const.c (fold_unary_loc): Handle REDUC_MIN_EXPR,
	REDUC_MAX_EXPR and REDUC_PLUS_EXPR.


	Jakub
Richard Guenther - Oct. 11, 2012, 1:42 p.m.
On Wed, Oct 10, 2012 at 7:49 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> This patch folds REDUC_*_EXPR (e.g. on pr54877.c -Ofast -mavx
> testcase we end up with unfolded REDUC_PLUS_EXPR till *.optimized).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Eh, I didn't realize the result type is a vector ... can you adjust the docs
in tree.def to reflect that the _result_ is a vector with the first element
having the result and operand 0 is the vector being reduced?

Ok.

Thanks,
Richard.

> 2012-10-10  Jakub Jelinek  <jakub@redhat.com>
>
>         * fold-const.c (fold_unary_loc): Handle REDUC_MIN_EXPR,
>         REDUC_MAX_EXPR and REDUC_PLUS_EXPR.
>
> --- gcc/fold-const.c.jj 2012-10-09 13:42:30.000000000 +0200
> +++ gcc/fold-const.c    2012-10-10 12:28:11.768999520 +0200
> @@ -8294,6 +8294,40 @@ fold_unary_loc (location_t loc, enum tre
>         return build_vector (type, elts);
>        }
>
> +    case REDUC_MIN_EXPR:
> +    case REDUC_MAX_EXPR:
> +    case REDUC_PLUS_EXPR:
> +      {
> +       unsigned int nelts = TYPE_VECTOR_SUBPARTS (type), i;
> +       tree *elts;
> +       enum tree_code subcode;
> +
> +       if (TREE_CODE (op0) != VECTOR_CST)
> +         return NULL_TREE;
> +
> +       elts = XALLOCAVEC (tree, nelts);
> +       if (!vec_cst_ctor_to_array (op0, elts))
> +         return NULL_TREE;
> +
> +       switch (code)
> +         {
> +         case REDUC_MIN_EXPR: subcode = MIN_EXPR; break;
> +         case REDUC_MAX_EXPR: subcode = MAX_EXPR; break;
> +         case REDUC_PLUS_EXPR: subcode = PLUS_EXPR; break;
> +         default: gcc_unreachable ();
> +         }
> +
> +       for (i = 1; i < nelts; i++)
> +         {
> +           elts[0] = const_binop (subcode, elts[0], elts[i]);
> +           if (elts[0] == NULL_TREE || !CONSTANT_CLASS_P (elts[0]))
> +             return NULL_TREE;
> +           elts[i] = build_zero_cst (TREE_TYPE (type));
> +         }
> +
> +       return build_vector (type, elts);
> +      }
> +
>      default:
>        return NULL_TREE;
>      } /* switch (code) */
>
>         Jakub
Jakub Jelinek - Oct. 11, 2012, 1:47 p.m.
On Thu, Oct 11, 2012 at 03:42:44PM +0200, Richard Biener wrote:
> On Wed, Oct 10, 2012 at 7:49 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > This patch folds REDUC_*_EXPR (e.g. on pr54877.c -Ofast -mavx
> > testcase we end up with unfolded REDUC_PLUS_EXPR till *.optimized).
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> Eh, I didn't realize the result type is a vector ... can you adjust the docs
> in tree.def to reflect that the _result_ is a vector with the first element
> having the result and operand 0 is the vector being reduced?

The comment wants to say that, but doesn't do it correctly:
   Operand 0 is a vector; the first element in the vector has the result.
   Operand 1 is a vector.  */
because obviously it doesn't have two operands, just one.  So it should be
perhaps
   Operand 0 is a vector.
   The expression returns a vector of the same type, with the first
   element in the vector holding the result of the reduction.  */
?

	Jakub
Richard Guenther - Oct. 11, 2012, 2:30 p.m.
On Thu, Oct 11, 2012 at 3:47 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Oct 11, 2012 at 03:42:44PM +0200, Richard Biener wrote:
>> On Wed, Oct 10, 2012 at 7:49 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > This patch folds REDUC_*_EXPR (e.g. on pr54877.c -Ofast -mavx
>> > testcase we end up with unfolded REDUC_PLUS_EXPR till *.optimized).
>> >
>> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>>
>> Eh, I didn't realize the result type is a vector ... can you adjust the docs
>> in tree.def to reflect that the _result_ is a vector with the first element
>> having the result and operand 0 is the vector being reduced?
>
> The comment wants to say that, but doesn't do it correctly:
>    Operand 0 is a vector; the first element in the vector has the result.
>    Operand 1 is a vector.  */
> because obviously it doesn't have two operands, just one.  So it should be
> perhaps
>    Operand 0 is a vector.
>    The expression returns a vector of the same type, with the first
>    element in the vector holding the result of the reduction.  */
> ?

Yes.  It also should specify that the other elements are zero (or is that
just your choice of "arbitrary"?)

Richard.

>         Jakub
Jakub Jelinek - Oct. 11, 2012, 2:33 p.m.
On Thu, Oct 11, 2012 at 04:30:42PM +0200, Richard Biener wrote:
> > The comment wants to say that, but doesn't do it correctly:
> >    Operand 0 is a vector; the first element in the vector has the result.
> >    Operand 1 is a vector.  */
> > because obviously it doesn't have two operands, just one.  So it should be
> > perhaps
> >    Operand 0 is a vector.
> >    The expression returns a vector of the same type, with the first
> >    element in the vector holding the result of the reduction.  */
> > ?
> 
> Yes.  It also should specify that the other elements are zero (or is that
> just your choice of "arbitrary"?)

That was my choice of arbitrary.  Guess what the hw insns leave in there is
pretty much random, could be copies of the result in all elements, or
temporaries from pairwise reductions, etc.

	Jakub
Richard Guenther - Oct. 11, 2012, 2:38 p.m.
On Thu, Oct 11, 2012 at 4:33 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Oct 11, 2012 at 04:30:42PM +0200, Richard Biener wrote:
>> > The comment wants to say that, but doesn't do it correctly:
>> >    Operand 0 is a vector; the first element in the vector has the result.
>> >    Operand 1 is a vector.  */
>> > because obviously it doesn't have two operands, just one.  So it should be
>> > perhaps
>> >    Operand 0 is a vector.
>> >    The expression returns a vector of the same type, with the first
>> >    element in the vector holding the result of the reduction.  */
>> > ?
>>
>> Yes.  It also should specify that the other elements are zero (or is that
>> just your choice of "arbitrary"?)
>
> That was my choice of arbitrary.  Guess what the hw insns leave in there is
> pretty much random, could be copies of the result in all elements, or
> temporaries from pairwise reductions, etc.

Ok - in other places we try to match HW behavior during constant folding
(famous SHIFT_COUNT_TRUNCATED handling ...), but I suppose it's
not even documented.

Richard.

>         Jakub

Patch

--- gcc/fold-const.c.jj	2012-10-09 13:42:30.000000000 +0200
+++ gcc/fold-const.c	2012-10-10 12:28:11.768999520 +0200
@@ -8294,6 +8294,40 @@  fold_unary_loc (location_t loc, enum tre
 	return build_vector (type, elts);
       }
 
+    case REDUC_MIN_EXPR:
+    case REDUC_MAX_EXPR:
+    case REDUC_PLUS_EXPR:
+      {
+	unsigned int nelts = TYPE_VECTOR_SUBPARTS (type), i;
+	tree *elts;
+	enum tree_code subcode;
+
+	if (TREE_CODE (op0) != VECTOR_CST)
+	  return NULL_TREE;
+
+	elts = XALLOCAVEC (tree, nelts);
+	if (!vec_cst_ctor_to_array (op0, elts))
+	  return NULL_TREE;
+
+	switch (code)
+	  {
+	  case REDUC_MIN_EXPR: subcode = MIN_EXPR; break;
+	  case REDUC_MAX_EXPR: subcode = MAX_EXPR; break;
+	  case REDUC_PLUS_EXPR: subcode = PLUS_EXPR; break;
+	  default: gcc_unreachable ();
+	  }
+
+	for (i = 1; i < nelts; i++)
+	  {
+	    elts[0] = const_binop (subcode, elts[0], elts[i]);
+	    if (elts[0] == NULL_TREE || !CONSTANT_CLASS_P (elts[0]))
+	      return NULL_TREE;
+	    elts[i] = build_zero_cst (TREE_TYPE (type));
+	  }
+
+	return build_vector (type, elts);
+      }
+
     default:
       return NULL_TREE;
     } /* switch (code) */