[Bug rtl-optimization/47008] New: [4.6 Regression] gfortran.dg/extends_2.f03 FAIL with -Os -fschedule-insns

[Bug rtl-optimization/47008] New: [4.6 Regression] gfortran.dg/extends_2.f03 FAIL with -Os -fschedule-insns

[zsojka at seznam dot cz]

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

           Summary: [4.6 Regression] gfortran.dg/extends_2.f03 FAIL with
                    -Os -fschedule-insns
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: zsojka@seznam.cz
              Host: x86_64-pc-linux-gnu
            Target: x86_64-pc-linux-gnu


Created attachment 22814
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22814
reduced testcase

Output:
$ gfortran -Os -fno-asynchronous-unwind-tables
-fno-inline-functions-called-once -fschedule-insns testcase.f03
$ ./a.out 
Aborted

$ gfortran -Os -fno-asynchronous-unwind-tables
-fno-inline-functions-called-once -fschedule-insns extends_2.f03
$ ./a.out 
Aborted

$ gfortran -Os -fno-asynchronous-unwind-tables
-fno-inline-functions-called-once -fschedule-insns -fsched-pressure
extends_3.f03
$ ./a.out 
Aborted

For testcase.f03, in the assembly, the problem is there:

# store 123455 to [rsp+8], &supervisor == rsp+8
mov    DWORD PTR [rsp+8], 123455    # person.1.ss,
lea    rsi, [rsp+8]    # tmp64,
# rsp -= 8 (parameter for entry() ?)
push    0    #
# &supervisor == rsp+8+8 == rsp+16
mov    eax, 538976288    # tmp67,
# loads old value of &supervisor(+4)
lea    rdx, [rsp+12]    # tmp65,
lea    rbx, [rsp+56]    # tmp72,
mov    rdi, rdx    # tmp66, tmp65
mov    edx, OFFSET FLAT:.LC3    #,
# starts storing 4 bytes under previously used &supervisor, overwrites 123455
rep stosd


Tested revisions (for reduced testcase):
r168026 - fail
r165699 - OK
4.5 r166509 - OK

[Bug rtl-optimization/47008] [4.6 Regression] gfortran.dg/extends_{23}.f03 FAIL with -Os -fschedule-insns

[hjl.tools at gmail dot com]

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

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2010.12.18 23:13:40
                 CC|                            |hubicka at gcc dot gnu.org
   Target Milestone|---                         |4.6.0
     Ever Confirmed|0                           |1

--- Comment #1 from H.J. Lu <hjl.tools at gmail dot com> 2010-12-18 23:13:40 UTC ---
It is caused by revision 166624:

http://gcc.gnu.org/ml/gcc-cvs/2010-11/msg00513.html

[Bug rtl-optimization/47008] [4.6 Regression] gfortran.dg/extends_{23}.f03 FAIL with -Os -fschedule-insns

[steven at gcc dot gnu.org]

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

Steven Bosscher <steven at gcc dot gnu.org> changed:

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

--- Comment #2 from Steven Bosscher <steven at gcc dot gnu.org> 2010-12-19 00:07:07 UTC ---
I doubt that revision did any more than just expose the underlying issue in
sched-deps.

[Bug rtl-optimization/47008] [4.6 Regression] gfortran.dg/extends_{23}.f03 FAIL with -Os -fschedule-insns

[hubicka at gcc dot gnu.org]

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

--- Comment #3 from Jan Hubicka <hubicka at gcc dot gnu.org> 2010-12-19 11:34:22 UTC ---
Yep, this indeed looks like aliasing bug...

[Bug rtl-optimization/47008] [4.6 Regression] gfortran.dg/extends_{23}.f03 FAIL with -Os -fschedule-insns

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2010-12-20 10:30:43 UTC ---
It doesn't, it looks like a RA/reload bug.
In *.sched1 we have:
   17 NOTE_INSN_BASIC_BLOCK
   31 [pre sp:DI+=0xfffffffffffffff8]=0
   20 {r64:DI=frame:DI-0x30;clobber flags:CC;}
      REG_UNUSED: flags:CC
   23 r67:SI=0x20202020
   24 r68:DI=0x6
   18 [frame:DI-0x8]=r63:DI
   19 [frame:DI-0x30]=0x1e23f
   32 [pre sp:DI+=0xfffffffffffffff8]=0
   21 {r65:DI=r64:DI+0x4;clobber flags:CC;}
      REG_UNUSED: flags:CC
   34 {r72:DI=frame:DI-0x8;clobber flags:CC;}
      REG_UNUSED: flags:CC
   29 r71:DI=0xa
   36 r9:DI=`*.LC1'
   37 r8:DI=`*.LC2'
   25 {r68:DI=0;r66:DI=r68:DI<<0x2+r65:DI;[r65:DI]=0;use r67:SI;use r68:DI;}
      REG_DEAD: r67:SI
      REG_DEAD: r65:DI
      REG_UNUSED: r68:DI
      REG_UNUSED: r66:DI
