[Bug middle-end/103802] New: [12 regression] recip-3.c fails after r12-6087 on Power m32

[Bug middle-end/103802] New: [12 regression] recip-3.c fails after r12-6087 on Power m32

[luoxhu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103802

            Bug ID: 103802
           Summary: [12 regression] recip-3.c fails after  r12-6087 on
                    Power m32
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: luoxhu at gcc dot gnu.org
  Target Milestone: ---

Invoking the compiler as /home/luoxhu/workspace/gcc-master_build/gcc/xgcc
-B/home/luoxhu/workspace/gcc-master_build/gcc/
/home/luoxhu/workspace/gcc-master/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c 
-fdiagnostics-plain-output   -O1 -fno-trapping-math -funsafe-math-optimizations
-fdump-tree-recip -S  -m32  -o recip-3.s
Executing on host: /home/luoxhu/workspace/gcc-master_build/gcc/xgcc
-B/home/luoxhu/workspace/gcc-master_build/gcc/
/home/luoxhu/workspace/gcc-master/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c 
-fdiagnostics-plain-output   -O1 -fno-trapping-math -funsafe-math-optimizations
-fdump-tree-recip -S  -m32  -o recip-3.s    (timeout = 300)
gcc.dg/tree-ssa/recip-3.c: pattern found 3 times
FAIL: gcc.dg/tree-ssa/recip-3.c scan-tree-dump-times recip " / " 5


Reson is m32 fail to cunroll due to
 recip-3.m32.c.172t.cunroll:   Not unrolling loop 1: size would grow.

[Bug middle-end/103802] [12 regression] recip-3.c fails after r12-6087 on Power m32

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103802

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization,
                   |                            |testsuite-fail
   Target Milestone|---                         |12.0

[Bug tree-optimization/103802] [12 regression] recip-3.c fails after r12-6087 on Power m32

[luoxhu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103802

--- Comment #1 from luoxhu at gcc dot gnu.org ---
MOVE_MAX_PIECES is 4 on m32 but it is 8 on m64, then estimate_move_cost is
different between them 2 vs 1 for “((size + MOVE_MAX_PIECES - 1) /
MOVE_MAX_PIECES)".

recip-3.m32.c.172t.cunroll:

 BB: 11, after_exit: 0
 BB: 7, after_exit: 0
  size:   2 _4 = F[i_23];
  size:   1 _5 = _4 + iftmp.1_10;
  size:   2 F[i_23] = _5;
 BB: 5, after_exit: 0
  size:   1 _2 = d_14 +
1.00000000000000088817841970012523233890533447265625e-1;
  size:   1 reciptmp_12 = 1.0e+0 / d_14;
  size:   1 iftmp.1_18 = reciptmp_12 * _2;
 BB: 6, after_exit: 0
  size:   1 _3 = -1.00000000000000088817841970012523233890533447265625e-1 -
d_14;
  size:   1 reciptmp_25 = 1.0e+0 / d_14;
  size:   1 iftmp.1_17 = reciptmp_25 * _3;
 BB: 4, after_exit: 0
  size:   2 if (e.0_1 <
-5.00000000000000444089209850062616169452667236328125e-2)
size: 19-4, last_iteration: 19-4
  Loop size: 19
  Estimated size after unrolling: 20
Not unrolling loop 1: size would grow.


But recip-3.m64.c.172t.cunroll:

 BB: 11, after_exit: 0
 BB: 7, after_exit: 0
  size:   1 _4 = F[i_23];
  size:   1 _5 = _4 + iftmp.1_10;
  size:   1 F[i_23] = _5;
 BB: 5, after_exit: 0
  size:   1 _2 = d_14 +
1.00000000000000088817841970012523233890533447265625e-1;
  size:   1 reciptmp_12 = 1.0e+0 / d_14;
  size:   1 iftmp.1_18 = reciptmp_12 * _2;
 BB: 6, after_exit: 0
  size:   1 _3 = -1.00000000000000088817841970012523233890533447265625e-1 -
d_14;
  size:   1 reciptmp_25 = 1.0e+0 / d_14;
  size:   1 iftmp.1_17 = reciptmp_25 * _3;
 BB: 4, after_exit: 0
  size:   2 if (e.0_1 <
-5.00000000000000444089209850062616169452667236328125e-2)
size: 17-4, last_iteration: 17-4
  Loop size: 17
  Estimated size after unrolling: 17
Making edge 18->9 impossible by redistributing probability to other edges.
Making edge 8->10 impossible by redistributing probability to other edges.
/home/luoxhu/workspace/gcc-master/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c:16:14:
optimized: loop with 1 iterations completely unrolled (header execution count
357878154)

[Bug tree-optimization/103802] [12 regression] recip-3.c fails after r12-6087 on Power m32

[luoxhu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103802

--- Comment #2 from luoxhu at gcc dot gnu.org ---
-funroll-loops could work around this, is this reasonable?

[Bug tree-optimization/103802] [12 regression] recip-3.c fails after r12-6087 on Power m32

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103802

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-12-28

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
FAILs on i686-linux too.

[Bug tree-optimization/103802] [12 regression] recip-3.c fails after r12-6087 on Power m32

[luoxhu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103802

--- Comment #4 from luoxhu at gcc dot gnu.org ---
Or restore the previous recip count check by comment out the if condition to
avoid bb in loop turns cold?

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c
b/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c
index 641c91e719e..d3c3053486d 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c
@@ -14,7 +14,13 @@ float h ()
        E = 1. - d;

        for( i=0; i < 2; i++ )
-               if( d > 0.01 )
+               // if( d > 0.01 )
+               /* The if condition will make followed bb cold (profile count
+                  less then the loop preheader), while r12-6087 is a
+                  optimization that avoids move COLD invariant expression out
+                  of loop, since this test case is to test recip expression
+                  could be CSE and eliminated, so comment the condition to
keep
+                  the test point.  */
                {
                        P = ( W < E ) ? (W - E)/d : (E - W)/d;
                        F[i] += P;
@@ -23,4 +29,4 @@ float h ()
        F[0] += E / d;
 }

-/* { dg-final { scan-tree-dump-times " / " 5 "recip" } } */
+/* { dg-final { scan-tree-dump-times " / " 1 "recip" } } */

[Bug tree-optimization/103802] [12 regression] recip-3.c fails after r12-6087 on Power m32

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103802

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
So the point is that P is invariant but we do not hoist it because it's
computed in a (estimated) cold block?  I notice that the condition is
invariant, too, so
in principle we could hoist as

  if (d > 0.01)
    P = ( W < E ) ? (W - E)/d : (E - W)/d;
  for (i=0; i < 2; i++ )
    if( d > 0.01 )
      F[i] += P;

alternatively one might argue that invariant expressions (unconditionally
computed or in a special way under invariant conditions) should be costed
differently.

I think best would be to restore the original intent of the testcase which
was added with the fix for PRs 23109, 23948 and 24123.  I suppose there
we saw the invariant hoisted(?) and the loop unrolled so I would suggest
to either apply the hoisting or the unrolling manually to the testcase.
(just look at the PRs whether you get a better idea of the origin of the
testcase).

[Bug tree-optimization/103802] [12 regression] recip-3.c fails after r12-6087 on Power m32

[luoxhu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103802

--- Comment #6 from luoxhu at gcc dot gnu.org ---
(In reply to Richard Biener from comment #5)
> So the point is that P is invariant but we do not hoist it because it's
> computed in a (estimated) cold block?  I notice that the condition is
> invariant, too, so
> in principle we could hoist as
> 
>   if (d > 0.01)
>     P = ( W < E ) ? (W - E)/d : (E - W)/d;
>   for (i=0; i < 2; i++ )
>     if( d > 0.01 )
>       F[i] += P;


Yes. But this loop only iterates twice, so bbs in loop is colder than
preheader.
-funswitch-loops should move the condition out of loop, but also need increase
the loop iteration count:

"/home/luoxhu/workspace/gcc-master/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c:16:14:
note: Not unswitching, loop is not expected to iterate"

> 
> alternatively one might argue that invariant expressions (unconditionally
> computed or in a special way under invariant conditions) should be costed
> differently.
> 
> I think best would be to restore the original intent of the testcase which
> was added with the fix for PRs 23109, 23948 and 24123.  I suppose there
> we saw the invariant hoisted(?) and the loop unrolled so I would suggest
> to either apply the hoisting or the unrolling manually to the testcase.
> (just look at the PRs whether you get a better idea of the origin of the
> testcase).

To restore the original intent of the testcase, increase the loop count is
better than "either apply the hoisting or unrolling".  Change it from "2" to at
least "5" will turn the cold bb to hot bb, then the two divides could be
hoisted out in LIM pass again(Verified below change could both pass on
power-m32 and x86-i686):

(It is much reasonable than the other two directions as loop iteration count is
not key for the test code.)

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c
b/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c
index 641c91e..a1d2d87 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c
@@ -1,7 +1,7 @@
 /* { dg-do compile } */
 /* { dg-options "-O1 -fno-trapping-math -funsafe-math-optimizations
-fdump-tree-recip" } */

-double F[2] = { 0.0, 0.0 }, e;
+double F[5] = { 0.0, 0.0 }, e;

 /* In this case the optimization is interesting.  */
 float h ()
@@ -13,7 +13,7 @@ float h ()
        d = 2.*e;
        E = 1. - d;

-       for( i=0; i < 2; i++ )
+       for( i=0; i < 5; i++ )
                if( d > 0.01 )
                {
                        P = ( W < E ) ? (W - E)/d : (E - W)/d;
@@ -23,4 +23,4 @@ float h ()
        F[0] += E / d;
 }

-/* { dg-final { scan-tree-dump-times " / " 5 "recip" } } */
+/* { dg-final { scan-tree-dump-times " / " 1 "recip" } } */

[Bug tree-optimization/103802] [12 regression] recip-3.c fails after r12-6087 on Power m32

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103802

--- Comment #7 from rguenther at suse dot de <rguenther at suse dot de> ---
On Fri, 7 Jan 2022, luoxhu at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103802
> 
> --- Comment #6 from luoxhu at gcc dot gnu.org ---
> (In reply to Richard Biener from comment #5)
> > So the point is that P is invariant but we do not hoist it because it's
> > computed in a (estimated) cold block?  I notice that the condition is
> > invariant, too, so
> > in principle we could hoist as
> > 
> >   if (d > 0.01)
> >     P = ( W < E ) ? (W - E)/d : (E - W)/d;
> >   for (i=0; i < 2; i++ )
> >     if( d > 0.01 )
> >       F[i] += P;
> 
> 
> Yes. But this loop only iterates twice, so bbs in loop is colder than
> preheader.
> -funswitch-loops should move the condition out of loop, but also need increase
> the loop iteration count:
> 
> "/home/luoxhu/workspace/gcc-master/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c:16:14:
> note: Not unswitching, loop is not expected to iterate"
> 
> > 
> > alternatively one might argue that invariant expressions (unconditionally
> > computed or in a special way under invariant conditions) should be costed
> > differently.
> > 
> > I think best would be to restore the original intent of the testcase which
> > was added with the fix for PRs 23109, 23948 and 24123.  I suppose there
> > we saw the invariant hoisted(?) and the loop unrolled so I would suggest
> > to either apply the hoisting or the unrolling manually to the testcase.
> > (just look at the PRs whether you get a better idea of the origin of the
> > testcase).
> 
> To restore the original intent of the testcase, increase the loop count is
> better than "either apply the hoisting or unrolling".  Change it from "2" to at
> least "5" will turn the cold bb to hot bb, then the two divides could be
> hoisted out in LIM pass again(Verified below change could both pass on
> power-m32 and x86-i686):
> 
> (It is much reasonable than the other two directions as loop iteration count is
> not key for the test code.)
> 
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c
> b/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c
> index 641c91e..a1d2d87 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c
> @@ -1,7 +1,7 @@
>  /* { dg-do compile } */
>  /* { dg-options "-O1 -fno-trapping-math -funsafe-math-optimizations
> -fdump-tree-recip" } */
> 
> -double F[2] = { 0.0, 0.0 }, e;
> +double F[5] = { 0.0, 0.0 }, e;
> 
>  /* In this case the optimization is interesting.  */
>  float h ()
> @@ -13,7 +13,7 @@ float h ()
>         d = 2.*e;
>         E = 1. - d;
> 
> -       for( i=0; i < 2; i++ )
> +       for( i=0; i < 5; i++ )
>                 if( d > 0.01 )
>                 {
>                         P = ( W < E ) ? (W - E)/d : (E - W)/d;
> @@ -23,4 +23,4 @@ float h ()
>         F[0] += E / d;
>  }
> 
> -/* { dg-final { scan-tree-dump-times " / " 5 "recip" } } */
> +/* { dg-final { scan-tree-dump-times " / " 1 "recip" } } */

That looks reasonable to me.

[Bug tree-optimization/103802] [12 regression] recip-3.c fails after r12-6087 on Power m32

[luoxhu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103802

luoxhu at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from luoxhu at gcc dot gnu.org ---
Fixed by

The master branch has been updated by Xiong Hu Luo <luoxhu@gcc.gnu.org>:

https://gcc.gnu.org/g:0552605b7b27dc6beed62e71bd05bc1efd191c0d

commit r12-6430-g0552605b7b27dc6beed62e71bd05bc1efd191c0d
Author: Xionghu Luo <luoxhu@linux.ibm.com>
Date:   Mon Jan 10 20:05:56 2022 -0600

    testsuite: Fix regression on m32 by r12-6087 [PR103820]

    r12-6087 will avoid move cold bb out of hot loop, while the original
    intent of this testcase is to hoist divides out of loop and CSE them to
    only one divide.  So increase the loop count to turn the cold bb to hot
    bb again.  Then the 3 divides could be rewritten with same reciptmp.

    Tested pass on Power-Linux {32,64}, x86 {64,32} and i686-linux.

    gcc/testsuite/ChangeLog:

            PR testsuite/103820
            * gcc.dg/tree-ssa/recip-3.c: Adjust.