diff mbox

Try simplifying memory operands during optabs expansion

Message ID g4mxjtar10.fsf@linaro.org
State Accepted
Headers show

Commit Message

Richard Sandiford April 14, 2011, 1:42 p.m. UTC 
Richard Henderson <rth@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.

Comments

Richard Henderson April 14, 2011, 3:23 p.m. UTC | #1 
On 04/14/2011 06:42 AM, Richard Sandiford wrote:
> 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.

Ok.


r~
diff mbox

Patch

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