[PATCH][RFC] Fix P1 PR77498

[PATCH][RFC] Fix P1 PR77498

[Richard Biener]

After quite some pondering over this and other related bugs I propose
the following for GCC 7 which tames down PRE a bit (back to levels
of GCC 6).  Technically it's the wrong place to fix this, we do
have measures in place during elimination but they are not in effect
at -O2.  For GCC 8 I'd like to be more aggressive there but that
would require to enable predictive commoning at -O2 (with some
limits to its unrolling) to not lose optimization opportunities.

The other option is to ignore this issue and postpone the solution
to GCC 8.

Bootstrapped / tested on x86_64-unknown-linux-gnu.

Any preference?

Thanks,
Richard.

2017-03-29  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/77498
	* tree-ssa-pre.c (phi_translate_1): Do not allow simplifications
	to non-constants over backedges.

	* gfortran.dg/pr77498.f: New testcase.

Index: gcc/tree-ssa-pre.c
===================================================================
*** gcc/tree-ssa-pre.c	(revision 246026)
--- gcc/tree-ssa-pre.c	(working copy)
*************** phi_translate_1 (pre_expr expr, bitmap_s
*** 1468,1477 ****
  		   leader for it.  */
  		if (constant->kind != CONSTANT)
  		  {
! 		    unsigned value_id = get_expr_value_id (constant);
! 		    constant = find_leader_in_sets (value_id, set1, set2);
! 		    if (constant)
! 		      return constant;
  		  }
  		else
  		  return constant;
--- 1468,1487 ----
  		   leader for it.  */
  		if (constant->kind != CONSTANT)
  		  {
! 		    /* Do not allow simplifications to non-constants over
! 		       backedges as this will likely result in a loop PHI node
! 		       to be inserted and increased register pressure.
! 		       See PR77498 - this avoids doing predcoms work in
! 		       a less efficient way.  */
! 		    if (find_edge (pred, phiblock)->flags & EDGE_DFS_BACK)
! 		      ;
! 		    else
! 		      {
! 			unsigned value_id = get_expr_value_id (constant);
! 			constant = find_leader_in_sets (value_id, set1, set2);
! 			if (constant)
! 			  return constant;
! 		      }
  		  }
  		else
  		  return constant;
Index: gcc/testsuite/gfortran.dg/pr77498.f
===================================================================
--- gcc/testsuite/gfortran.dg/pr77498.f	(nonexistent)
+++ gcc/testsuite/gfortran.dg/pr77498.f	(working copy)
@@ -0,0 +1,36 @@
+! { dg-do compile }
+! { dg-options "-O2 -ffast-math -fdump-tree-pre" }
+
+      subroutine foo(U,V,R,N,A)
+      integer N
+      real*8 U(N,N,N),V(N,N,N),R(N,N,N),A(0:3)
+      integer I3, I2, I1
+C
+      do I3=2,N-1
+       do I2=2,N-1
+        do I1=2,N-1
+         R(I1,I2,I3)=V(I1,I2,I3)
+     *      -A(0)*( U(I1,  I2,  I3  ) )
+     *      -A(1)*( U(I1-1,I2,  I3  ) + U(I1+1,I2,  I3  )
+     *                 +  U(I1,  I2-1,I3  ) + U(I1,  I2+1,I3  )
+     *                 +  U(I1,  I2,  I3-1) + U(I1,  I2,  I3+1) )
+     *      -A(2)*( U(I1-1,I2-1,I3  ) + U(I1+1,I2-1,I3  )
+     *                 +  U(I1-1,I2+1,I3  ) + U(I1+1,I2+1,I3  )
+     *                 +  U(I1,  I2-1,I3-1) + U(I1,  I2+1,I3-1)
+     *                 +  U(I1,  I2-1,I3+1) + U(I1,  I2+1,I3+1)
+     *                 +  U(I1-1,I2,  I3-1) + U(I1-1,I2,  I3+1)
+     *                 +  U(I1+1,I2,  I3-1) + U(I1+1,I2,  I3+1) )
+     *      -A(3)*( U(I1-1,I2-1,I3-1) + U(I1+1,I2-1,I3-1)
+     *                 +  U(I1-1,I2+1,I3-1) + U(I1+1,I2+1,I3-1)
+     *                 +  U(I1-1,I2-1,I3+1) + U(I1+1,I2-1,I3+1)
+     *                 +  U(I1-1,I2+1,I3+1) + U(I1+1,I2+1,I3+1) )
+        enddo
+       enddo
+      enddo
+      return
+      end
+
+! PRE shouldn't do predictive commonings job here (and in a bad way)
+! ???  It still does but not as bad as it could.  Less prephitmps
+! would be better, pcom does it with 6.
+! { dg-final { scan-tree-dump-times "# prephitmp" 9 "pre" } }

Re: [PATCH][RFC] Fix P1 PR77498

[Bin.Cheng]

On Wed, Mar 29, 2017 at 11:05 AM, Richard Biener <rguenther@suse.de> wrote:
>
> After quite some pondering over this and other related bugs I propose
> the following for GCC 7 which tames down PRE a bit (back to levels
> of GCC 6).  Technically it's the wrong place to fix this, we do
> have measures in place during elimination but they are not in effect
> at -O2.  For GCC 8 I'd like to be more aggressive there but that
> would require to enable predictive commoning at -O2 (with some
> limits to its unrolling) to not lose optimization opportunities.
>
> The other option is to ignore this issue and postpone the solution
> to GCC 8.
>
> Bootstrapped / tested on x86_64-unknown-linux-gnu.
>
> Any preference?
+1 enabling predcom at O2 for this kind of transformation, remember I
once suggested that.

Thanks,
bin
>
> Thanks,
> Richard.
>
> 2017-03-29  Richard Biener  <rguenther@suse.de>
>
>         PR tree-optimization/77498
>         * tree-ssa-pre.c (phi_translate_1): Do not allow simplifications
>         to non-constants over backedges.
>
>         * gfortran.dg/pr77498.f: New testcase.
>
> Index: gcc/tree-ssa-pre.c
> ===================================================================
> *** gcc/tree-ssa-pre.c  (revision 246026)
> --- gcc/tree-ssa-pre.c  (working copy)
> *************** phi_translate_1 (pre_expr expr, bitmap_s
> *** 1468,1477 ****
>                    leader for it.  */
>                 if (constant->kind != CONSTANT)
>                   {
> !                   unsigned value_id = get_expr_value_id (constant);
> !                   constant = find_leader_in_sets (value_id, set1, set2);
> !                   if (constant)
> !                     return constant;
>                   }
>                 else
>                   return constant;
> --- 1468,1487 ----
>                    leader for it.  */
>                 if (constant->kind != CONSTANT)
>                   {
> !                   /* Do not allow simplifications to non-constants over
> !                      backedges as this will likely result in a loop PHI node
> !                      to be inserted and increased register pressure.
> !                      See PR77498 - this avoids doing predcoms work in
> !                      a less efficient way.  */
> !                   if (find_edge (pred, phiblock)->flags & EDGE_DFS_BACK)
> !                     ;
> !                   else
> !                     {
> !                       unsigned value_id = get_expr_value_id (constant);
> !                       constant = find_leader_in_sets (value_id, set1, set2);
> !                       if (constant)
> !                         return constant;
> !                     }
>                   }
>                 else
>                   return constant;
> Index: gcc/testsuite/gfortran.dg/pr77498.f
> ===================================================================
> --- gcc/testsuite/gfortran.dg/pr77498.f (nonexistent)
> +++ gcc/testsuite/gfortran.dg/pr77498.f (working copy)
> @@ -0,0 +1,36 @@
> +! { dg-do compile }
> +! { dg-options "-O2 -ffast-math -fdump-tree-pre" }
> +
> +      subroutine foo(U,V,R,N,A)
> +      integer N
> +      real*8 U(N,N,N),V(N,N,N),R(N,N,N),A(0:3)
> +      integer I3, I2, I1
> +C
> +      do I3=2,N-1
> +       do I2=2,N-1
> +        do I1=2,N-1
> +         R(I1,I2,I3)=V(I1,I2,I3)
> +     *      -A(0)*( U(I1,  I2,  I3  ) )
> +     *      -A(1)*( U(I1-1,I2,  I3  ) + U(I1+1,I2,  I3  )
> +     *                 +  U(I1,  I2-1,I3  ) + U(I1,  I2+1,I3  )
> +     *                 +  U(I1,  I2,  I3-1) + U(I1,  I2,  I3+1) )
> +     *      -A(2)*( U(I1-1,I2-1,I3  ) + U(I1+1,I2-1,I3  )
> +     *                 +  U(I1-1,I2+1,I3  ) + U(I1+1,I2+1,I3  )
> +     *                 +  U(I1,  I2-1,I3-1) + U(I1,  I2+1,I3-1)
> +     *                 +  U(I1,  I2-1,I3+1) + U(I1,  I2+1,I3+1)
> +     *                 +  U(I1-1,I2,  I3-1) + U(I1-1,I2,  I3+1)
> +     *                 +  U(I1+1,I2,  I3-1) + U(I1+1,I2,  I3+1) )
> +     *      -A(3)*( U(I1-1,I2-1,I3-1) + U(I1+1,I2-1,I3-1)
> +     *                 +  U(I1-1,I2+1,I3-1) + U(I1+1,I2+1,I3-1)
> +     *                 +  U(I1-1,I2-1,I3+1) + U(I1+1,I2-1,I3+1)
> +     *                 +  U(I1-1,I2+1,I3+1) + U(I1+1,I2+1,I3+1) )
> +        enddo
> +       enddo
> +      enddo
> +      return
> +      end
> +
> +! PRE shouldn't do predictive commonings job here (and in a bad way)
> +! ???  It still does but not as bad as it could.  Less prephitmps
> +! would be better, pcom does it with 6.
> +! { dg-final { scan-tree-dump-times "# prephitmp" 9 "pre" } }

Re: [PATCH][RFC] Fix P1 PR77498

[Jeff Law]

On 03/29/2017 04:05 AM, Richard Biener wrote:
>
> After quite some pondering over this and other related bugs I propose
> the following for GCC 7 which tames down PRE a bit (back to levels
> of GCC 6).  Technically it's the wrong place to fix this, we do
> have measures in place during elimination but they are not in effect
> at -O2.  For GCC 8 I'd like to be more aggressive there but that
> would require to enable predictive commoning at -O2 (with some
> limits to its unrolling) to not lose optimization opportunities.
>
> The other option is to ignore this issue and postpone the solution
> to GCC 8.
>
> Bootstrapped / tested on x86_64-unknown-linux-gnu.
>
> Any preference?
>
> Thanks,
> Richard.
>
> 2017-03-29  Richard Biener  <rguenther@suse.de>
>
> 	PR tree-optimization/77498
> 	* tree-ssa-pre.c (phi_translate_1): Do not allow simplifications
> 	to non-constants over backedges.
>
> 	* gfortran.dg/pr77498.f: New testcase.
I've got a slight preference for this patch.

If you had a good start on the real fix then I'd lean more towards 
postponing.

jeff

Re: [PATCH][RFC] Fix P1 PR77498

[Richard Biener]

On Wed, 29 Mar 2017, Jeff Law wrote:

> On 03/29/2017 04:05 AM, Richard Biener wrote:
> > 
> > After quite some pondering over this and other related bugs I propose
> > the following for GCC 7 which tames down PRE a bit (back to levels
> > of GCC 6).  Technically it's the wrong place to fix this, we do
> > have measures in place during elimination but they are not in effect
> > at -O2.  For GCC 8 I'd like to be more aggressive there but that
> > would require to enable predictive commoning at -O2 (with some
> > limits to its unrolling) to not lose optimization opportunities.
> > 
> > The other option is to ignore this issue and postpone the solution
> > to GCC 8.
> > 
> > Bootstrapped / tested on x86_64-unknown-linux-gnu.
> > 
> > Any preference?
> > 
> > Thanks,
> > Richard.
> > 
> > 2017-03-29  Richard Biener  <rguenther@suse.de>
> > 
> > 	PR tree-optimization/77498
> > 	* tree-ssa-pre.c (phi_translate_1): Do not allow simplifications
> > 	to non-constants over backedges.
> > 
> > 	* gfortran.dg/pr77498.f: New testcase.
> I've got a slight preference for this patch.
> 
> If you had a good start on the real fix then I'd lean more towards postponing.

I wouldn't it yet call "real fix" but just an idea (where I tested the
PRE side already, with the expected testsuite regressions).  I've done
no benchmarking at all for that.  For the proposed patch the situation
is that we're only going to remove some PRE that was done additionally
over GCC 6 because of the rev. that caused the regression, so I have
confidence that it won't make things worse when comparing to GCC 6.

Thus I've now applied the patch.

Richard.

Re: [PATCH][RFC] Fix P1 PR77498

[Christophe Lyon]

Hi Richard,


On 30 March 2017 at 09:13, Richard Biener <rguenther@suse.de> wrote:
> On Wed, 29 Mar 2017, Jeff Law wrote:
>
>> On 03/29/2017 04:05 AM, Richard Biener wrote:
>> >
>> > After quite some pondering over this and other related bugs I propose
>> > the following for GCC 7 which tames down PRE a bit (back to levels
>> > of GCC 6).  Technically it's the wrong place to fix this, we do
>> > have measures in place during elimination but they are not in effect
>> > at -O2.  For GCC 8 I'd like to be more aggressive there but that
>> > would require to enable predictive commoning at -O2 (with some
>> > limits to its unrolling) to not lose optimization opportunities.
>> >
>> > The other option is to ignore this issue and postpone the solution
>> > to GCC 8.
>> >
>> > Bootstrapped / tested on x86_64-unknown-linux-gnu.
>> >
>> > Any preference?
>> >
>> > Thanks,
>> > Richard.
>> >
>> > 2017-03-29  Richard Biener  <rguenther@suse.de>
>> >
>> >     PR tree-optimization/77498
>> >     * tree-ssa-pre.c (phi_translate_1): Do not allow simplifications
>> >     to non-constants over backedges.
>> >
>> >     * gfortran.dg/pr77498.f: New testcase.
>> I've got a slight preference for this patch.
>>
>> If you had a good start on the real fix then I'd lean more towards postponing.
>
> I wouldn't it yet call "real fix" but just an idea (where I tested the
> PRE side already, with the expected testsuite regressions).  I've done
> no benchmarking at all for that.  For the proposed patch the situation
> is that we're only going to remove some PRE that was done additionally
> over GCC 6 because of the rev. that caused the regression, so I have
> confidence that it won't make things worse when comparing to GCC 6.
>
> Thus I've now applied the patch.
>

