[Bug tree-optimization/52976] New: [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[Bug tree-optimization/52976] New: [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

             Bug #: 52976
           Summary: [4.8 Regression] Revision 186384 breaks the polyhedron
                    tests aermod.f90 and doduc.f90 at -O3 -ffast-math
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: dominiq@lps.ens.fr
                CC: rguenth@gcc.gnu.org, tkoenig@gcc.gnu.org,
                    wschmidt@gcc.gnu.org


On powerpc-apple-darwin9 and x86_64-apple-darwin10, the polyhedron tests
aermod.f90 and doduc.f90 are broken after revision 186384 at -O3 -ffast-math:

[macbook] lin/test% gfc -O3 -ffast-math -fno-tree-vectorize aermod.f90
aermod.f90: In function 'grdurban':
aermod.f90:27214:0: error: definition in block 9 follows the use
       SUBROUTINE GRDURBAN
 ^
for SSA_NAME: reassocpow.22077_36 in statement:
D.56565_32 = D.56564_31 * reassocpow.22077_36;
aermod.f90:27214:0: internal compiler error: verify_ssa failed
       SUBROUTINE GRDURBAN
 ^
[macbook] lin/test% gfc -O3 -ffast-math -fno-tree-vectorize doduc.f90
doduc.f90: In function 's55199':
doduc.f90:4384:0: internal compiler error: gimple check: expected
gimple_assign(error_mark), have gimple_call() in gimple_assign_rhs1, at
gimple.h:1850
       SUBROUTINE S55199(H,P,T,Rho,Dvdhp,Dvdph,Dtdh,Dtdp)
 ^

The SUBROUTINE S55199 can be reduced to

      SUBROUTINE S55199(P,Dvdph)
      implicit none
      real(8) :: c1,c2,c3,P,Dvdph
      c1=0.1d0
      c2=0.2d0
      c3=0.3d0
      Dvdph = c1 + 2.*P*c2 + 3.*P**2*c3
      END

which gives the following ICE at r186417

[macbook] test/doduc_tst% gfc -O3 -ffast-math -fno-tree-vectorize
s55199_red.f90 -c
s55199_red.f90: In function 's55199':
s55199_red.f90:1:0: internal compiler error: gimple check: expected
gimple_assign(error_mark), have gimple_call() in gimple_assign_rhs1, at
gimple.h:1850
       SUBROUTINE S55199(P,Dvdph)
 ^

or

s55199_red.f90: In function 's55199':
s55199_red.f90:1:0: internal compiler error: in zero_one_operation, at
tree-ssa-reassoc.c:1041
       SUBROUTINE S55199(P,Dvdph)
 ^

for gcc configured with --enable-checking=release.

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

--- Comment #1 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2012-04-13 18:10:05 UTC ---
PR52980 looks like a duplicate.

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[wschmidt at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

William J. Schmidt <wschmidt at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2012-04-13
                 CC|                            |bergner at gcc dot gnu.org
         AssignedTo|unassigned at gcc dot       |wschmidt at gcc dot gnu.org
                   |gnu.org                     |
     Ever Confirmed|0                           |1

--- Comment #2 from William J. Schmidt <wschmidt at gcc dot gnu.org> 2012-04-13 19:06:03 UTC ---
Investigating.

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[wschmidt at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

--- Comment #3 from William J. Schmidt <wschmidt at gcc dot gnu.org> 2012-04-13 19:41:18 UTC ---
When changing from the original form of the patch to the form that recorded
repeated ops in the ops table, I missed the effect on undistribution where a
term contains a multiply of a repeated op.  Those terms should be excluded from
undistribution to return to the former behavior, and I'll work on fixing that
for the short term.  However, it would eventually be better to factor a P out
of the example in this case:

c1 + 2.*P*c2 + 3.*P**2*c3 ==>  c1 + P * (2.*c2 + 3.*P*c3)

In the original form of the patch, this was handled naturally because P**2 was
expanded into P*P and the existing logic was sufficient.  I just missed the
need to add some logic to undistribution.

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[wschmidt at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

--- Comment #4 from William J. Schmidt <wschmidt at gcc dot gnu.org> 2012-04-13 20:46:27 UTC ---
Here's a patch that fixes the reduced test case but is otherwise untested.  I
don't know what "the polyhedron tests aermod.490 and doduc.f90" are; I don't
find them in the gcc testsuite.  Dominique, can you please try this patch on
those?  I will do some regression testing and try the patch out on SPEC in the
meanwhile.

Index: gcc/tree-ssa-reassoc.c
===================================================================
--- gcc/tree-ssa-reassoc.c    (revision 186393)
+++ gcc/tree-ssa-reassoc.c    (working copy)
@@ -1200,6 +1200,7 @@ undistribute_ops_list (enum tree_code opcode,
       dcode = gimple_assign_rhs_code (oe1def);
       if ((dcode != MULT_EXPR
        && dcode != RDIV_EXPR)
+      || oe1->count != 1
       || !is_reassociable_op (oe1def, dcode, loop))
     continue;

@@ -1243,6 +1244,8 @@ undistribute_ops_list (enum tree_code opcode,
       oecount c;
       void **slot;
       size_t idx;
+      if (oe1->count != 1)
+        continue;
       c.oecode = oecode;
       c.cnt = 1;
       c.id = next_oecount_id++;
@@ -1311,7 +1314,7 @@ undistribute_ops_list (enum tree_code opcode,

       FOR_EACH_VEC_ELT (operand_entry_t, subops[i], j, oe1)
         {
-          if (oe1->op == c->op)
+          if (oe1->op == c->op && oe1->count == 1)
         {
           SET_BIT (candidates2, i);
           ++nr_candidates2;

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

--- Comment #5 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2012-04-13 22:11:24 UTC ---
The patch in comment #4 fixes doduc.f90, but not aermod.f90 (the sources are
available at http://polyhedron.com/pb05-lin64-f90bench_SBhtml ). A reduced test
for aermod is

[macbook] lin/test% cat grdurban_red.f90
      SUBROUTINE GRDURBAN(URBWSTR, ZIURB, GRIDHT)

      IMPLICIT NONE
      INTEGER :: I
      REAL :: SW2, URBWSTR, ZIURB, GRIDHT(87)

      SAVE 

      SW2 = 1.6*(GRIDHT(I)/ZIURB)**0.667*URBWSTR**2

      END
[macbook] lin/test% gfc -c -O3 -ffast-math grdurban_red.f90
grdurban_red.f90: In function 'grdurban':
grdurban_red.f90:1:0: error: definition in block 2 follows the use
       SUBROUTINE GRDURBAN(URBWSTR, ZIURB, GRIDHT)
 ^
for SSA_NAME: reassocpow.4_1 in statement:
D.1875_14 = reassocpow.4_1 * D.1874_13;
grdurban_red.f90:1:0: internal compiler error: verify_ssa failed
       SUBROUTINE GRDURBAN(URBWSTR, ZIURB, GRIDHT)
 ^

Note that the ICE disappears if I remove the SAVE.

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[wschmidt at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

--- Comment #6 from William J. Schmidt <wschmidt at gcc dot gnu.org> 2012-04-14 15:05:37 UTC ---
The following patch fixes both reduced test cases and appears to fix the SPEC
problems in PR52980 as well.  Bootstrap/regression test in progress, and will
then do a complete set of SPEC builds to be sure.

The second problem involved a subtlety in the rank order of operands.  When
introducing the calls to __builtin_powi, I needed to fiddle with the resulting
operand rank to ensure uses of the call result appear after the call.

Dominique, please let me know whether this cleans up all your polyhedron tests
without introducing new failures.  Thanks!


Index: gcc/tree-ssa-reassoc.c
===================================================================
--- gcc/tree-ssa-reassoc.c    (revision 186393)
+++ gcc/tree-ssa-reassoc.c    (working copy)
@@ -544,6 +544,28 @@ add_repeat_to_ops_vec (VEC(operand_entry_t, heap)
   reassociate_stats.pows_encountered++;
 }

+/* Add an operand entry to *OPS for the tree operand OP, giving the
+   new entry a larger rank than any other operand already in *OPS.  */
+
+static void
+add_to_ops_vec_max_rank (VEC(operand_entry_t, heap) **ops, tree op)
+{
+  operand_entry_t oe = (operand_entry_t) pool_alloc (operand_entry_pool);
+  operand_entry_t oe1;
+  unsigned i;
+  unsigned max_rank = 0;
+
+  FOR_EACH_VEC_ELT (operand_entry_t, *ops, i, oe1)
+    if (oe1->rank > max_rank)
+      max_rank = oe1->rank;
+
+  oe->op = op;
+  oe->rank = max_rank + 1;
+  oe->id = next_operand_entry_id++;
+  oe->count = 1;
+  VEC_safe_push (operand_entry_t, heap, *ops, oe);
+}
+
 /* Return true if STMT is reassociable operation containing a binary
    operation with tree code CODE, and is inside LOOP.  */

@@ -1200,6 +1222,7 @@ undistribute_ops_list (enum tree_code opcode,
       dcode = gimple_assign_rhs_code (oe1def);
       if ((dcode != MULT_EXPR
        && dcode != RDIV_EXPR)
+      || oe1->count != 1
       || !is_reassociable_op (oe1def, dcode, loop))
     continue;

@@ -1243,6 +1266,8 @@ undistribute_ops_list (enum tree_code opcode,
       oecount c;
       void **slot;
       size_t idx;
+      if (oe1->count != 1)
+        continue;
       c.oecode = oecode;
       c.cnt = 1;
       c.id = next_oecount_id++;
@@ -1311,7 +1336,7 @@ undistribute_ops_list (enum tree_code opcode,

       FOR_EACH_VEC_ELT (operand_entry_t, subops[i], j, oe1)
         {
-          if (oe1->op == c->op)
+          if (oe1->op == c->op && oe1->count == 1)
         {
           SET_BIT (candidates2, i);
           ++nr_candidates2;
@@ -3275,8 +3300,10 @@ attempt_builtin_powi (gimple stmt, VEC(operand_ent
       gsi_insert_before (&gsi, pow_stmt, GSI_SAME_STMT);
     }

-      /* Append the result of this iteration to the ops vector.  */
-      add_to_ops_vec (ops, iter_result);
+      /* Append the result of this iteration to the ops vector.
+         Give it a rank higher than all other ranks in the ops vector
+         so that all uses of it will be forced to come after it.  */
+      add_to_ops_vec_max_rank (ops, iter_result);

       /* Decrement the occurrence count of each element in the product
      by the count found above, and remove this many copies of each

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[wschmidt at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

William J. Schmidt <wschmidt at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hjl.tools at gmail dot com

--- Comment #7 from William J. Schmidt <wschmidt at gcc dot gnu.org> 2012-04-14 15:11:43 UTC ---
*** Bug 52980 has been marked as a duplicate of this bug. ***

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[wschmidt at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

--- Comment #8 from William J. Schmidt <wschmidt at gcc dot gnu.org> 2012-04-14 16:54:02 UTC ---
Patch bootstraps and passes regressions; all SPEC tests build cleanly.  Will
submit today to gcc-patches.

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

--- Comment #9 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2012-04-15 10:42:36 UTC ---
> Patch bootstraps and passes regressions; ...

And also the polyhedron tests. Thanks.

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.0

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[wschmidt at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

--- Comment #10 from William J. Schmidt <wschmidt at gcc dot gnu.org> 2012-04-16 12:16:04 UTC ---
Author: wschmidt
Date: Mon Apr 16 12:15:50 2012
New Revision: 186493

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186493
Log:
2012-04-16  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

    PR tree-optimization/52976
    * tree-ssa-reassoc.c (add_to_ops_vec_max_rank): New function.
    (undistribute_ops_list): Ops with repeat counts aren't eligible for
    undistribution.
    (attempt_builtin_powi): Call add_to_ops_vec_max_rank.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-ssa-reassoc.c

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[wschmidt at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

William J. Schmidt <wschmidt at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|4.8.0                       |---

--- Comment #11 from William J. Schmidt <wschmidt at gcc dot gnu.org> 2012-04-16 12:17:48 UTC ---
Fixed.

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vincenzo.innocente at cern
                   |                            |dot ch

--- Comment #12 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-04-16 13:22:49 UTC ---
*** Bug 53007 has been marked as a duplicate of this bug. ***

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[vincenzo.innocente at cern dot ch]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

--- Comment #13 from vincenzo Innocente <vincenzo.innocente at cern dot ch> 2012-04-16 13:53:30 UTC ---
I confirm that "revision 186494" fixed PR53007.
btw:
would it be possible to add the revision number to the oyuout of "c++ -v"?
the current "version id" 
gcc version 4.8.0 20120416 (experimental) (GCC) 
does not uniquely identify the content of the build…

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

--- Comment #14 from rguenther at suse dot de <rguenther at suse dot de> 2012-04-16 13:55:58 UTC ---
On Mon, 16 Apr 2012, vincenzo.innocente at cern dot ch wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976
> 
> --- Comment #13 from vincenzo Innocente <vincenzo.innocente at cern dot ch> 2012-04-16 13:53:30 UTC ---
> I confirm that "revision 186494" fixed PR53007.
> btw:
> would it be possible to add the revision number to the oyuout of "c++ -v"?
> the current "version id" 
> gcc version 4.8.0 20120416 (experimental) (GCC) 
> does not uniquely identify the content of the build...

It does if you use ./contrib/gcc_update

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dcb314 at hotmail dot com

--- Comment #15 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-04-18 10:22:37 UTC ---
*** Bug 53022 has been marked as a duplicate of this bug. ***

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[wschmidt at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

--- Comment #16 from William J. Schmidt <wschmidt at gcc dot gnu.org> 2012-04-18 12:25:30 UTC ---
Author: wschmidt
Date: Wed Apr 18 12:25:17 2012
New Revision: 186567

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186567
Log:
gcc:

2012-04-18  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

    PR tree-optimization/52976
    * tree-ssa-reassoc.c (stmt_is_power_of_op): New function.
    (decrement_power): Likewise.
    (propagate_op_to_single_use): Likewise.
    (zero_one_operation): Handle __builtin_pow* calls in linearized
    expression trees; factor logic into propagate_op_to_single_use.
    (undistribute_ops_list): Allow operands with repeat counts > 1.


gcc/testsuite:

2012-04-18  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

    PR tree-optimization/52976
    gfortran.dg/reassoc_7.f: New test.
    gfortran.dg/reassoc_8.f: Likewise.
    gfortran.dg/reassoc_9.f: Likewise.
    gfortran.dg/reassoc_10.f: Likewise.

Added:
    trunk/gcc/testsuite/gfortran.dg/reassoc_10.f
    trunk/gcc/testsuite/gfortran.dg/reassoc_7.f
    trunk/gcc/testsuite/gfortran.dg/reassoc_8.f
    trunk/gcc/testsuite/gfortran.dg/reassoc_9.f
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-reassoc.c

[Bug tree-optimization/52976] [4.8 Regression] Revision 186384 breaks the polyhedron tests aermod.f90 and doduc.f90 at -O3 -ffast-math

[wschmidt at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52976

--- Comment #17 from William J. Schmidt <wschmidt at gcc dot gnu.org> 2012-04-18 12:29:39 UTC ---
Author: wschmidt
Date: Wed Apr 18 12:29:23 2012
New Revision: 186568

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186568
Log:
gcc:

2012-04-18  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

    PR tree-optimization/52976
    * tree-ssa-reassoc.c (add_to_ops_vec_max_rank): Delete.
    (possibly_move_powi): New function.
    (rewrite_expr_tree): Call possibly_move_powi.
    (rewrite_expr_tree_parallel): Likewise.
    (attempt_builtin_powi): Change call of add_to_ops_vec_max_rank to
    call add_to_ops_vec instead.


gcc/testsuite:

2012-04-18  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

    PR tree-optimization/52976
    gfortran.dg/reassoc_11.f: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/reassoc_11.f
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-reassoc.c