[Bug target/25603] New: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[Bug target/25603] New: [4.1/4.2 Regression]: Miscompiled FORTRAN program

[hjl at lucon dot org]

I have identified that

http://gcc.gnu.org/ml/gcc-patches/2005-12/msg01261.html

causes FORTRAN program to generate wrong result on ia64


-- 
           Summary: [4.1/4.2 Regression]: Miscompiled FORTRAN program
           Product: gcc
           Version: 4.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: hjl at lucon dot org
 GCC build triplet: ia64-unknown-linux-gnu
  GCC host triplet: ia64-unknown-linux-gnu
GCC target triplet: ia64-unknown-linux-gnu


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[hjl at lucon dot org]

------- Comment #1 from hjl at lucon dot org  2005-12-30 17:40 -------
Created an attachment (id=10571)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10571&action=view)
A testcase

I got

/usr/gcc-4.2/bin/gfortran -O -fschedule-insns -o bar bar.f
./bar
 PROGRAM XSTART   201.000000000000
 BAR1 XSTART   0.00000000000000
make: *** [bar] Aborted
make: *** Deleting file `bar'


-- 


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 GCC target triplet|ia64-unknown-linux-gnu      |ia64-linux-gnu
           Keywords|                            |wrong-code
   Target Milestone|---                         |4.1.0


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[hjl at lucon dot org]

------- Comment #2 from hjl at lucon dot org  2005-12-30 18:42 -------
*** Bug 25585 has been marked as a duplicate of this bug. ***


-- 


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[jakub at gcc dot gnu dot org]

------- Comment #3 from jakub at gcc dot gnu dot org  2006-01-02 13:57 -------
This seems to be a reload bug, but Alan Modra's patch is correct.
In *.lreg we have:
(insn 577 78 11 2 (set (reg/f:DI 391)
        (reg/f:DI 0 ap)) 5 {*movdi_internal} (nil)
    (nil))

(insn 11 577 14 2 (set (reg/v/f:DI 388 [ nfilt ])
        (mem/c/i:DI (post_inc:DI (reg/f:DI 391)) [0 nfilt+0 S8 A64])) 5
{*movdi_internal} (insn_list:REG_DEP_TRUE 577 (nil))
    (expr_list:REG_INC (reg/f:DI 391)
        (expr_list:REG_EQUIV (mem/f/c/i:DI (reg/f:DI 0 ap) [0 nfilt+0 S8 A64])
            (nil))))

(insn 14 11 83 2 (set (reg/v/f:DI 389 [ xstart ])
        (mem/c/i:DI (reg/f:DI 391) [0 xstart+0 S8 A64])) 5 {*movdi_internal}
(insn_list:REG_DEP_TRUE 11 (insn_list:REG_DEP_TRUE
577 (nil)))
    (expr_list:REG_DEAD (reg/f:DI 391)
        (expr_list:REG_EQUIV (mem/f/c/i:DI (plus:DI (reg/f:DI 0 ap)
                    (const_int 8 [0x8])) [0 xstart+0 S8 A64])
            (nil))))

ap is in this case r12 + 1088, and the difference coming from Alan Modra's
patch is that before insn 577 was reloaded to an identical insn (as its
INSN_CODE
was wrong), while with that fix the insn is kept as is.  The only real effect
of the reloading resp. not reloading in insn 577 is register choice for
reloading
of subsequent insn,
(insn 11 577 14 2 (set (mem/c/i:DI (plus:DI (reg/f:DI 12 r12)
                (const_int 1088 [0x440])) [0 nfilt+0 S8 A64])
        (mem/c/i:DI (post_inc:DI (reg/f:DI 14 r14 [391])) [0 nfilt+0 S8 A64]))
5 {*movdi_internal} (insn_list:REG_DEP_TRUE 577 (nil))
    (expr_list:REG_INC (reg/f:DI 14 r14 [391])
        (expr_list:REG_EQUIV (mem/f/c/i:DI (reg/f:DI 0 ap) [0 nfilt+0 S8 A64])
            (nil))))

In one case, choose_reload_regs picks up:
Reload 0: reload_in (DI) = (plus:DI (reg/f:DI 12 r12)
                                                    (const_int 1088 [0x440]))
        GR_REGS, RELOAD_FOR_OPERAND_ADDRESS (opnum = 0), can't combine
        reload_in_reg: (plus:DI (reg/f:DI 12 r12)
                                                    (const_int 1088 [0x440]))
        reload_reg_rtx: (reg/f:DI 14 r14 [391])
Reload 1: reload_out (DI) = (mem/c/i:DI (plus:DI (reg/f:DI 12 r12)
                                                        (const_int 1088
[0x440])) [0 nfilt+0 S8 A64])
        NO_REGS, RELOAD_FOR_OUTPUT (opnum = 0), optional
        reload_out_reg: (reg/v/f:DI 388 [ nfilt ])
Reload 2: reload_in (DI) = (mem/c/i:DI (post_inc:DI (reg/f:DI 14 r14 [391])) [0
nfilt+0 S8 A64])
        GR_REGS, RELOAD_FOR_INPUT (opnum = 1)
        reload_in_reg: (mem/c/i:DI (post_inc:DI (reg/f:DI 14 r14 [391])) [0
nfilt+0 S8 A64])
        reload_reg_rtx: (reg:DI 15 r15)
and with Alan's patch reverted it chooses reload_reg_rtx for reload 0 and 2
r15 and r16 instead.  It seems reload is confused by the POST_INC and when
it chooses r14 is rld[0].reg_rtx, we get:
(insn 611 577 610 2 (set (reg:DI 15 r15)
        (mem/c/i:DI (post_inc:DI (reg/f:DI 14 r14 [391])) [0 nfilt+0 S8 A64]))
5 {*movdi_internal} (nil)
    (expr_list:REG_INC (reg/f:DI 14 r14 [391])
        (nil)))

(insn 610 611 11 2 (set (reg/f:DI 14 r14 [391])
        (plus:DI (reg/f:DI 12 r12)
            (const_int 1088 [0x440]))) 105 {adddi3} (nil)
    (nil))

(insn 11 610 14 2 (set (mem/c/i:DI (reg/f:DI 14 r14 [391]) [0 nfilt+0 S8 A64])
        (reg:DI 15 r15)) 5 {*movdi_internal} (insn_list:REG_DEP_TRUE 577 (nil))
    (expr_list:REG_EQUIV (mem/f/c/i:DI (reg/f:DI 0 ap) [0 nfilt+0 S8 A64])
        (nil)))

(insn 14 11 83 2 (set (reg/v/f:DI 107 loc75 [orig:389 xstart ] [389])
        (mem/c/i:DI (reg/f:DI 14 r14 [391]) [0 xstart+0 S8 A64])) 5
{*movdi_internal} (insn_list:REG_DEP_TRUE 11 (insn_list:REG_DEP_TRUE 577
(nil)))
    (expr_list:REG_EQUIV (mem/f/c/i:DI (plus:DI (reg/f:DI 0 ap)
                (const_int 8 [0x8])) [0 xstart+0 S8 A64])
        (nil)))
which is wrong, as r14 should have contained r12 + 1088 + 8 in insn 14, not
r12 + 1088.


-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2006-01-02 13:57:12
               date|                            |


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[steven at gcc dot gnu dot org]

------- Comment #4 from steven at gcc dot gnu dot org  2006-01-09 22:02 -------
IA-64 is a secondary platform, and Fortran is not a release critical language.


-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[steven at gcc dot gnu dot org]

------- Comment #5 from steven at gcc dot gnu dot org  2006-01-10 22:07 -------
Adding Jim Wilson to the CC: because he is the listed IA-64 maintainer.


-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wilson at gcc dot gnu dot
                   |                            |org


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[sje at cup dot hp dot com]

------- Comment #6 from sje at cup dot hp dot com  2006-01-23 16:58 -------
I have also reproduced this bug on IA64 HP-UX in LP64 mode (but not in ILP32
mode).


-- 

sje at cup dot hp dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sje at cup dot hp dot com


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[wilson at gcc dot gnu dot org]

------- Comment #7 from wilson at gcc dot gnu dot org  2006-01-31 03:49 -------
Things start going wrong in the reload inheritance code.  For reload 0, for
insn 11, we choose to inherit the value already in r14 in an inner loop in
choose_reload_regs.  Near the end, we double check to make sure we can still
inherit it, and notice we can't because the reload_reg_unavailable test fails. 
We cancel the inheritance, but we continue to use r14 as the reload reg.  We
then have to reload the value into r14, possibly hoping that post reload cse
can simplify the code.  Unfortunately, r14 is used in an auto-inc address in
this instruction which gets reloaded, so we have two reloads using the same
reg, one of which modifies the reg, and this fails miserably.

The inheritance code is calling find_equiv_reg, and then doing a number of
validity tests on the returned result.  One of the tests it does is to call
regno_clobbered_p.  This makes sure the reg isn't used in a clobber or set, but
it makes no attempt to look for other kinds of side-effects, such as auto-inc
addresses.  I think this is where the error lies.  If we modify
regno_clobberred_p to handle a REG_INC note the same as a clobber, then we
would get the right result here for this testcase, as reload 0 is
RELOAD_FOR_OPERAND_ADDRESS.  I'm not sure if this works in general though.  We
might need to be more strict for regs used in a REG_INC note, e.g. we might
need to disallow them always.

I'm curious whether Bernd's new reload-branch works for this testcase.  I don't
have a post-svn-conversion copy, so I had to check out a new copy and I'm doing
a build now.


-- 


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[grigory_zagorodnev at linux dot intel dot com]

------- Comment #8 from grigory_zagorodnev at linux dot intel dot com  2006-02-08 06:40 -------
I've verified that reload-branch works fine on the regression test-case. It
either works with that Alan's obvious reload fix
(http://gcc.gnu.org/ml/gcc-patches/2005-12/msg01261.html) applied. That makes
me think the root cause of the problem is fixed in the reload-branch.


-- 


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[wilson at gcc dot gnu dot org]

------- Comment #9 from wilson at gcc dot gnu dot org  2006-02-10 02:03 -------
I've looked at the reload-branch also.  I need two patches in order to
bootstrap it.  One trivial obvious patch, and one hacked not technically
correct patch.  It then bootstrapped and compiled this testcase correctly.

Since a major goal of the reload-branch is a complete rewrite of how
inheritance works, it isn't clear that the problem is fixed just because it
happened to work.  So I took a look at the sources.  The key detail here is
that reload-branch uses the scan_rtx function to determine what registers are
inputs and/or outputs of an instruction.  The scan_rtx function does handle
{POST,PRE}_{INC,DEC} correctly, treating them the same as SET, so that the
operand gets marked as an output register.  Although, looking at this, I can't
help but notice that {POST,PRE}_MODIFY do not get equivalent treatment.  I
think this is a bug.  Anyways, I'm convinced that reload-branch has a proper
solution to this bug, and I've got 3 bug reports against reload-branch that I
have to do something with now.

I remain convinced that my earlier suggestion for how to fix current reload is
correct.  I just haven't gotten around to trying to write a patch yet.  In
choose_reload_regs, where it calls regno_clobbered_p, we need an additional
check for registers modified by side-effects, as regno_clobbered_p does not
handle that itself.  This could probably be as simple as checking for a REG_INC
note that mentions the equiv register.


-- 


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[hjl at lucon dot org]

------- Comment #10 from hjl at lucon dot org  2006-02-10 03:37 -------
Thanks, Jim. Denis and I are working on a patch based on your suggestion to
modify regno_clobbered_p to handle REG_INC. We will submit it next week.


-- 


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[amylaar at gcc dot gnu dot org]

------- Comment #11 from amylaar at gcc dot gnu dot org  2006-02-14 18:55 -------
(In reply to comment #4)
> IA-64 is a secondary platform, and Fortran is not a release critical language.
>

But reload is neither specific to IA-64 nor Fortran, so this is a generic
latent wrong-code bug.  Shouldn't this be P2 or P3?


-- 

amylaar at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |joern dot rennecke at st dot
                   |                            |com


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[hjl at lucon dot org]

------- Comment #12 from hjl at lucon dot org  2006-02-14 19:33 -------
Our patch is ready. Denis, please submit your patch now. Thanks.


-- 


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[hjl at lucon dot org]

------- Comment #13 from hjl at lucon dot org  2006-02-14 20:04 -------
A patch is posted at

http://gcc.gnu.org/ml/gcc-patches/2006-02/msg01086.html


-- 

hjl at lucon dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[pinskia at gcc dot gnu dot org]

------- Comment #14 from pinskia at gcc dot gnu dot org  2006-02-14 21:15 -------
I bet this is related to PR 20972.


-- 


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[hjl at lucon dot org]

------- Comment #15 from hjl at lucon dot org  2006-02-15 21:29 -------
The updated patch is posted at

http://gcc.gnu.org/ml/gcc-patches/2006-02/msg01225.html


-- 


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

[Bug target/25603] [4.1/4.2 Regression]: Miscompiled FORTRAN program

[hjl at gcc dot gnu dot org]

------- Comment #16 from hjl at gcc dot gnu dot org  2006-02-17 00:03 -------
Subject: Bug 25603

Author: hjl
Date: Fri Feb 17 00:03:06 2006
New Revision: 111162

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=111162
Log:
2006-02-16  Denis Nagorny <denis_nagorny@linux.intel.com>

        PR rtl-optimization/25603
        * reload.c (reg_inc_found_and_valid_p): New. Check REG_INC note.
        (regno_clobbered_p): Use it. Reusing SETS argument for REG_INC case.
        * reload1.c (choose_reload_regs): Added call of regno_clobbered_p 
        with new meaning of SETS.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/reload.c
    trunk/gcc/reload1.c


-- 


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

[Bug target/25603] [4.1 Regression]: Miscompiled FORTRAN program

[hjl at lucon dot org]

------- Comment #17 from hjl at lucon dot org  2006-02-20 17:46 -------
A testcase patch is posted at

http://gcc.gnu.org/ml/gcc-patches/2006-02/msg01391.html


-- 


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

[Bug target/25603] [4.1 Regression]: Miscompiled FORTRAN program

[rguenth at gcc dot gnu dot org]

------- Comment #18 from rguenth at gcc dot gnu dot org  2006-02-22 16:30 -------
As this is a wrong-code regression for 4.1 (albeit on ia64-linux), Mark should
re-consider the priority and maybe allow this patch in.  And the patch
submitter should have asked for that in the first place (and can now ask for it
please).


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mmitchel at gcc dot gnu dot
                   |                            |org


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

[Bug target/25603] [4.1 Regression]: Miscompiled FORTRAN program

[hjl at gcc dot gnu dot org]

------- Comment #19 from hjl at gcc dot gnu dot org  2006-02-22 16:59 -------
Subject: Bug 25603

Author: hjl
Date: Wed Feb 22 16:59:45 2006
New Revision: 111365

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=111365
Log:
2006-02-22  H.J. Lu  <hongjiu.lu@intel.com>

        PR target/25603
        * gfortran.dg/pr25603.f: New testcase.

Added:
    trunk/gcc/testsuite/gfortran.dg/pr25603.f
Modified:
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug target/25603] [4.1 Regression]: Miscompiled FORTRAN program

[hjl at lucon dot org]

------- Comment #20 from hjl at lucon dot org  2006-02-22 17:04 -------
I don't want to see this patch hold up 4.1.0 release. I will ask it be
applied to the 4.1 branch when it is open. But it is Mark's call.


-- 


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

[Bug target/25603] [4.1 Regression]: Miscompiled FORTRAN program

[mmitchel at gcc dot gnu dot org]

------- Comment #21 from mmitchel at gcc dot gnu dot org  2006-02-22 22:51 -------
Set back to P3 so that I will be sure to reconsider it.


-- 

mmitchel at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P4                          |P3


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

[Bug target/25603] [4.1 Regression]: Miscompiled FORTRAN program

[mmitchel at gcc dot gnu dot org]

------- Comment #22 from mmitchel at gcc dot gnu dot org  2006-02-24 01:18 -------
Subject: Bug 25603

Author: mmitchel
Date: Fri Feb 24 01:18:27 2006
New Revision: 111409

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=111409
Log:
        Backport:
        2006-02-16  Denis Nagorny <denis_nagorny@linux.intel.com>
        PR rtl-optimization/25603
        * reload.c (reg_inc_found_and_valid_p): New. Check REG_INC note.
        (regno_clobbered_p): Use it. Reusing SETS argument for REG_INC case.
        * reload1.c (choose_reload_regs): Added call of regno_clobbered_p 
        with new meaning of SETS.

        Backport:
        2006-02-22  H.J. Lu  <hongjiu.lu@intel.com>
        PR target/25603
        * gfortran.dg/pr25603.f: New testcase.

Added:
    branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/pr25603.f
      - copied unchanged from r111365,
trunk/gcc/testsuite/gfortran.dg/pr25603.f
Modified:
    branches/gcc-4_1-branch/gcc/ChangeLog
    branches/gcc-4_1-branch/gcc/reload.c
    branches/gcc-4_1-branch/gcc/reload1.c
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog


-- 


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

[Bug target/25603] [4.1 Regression]: Miscompiled FORTRAN program

[mmitchel at gcc dot gnu dot org]

------- Comment #23 from mmitchel at gcc dot gnu dot org  2006-02-24 01:20 -------
Fixed in 4.1.0.


-- 

mmitchel at gcc dot gnu dot org changed:

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


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