With this patch, the following testcase now fails on arm* targets:
gcc.dg/tree-ssa/pr71347.c scan-tree-dump-not optimized ".* = MEM.*;"

I'll have to rebuild manually if you need the dumps, let me know.

Thanks,

Christophe

> Richard.

Re: [PATCH][RFC] Fix P1 PR77498

[Rainer Orth]

Hi Christophe,

> With this patch, the following testcase now fails on arm* targets:
> gcc.dg/tree-ssa/pr71347.c scan-tree-dump-not optimized ".* = MEM.*;"

same on Solaris/SPARC.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH][RFC] Fix P1 PR77498

[Richard Biener]

On Fri, 31 Mar 2017, Rainer Orth wrote:

> Hi Christophe,
> 
> > With this patch, the following testcase now fails on arm* targets:
> > gcc.dg/tree-ssa/pr71347.c scan-tree-dump-not optimized ".* = MEM.*;"
> 
> same on Solaris/SPARC.

I've reverted r241968 with (patch reverted).  It doesn't include
sparc, so please amend as you see fit.

Richard.

2017-03-31  Richard Biener  <rguenther@suse.de>
 
	* gcc.dg/tree-ssa/pr71347.c: Put back XFAIL.

Index: gcc/testsuite/gcc.dg/tree-ssa/pr71347.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/pr71347.c	(revision 241967)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr71347.c	(revision 241968)
@@ -14,4 +14,4 @@ void foo (void)
 }
 
 /* Load of X[i - i] can be omitted by reusing X[i] in previous iteration.  */
