Richard Henderson <r...@redhat.com> writes:
> On 03/31/2011 03:19 AM, Richard Sandiford wrote:
>> * recog.h (insn_operand_data): Add an "allows_mem" field.
>> * genoutput.c (output_operand_data): Initialize it.
>> * optabs.c (maybe_legitimize_operand_same_code): New function.
>> (maybe_legitimize_operand): Use it when matching the original
>> op->value.
>
> Ok, with
>
>> old_volatile_ok = volatile_ok;
>> mode = op->mode;
>> - result = false;
>> switch (op->type)
>> {
>> case EXPAND_FIXED:
>> volatile_ok = true;
>> - break;
>> + result = maybe_legitimize_operand_same_code (icode, opno, op);
>> + volatile_ok = old_volatile_ok;
>> + return result;
>
> You can now move all references to volatile_ok into this case.
>
>> + return insn_operand_matches (icode, opno, op->value);
> ...
>> + return insn_operand_matches (icode, opno, op->value);
> ...
>> + return insn_operand_matches (icode, opno, op->value);
>
> And leave this common call at the end of the function.
Thanks. The original patch caused:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48573
so I had to revert it. The problem was that we were trying to force
a stack predecrement into a register.
For the record, The call path was:
emit_single_push_insn
if (GET_MODE_SIZE (mode) == rounded_size)
dest_addr = gen_rtx_fmt_e (STACK_PUSH_CODE, Pmode, stack_pointer_rtx);
...
dest = gen_rtx_MEM (mode, dest_addr);
...
emit_move_insn (dest, x);
emit_move_insn
if (optimize
&& SCALAR_FLOAT_MODE_P (GET_MODE (x))
&& (last_insn = compress_float_constant (x, y)))
return last_insn;
compress_float_constant
emit_unop_insn (ic, x, trunc_y, UNKNOWN);
and emit_move_insn certainly handles push_operands in some contexts.
I see now that this code should too.
I wondered about adding a check specifically for RTX_AUTOINC, or a
check specifically for push_operand. In the end I went for the more
general !side_effects_p, since if we do have something bizarre like
a (mem (mem ...)) address whose inner MEM is volatile, we probably
shouldn't be messing with that either.
The !side_effects_p check is the only change from last time.
Tested on x86_64-linux-gnu, this time on a machine set up for building
the 32-bit multilibs. Also tested on arm-linux-gnueabi. OK to install?
Richard
gcc/
* recog.h (insn_operand_data): Add an "allows_mem" field.
* genoutput.c (output_operand_data): Initialize it.
* optabs.c (maybe_legitimize_operand_same_code): New function.
(maybe_legitimize_operand): Use it when matching the original
op->value.
Index: gcc/recog.h
===================================================================
--- gcc/recog.h 2011-04-13 09:22:44.000000000 +0100
+++ gcc/recog.h 2011-04-13 09:23:39.000000000 +0100
@@ -272,6 +272,8 @@ struct insn_operand_data
const char is_operator;
const char eliminable;
+
+ const char allows_mem;
};
/* Legal values for insn_data.output_format. Indicate what type of data
Index: gcc/genoutput.c
===================================================================
--- gcc/genoutput.c 2011-04-13 09:22:44.000000000 +0100
+++ gcc/genoutput.c 2011-04-13 09:23:39.000000000 +0100
@@ -66,6 +66,8 @@ Software Foundation; either version 3, o
MATCH_OPERAND; it is zero for operands that should not be changed during
register elimination such as MATCH_OPERATORs.
+ g. `allows_mem', is true for operands that accept MEM rtxes.
+
The code number of an insn is simply its position in the machine
description; code numbers are assigned sequentially to entries in
the description, starting with code number 0.
@@ -256,6 +258,8 @@ output_operand_data (void)
for (d = odata; d; d = d->next)
{
+ struct pred_data *pred;
+
printf (" {\n");
printf (" %s,\n",
@@ -269,7 +273,12 @@ output_operand_data (void)
printf (" %d,\n", d->constraint == NULL ? 1 : 0);
- printf (" %d\n", d->eliminable);
+ printf (" %d,\n", d->eliminable);
+
+ pred = NULL;
+ if (d->predicate)
+ pred = lookup_predicate (d->predicate);
+ printf (" %d\n", pred && pred->codes[MEM]);
printf(" },\n");
}
Index: gcc/optabs.c
===================================================================
--- gcc/optabs.c 2011-04-13 09:22:44.000000000 +0100
+++ gcc/optabs.c 2011-04-13 09:28:40.000000000 +0100
@@ -7001,6 +7001,41 @@ insn_operand_matches (enum insn_code ico
(operand, insn_data[(int) icode].operand[opno].mode)));
}
+/* Like maybe_legitimize_operand, but do not change the code of the
+ current rtx value. */
+
+static bool
+maybe_legitimize_operand_same_code (enum insn_code icode, unsigned int opno,
+ struct expand_operand *op)
+{
+ /* See if the operand matches in its current form. */
+ if (insn_operand_matches (icode, opno, op->value))
+ return true;
+
+ /* If the operand is a memory whose address has no side effects,
+ try forcing the address into a register. The check for side
+ effects is important because force_reg cannot handle things
+ like auto-modified addresses. */
+ if (insn_data[(int) icode].operand[opno].allows_mem
+ && MEM_P (op->value)
+ && !side_effects_p (XEXP (op->value, 0)))
+ {
+ rtx addr, mem, last;
+
+ last = get_last_insn ();
+ addr = force_reg (Pmode, XEXP (op->value, 0));
+ mem = replace_equiv_address (op->value, addr);
+ if (insn_operand_matches (icode, opno, mem))
+ {
+ op->value = mem;
+ return true;
+ }
+ delete_insns_since (last);
+ }
+
+ return false;
+}
+
/* Try to make OP match operand OPNO of instruction ICODE. Return true
on success, storing the new operand value back in OP. */
@@ -7011,22 +7046,25 @@ maybe_legitimize_operand (enum insn_code
enum machine_mode mode, imode;
bool old_volatile_ok, result;
- old_volatile_ok = volatile_ok;
mode = op->mode;
- result = false;
switch (op->type)
{
case EXPAND_FIXED:
+ old_volatile_ok = volatile_ok;
volatile_ok = true;
- break;
+ result = maybe_legitimize_operand_same_code (icode, opno, op);
+ volatile_ok = old_volatile_ok;
+ return result;
case EXPAND_OUTPUT:
gcc_assert (mode != VOIDmode);
- if (!op->value
- || op->value == const0_rtx
- || GET_MODE (op->value) != mode
- || !insn_operand_matches (icode, opno, op->value))
- op->value = gen_reg_rtx (mode);
+ if (op->value
+ && op->value != const0_rtx
+ && GET_MODE (op->value) == mode
+ && maybe_legitimize_operand_same_code (icode, opno, op))
+ return true;
+
+ op->value = gen_reg_rtx (mode);
break;
case EXPAND_INPUT:
@@ -7034,9 +7072,10 @@ maybe_legitimize_operand (enum insn_code
gcc_assert (mode != VOIDmode);
gcc_assert (GET_MODE (op->value) == VOIDmode
|| GET_MODE (op->value) == mode);
- result = insn_operand_matches (icode, opno, op->value);
- if (!result)
- op->value = copy_to_mode_reg (mode, op->value);
+ if (maybe_legitimize_operand_same_code (icode, opno, op))
+ return true;
+
+ op->value = copy_to_mode_reg (mode, op->value);
break;
case EXPAND_CONVERT_TO:
@@ -7070,10 +7109,7 @@ maybe_legitimize_operand (enum insn_code
goto input;
break;
}
- if (!result)
- result = insn_operand_matches (icode, opno, op->value);
- volatile_ok = old_volatile_ok;
- return result;
+ return insn_operand_matches (icode, opno, op->value);
}
/* Make OP describe an input operand that should have the same value
