RFA: displacement handling in equiv_address_substitution

[Richard Sandiford]

Hi Vlad,

As discussed in the reviews, one of the things that worried me was the
combination of:

1) the displacement fixup code in process_address assumes that the address
   is exactly equal to BASE_LOC + INDEX_LOC + DISP (with null values
   being equivalent to 0).

2) extract_address_regs allows (and has to allow) much more general forms
   than that.

3) equiv_address_substitution folds base and index displacements
   without checking the shape of the address.

So the code from (1) could end up fixing a displacement created by (3)
even if the address isn't a simple BASE_LOC + INDEX_LOC + DISP.

There's a similar problem with the relationship between INDEX_LOC
and INDEX_REG_LOC: we assume the scale is 1 unless INDEX is a MULT
of the index register.

This patch adds two utility functions, one to test whether the address
is simple enough for the fixup code to handle, and one to see whether
the index scale is known.  The latter handles ASHIFT as well as MULT,
since ASHIFT can be used in out-of-MEM address operands.

The condition:

  /* If the address already has a displacement, we can simply add the
     new displacement to it.  */
  if (ad->disp_loc)
    return true;

might leave a bit of a hole, but the asserts you added to
extract_address_regs should mean that disp_loc really is a
displacement (thanks for those btw).

In principle, we could apply a base or index displacement even in cases
that process_address can't fix up, test whether the result is valid,
and revert to the normal behaviour of reloading the full base or index
value if not.  That's not going to be useful for x86 (or for MIPS :-))
and I'm not planning to do it, but the valid_address_p patch I just sent
ought to help.

Tested on x86_64-linux-gnu.  Also tested by making sure that there
were no changes in assembly output for a set of gcc .ii files
(i.e. these extra checks didn't affect the code on x86_64 at least).
OK to install?

Richard


gcc/
	* lra-constraints.c (get_index_scale, can_add_disp_p): New functions.
	(equiv_address_substitution): Use them.

Index: gcc/lra-constraints.c
===================================================================
--- gcc/lra-constraints.c	2012-10-25 09:50:13.005284668 +0100
+++ gcc/lra-constraints.c	2012-10-25 09:55:17.376283922 +0100
@@ -756,6 +756,28 @@ extract_address_regs (enum machine_mode
     ad->index_loc = ad->index_reg_loc;
 }
 
+/* Return the scale applied to *AD->INDEX_REG_LOC, or 0 if the index is
+   more complicated than that.  */
+static HOST_WIDE_INT
+get_index_scale (struct address *ad)
+{
+  rtx index = *ad->index_loc;
+  if (GET_CODE (index) == MULT
+      && CONST_INT_P (XEXP (index, 1))
+      && ad->index_reg_loc == &XEXP (index, 0))
+    return INTVAL (XEXP (index, 1));
+
+  if (GET_CODE (index) == ASHIFT
+      && CONST_INT_P (XEXP (index, 1))
+      && ad->index_reg_loc == &XEXP (index, 0))
+    return (HOST_WIDE_INT) 1 << INTVAL (XEXP (index, 1));
+
+  if (ad->index_reg_loc == ad->index_loc)
+    return 1;
+
+  return 0;
+}
+
 
 
 /* The page contains major code to choose the current insn alternative
@@ -2430,6 +2452,40 @@ base_plus_disp_to_reg (enum machine_mode
   return new_reg;
 }
 
+/* Return true if we can add a displacement to address ADDR_LOC,
+   which is described by AD, even if that makes the address invalid.
+   The fix-up code requires any new address to be the sum of the base,
+   index and displacement fields of an AD-like structure.  */
+static bool
+can_add_disp_p (struct address *ad, rtx *addr_loc)
+{
+  /* Automodified addresses have a fixed form.  */
+  if (ad->base_modify_p)
+    return false;
+
+  /* If the address already has a displacement, we can simply add the
+     new displacement to it.  */
+  if (ad->disp_loc)
+    return true;
+
+  /* If the address is entirely a base or index, we can try adding
+     a constant to it.  */
+  if (addr_loc == ad->base_reg_loc || addr_loc == ad->index_loc)
+    return true;
+
+  /* Likewise if the address is entirely a sum of the base and index.  */
+  if (GET_CODE (*addr_loc) == PLUS)
+    {
+      rtx *op0 = &XEXP (*addr_loc, 0);
+      rtx *op1 = &XEXP (*addr_loc, 1);
+      if (op0 == ad->base_reg_loc && op1 == ad->index_loc)
+	return true;
+      if (op1 == ad->base_reg_loc && op0 == ad->index_loc)
+	return true;
+    }
+  return false;
+}
+
 /* Make substitution in address AD in space AS with location ADDR_LOC.
    Update AD and ADDR_LOC if it is necessary.  Return true if a
    substitution was made.  */
@@ -2475,7 +2531,8 @@ equiv_address_substitution (struct addre
 	}
       else if (GET_CODE (new_base_reg) == PLUS
 	       && REG_P (XEXP (new_base_reg, 0))
-	       && CONST_INT_P (XEXP (new_base_reg, 1)))
+	       && CONST_INT_P (XEXP (new_base_reg, 1))
+	       && can_add_disp_p (ad, addr_loc))
 	{
 	  disp += INTVAL (XEXP (new_base_reg, 1));
 	  *ad->base_reg_loc = XEXP (new_base_reg, 0);
@@ -2484,12 +2541,6 @@ equiv_address_substitution (struct addre
       if (ad->base_reg_loc2 != NULL)
 	*ad->base_reg_loc2 = *ad->base_reg_loc;
     }
-  scale = 1;
-  if (ad->index_loc != NULL && GET_CODE (*ad->index_loc) == MULT)
-    {
-      lra_assert (CONST_INT_P (XEXP (*ad->index_loc, 1)));
-      scale = INTVAL (XEXP (*ad->index_loc, 1));
-    }
   if (index_reg != new_index_reg)
     {
       if (REG_P (new_index_reg))
@@ -2499,7 +2550,9 @@ equiv_address_substitution (struct addre
 	}
       else if (GET_CODE (new_index_reg) == PLUS
 	       && REG_P (XEXP (new_index_reg, 0))
-	       && CONST_INT_P (XEXP (new_index_reg, 1)))
+	       && CONST_INT_P (XEXP (new_index_reg, 1))
+	       && can_add_disp_p (ad, addr_loc)
+	       && (scale = get_index_scale (ad)))
 	{
 	  disp += INTVAL (XEXP (new_index_reg, 1)) * scale;
 	  *ad->index_reg_loc = XEXP (new_index_reg, 0);