-/* { dg-final { scan-tree-dump-not ".* = MEM.*;" "optimized" { xfail { ia64-*-* arm*-*-* m68k*-*-* } } } } */
+/* { dg-final { scan-tree-dump-not ".* = MEM.*;" "optimized" } } */

Re: [PATCH][RFC] Fix P1 PR77498

[Richard Biener]

On Fri, 31 Mar 2017, Richard Biener wrote:

> On Fri, 31 Mar 2017, Rainer Orth wrote:
> 
> > Hi Christophe,
> > 
> > > With this patch, the following testcase now fails on arm* targets:
> > > gcc.dg/tree-ssa/pr71347.c scan-tree-dump-not optimized ".* = MEM.*;"
> > 
> > same on Solaris/SPARC.
> 
> I've reverted r241968 with (patch reverted).  It doesn't include
> sparc, so please amend as you see fit.

Ah, r241441.  I'll fixup myself then.

Richard.

> Richard.
> 
> 2017-03-31  Richard Biener  <rguenther@suse.de>
>  
> 	* gcc.dg/tree-ssa/pr71347.c: Put back XFAIL.
> 
> Index: gcc/testsuite/gcc.dg/tree-ssa/pr71347.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/pr71347.c	(revision 241967)
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr71347.c	(revision 241968)
> @@ -14,4 +14,4 @@ void foo (void)
>  }
>  
>  /* Load of X[i - i] can be omitted by reusing X[i] in previous iteration.  */
> -/* { dg-final { scan-tree-dump-not ".* = MEM.*;" "optimized" { xfail { ia64-*-* arm*-*-* m68k*-*-* } } } } */
> +/* { dg-final { scan-tree-dump-not ".* = MEM.*;" "optimized" } } */
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH][RFC] Fix P1 PR77498

