[Bug tree-optimization/41212] New: miscompilation at -O2

[Bug tree-optimization/41212] New: miscompilation at -O2

[jpr at csc dot fi]

The program below is miscompiled using
gfortran -O2 -o m m.f90; ./m
gives:
y=   0.60653065945526063       2*y=   2.0000000000000000

(the second of the printed numbers should equal twice the first). Using
gfortran -fno-inline -O2 -o m m.f90
works OK.

The compiler is:

Using built-in specs.
Target: i686-pc-linux-gnu
Configured with: /home/jerry/gcc/trunk/configure --prefix=/usr/local/gfortran
--enable-languages=c,fortran --disable-libmudflap --enable-libgomp
--disable-shared
Thread model: posix
gcc version 4.5.0 20090831 (experimental) [trunk revision 151238] (GCC)



program m

   double precision :: y,z

   call b(1.0d0,y,z)
   print*,'y= ', y, ' 2*y=', z

contains

     subroutine b( x, y, z)
       implicit none

       double precision :: x,y,z

       integer :: i, k
       double precision :: h, r

       y = 1.0d0
       z = 0.0d0

       h = 0
       DO k = 1,10
          h = h + 1.0d0/k

          r = 1
          DO i = 1,k
             r = (x/(2*i) ) * r
          END DO

          y = y + (-1)**k * r
          z = z + (-1)**(k+1) * h * r

          IF ( ABS(2*k/x*r) < 1d-6 ) EXIT
       END DO

       z = 2*y
     end subroutine b
end program m


-- 
           Summary: miscompilation  at  -O2
           Product: gcc
           Version: 4.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: jpr at csc dot fi


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

[Bug tree-optimization/41212] miscompilation at -O2

[jpr at csc dot fi]

------- Comment #1 from jpr at csc dot fi  2009-09-01 12:15 -------
*** Bug 41213 has been marked as a duplicate of this bug. ***


-- 


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

[Bug tree-optimization/41212] miscompilation at -O2

[dominiq at lps dot ens dot fr]

------- Comment #2 from dominiq at lps dot ens dot fr  2009-09-01 13:37 -------
Confirmed on (powerpc|i686)-apple-darwin9 in 32 bit mode and -O2 or above. This
is a regression: I get 1.21306131891052 with gcc 4.2.4, 4.3.4, and 4.4.1. I
also get 1.21306131891052 with recent revisions of trunk with -m64.


-- 


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

[Bug tree-optimization/41212] [4.5 Regression] miscompilation at -O2

[ubizjak at gmail dot com]

------- Comment #3 from ubizjak at gmail dot com  2009-09-01 17:31 -------
Per comment #2.


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2009-09-01 17:31:02
               date|                            |
            Summary|miscompilation  at  -O2     |[4.5 Regression]
                   |                            |miscompilation  at  -O2
   Target Milestone|---                         |4.5.0


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

[Bug tree-optimization/41212] [4.5 Regression] miscompilation at -O2

[jpr at csc dot fi]

------- Comment #4 from jpr at csc dot fi  2009-09-03 07:10 -------
FWIW: I had a look at the -fdump-tree-all files.
All seems well (to my untrained eyes), until


<bb 8>:
  D.1412_51 = y;
  D.1430_52 = D.1412_51 * 2.0e+0;
  z = D.1430_52;
in m.f90.060t.phiprop

is transformed to
<bb 8>:
  D.1412_51 = 1.0e+0;
  D.1430_52 = 2.0e+0;
  z = D.1430_52;
in m.f90.061t.fre

Regards,Juha


-- 

jpr at csc dot fi changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jpr at csc dot fi


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

[Bug tree-optimization/41212] [4.5 Regression] miscompilation at -O2

[jpr at csc dot fi]

------- Comment #5 from jpr at csc dot fi  2009-09-03 10:40 -------
Adding -fstrict-aliasing to -O1 breaks the code:

