public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug rtl-optimization/47166] New: [4.5 4.6 Regression] SpecCpu2000 Ammp segfaults for ARM with -O3 -mthumb
@ 2011-01-04 11:53 ibolton at gcc dot gnu.org
  2011-01-04 12:03 ` [Bug rtl-optimization/47166] " bernds at gcc dot gnu.org
                   ` (33 more replies)
  0 siblings, 34 replies; 35+ messages in thread
From: ibolton at gcc dot gnu.org @ 2011-01-04 11:53 UTC (permalink / raw)
  To: gcc-bugs

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

           Summary: [4.5 4.6 Regression] SpecCpu2000 Ammp segfaults for
                    ARM with -O3 -mthumb
           Product: gcc
           Version: 4.5.2
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: rtl-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: ibolton@gcc.gnu.org
        Depends on: 45051
              Host: arm-linux-gnueabi
            Target: arm-linux-gnueabi
             Build: arm-linux-gnueabi


As of r162558 on trunk, and r164498 on 4.5 branch, SpecCPU2000 Ammp has failed
for ARM -O3 thumb.  The same patch was committed for both of these revisions,
intending to fix pr45051:

Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c       (revision 164495)
+++ gcc/reload1.c       (revision 164498)
@@ -8325,6 +8325,8 @@ delete_output_reload (rtx insn, int j, i
   int n_inherited = 0;
   rtx i1;
   rtx substed;
+  unsigned regno;
+  int nregs;

   /* It is possible that this reload has been only used to set another reload
      we eliminated earlier and thus deleted this instruction too.  */
@@ -8376,6 +8378,12 @@ delete_output_reload (rtx insn, int j, i
   if (n_occurrences > n_inherited)
     return;

+  regno = REGNO (reg);
+  if (regno >= FIRST_PSEUDO_REGISTER)
+    nregs = 1;
+  else
+    nregs = hard_regno_nregs[regno][GET_MODE (reg)];
+
   /* If the pseudo-reg we are reloading is no longer referenced
      anywhere between the store into it and here,
      and we're within the same basic block, then the value can only
@@ -8387,7 +8395,7 @@ delete_output_reload (rtx insn, int j, i
       if (NOTE_INSN_BASIC_BLOCK_P (i1))
        return;
       if ((NONJUMP_INSN_P (i1) || CALL_P (i1))
-         && reg_mentioned_p (reg, PATTERN (i1)))
+         && refers_to_regno_p (regno, regno + nregs, PATTERN (i1), NULL))
        {
          /* If this is USE in front of INSN, we only have to check that
             there are no more references than accounted for by inheritance. 
*/

I assume the patch was meant to prevent deletions that shouldn't occur.  This
might be what happens for the original symptomatic test, but I am now seeing
extra deletions that shouldn't happen for Ammp.

For example, without this patch, you get these insns somewhere in the ira dump
for mm_fv_update_nonbon() from rectmm.c:

 (insn 3163 3161 3164 107 rectmm.c:1041 (set (reg:SI 1 r1) 
    (plus:SI (reg:SI 1 r1) 
       (const_int 280 [0x118]))) 4 {*arm_addsi3} (nil))

 (insn 3164 3163 1730 107 rectmm.c:1041 (set (reg:SI 3 r3) 
    (reg:SI 1 r1)) 586 {*thumb2_movsi_vfp} (nil))

With the patch, you lose the add and just get this:

 (insn 3164 3161 1730 107 rectmm.c:1041 (set (reg:SI 3 r3)
    (reg:SI 1 r1)) 586 {*thumb2_movsi_vfp} (nil)) 

The incrementing of r1 is perfectly legitimate and useful and removing it is a
bug.  Other increments of r9, ip, r0 and r3 are also lost.

I think the issue might be that reg_mentioned_p() considers output registers to
have been "mentioned", whereas the refers_to_regno_p() does not consider an
output register to have been "referred to".  I can see the problem with only
using reg_mentioned_p() because it doesn't handle subregs, but there is also a
problem with only using refers_to_regno_p(), as we can see with this segfault
in Ammp.

I therefore wonder if the fix might be this:

Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c       (revision 168082)
+++ gcc/reload1.c       (working copy)
@@ -8395,7 +8395,8 @@ delete_output_reload (rtx insn, int j, i
       if (NOTE_INSN_BASIC_BLOCK_P (i1))
        return;
       if ((NONJUMP_INSN_P (i1) || CALL_P (i1))
-         && refers_to_regno_p (regno, regno + nregs, PATTERN (i1), NULL))
+         && (refers_to_regno_p (regno, regno + nregs, PATTERN (i1), NULL)
+             || reg_mentioned_p (reg, PATTERN (i1))))
        {
          /* If this is USE in front of INSN, we only have to check that
             there are no more references than accounted for by inheritance. 
*/

It would be helpful to have some input from someone who understands reload
better than I do, so we can come up with a fix that we have confidence in.


^ permalink raw reply	[flat|nested] 35+ messages in thread
end of thread, other threads:[~2011-03-15  9:38 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-04 11:53 [Bug rtl-optimization/47166] New: [4.5 4.6 Regression] SpecCpu2000 Ammp segfaults for ARM with -O3 -mthumb ibolton at gcc dot gnu.org
2011-01-04 12:03 ` [Bug rtl-optimization/47166] " bernds at gcc dot gnu.org
2011-01-04 12:30 ` ibolton at gcc dot gnu.org
2011-01-04 12:32 ` ibolton at gcc dot gnu.org
2011-01-05  6:10 ` hp at gcc dot gnu.org
2011-01-05  8:21 ` hp at gcc dot gnu.org
2011-01-05 14:42 ` ibolton at gcc dot gnu.org
2011-01-05 16:50 ` ibolton at gcc dot gnu.org
2011-01-11 12:35 ` [Bug rtl-optimization/47166] [4.5/4.6 " rguenth at gcc dot gnu.org
2011-01-11 16:16 ` bernds at gcc dot gnu.org
2011-01-11 17:30 ` bernds at gcc dot gnu.org
2011-01-11 18:06 ` ibolton at gcc dot gnu.org
2011-01-12 10:43 ` ibolton at gcc dot gnu.org
2011-01-12 10:49 ` ibolton at gcc dot gnu.org
2011-01-15 16:15 ` [Bug rtl-optimization/47166] [4.5.2/4.6 " ebotcazou at gcc dot gnu.org
2011-01-19 20:01 ` bernds at gcc dot gnu.org
2011-01-19 20:16 ` ebotcazou at gcc dot gnu.org
2011-01-19 20:49 ` hp at gcc dot gnu.org
2011-01-19 23:14 ` ebotcazou at gcc dot gnu.org
2011-01-20 11:06 ` [Bug rtl-optimization/47166] [4.5/4.6 " rguenth at gcc dot gnu.org
2011-01-20 18:06 ` hp at gcc dot gnu.org
2011-01-23 22:41 ` bernds at gcc dot gnu.org
2011-01-24 14:47 ` jakub at gcc dot gnu.org
2011-01-24 15:30 ` ibolton at gcc dot gnu.org
2011-01-25 14:11 ` ibolton at gcc dot gnu.org
2011-01-25 16:19 ` [Bug rtl-optimization/47166] [4.5 " jakub at gcc dot gnu.org
2011-02-01  1:16 ` ramana at gcc dot gnu.org
2011-02-01  1:35 ` bernds at gcc dot gnu.org
2011-02-01  1:39 ` ramana.r at gmail dot com
2011-02-11 16:05 ` bernds at gcc dot gnu.org
2011-02-11 16:15 ` bernds at gcc dot gnu.org
2011-02-15 15:48 ` ibolton at gcc dot gnu.org
2011-03-14 13:46 ` rsandifo at gcc dot gnu.org
2011-03-14 13:49 ` rsandifo at gcc dot gnu.org
2011-03-15  9:40 ` rsandifo at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).