[Markus Trippelsdorf]

On 2017.03.31 at 11:16 +0200, Richard Biener wrote:
> On Fri, 31 Mar 2017, Richard Biener wrote:
> 
> > On Fri, 31 Mar 2017, Rainer Orth wrote:
> > 
> > > Hi Christophe,
> > > 
> > > > With this patch, the following testcase now fails on arm* targets:
> > > > gcc.dg/tree-ssa/pr71347.c scan-tree-dump-not optimized ".* = MEM.*;"
> > > 
> > > same on Solaris/SPARC.
> > 
> > I've reverted r241968 with (patch reverted).  It doesn't include
> > sparc, so please amend as you see fit.
> 
> Ah, r241441.  I'll fixup myself then.

It also fails on some X86 configurations:
https://gcc.gnu.org/ml/gcc-regression/2017-03/msg00237.html
https://gcc.gnu.org/ml/gcc-regression/2017-03/msg00238.html

-- 
Markus

Re: [PATCH][RFC] Fix P1 PR77498

[Richard Biener]

On Fri, 31 Mar 2017, Markus Trippelsdorf wrote:

> On 2017.03.31 at 11:16 +0200, Richard Biener wrote:
> > On Fri, 31 Mar 2017, Richard Biener wrote:
> > 
> > > On Fri, 31 Mar 2017, Rainer Orth wrote:
> > > 
> > > > Hi Christophe,
> > > > 
> > > > > With this patch, the following testcase now fails on arm* targets:
> > > > > gcc.dg/tree-ssa/pr71347.c scan-tree-dump-not optimized ".* = MEM.*;"
> > > > 
> > > > same on Solaris/SPARC.
> > > 
> > > I've reverted r241968 with (patch reverted).  It doesn't include
> > > sparc, so please amend as you see fit.
> > 
> > Ah, r241441.  I'll fixup myself then.
> 
> It also fails on some X86 configurations:
> https://gcc.gnu.org/ml/gcc-regression/2017-03/msg00237.html
> https://gcc.gnu.org/ml/gcc-regression/2017-03/msg00238.html