so, the 0x1e23f store in insn 19 is to frame-48 and then there is memset
(frame-48+4, ' ', 24)
In *.ira this is:
   17 NOTE_INSN_BASIC_BLOCK
   31 [pre sp:DI+=0xfffffffffffffff8]=0
   20 {si:DI=sp:DI+0x8;clobber flags:CC;}
      REG_EQUIV: sp:DI+0x8
   24 cx:DI=0x6
      REG_EQUAL: 0x6
   18 [sp:DI+0x30]=r10:DI
   19 [sp:DI+0x8]=0x1e23f
   32 [pre sp:DI+=0xfffffffffffffff8]=0
   21 {dx:DI=si:DI+0x4;clobber flags:CC;}
   34 {bx:DI=sp:DI+0x38;clobber flags:CC;}
      REG_EQUIV: sp:DI+0x38
   29 r11:DI=0xa
      REG_EQUAL: 0xa
   36 r9:DI=`*.LC1'
   37 r8:DI=`*.LC2'
   57 ax:SI=0x20202020
      REG_EQUIV: 0x20202020
   59 di:DI=dx:DI
   25 {cx:DI=0;di:DI=cx:DI<<0x2+di:DI;[di:DI]=0;use ax:SI;use cx:DI;}
i.e. frame-48 is rsp+0 at the beginning of the bb, and the 0x1e234 store is to
that location and memset 4 bytes above it (as there are intervening push insns,
the store is to %rsp+8 at that point and memset is to %rsp+16+4).  The
REG_EQUIV on insn 20 is wrong though, %rsi is equal to %rsp+8 only until %rsp
is changed, not always.
And in *.postreload it uses REG_EQUIV to optimize rdx = rsi + 4 into rdx = rsp
+ 12, which is wrong in that spot, it should have been rsp + 20:
   31 [pre sp:DI+=0xfffffffffffffff8]=0
   20 {si:DI=sp:DI+0x8;clobber flags:CC;}
      REG_EQUIV: sp:DI+0x8
   24 cx:DI=0x6
      REG_EQUAL: 0x6
   18 [sp:DI+0x30]=r10:DI
   19 [sp:DI+0x8]=0x1e23f
   32 [pre sp:DI+=0xfffffffffffffff8]=0
   21 {dx:DI=sp:DI+0xc;clobber flags:CC;}
   34 {bx:DI=sp:DI+0x38;clobber flags:CC;}
      REG_EQUIV: sp:DI+0x38
   29 r11:DI=0xa
      REG_EQUAL: 0xa
   36 r9:DI=`*.LC1'
   37 r8:DI=`*.LC2'
   57 ax:SI=0x20202020
      REG_EQUIV: 0x20202020
   59 di:DI=dx:DI
   25 {cx:DI=0;di:DI=cx:DI<<0x2+di:DI;[di:DI]=0;use ax:SI;use cx:DI;}

[Bug rtl-optimization/47008] [4.6 Regression] gfortran.dg/extends_{23}.f03 FAIL with -Os -fschedule-insns

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at gcc dot gnu.org,
                   |                            |vmakarov at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2010-12-20 11:10:00 UTC ---
The note is added in update_equiv_regs called from ira.
          /* cse sometimes generates function invariants, but doesn't put a
             REG_EQUAL note on the insn.  Since this note would be redundant,
             there's no point creating it earlier than here.  */
          if (! note && ! rtx_varies_p (src, 0))
            note = set_unique_reg_note (insn, REG_EQUAL, copy_rtx (src));
...
          /* If this register is known to be equal to a constant, record that
             it is always equivalent to the constant.  */
          if (DF_REG_DEF_COUNT (regno) == 1
              && note && ! rtx_varies_p (XEXP (note, 0), 0))
            {
              rtx note_value = XEXP (note, 0);
              remove_note (insn, note);
              set_unique_reg_note (insn, REG_EQUIV, note_value);
            }
This is correct at that point, because note_value is frame - 48, which really
is constant through the whole function.

