On Wed, Mar 26, 2014 at 09:53:47PM +0000, Richard Sandiford wrote:
> Richard Henderson <r...@redhat.com> writes:
> > On 03/26/2014 12:40 PM, Jakub Jelinek wrote:
> >> On Wed, Mar 26, 2014 at 01:32:44PM -0600, Jeff Law wrote:
> >>> On 03/26/14 12:28, Jakub Jelinek wrote:
> >>>> (mult:SI (const_int 0) (const_int 4)) is IMHO far from being canonical.
> >>>> And, I'd say it is likely other target legitimization hooks would also
> >>>> try
> >>>> to simplify it similarly.
> >>>> simplify_gen_binary is used in several other places during expansion,
> >>>> so I don't see why it couldn't be desirable here.
> >>> No particular reason. I'll try that since we disagree about the
> >>> validity of the RTL and we can both agree that using
> >>> simplify_gen_binary is reasonable.
> >>
> >> Other possibility if you want to change it in the i386.c legitimize_address
> >> hook would be IMHO using force_reg instead of force_operand, it should be
> >> the same thing in most cases, except for these corner cases, and there
> >> would
> >> be no need to canonizalize anything afterwards.
> >> But, if the i?86 maintainers feel otherwise on this and think your patch is
> >> ok, I don't feel that strongly about this.
> >
> > I like this as a solution. Let the combiner clean things up if it's
> > gotten so far.
Did you mean Jeff's original change, or say:
--- gcc/config/i386/i386.c 2014-03-20 17:41:45.917689676 +0100
+++ gcc/config/i386/i386.c 2014-03-27 14:47:21.876254288 +0100
@@ -13925,13 +13925,13 @@ ix86_legitimize_address (rtx x, rtx oldx
if (GET_CODE (XEXP (x, 0)) == MULT)
{
changed = 1;
- XEXP (x, 0) = force_operand (XEXP (x, 0), 0);
+ XEXP (x, 0) = copy_addr_to_reg (XEXP (x, 0));
}
if (GET_CODE (XEXP (x, 1)) == MULT)
{
changed = 1;
- XEXP (x, 1) = force_operand (XEXP (x, 1), 0);
+ XEXP (x, 1) = copy_addr_to_reg (XEXP (x, 1));
}
if (changed
(or copy_to_reg, should be the same thing).
> How about doing both? Jakub's simplify_gen_binary change looked like a good
> idea regardless of whatever else happens. Seems a shame not to go with it.
Agreed.
Jakub