I see.  The test is somewhat strange (well, it's an IVOPTS test).  To
simulate the effect of the PRE changes we could simply enable
-fpredictive-commoning on it.

Let's ask Bin what the testcase was supposed to test... (the testcase
comment suggests that pcom is applied but it certainly wasn't before
the xfails were removed).

Richard.

Re: [PATCH][RFC] Fix P1 PR77498

[Bin.Cheng]

On Fri, Mar 31, 2017 at 11:37 AM, Richard Biener <rguenther@suse.de> wrote:
> On Fri, 31 Mar 2017, Markus Trippelsdorf wrote:
>
>> On 2017.03.31 at 11:16 +0200, Richard Biener wrote:
>> > On Fri, 31 Mar 2017, Richard Biener wrote:
>> >
>> > > On Fri, 31 Mar 2017, Rainer Orth wrote:
>> > >
>> > > > Hi Christophe,
>> > > >
>> > > > > With this patch, the following testcase now fails on arm* targets:
>> > > > > gcc.dg/tree-ssa/pr71347.c scan-tree-dump-not optimized ".* = MEM.*;"
>> > > >
>> > > > same on Solaris/SPARC.
>> > >
>> > > I've reverted r241968 with (patch reverted).  It doesn't include
>> > > sparc, so please amend as you see fit.
>> >
>> > Ah, r241441.  I'll fixup myself then.
>>
>> It also fails on some X86 configurations:
>> https://gcc.gnu.org/ml/gcc-regression/2017-03/msg00237.html
>> https://gcc.gnu.org/ml/gcc-regression/2017-03/msg00238.html
>
> I see.  The test is somewhat strange (well, it's an IVOPTS test).  To
> simulate the effect of the PRE changes we could simply enable
> -fpredictive-commoning on it.
>
> Let's ask Bin what the testcase was supposed to test... (the testcase
> comment suggests that pcom is applied but it certainly wasn't before
> the xfails were removed).