gfortran -o m m.f90 -O1; ./m
 y=   0.60653065945526063       2*y=   1.2130613189105213

gfortran -o m m.f90 -O1 -fstrict-aliasing ; ./m
 y=   0.60653065945526063       2*y=   2.0000000000000000

and equally adding -fno-strict-aliasing to -O2 saves the day

gfortran -o m m.f90 -O2; ./m
 y=   0.60653065945526063       2*y=   2.0000000000000000

gfortran -o m m.f90 -O2 -fno-strict-aliasing;./m
 y=   0.60653065945526063       2*y=   1.2130613189105213

Juha


-- 


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

[Bug tree-optimization/41212] [4.5 Regression] miscompilation at -O2

[rguenth at gcc dot gnu dot org]

------- Comment #6 from rguenth at gcc dot gnu dot org  2009-09-03 11:40 -------
This looks like your restrict changes get in the way.  We have

  y_13 = (real(kind=8) & restrict) &y;
..
  *y_13 = D.1416_35;
...
  D.1412_51 = y;

so one access via the plain y gets re-introduced by forwprop somehow and
somehow we conclude y_13 cannot point to it.  Hmmm.  My bet is on the
TBAA "hack" by micha here ... the local variables have a different alias-set
than the pointed-to type in the parameter.

Micha - that's quite unfortunate, and I think the only way out is to not
do this TBAA trick if the address of such variable is taken - which also
means "always" for global vars.  Ugh.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu dot
                   |                            |org, matz at gcc dot gnu dot
                   |                            |org


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

[Bug fortran/41212] [4.5 Regression] miscompilation at -O2

[matz at gcc dot gnu dot org]

------- Comment #7 from matz at gcc dot gnu dot org  2009-09-03 16:39 -------
Hmm, in a sense what forward prop is doing is at least fishy.  It forwards
the casted ADDR_EXPR into some references of y_13, but not into all of them.
As y_13 is a restrict pointer this is always going to be a problem, because
suddenly you have accesses to the object via that restrict pointer and via
something else not based on that pointer.

Either forward prop needs to forward this into _all_ y_13 uses or into none.


-- 


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

[Bug fortran/41212] [4.5 Regression] miscompilation at -O2

[rguenther at suse dot de]

------- Comment #8 from rguenther at suse dot de  2009-09-03 16:50 -------
Subject: Re:  [4.5 Regression] miscompilation  at  -O2

On Thu, 3 Sep 2009, matz at gcc dot gnu dot org wrote:

> ------- Comment #7 from matz at gcc dot gnu dot org  2009-09-03 16:39 -------
> Hmm, in a sense what forward prop is doing is at least fishy.  It forwards
> the casted ADDR_EXPR into some references of y_13, but not into all of them.
> As y_13 is a restrict pointer this is always going to be a problem, because
> suddenly you have accesses to the object via that restrict pointer and via
> something else not based on that pointer.
> 
> Either forward prop needs to forward this into _all_ y_13 uses or into none.

That won't help.  The canonical counter-example would be that the
caller already accesses the plain y and z variables.  That forwprop
"simulates" this is just unfortunate.  I'll nevertheless look into
the forwprop issue - but that will just fix this particular testcase
without addressing the real problem.

Richard.


-- 


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

[Bug fortran/41212] [4.5 Regression] miscompilation at -O2

[matz at gcc dot gnu dot org]

------- Comment #9 from matz at gcc dot gnu dot org  2009-09-03 17:13 -------
Indeed. For fixing fwprop this:

