On Wed, Mar 26, 2014 at 09:53:47PM +0000, Richard Sandiford wrote:
Richard Henderson <rth@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).