It's supposed to test that both loads inside loop can be optimized,
X[i-1] by predcom or pre; X[1] by loop invariant.  When the test is
added, neither pre nor predcom (not at O2) can do this, but we have
another chance that pre + ivopts + dom together can eliminate X[i-1].
But this really depends on each pass does the right thing.  That's
also why it is added with ivopts change.  Back in time, it was ivopts
did "wrong".  Apparently, having a test on different passes is
fragile.  I will send a patch adding option "-fpredictive-commoning"
because predcom seems the right pass to do that.  Also given we are
considering enabling predcom at O2 for GCC 8.

Thanks,
bin
>
> Richard.

Re: [PATCH][RFC] Fix P1 PR77498

[Bin.Cheng]

[-- Attachment #1: Type: text/plain, Size: 2217 bytes --]

On Fri, Mar 31, 2017 at 2:57 PM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Fri, Mar 31, 2017 at 11:37 AM, Richard Biener <rguenther@suse.de> wrote:
>> On Fri, 31 Mar 2017, Markus Trippelsdorf wrote:
>>
>>> On 2017.03.31 at 11:16 +0200, Richard Biener wrote:
>>> > On Fri, 31 Mar 2017, Richard Biener wrote:
>>> >
>>> > > On Fri, 31 Mar 2017, Rainer Orth wrote:
>>> > >
>>> > > > Hi Christophe,
>>> > > >
>>> > > > > With this patch, the following testcase now fails on arm* targets:
>>> > > > > gcc.dg/tree-ssa/pr71347.c scan-tree-dump-not optimized ".* = MEM.*;"
>>> > > >
>>> > > > same on Solaris/SPARC.
>>> > >
>>> > > I've reverted r241968 with (patch reverted).  It doesn't include
>>> > > sparc, so please amend as you see fit.
>>> >
>>> > Ah, r241441.  I'll fixup myself then.
>>>
>>> It also fails on some X86 configurations:
>>> https://gcc.gnu.org/ml/gcc-regression/2017-03/msg00237.html
>>> https://gcc.gnu.org/ml/gcc-regression/2017-03/msg00238.html
>>
>> I see.  The test is somewhat strange (well, it's an IVOPTS test).  To
>> simulate the effect of the PRE changes we could simply enable
>> -fpredictive-commoning on it.
>>
>> Let's ask Bin what the testcase was supposed to test... (the testcase
>> comment suggests that pcom is applied but it certainly wasn't before
>> the xfails were removed).
>
> It's supposed to test that both loads inside loop can be optimized,
> X[i-1] by predcom or pre; X[1] by loop invariant.  When the test is
> added, neither pre nor predcom (not at O2) can do this, but we have
> another chance that pre + ivopts + dom together can eliminate X[i-1].
> But this really depends on each pass does the right thing.  That's
> also why it is added with ivopts change.  Back in time, it was ivopts
> did "wrong".  Apparently, having a test on different passes is
> fragile.  I will send a patch adding option "-fpredictive-commoning"
> because predcom seems the right pass to do that.  Also given we are
> considering enabling predcom at O2 for GCC 8.
Here is the patch.  Test result checked on arm-none-eabi.  Is it OK?

Thanks,
bin
gcc/testsuite/ChangeLog
2017-03-31  Bin Cheng  <bin.cheng@arm.com>

* gcc.dg/tree-ssa/pr71347.c: Add predcom and drop XFAIL.

Thanks,
bin

[-- Attachment #2: pr71347-add-predcom.txt --]
[-- Type: text/plain, Size: 736 bytes --]

Index: gcc/testsuite/gcc.dg/tree-ssa/pr71347.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/pr71347.c	(revision 246615)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr71347.c	(working copy)
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-options "-O2 -fdump-tree-optimized -fpredictive-commoning" } */
 
 double in;
 extern void Write (double);
@@ -14,4 +14,4 @@
 }
 
 /* Load of X[i - i] can be omitted by reusing X[i] in previous iteration.  */
