This is the mail archive of the
gcc-bugs@gcc.gnu.org
mailing list for the GCC project.
[Bug rtl-optimization/47166] New: [4.5 4.6 Regression] SpecCpu2000 Ammp segfaults for ARM with -O3 -mthumb
- From: "ibolton at gcc dot gnu.org" <gcc-bugzilla at gcc dot gnu dot org>
- To: gcc-bugs at gcc dot gnu dot org
- Date: Tue, 4 Jan 2011 11:53:02 +0000
- Subject: [Bug rtl-optimization/47166] New: [4.5 4.6 Regression] SpecCpu2000 Ammp segfaults for ARM with -O3 -mthumb
- Auto-submitted: auto-generated
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.