[Bug middle-end/50628] New: [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f fails

[Bug middle-end/50628] New: [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f fails

[burnus at gcc dot gnu.org]

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

             Bug #: 50628
           Summary: [4.7 Regression]
                    gfortran.fortran-torture/execute/entry_4.f fails
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: burnus@gcc.gnu.org


gfortran.fortran-torture/execute/entry_4.f fails with -O2, it works with -O.
The failure is an ABORT in line 54, i.e. in the main program's
        if (f2 (0) .ne. 45) call abort ()
check. With -O, the complex number is "(45.0000000, 0.00000000)", but with -O2
one has "(0.00000000, 0.00000000)".

FAILING: Rev. 179603 of today (x86-64-linux)
WORKING: Same system with -m32.

FAILING: Rev. 179594,  4.7.0 20111006, x86_64-unknown-linux-gnu
         http://gcc.gnu.org/ml/gcc-testresults/2011-10/msg00659.html
FAILING: Rev. 179566,  4.7.0 20111005, x86_64-unknown-linux-gnu
         http://gcc.gnu.org/ml/gcc-testresults/2011-10/msg00565.html
WORKING: Rev. 179545,  4.7.0 20111005, x86_64-unknown-linux-gnu
         http://gcc.gnu.org/ml/gcc-testresults/2011-10/msg00550.html

[Bug middle-end/50628] [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f fails

[burnus at gcc dot gnu.org]

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

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.7.0

--- Comment #1 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-10-06 13:09:03 UTC ---
For completeness, the test case also works with NAG 5.1 (-C=all and -O4), ifort
11.1 (-xHost -O3), pathf95 3.2.99 (-Ofast), openf95 4.2.2.1 (-Ofast), pgf90
10.1-0 (-O3) and 11.5-0 (-fast) and Crayftn 7.1.4.111 (-O3).

[Bug middle-end/50628] [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f fails

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011-10-06
     Ever Confirmed|0                           |1

--- Comment #2 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-10-06 13:34:38 UTC ---
PRE now changes

-<bb 6>:
   D.2317 = VIEW_CONVERT_EXPR<union munion.1.f2>(__complex__ (4.5e+1, 0.0));
   D.2317.e2 = 0;
-  D.2329_19 = REALPART_EXPR <VIEW_CONVERT_EXPR<complex(kind=4)>(D.2317)>;
+  D.2329_19 = 0.0;
   D.2330_20 = IMAGPART_EXPR <VIEW_CONVERT_EXPR<complex(kind=4)>(D.2317)>;
-  D.2331_21 = D.2329_19 != 4.5e+1;
+  D.2331_21 = 1;
   D.2332_22 = D.2330_20 != 0.0;
-  D.2333_23 = D.2332_22 | D.2331_21;
-  if (D.2333_23 == 1)

but you can see that the transform is correct - __complex__ (4.5e+1, 0.0)
is stored to D.2317 but then the realpart is overwritten by a boolean(kind=4)
false.  Which is interpreted as 0.0 by REALPART_EXPR
<VIEW_CONVERT_EXPR<complex(kind=4)>(D.2317)>; thus the D.2329_19 != 4.5e+1
is always true.

that's the e1 (7) test.  Without PRE it fails at the g4 (2) test (via FRE).

I suppose the frontend eventually fails foul of aliasing issues here?


Reduced testcase:

        complex function f2 (a)
        integer a
        logical e2
        entry e2 (a)
        if (a .gt. 0) then
          e2 = a .lt. 46
        else
          f2 = 45
        endif
        end function

        program entrytest
        complex f2
        if (f2 (0) .ne. 45) call abort ()
        end

[Bug middle-end/50628] [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f fails

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jamborm at gcc dot gnu.org

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-10-06 13:50:42 UTC ---
Looks like SRA messes up:

 f2 (integer(kind=4) * a)
 {
+  complex(kind=4) D.1794;
+  complex(kind=4) SR.5;
+  logical(kind=4) SR.4;
   integer(kind=4) D.1791;
   logical(kind=4) D.1790;
   union munion.0.f2 D.1789;
@@ -181,8 +79,11 @@

 <bb 5>:
   D.1789 = __result_master.0.f2;
-  __result = D.1789;
-  D.1779_2 = __result.f2;
+  SR.4_11 = __result_master.0.f2.e2;
+  D.1789.e2 = SR.4_11;
+  D.1794_12 = VIEW_CONVERT_EXPR<complex(kind=4)>(D.1789);
+  SR.5_14 = D.1794_12;
+  D.1779_2 = SR.5_14;
   return D.1779_2;

that looks like quite a stupit transform (the load and the store, using
logical(kind=4) which has the right size but precision 1 - and thus
truncates!)!

FRE then goes and does

   __result_master.0.f2.f2 = __complex__ (4.5e+1, 0.0);
   D.1808 = __result_master.0.f2;
   SR.7_12 = __result_master.0.f2.e2;
-  SR.4_6 = __result_master.0.f2.e2;
+  SR.4_6 = 0;
   SR.7_8 = SR.4_6;
   D.1808.e2 = SR.7_8;
   D.1802_7 = VIEW_CONVERT_EXPR<complex(kind=4)>(D.1808);

because it simply interprets bit zero of 4.5e+1.

Martin?

[Bug middle-end/50628] [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f fails

[dominiq at lps dot ens.fr]

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

--- Comment #4 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2011-10-06 13:55:24 UTC ---
This is due to/exposed by revision 179566. The test succeeds with  -O2
-fno-inline-small-functions, but fails with -O1 -finline-small-functions.

[Bug middle-end/50628] [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f fails

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-10-06 14:08:08 UTC ---
Basically SRA transforms

union u { char c; bool b; };

u.c = ...;
if (u.c ...)

to

u.c = ...;
bool reg = u.b;
u.b = reg;
if (u.c ...)

which is at least suspicious. If a user would write this I'd say it
is simply undefined (because u.b cannot represent all values that
are in the memory location of u.c).

FRE exposes this via native_interpret_int which at the end does

  return double_int_to_tree (type, result);
}

which truncates the value to 1-bit precision.  We probably don't want
out-of-precision INTEGER_CSTs here, so we could at most fail here.
But clearly SRA should avoid doing what it does.

[Bug middle-end/50628] [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f fails

[mikael at gcc dot gnu.org]

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

Mikael Morin <mikael at gcc dot gnu.org> changed:

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

--- Comment #6 from Mikael Morin <mikael at gcc dot gnu.org> 2011-10-06 18:13:08 UTC ---
*** Bug 50634 has been marked as a duplicate of this bug. ***

[Bug middle-end/50628] [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f90 fails

[mikael at gcc dot gnu.org]

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

Mikael Morin <mikael at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikael at gcc dot gnu.org
            Summary|[4.7 Regression]            |[4.7 Regression]
                   |gfortran.fortran-torture/ex |gfortran.fortran-torture/ex
                   |ecute/entry_4.f fails       |ecute/entry_4.f90 fails

--- Comment #7 from Mikael Morin <mikael at gcc dot gnu.org> 2011-10-06 18:16:53 UTC ---
(In reply to comment #4)
> This is due to/exposed by revision 179566. 

According to H.J. in bug 50634:
> On Linux/x86-64, revision 179556:

There is certainly a typo in one of the reported revisions.

[Bug middle-end/50628] [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f90 fails

[dominiq at lps dot ens.fr]

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

--- Comment #8 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2011-10-06 18:28:00 UTC ---
> There is certainly a typo in one of the reported revisions.

Yes, it is 179556.

[Bug middle-end/50628] [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f90 fails

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug middle-end/50628] [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f90 fails

[dominiq at lps dot ens.fr]

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

Dominique d'Humieres <dominiq at lps dot ens.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |danglin at gcc dot gnu.org

--- Comment #9 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2011-11-03 18:45:34 UTC ---
*** Bug 50985 has been marked as a duplicate of this bug. ***

[Bug middle-end/50628] [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f90 fails

[dominiq at lps dot ens.fr]

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

--- Comment #10 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2011-11-12 12:21:27 UTC ---
On x86_64-apple-darwin10 the test was failing for both -m32 and -m64 up to
revision 181046. From revision 181258 it fails only with -m64.

[Bug middle-end/50628] [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f90 fails

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |jamborm at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #11 from Martin Jambor <jamborm at gcc dot gnu.org> 2011-11-23 16:01:37 UTC ---
What happens in SRA is this.  This a part of the dump before SRA
(ealias):

<bb 2>:
  D.2127_5 = *a_1(D);
  if (D.2127_5 > 0)
    goto <bb 3>;
  else
    goto <bb 4>;

<bb 3>:
  D.2126_6 = D.2127_5 <= 45;
  /* In the following statement SRA notices a logical access
     to __result_master.1.f2. */
  __result_master.1.f2.e2 = D.2126_6; 
  goto <bb 5>;

<bb 4>:
  /* In the following statement SRA notices a complex access
     to __result_master.1.f2.  If a scalar type access has a scalar type
     sub-access, they cannot be made into replacements.  */
  __result_master.1.f2.f2 = __complex__ (4.5e+1, 0.0);

<bb 5>:

  /* However, before SRA figures that out, it propagates the logical
  sub-accesses across the following aggregate assignment in order to
  facilitate pseudo copy propagation of aggregates.  Thus, D.2125 gets an
  artificial logical sub-access which is a child of an aggregate and qualifies
  for replacement creation.  And that's why the weird logical variable gets
  there.  */

  D.2125 = __result_master.1.f2;
  __result_master.1.f2 ={v} {CLOBBER};
  __result = D.2125;
  D.2056_2 = __result.f2;
}

We should not do propagation of sub-accesses of scalar accesses because
that's never profitable and it would also fix this testcase.

However, I'm afraid that if we encapsulated 

  __result_master.1.f2.f2 = __complex__ (4.5e+1, 0.0);

into the union type and moved the last two statements of the function
into a different function, we would get the logical replacements of
__result_master.1.f2 and D.2125 again and might incur the same problems
(depending on whether FRE would be able to look through this).
SRA on unions and type-conversions is tough.

[Bug middle-end/50628] [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f90 fails

[jamborm at gcc dot gnu.org]

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

--- Comment #12 from Martin Jambor <jamborm at gcc dot gnu.org> 2011-11-24 12:18:46 UTC ---
Created attachment 25909
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25909
Fix

This is the fix I wrote about yesterday.  It bootstraps and tests fine
on x86_64 and we should want to have it because it always avoids
unnecessary work, even though I find it difficult to see it as a
correctness check.  Well...

[Bug middle-end/50628] [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f90 fails

[dominiq at lps dot ens.fr]

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

--- Comment #13 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2011-12-03 15:18:06 UTC ---
On x86_64-apple-darwin10, the patch in comment #12 fixes the pr without new
regression. Thanks.

[Bug middle-end/50628] [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f90 fails

[jamborm at gcc dot gnu.org]

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

--- Comment #14 from Martin Jambor <jamborm at gcc dot gnu.org> 2011-12-13 16:08:19 UTC ---
Author: jamborm
Date: Tue Dec 13 16:08:14 2011
New Revision: 182289

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182289
Log:
2011-12-13  Martin Jambor  <mjambor@suse.cz>

    PR middle-end/50628
    * tree-sra.c (propagate_subaccesses_across_link): Do not propagate
    sub-accesses of scalar accesses.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-sra.c

[Bug middle-end/50628] [4.7 Regression] gfortran.fortran-torture/execute/entry_4.f90 fails

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
                URL|                            |http://gcc.gnu.org/ml/gcc-p
                   |                            |atches/2011-12/msg01000.htm
                   |                            |l
         Resolution|                            |FIXED

--- Comment #15 from Martin Jambor <jamborm at gcc dot gnu.org> 2011-12-13 16:16:28 UTC ---
This is now fixed with the patch from comment #12.
However, this PR helped to uncover a more fundamental bug which is now
PR 51528.  

For reference, the message in the mailing list is:
http://gcc.gnu.org/ml/gcc-patches/2011-12/msg01000.html