-/* { dg-final { scan-tree-dump-not ".* = MEM.*;" "optimized" { xfail { ia64-*-* arm*-*-* m68k*-*-* sparc*-*-* } } } } */
+/* { dg-final { scan-tree-dump-not ".* = MEM.*;" "optimized" } } */

Re: [PATCH][RFC] Fix P1 PR77498

[Rainer Orth]

Hi Bin,

> Here is the patch.  Test result checked on arm-none-eabi.  Is it OK?

it passes on sparc-sun-solaris2.12, too.

Thanks.
        Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH][RFC] Fix P1 PR77498

[Richard Biener]

On Fri, 31 Mar 2017, Bin.Cheng wrote:

> On Fri, Mar 31, 2017 at 2:57 PM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> > On Fri, Mar 31, 2017 at 11:37 AM, Richard Biener <rguenther@suse.de> wrote:
> >> On Fri, 31 Mar 2017, Markus Trippelsdorf wrote:
> >>
> >>> On 2017.03.31 at 11:16 +0200, Richard Biener wrote:
> >>> > On Fri, 31 Mar 2017, Richard Biener wrote:
> >>> >
> >>> > > On Fri, 31 Mar 2017, Rainer Orth wrote:
> >>> > >
> >>> > > > Hi Christophe,
> >>> > > >
> >>> > > > > With this patch, the following testcase now fails on arm* targets:
> >>> > > > > gcc.dg/tree-ssa/pr71347.c scan-tree-dump-not optimized ".* = MEM.*;"
> >>> > > >
> >>> > > > same on Solaris/SPARC.
> >>> > >
> >>> > > I've reverted r241968 with (patch reverted).  It doesn't include
> >>> > > sparc, so please amend as you see fit.
> >>> >
> >>> > Ah, r241441.  I'll fixup myself then.
> >>>
> >>> It also fails on some X86 configurations:
> >>> https://gcc.gnu.org/ml/gcc-regression/2017-03/msg00237.html
> >>> https://gcc.gnu.org/ml/gcc-regression/2017-03/msg00238.html
> >>
> >> I see.  The test is somewhat strange (well, it's an IVOPTS test).  To
> >> simulate the effect of the PRE changes we could simply enable
> >> -fpredictive-commoning on it.
> >>
> >> Let's ask Bin what the testcase was supposed to test... (the testcase
> >> comment suggests that pcom is applied but it certainly wasn't before
> >> the xfails were removed).
> >
> > It's supposed to test that both loads inside loop can be optimized,
> > X[i-1] by predcom or pre; X[1] by loop invariant.  When the test is
> > added, neither pre nor predcom (not at O2) can do this, but we have
> > another chance that pre + ivopts + dom together can eliminate X[i-1].
> > But this really depends on each pass does the right thing.  That's
> > also why it is added with ivopts change.  Back in time, it was ivopts
> > did "wrong".  Apparently, having a test on different passes is
> > fragile.  I will send a patch adding option "-fpredictive-commoning"
> > because predcom seems the right pass to do that.  Also given we are
> > considering enabling predcom at O2 for GCC 8.
> Here is the patch.  Test result checked on arm-none-eabi.  Is it OK?

