Fix PR tree-optimization/46213

Hi,

MINUS_EXPR in reduction is supposed to be supported for computations of the
form: res = res - a[i], and not for res = a[i] - res, but this is not
checked. The attached patch adds such check.

Bootstrapped on powerpc64-suse-linux and tested on x86_64-suse-linux.
Committed.

Ira


ChangeLog:

	PR tree-optimization/46213
	* tree-vect-loop.c (vect_is_simple_reduction_1): Handle
	MINUS_EXPR only if the first operand is reduction operand.

testsuite/ChangeLog:

	PR tree-optimization/46213
	* gfortran.dg/vect/pr46213.f90: New.

On Thu, Nov 4, 2010 at 12:51 PM, Ira Rosen <IRAR@il.ibm.com> wrote:
>
> Hi,
>
> MINUS_EXPR in reduction is supposed to be supported for computations of the
> form: res = res - a[i], and not for res = a[i] - res, but this is not
> checked. The attached patch adds such check.

We can simply support this by treating is as

  tmp = -a[i];
  res = res + tmp;

Richard.

> Bootstrapped on powerpc64-suse-linux and tested on x86_64-suse-linux.
> Committed.
>
> Ira
>
>
> ChangeLog:
>
>        PR tree-optimization/46213
>        * tree-vect-loop.c (vect_is_simple_reduction_1): Handle
>        MINUS_EXPR only if the first operand is reduction operand.
>
> testsuite/ChangeLog:
>
>        PR tree-optimization/46213
>        * gfortran.dg/vect/pr46213.f90: New.
>
>
> Index: testsuite/gfortran.dg/vect/pr46213.f90
> ===================================================================
> --- testsuite/gfortran.dg/vect/pr46213.f90      (revision 0)
> +++ testsuite/gfortran.dg/vect/pr46213.f90      (revision 0)
> @@ -0,0 +1,25 @@
> +! { dg-do compile }
> +! { dg-options "-O -fno-tree-loop-ivcanon -ftree-vectorize -fno-tree-ccp
> -fno-tree-ch -finline-small-functions" }
> +
> +module foo
> +  INTEGER, PARAMETER :: ONE = 1
> +end module foo
> +program test
> +  use foo
> +  integer :: a(ONE), b(ONE), c(ONE), d(ONE)
> +  interface
> +    function h_ext()
> +    end function h_ext
> +  end interface
> +  c = j()
> +  if (any (c .ne. check)) call myabort (7)
> +contains
> +  function j()
> +     integer :: j(ONE), cc(ONE)
> +     j = cc - j
> +  end function j
> +  function get_d()
> +  end function get_d
> +end program test
> +
> +! { dg-final { cleanup-tree-dump "vect" } }
> Index: tree-vect-loop.c
> ===================================================================
> --- tree-vect-loop.c    (revision 166172)
> +++ tree-vect-loop.c    (working copy)
> @@ -1800,7 +1800,11 @@ vect_is_simple_reduction_1 (loop_vec_inf
>      simply rewriting this into "res += -x[i]".  Avoid changing
>      gimple instruction for the first simple tests and only do this
>      if we're allowed to change code at all.  */
> -  if (code == MINUS_EXPR && modify)
> +  if (code == MINUS_EXPR
> +      && modify
> +      && (op1 = gimple_assign_rhs1 (def_stmt))
> +      && TREE_CODE (op1) == SSA_NAME
> +      && SSA_NAME_DEF_STMT (op1) == phi)
>     code = PLUS_EXPR;
>
>   if (check_reduction
>
>

Hi,

On Thu, 4 Nov 2010, Richard Guenther wrote:

> On Thu, Nov 4, 2010 at 12:51 PM, Ira Rosen <IRAR@il.ibm.com> wrote:
> >
> > Hi,
> >
> > MINUS_EXPR in reduction is supposed to be supported for computations of the
> > form: res = res - a[i], and not for res = a[i] - res, but this is not
> > checked. The attached patch adds such check.
> 
> We can simply support this by treating is as
> 
>   tmp = -a[i];
>   res = res + tmp;

That would be res = res - a[i], which we _do_ support :)  res = X - res is 
an alternating progression, not so easily supported.


Ciao,
Michael.

Richard Guenther <richard.guenther@gmail.com> wrote on 04/11/2010 04:18:34
PM:

> On Thu, Nov 4, 2010 at 12:51 PM, Ira Rosen <IRAR@il.ibm.com> wrote:
> >
> > Hi,
> >
> > MINUS_EXPR in reduction is supposed to be supported for computations of
the
> > form: res = res - a[i], and not for res = a[i] - res, but this is not
> > checked. The attached patch adds such check.
>
> We can simply support this by treating is as
>
>   tmp = -a[i];
>   res = res + tmp;

But isn't that the res = res - a[i] case? It is supported. This patch
prevents attempts to vectorize res = a[i] - res.

Ira

>
> Richard.
>
> > Bootstrapped on powerpc64-suse-linux and tested on x86_64-suse-linux.
> > Committed.
> >
> > Ira
> >
> >
> > ChangeLog:
> >
> >        PR tree-optimization/46213
> >        * tree-vect-loop.c (vect_is_simple_reduction_1): Handle
> >        MINUS_EXPR only if the first operand is reduction operand.
> >
> > testsuite/ChangeLog:
> >
> >        PR tree-optimization/46213
> >        * gfortran.dg/vect/pr46213.f90: New.
> >
> >
> > Index: testsuite/gfortran.dg/vect/pr46213.f90
> > ===================================================================
> > --- testsuite/gfortran.dg/vect/pr46213.f90      (revision 0)
> > +++ testsuite/gfortran.dg/vect/pr46213.f90      (revision 0)
> > @@ -0,0 +1,25 @@
> > +! { dg-do compile }
> > +! { dg-options "-O -fno-tree-loop-ivcanon -ftree-vectorize
-fno-tree-ccp
> > -fno-tree-ch -finline-small-functions" }
> > +
> > +module foo
> > +  INTEGER, PARAMETER :: ONE = 1
> > +end module foo
> > +program test
> > +  use foo
> > +  integer :: a(ONE), b(ONE), c(ONE), d(ONE)
> > +  interface
> > +    function h_ext()
> > +    end function h_ext
> > +  end interface
> > +  c = j()
> > +  if (any (c .ne. check)) call myabort (7)
> > +contains
> > +  function j()
> > +     integer :: j(ONE), cc(ONE)
> > +     j = cc - j
> > +  end function j
> > +  function get_d()
> > +  end function get_d
> > +end program test
> > +
> > +! { dg-final { cleanup-tree-dump "vect" } }
> > Index: tree-vect-loop.c
> > ===================================================================
> > --- tree-vect-loop.c    (revision 166172)
> > +++ tree-vect-loop.c    (working copy)
> > @@ -1800,7 +1800,11 @@ vect_is_simple_reduction_1 (loop_vec_inf
> >      simply rewriting this into "res += -x[i]".  Avoid changing
> >      gimple instruction for the first simple tests and only do this
> >      if we're allowed to change code at all.  */
> > -  if (code == MINUS_EXPR && modify)
> > +  if (code == MINUS_EXPR
> > +      && modify
> > +      && (op1 = gimple_assign_rhs1 (def_stmt))
> > +      && TREE_CODE (op1) == SSA_NAME
> > +      && SSA_NAME_DEF_STMT (op1) == phi)
> >     code = PLUS_EXPR;
> >
> >   if (check_reduction
> >
> >