--- tree-ssa-forwprop.c (Revision 151348)
+++ tree-ssa-forwprop.c (Arbeitskopie)
@@ -958,7 +958,7 @@ forward_propagate_addr_expr (tree name,
       /* If the use is in a deeper loop nest, then we do not want
         to propagate the ADDR_EXPR into the loop as that is likely
         adding expression evaluations into the loop.  */
-      if (gimple_bb (use_stmt)->loop_depth > stmt_loop_depth)
+      if (0 && gimple_bb (use_stmt)->loop_depth > stmt_loop_depth)
        {
          all = false;
          continue;

or this:

--- tree-ssa-forwprop.c (Revision 151348)
+++ tree-ssa-forwprop.c (Arbeitskopie)
@@ -1281,7 +1281,8 @@ tree_ssa_forward_propagate_single_use_va
                     as well, as this is valid gimple.  */
                  || (CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (stmt))
                      && TREE_CODE (rhs) == ADDR_EXPR
-                     && POINTER_TYPE_P (TREE_TYPE (lhs))))
+                     && POINTER_TYPE_P (TREE_TYPE (lhs))
+                     && !TYPE_RESTRICT (TREE_TYPE (lhs))))
                {
                  STRIP_NOPS (rhs);
                  if (!stmt_references_abnormal_ssa_name (stmt)

will both fix the issue at hand.  The first will propagate to everywhere
(with the problem mentioned in the comment, so that would need refinement),
the second will not touch this.

Unfortunately you are right with pre-existing accesses outside the restrict
region.


-- 


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

[Bug fortran/41212] [4.5 Regression] miscompilation at -O2

[rguenther at suse dot de]

------- Comment #10 from rguenther at suse dot de  2009-09-03 17:23 -------
Subject: Re:  [4.5 Regression] miscompilation  at  -O2

On Thu, 3 Sep 2009, matz at gcc dot gnu dot org wrote:

> ------- Comment #9 from matz at gcc dot gnu dot org  2009-09-03 17:13 -------
> Indeed. For fixing fwprop this:
> 
> --- tree-ssa-forwprop.c (Revision 151348)
> +++ tree-ssa-forwprop.c (Arbeitskopie)
> @@ -958,7 +958,7 @@ forward_propagate_addr_expr (tree name,
>        /* If the use is in a deeper loop nest, then we do not want
>          to propagate the ADDR_EXPR into the loop as that is likely
>          adding expression evaluations into the loop.  */
> -      if (gimple_bb (use_stmt)->loop_depth > stmt_loop_depth)
> +      if (0 && gimple_bb (use_stmt)->loop_depth > stmt_loop_depth)
>         {
>           all = false;
>           continue;

Ah ... something I wanted to remove since long ago.  The theory is
that less addresses are always good and will improve alias information.

For the restrict pointer case as well, if the address is
based on a decl (thus not on an indirect reference), even if we
loose the restrict qualification that way.

> or this:
> 
> --- tree-ssa-forwprop.c (Revision 151348)
> +++ tree-ssa-forwprop.c (Arbeitskopie)
> @@ -1281,7 +1281,8 @@ tree_ssa_forward_propagate_single_use_va
>                      as well, as this is valid gimple.  */
>                   || (CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (stmt))
>                       && TREE_CODE (rhs) == ADDR_EXPR
> -                     && POINTER_TYPE_P (TREE_TYPE (lhs))))
> +                     && POINTER_TYPE_P (TREE_TYPE (lhs))
> +                     && !TYPE_RESTRICT (TREE_TYPE (lhs))))
>                 {
>                   STRIP_NOPS (rhs);
>                   if (!stmt_references_abnormal_ssa_name (stmt)
> 
> will both fix the issue at hand.  The first will propagate to everywhere
> (with the problem mentioned in the comment, so that would need refinement),
> the second will not touch this.
> 
> Unfortunately you are right with pre-existing accesses outside the restrict
> region.

Indeed.

Richard.


-- 


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

[Bug fortran/41212] [4.5 Regression] miscompilation at -O2

[jv244 at cam dot ac dot uk]

------- Comment #11 from jv244 at cam dot ac dot uk  2009-09-12 08:29 -------
(In reply to comment #9)
> Indeed. For fixing fwprop this:
[...]
> or this:
[...]
> will both fix the issue at hand.  The first will propagate to everywhere
> (with the problem mentioned in the comment, so that would need refinement),
> the second will not touch this.

I've tested both patches on CP2K, which is currently miscompiled due to
aliasing issues (PR41312). The second patch fixes some issues, but not all
(i.e. shifts the segfault to a later point). The first patch is needed to get a
correct run. If there is a refined version of patch 1, I'm happy to give it
another round of testing... some fix is need to get the CP2K/gfortran
performance tester (http://cp2k.berlios.de/gfortran/) back in shape.


-- 


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

[Bug fortran/41212] [4.5 Regression] miscompilation at -O2

[rguenth at gcc dot gnu dot org]

------- Comment #12 from rguenth at gcc dot gnu dot org  2009-09-12 11:46 -------
We should instead revert

2009-08-18  Michael Matz  <matz@suse.de>

        * trans-expr.c (gfc_conv_substring): Don't evaluate casted decl early,
        change order of length calculation to (end - start) + 1.
        (gfc_get_interface_mapping_array): Adjust call to
        gfc_get_nodesc_array_type.
...
        * trans-decl.c (gfc_finish_var_decl): Give scalars that can't be
        aliased a type with a different alias set than the base type.
        (gfc_build_dummy_array_decl): Adjust call to gfc_get_nodesc_array_type.

for now.

Richard.


-- 


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

[Bug fortran/41212] [4.5 Regression] miscompilation at -O2

[matz at gcc dot gnu dot org]

------- Comment #13 from matz at gcc dot gnu dot org  2009-09-14 12:20 -------
Nah, not the full patch.  Maybe the part that gives separate alias sets to
scalar variables.  But there's plenty of time for reverting, let's try to
cobble up some other fixes first.


-- 


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

[Bug fortran/41212] [4.5 Regression] miscompilation at -O2

[matz at gcc dot gnu dot org]

------- Comment #14 from matz at gcc dot gnu dot org  2009-09-15 15:21 -------
http://gcc.gnu.org/ml/gcc-patches/2009-09/msg01001.html


-- 

matz at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |matz at gcc dot gnu dot org
                   |dot org                     |
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2009-09-01 17:31:02         |2009-09-15 15:21:44
               date|                            |


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

[Bug fortran/41212] [4.5 Regression] miscompilation at -O2

[jv244 at cam dot ac dot uk]

------- Comment #15 from jv244 at cam dot ac dot uk  2009-09-15 16:20 -------
(In reply to comment #14)
> http://gcc.gnu.org/ml/gcc-patches/2009-09/msg01001.html

This also fixes PR41312 (that I will close as a duplicate).


-- 


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

[Bug fortran/41212] [4.5 Regression] miscompilation at -O2

[kargl at gcc dot gnu dot org]

------- Comment #16 from kargl at gcc dot gnu dot org  2009-09-15 17:21 -------
(In reply to comment #14)
> http://gcc.gnu.org/ml/gcc-patches/2009-09/msg01001.html
> 

I'm not in a position to test this patch at the moment.
Does this PR 41347?


-- 


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

[Bug fortran/41212] [4.5 Regression] miscompilation at -O2

[jv244 at cam dot ac dot uk]

------- Comment #17 from jv244 at cam dot ac dot uk  2009-09-15 17:56 -------
(In reply to comment #16)
> Does this PR 41347?
no, it doesn't. 


-- 


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

[Bug fortran/41212] [4.5 Regression] miscompilation at -O2

[matz at gcc dot gnu dot org]

------- Comment #18 from matz at gcc dot gnu dot org  2009-09-16 16:12 -------
Subject: Bug 41212

Author: matz
Date: Wed Sep 16 16:12:18 2009
New Revision: 151761

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=151761
Log:
        PR fortran/41212
        * tree.h (struct tree_decl_common): Add decl_restricted_flag,
        shorten decl_common_unused.
        (DECL_RESTRICTED_P): New accessor.
        * tree-ssa-alias.c (ptr_deref_may_alias_decl_p): Use it
        to disambiguate marked decls and restrict pointers.

fortran/
        * trans.h (struct lang_type): Remove nontarget_type member.
        * trans.c (gfc_add_modify): Don't access it.
        * trans-decl.c (gfc_finish_var_decl): Don't allocate and set it,
        instead set DECL_RESTRICTED_P on affected decls.

testsuite/
        * gfortran.dg/pr41212.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/pr41212.f90
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-decl.c
    trunk/gcc/fortran/trans.c
    trunk/gcc/fortran/trans.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-alias.c
    trunk/gcc/tree.h


-- 


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

[Bug fortran/41212] [4.5 Regression] miscompilation at -O2

[matz at gcc dot gnu dot org]

------- Comment #19 from matz at gcc dot gnu dot org  2009-09-16 16:13 -------
Fixed.


-- 

matz at gcc dot gnu dot org changed:

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


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

[Bug fortran/41212] [4.5 Regression] miscompilation at -O2

[hjl at gcc dot gnu dot org]

------- Comment #20 from hjl at gcc dot gnu dot org  2009-09-28 20:39 -------
Subject: Bug 41212

Author: hjl
Date: Mon Sep 28 20:38:53 2009
New Revision: 152250

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=152250
Log:
2009-09-28  H.J. Lu  <hongjiu.lu@intel.com>

        Backport from mainline:
        2009-09-26  Andreas Schwab  <schwab@linux-m68k.org>

        PR c/41476
        * gcc.dg/cond-constqual-1.c: New test.

        2009-09-26  Michael Matz  <matz@suse.de>

        PR lto/40758
        PR middle-end/41470
        * gcc.dg/pr41470.c: New test.

        2009-09-23  Alexandre Oliva  <aoliva@redhat.com>

        PR debug/41248
        * gcc.dg/pr41248.c: New.

        2009-09-22  Alexandre Oliva  <aoliva@redhat.com>

        PR debug/41295
        * gcc.dg/pr41295.c: New.

        2009-09-20  Andrew Pinski  <pinskia@gcc.gnu.org>

        PR middle-end/40642
        * g++.dg/torture/pr40642.C: New testcase.

        2009-09-17  Michael Matz  <matz@suse.de>

        PR middle-end/41347
        * gfortran.dg/pr41347.f90: New test.

        2009-09-16  Michael Matz  <matz@suse.de>

        PR fortran/41212
        * gfortran.dg/pr41212.f90: New test.

        2009-09-11  Michael Matz  <matz@suse.de>

        PR middle-end/41275
        * g++.dg/tree-ssa/pr41275.C: New test.

        2009-09-11  Janus Weil  <janus@gcc.gnu.org>

        PR fortran/41242
        * gfortran.dg/proc_ptr_comp_21.f90: New.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/torture/pr40642.C
      - copied unchanged from r152249,
trunk/gcc/testsuite/g++.dg/torture/pr40642.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/tree-ssa/pr41275.C
      - copied unchanged from r152249,
trunk/gcc/testsuite/g++.dg/tree-ssa/pr41275.C
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/cond-constqual-1.c
      - copied unchanged from r152248,
trunk/gcc/testsuite/gcc.dg/cond-constqual-1.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr41248.c
      - copied unchanged from r152248, trunk/gcc/testsuite/gcc.dg/pr41248.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr41295.c
      - copied unchanged from r152249, trunk/gcc/testsuite/gcc.dg/pr41295.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr41470.c
      - copied unchanged from r152248, trunk/gcc/testsuite/gcc.dg/pr41470.c
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/pr41212.f90
      - copied unchanged from r152249,
trunk/gcc/testsuite/gfortran.dg/pr41212.f90
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/pr41347.f90
      - copied unchanged from r152249,
trunk/gcc/testsuite/gfortran.dg/pr41347.f90
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/proc_ptr_comp_21.f90
      - copied unchanged from r152249,
trunk/gcc/testsuite/gfortran.dg/proc_ptr_comp_21.f90
Modified:
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog


-- 


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