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

Ian Bolton <ibolton at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
                 CC|                            |ibolton at gcc dot gnu.org
         Resolution|FIXED                       |

--- Comment #7 from Ian Bolton <ibolton at gcc dot gnu.org> 2010-12-22 16:23:21 
UTC ---
(In reply to comment #5)
> Assuming fixed and closing.  Please reopen if you still have a problem.

This patch has caused SpecCPU2000 Ammp to fail for ARM -O3 thumb.

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. 
*/

I am heading off for Christmas vacation shortly, so cannot look into this any
further, but I wanted to record my findings so far publicly.  Apologies if
there is missing information.  I return to work Jan 4th.

Reply via email to