Is eliminate_regs_in_insn allowed to generate invalid instruction?

[Bingfeng Mei]

Hi, Ian,
I think I found the cause. find_reloads_address returns 0 even
it reloads both parts of address (see the patch). Therefore, 
corresponding address_reloaded[i] is not set and in
the following code of find_reloads, 

   if (EXTRA_CONSTRAINT_STR (operand, c, p))
    win = 1;
/* If the address was already reloaded,
   we win as well.  */
   else if (MEM_P (operand)
	      && address_reloaded[i] == 1) <-- address_reloaded[i] still 0
     win = 1;
    ...

Extra reload is thus generated even it is unnecessary
and causes ICE. 

It looks like a general GCC bug. But I couldn't produce
 a test for x86. The following patch is bootstrapped 
and passes tests on x86_64-unknown-linux-gnu. Is it
 OK to apply the patch?

Cheers,
Bingfeng 


Index: reload.c
===================================================================
--- reload.c    (revision 167979)
+++ reload.c    (working copy)
@@ -5188,7 +5188,7 @@ find_reloads_address (enum machine_mode
                                  &XEXP (ad, 1 - op_index), opnum,
                                  type, 0, insn);

-         return 0;
+         return 1;
        }
     }


> -----Original Message-----
> From: Ian Lance Taylor [mailto:iant@google.com]
> Sent: 17 December 2010 15:10
> To: Bingfeng Mei
> Cc: gcc@gcc.gnu.org
> Subject: Re: Is eliminate_regs_in_insn allowed to generate invalid
> instruction?
> 
> "Bingfeng Mei" <bmei@broadcom.com> writes:
> 
> > from gen_reload function.
> >   /* Otherwise, just write (set OUT IN) and hope for the best.  */
> >   else
> >     emit_insn (gen_rtx_SET (VOIDmode, out, in));
> 
> Those lines are one of the curses of reload.  When you hit them, you
> know something has gone wrong.  Unfortunately, exactly what has gone
> wrong can be difficult to determine.  It basically means that you are
> trying to reload something that the reload pass does not understand.
> 
> 
> > The comment doesn’t sound very convincing to me.
> >
> >
> > From debug message:
> > Reloads for insn # 680
> > Reload 0: reload_in (SI) = (plus:SI (reg/f:SI 57 r57)
> >                                                     (const_int 40
> [0x28]))
> > 	GR_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1)
> > 	reload_in_reg: (plus:SI (reg/f:SI 57 r57)
> >                                                     (const_int 40
> [0x28]))
> > 	reload_reg_rtx: (reg:SI 4 r4)
> > Reload 1: reload_in (SI) = (plus:SI (reg/f:SI 57 r57)
> >                                                     (const_int 78396
> [0x1323c]))
> > 	GR_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1), can't combine
> > 	reload_in_reg: (plus:SI (reg/f:SI 57 r57)
> >                                                     (const_int 78396
> [0x1323c]))
> > 	reload_reg_rtx: (reg:SI 6 r6)
> > Reload 2: reload_in (SI) = (mem/c:SI (plus:SI (reg/f:SI 57 r57)
> >                                                         (const_int
> 78396 [0x1323c])) [50 %sfp+78252 S4 A32])
> > 	GR_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1), can't combine
> > 	reload_in_reg: (reg:SI 596 [ ivtmp.474 ])
> > 	reload_reg_rtx: (reg:SI 9 r9)
> > Reload 3: reload_in (SI) = (plus:SI (mult:SI (reg:SI 596
> [ ivtmp.474 ])
> >                                                         (const_int 8
> [0x8]))
> >                                                     (plus:SI
> (reg/f:SI 57 r57)
> >                                                         (const_int 40
> [0x28])))
> > 	GR_REGS, RELOAD_FOR_INPUT (opnum = 1), inc by 8
> > 	reload_in_reg: (plus:SI (mult:SI (reg:SI 596 [ ivtmp.474 ])
> >                                                         (const_int 8
> [0x8]))
> >                                                     (plus:SI
> (reg/f:SI 57 r57)
> >                                                         (const_int 40
> [0x28])))
> > 	reload_reg_rtx: (reg:SI 4 r4)
> >
> >
> > I don’t understand why Reload 3 is necessary. After reload 0, 1, 2,
> > The following expression already fits into our addressing mode.
> >
> > (plus:SI (mult:SI (reg:SI 9 r9)  (const_int 8 [0x8]))
> >                                  (reg:SI 4 r4))
> >
> > Instead GCC tries to generate invalid
> > (insn 1716 1715 680 84 src/weighted_prediction.c:729 (set (reg:SI 4
> r4)
> >         (plus:SI (mult:SI (reg:SI 9 r9)
> >                 (const_int 8 [0x8]))
> >             (reg:SI 4 r4))) -1 (nil))
> > and load from [r4] subsequently.
> 
> Sounds like you're well on the way to tracking down the problem: you
> need to see why gcc decided to add reload 3 given the existence of the
> reloads 0 through 2.  In particular, look at the code after
>   if (strict_memory_address_addr_space_p (mode, ad, as))
> in find_reloads_address.
> 
> Ian