Ok.

> Thanks,
> bin
> gcc/testsuite/ChangeLog
> 2017-03-31  Bin Cheng  <bin.cheng@arm.com>
> 
> * gcc.dg/tree-ssa/pr71347.c: Add predcom and drop XFAIL.
> 
> Thanks,
> bin
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH][RFC] Fix P1 PR77498

[Richard Biener]

On Fri, 31 Mar 2017, Christophe Lyon wrote:

> Hi Richard,
> 
> 
> On 30 March 2017 at 09:13, Richard Biener <rguenther@suse.de> wrote:
> > On Wed, 29 Mar 2017, Jeff Law wrote:
> >
> >> On 03/29/2017 04:05 AM, Richard Biener wrote:
> >> >
> >> > After quite some pondering over this and other related bugs I propose
> >> > the following for GCC 7 which tames down PRE a bit (back to levels
> >> > of GCC 6).  Technically it's the wrong place to fix this, we do
> >> > have measures in place during elimination but they are not in effect
> >> > at -O2.  For GCC 8 I'd like to be more aggressive there but that
> >> > would require to enable predictive commoning at -O2 (with some
> >> > limits to its unrolling) to not lose optimization opportunities.
> >> >
> >> > The other option is to ignore this issue and postpone the solution
> >> > to GCC 8.
> >> >
> >> > Bootstrapped / tested on x86_64-unknown-linux-gnu.
> >> >
> >> > Any preference?
> >> >
> >> > Thanks,
> >> > Richard.
> >> >
> >> > 2017-03-29  Richard Biener  <rguenther@suse.de>
> >> >
> >> >     PR tree-optimization/77498
> >> >     * tree-ssa-pre.c (phi_translate_1): Do not allow simplifications
> >> >     to non-constants over backedges.
> >> >
> >> >     * gfortran.dg/pr77498.f: New testcase.
> >> I've got a slight preference for this patch.
> >>
> >> If you had a good start on the real fix then I'd lean more towards postponing.
> >
> > I wouldn't it yet call "real fix" but just an idea (where I tested the
> > PRE side already, with the expected testsuite regressions).  I've done
> > no benchmarking at all for that.  For the proposed patch the situation
> > is that we're only going to remove some PRE that was done additionally
> > over GCC 6 because of the rev. that caused the regression, so I have
> > confidence that it won't make things worse when comparing to GCC 6.
> >
> > Thus I've now applied the patch.
> >
> 
> With this patch, the following testcase now fails on arm* targets:
> gcc.dg/tree-ssa/pr71347.c scan-tree-dump-not optimized ".* = MEM.*;"
> 
> I'll have to rebuild manually if you need the dumps, let me know.

Ok, there was an XFAIL there which was removed after the PRE enhancement
and thus now needs to be put back.  I'll do that.

Thanks for noticing.

Richard.