From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32672 invoked by alias); 4 Jan 2011 11:53:09 -0000 Received: (qmail 32658 invoked by uid 22791); 4 Jan 2011 11:53:08 -0000 X-SWARE-Spam-Status: No, hits=-2.9 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from localhost (HELO gcc.gnu.org) (127.0.0.1) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 04 Jan 2011 11:53:04 +0000 From: "ibolton at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug rtl-optimization/47166] New: [4.5 4.6 Regression] SpecCpu2000 Ammp segfaults for ARM with -O3 -mthumb X-Bugzilla-Reason: CC X-Bugzilla-Type: new X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: rtl-optimization X-Bugzilla-Keywords: wrong-code X-Bugzilla-Severity: normal X-Bugzilla-Who: ibolton at gcc dot gnu.org X-Bugzilla-Status: UNCONFIRMED X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Changed-Fields: Message-ID: X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated Content-Type: text/plain; charset="UTF-8" MIME-Version: 1.0 Date: Tue, 04 Jan 2011 11:53:00 -0000 Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-bugs-owner@gcc.gnu.org X-SW-Source: 2011-01/txt/msg00259.txt.bz2 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.