Re: [RTL, ColdFire 24/63] Add support for a MODE_INDEX_REG_CLASS macro

[Jeffrey Law <law at redhat dot com>]

On Tue, 2007-02-27 at 12:24 -0800, Ian Lance Taylor wrote:
> Jeffrey Law <law@redhat.com> writes:

> The thing I think is odd about this approach is that as far as I know
> reg+reg addressing is not actually mode dependent; it's dependent on
> the register class which is the destination of the load.  We are using
> the mode as a proxy for that.  In principle we can do better.
True.  I'm well aware that I'm glossing over that.  GCC as a whole
really falls down when address validity has that level of context
dependency.    The PA is notorious in this regard and the problems
just about drove me nuts.

Even with that caveat, I still think fixing how we use
double_reg_address_ok is a much better fix and  removes
one of reload's (incorrect) assumptions about how addressing
modes can be used (if indexing is valid for QImode, then it
must be valid for other modes).



> I think you forgot to attach the patch.
> 
Not my day.  Attached


Index: postreload.c
===================================================================
--- postreload.c	(revision 122379)
+++ postreload.c	(working copy)
@@ -703,12 +703,15 @@
   int last_label_ruid;
   int min_labelno, n_labels;
   HARD_REG_SET ever_live_at_start, *label_live;
+  enum machine_mode mode;
 
   /* If reg+reg can be used in offsetable memory addresses, the main chunk of
      reload has already used it where appropriate, so there is no use in
      trying to generate it now.  */
-  if (double_reg_address_ok && INDEX_REG_CLASS != NO_REGS)
-    return;
+  for (mode = VOIDmode; (int) mode < NUM_MACHINE_MODES;
+       mode = (enum machine_mode) ((int) mode + 1))
+    if (double_reg_address_ok[mode] && INDEX_REG_CLASS != NO_REGS)
+      return;
 
   /* To avoid wasting too much time later searching for an index register,
      determine the minimum and maximum index register numbers.  */
Index: reload.c
===================================================================
--- reload.c	(revision 122379)
+++ reload.c	(working copy)
@@ -4965,7 +4965,7 @@
 	    loc = &XEXP (*loc, 0);
 	}
 
-      if (double_reg_address_ok)
+      if (double_reg_address_ok[mode])
 	{
 	  /* Unshare the sum as well.  */
 	  *loc = ad = copy_rtx (ad);
@@ -5008,8 +5008,8 @@
      find_reloads_subreg_address.
 
      If we decide to do something, it must be that `double_reg_address_ok'
-     is true.  We generate a reload of the base register + constant and
-     rework the sum so that the reload register will be added to the index.
+     is true for MODE.  We generate a reload of the base register + constant
+     and rework the sum so that the reload register will be added to the index.
      This is safe because we know the address isn't shared.
 
      We check for the base register as both the first and second operand of
Index: reload.h
===================================================================
--- reload.h	(revision 122379)
+++ reload.h	(working copy)
@@ -189,7 +189,7 @@
 extern char indirect_symref_ok;
 
 /* Nonzero if an address (plus (reg frame_pointer) (reg ...)) is valid.  */
-extern char double_reg_address_ok;
+extern char double_reg_address_ok[MAX_MACHINE_MODE];
 
 extern int num_not_at_initial_offset;
 
Index: reload1.c
===================================================================
--- reload1.c	(revision 122379)
+++ reload1.c	(working copy)
@@ -244,8 +244,9 @@
    which these are valid is the same as spill_indirect_levels, above.  */
 char indirect_symref_ok;
 
-/* Nonzero if an address (plus (reg frame_pointer) (reg ...)) is valid.  */
-char double_reg_address_ok;
+/* Nonzero if an address (plus (reg frame_pointer) (reg ...)) is valid,
+   indxed by the mode of the memory reference.  */
+char double_reg_address_ok[MAX_MACHINE_MODE];
 
 /* Record the stack slot for each spilled hard register.  */
 static rtx spill_stack_slot[FIRST_PSEUDO_REGISTER];
@@ -453,6 +454,7 @@
 init_reload (void)
 {
   int i;
+  enum machine_mode mode;
 
   /* Often (MEM (REG n)) is still valid even if (REG n) is put on the stack.
      Set spill_indirect_levels to the number of levels such addressing is
@@ -477,24 +479,25 @@
   tem = gen_rtx_MEM (Pmode, gen_rtx_SYMBOL_REF (Pmode, "foo"));
   indirect_symref_ok = memory_address_p (QImode, tem);
 
-  /* See if reg+reg is a valid (and offsettable) address.  */
+  /* See if reg+reg is a valid (and offsettable) address for each mode.  */
+  for (mode = VOIDmode; (int) mode < NUM_MACHINE_MODES;
+       mode = (enum machine_mode) ((int) mode + 1))
+    for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
+      {
+	tem = gen_rtx_PLUS (Pmode,
+			    gen_rtx_REG (Pmode, HARD_FRAME_POINTER_REGNUM),
+			    gen_rtx_REG (Pmode, i));
 
-  for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
-    {
-      tem = gen_rtx_PLUS (Pmode,
-			  gen_rtx_REG (Pmode, HARD_FRAME_POINTER_REGNUM),
-			  gen_rtx_REG (Pmode, i));
+	/* This way, we make sure that reg+reg is an offsettable address.  */
+	tem = plus_constant (tem, GET_MODE_SIZE (mode));
 
-      /* This way, we make sure that reg+reg is an offsettable address.  */
-      tem = plus_constant (tem, 4);
+	if (memory_address_p (mode, tem))
+	  {
+	    double_reg_address_ok[mode] = 1;
+	    break;
+	  }
+      }
 
-      if (memory_address_p (QImode, tem))
-	{
-	  double_reg_address_ok = 1;
-	  break;
-	}
-    }
-
   /* Initialize obstack for our rtl allocation.  */
   gcc_obstack_init (&reload_obstack);
   reload_startobj = obstack_alloc (&reload_obstack, 0);