But when
  /* If we changed something, perform elimination in REG_NOTES.  This is
     needed even when REPLACE is zero because a REG_DEAD note might refer
     to a register that we eliminate and could cause a different number  
     of spill registers to be needed in the final reload pass than in
     the pre-passes.  */
  if (val && REG_NOTES (insn) != 0)
    REG_NOTES (insn)
      = eliminate_regs_1 (REG_NOTES (insn), VOIDmode, REG_NOTES (insn), true,
                          false);
in eliminate_regs_in_insn changes the REG_EQUIV note content from frame -
something to %rsp + something, the expression is suddenly not constant anymore.

So, shouldn't eliminate_regs_1 here downgrade REG_EQUIV notes where
!rtx_varies_p before and rtx_varies_p afterwards to REG_EQUAL?

[Bug rtl-optimization/47008] [4.6 Regression] gfortran.dg/extends_{23}.f03 FAIL with -Os -fschedule-insns

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2010-12-20 13:03:56 UTC ---
--- reload1.c.jj       2010-11-25 18:50:45.000000000 +0100
+++ reload1.c   2010-12-20 12:54:34.924901692 +0100
@@ -3597,9 +3597,20 @@ eliminate_regs_in_insn (rtx insn, int re
      of spill registers to be needed in the final reload pass than in
      the pre-passes.  */
   if (val && REG_NOTES (insn) != 0)
-    REG_NOTES (insn)
-      = eliminate_regs_1 (REG_NOTES (insn), VOIDmode, REG_NOTES (insn), true,
-                         false);
+    {
+      rtx note;
+      REG_NOTES (insn)
+       = eliminate_regs_1 (REG_NOTES (insn), VOIDmode, REG_NOTES (insn), true,
+                           false);
+      /* Register elimination might change a REG_EQUIV note expression from
+        something that is constant to e.g. stack pointer based expression,
+        which can't be safely moved across stack pointer adjustments.
+        Downgrade such notes to REG_EQUAL.  */
+      for (note = REG_NOTES (insn); note; note = XEXP (note, 1))
+       if (REG_NOTE_KIND (note) == REG_EQUIV
+           && rtx_varies_p (XEXP (note, 0), 0))
+         PUT_REG_NOTE_KIND (note, REG_EQUAL);
+    }

   return val;
 }

unfortunately doesn't fix this.
--- postreload.c.jj    2010-12-02 13:15:24.000000000 +0100
+++ postreload.c        2010-12-20 14:01:23.404758077 +0100
@@ -1415,7 +1415,8 @@ reload_combine_note_store (rtx dst, cons
     {
       dst = XEXP (dst, 0);
       if (GET_CODE (dst) == PRE_INC || GET_CODE (dst) == POST_INC
-         || GET_CODE (dst) == PRE_DEC || GET_CODE (dst) == POST_DEC)
+         || GET_CODE (dst) == PRE_DEC || GET_CODE (dst) == POST_DEC
+         || GET_CODE (dst) == PRE_MODIFY || GET_CODE (dst) == POST_MODIFY)
        {
          regno = REGNO (XEXP (dst, 0));
          mode = GET_MODE (XEXP (dst, 0));
does though.  Perhaps just the latter patch would be good enough, dunno whether
something after reload ever makes a difference between REG_EQUIV and REG_EQUAL.

[Bug rtl-optimization/47008] [4.6 Regression] gfortran.dg/extends_{23}.f03 FAIL with -Os -fschedule-insns

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                URL|                            |http://gcc.gnu.org/ml/gcc-p
                   |                            |atches/2010-12/msg01588.htm
                   |                            |l
         AssignedTo|unassigned at gcc dot       |jakub at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2010-12-21 08:38:42 UTC ---
http://gcc.gnu.org/ml/gcc-patches/2010-12/msg01588.html

[Bug rtl-optimization/47008] [4.6 Regression] gfortran.dg/extends_{23}.f03 FAIL with -Os -fschedule-insns

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> 2010-12-21 14:51:46 UTC ---
Author: jakub
Date: Tue Dec 21 14:51:42 2010
New Revision: 168117

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168117
Log:
    PR rtl-optimization/47008
    * postreload.c (reload_combine_note_store): Also handle
    PRE_MODIFY and POST_MODIFY.

    * gfortran.dg/pr47008.f03: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/pr47008.f03
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/postreload.c
    trunk/gcc/testsuite/ChangeLog

[Bug rtl-optimization/47008] [4.6 Regression] gfortran.dg/extends_{23}.f03 FAIL with -Os -fschedule-insns

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> 2010-12-21 14:53:36 UTC ---
Fixed.