Patchwork [ARM] Refine scaled address expression on ARM

login
register
mail settings
Submitter Bin Cheng
Date Aug. 28, 2013, 7 a.m.
Message ID <002a01cea3bc$43d36670$cb7a3350$@arm.com>
Download mbox | patch
Permalink /patch/270375/
State New
Headers show

Comments

Bin Cheng - Aug. 28, 2013, 7 a.m.
Hi,

This patch refines scaled address expression on ARM.  It supports
"base+index*scale" in arm_legitimate_address_outer_p.  It also tries to
legitimize "base + index * scale + offset" with "reg <- base + offset;  reg
+ index * scale" by introducing thumb2_legitimize_address.  For now function
thumb2_legitimize_address is a kind of placeholder and just does the
mentioned transformation by calling to try_multiplier_address.  Hoping we
can improve it in the future.

With this patch:
1) "base+index*scale" is recognized.
2) PR57540 is fixed.

Bootstrapped and Tested on A15.  Is it OK?

Thanks.
Bin

2013-08-28  Bin Cheng  <bin.cheng@arm.com>

	* config/arm/arm.c (arm_legitimate_address_outer_p):
	Support addressing mode like "base + index * scale".
	(try_multiplier_address): New function.
	(arm_legitimize_address): Call try_multiplier_address.
Richard Earnshaw - Aug. 29, 2013, 1:06 p.m.
On 28/08/13 08:00, bin.cheng wrote:
> Hi,
> 
> This patch refines scaled address expression on ARM.  It supports
> "base+index*scale" in arm_legitimate_address_outer_p.  It also tries to
> legitimize "base + index * scale + offset" with "reg <- base + offset;  reg
> + index * scale" by introducing thumb2_legitimize_address.  For now function
> thumb2_legitimize_address is a kind of placeholder and just does the
> mentioned transformation by calling to try_multiplier_address.  Hoping we
> can improve it in the future.
> 
> With this patch:
> 1) "base+index*scale" is recognized.

That's because (PLUS (REG) (MULT (REG) (CONST))) is not canonical form.
 So this shouldn't be necessary.  Can you identify where this
non-canoncial form is being generated?

R.

> 2) PR57540 is fixed.
> 
> Bootstrapped and Tested on A15.  Is it OK?
> 
> Thanks.
> Bin
> 
> 2013-08-28  Bin Cheng  <bin.cheng@arm.com>
> 
> 	* config/arm/arm.c (arm_legitimate_address_outer_p):
> 	Support addressing mode like "base + index * scale".
> 	(try_multiplier_address): New function.
> 	(arm_legitimize_address): Call try_multiplier_address.
> 
> 
> 6-arm-scaled_address-20130828.txt
> 
> 
> Index: gcc/config/arm/arm.c
> ===================================================================
> --- gcc/config/arm/arm.c	(revision 200774)
> +++ gcc/config/arm/arm.c	(working copy)
> @@ -5931,7 +5931,9 @@ arm_legitimate_address_outer_p (enum machine_mode
>  		    && arm_legitimate_index_p (mode, xop1, outer, strict_p))
>  		   || (!strict_p && will_be_in_index_register (xop1))))
>  	      || (arm_address_register_rtx_p (xop1, strict_p)
> -		  && arm_legitimate_index_p (mode, xop0, outer, strict_p)));
> +		  && arm_legitimate_index_p (mode, xop0, outer, strict_p))
> +	      || (arm_address_register_rtx_p (xop0, strict_p)
> +		  && arm_legitimate_index_p (mode, xop1, outer, strict_p)));
>      }
>  
>  #if 0
> @@ -6652,6 +6654,106 @@ legitimize_tls_address (rtx x, rtx reg)
>      }
>  }
>  
> +/* Try to find address expression like base + index * scale + offset
> +   in X.  If we find one, force base + offset into register and
> +   construct new expression reg + index * scale; return the new
> +   address expression if it's valid.  Otherwise return X.  */
> +static rtx
> +try_multiplier_address (rtx x, enum machine_mode mode ATTRIBUTE_UNUSED)
> +{
> +  rtx tmp, base_reg, new_rtx;
> +  rtx base = NULL_RTX, index = NULL_RTX, scale = NULL_RTX, offset = NULL_RTX;
> +
> +  gcc_assert (GET_CODE (x) == PLUS);
> +
> +  /* Try to find and record base/index/scale/offset in X. */
> +  if (GET_CODE (XEXP (x, 1)) == MULT)
> +    {
> +      tmp = XEXP (x, 0);
> +      index = XEXP (XEXP (x, 1), 0);
> +      scale = XEXP (XEXP (x, 1), 1);
> +      if (GET_CODE (tmp) != PLUS)
> +	return x;
> +
> +      base = XEXP (tmp, 0);
> +      offset = XEXP (tmp, 1);
> +    }
> +  else
> +    {
> +      tmp = XEXP (x, 0);
> +      offset = XEXP (x, 1);
> +      if (GET_CODE (tmp) != PLUS)
> +	return x;
> +
> +      base = XEXP (tmp, 0);
> +      scale = XEXP (tmp, 1);
> +      if (GET_CODE (base) == MULT)
> +	{
> +	  tmp = base;
> +	  base = scale;
> +	  scale = tmp;
> +	}
> +      if (GET_CODE (scale) != MULT)
> +	return x;
> +
> +      index = XEXP (scale, 0);
> +      scale = XEXP (scale, 1);
> +    }
> +
> +  if (CONST_INT_P (base))
> +    {
> +      tmp = base;
> +      base = offset;
> +      offset = tmp;
> +    }
> +
> +  if (CONST_INT_P (index))
> +    {
> +      tmp = index;
> +      index = scale;
> +      scale = tmp;
> +    }
> +
> +  /* ARM only supports constant scale in address.  */
> +  if (!CONST_INT_P (scale))
> +    return x;
> +
> +  if (GET_MODE (base) != SImode || GET_MODE (index) != SImode)
> +    return x;
> +
> +  /* Only register/constant are allowed in each part.  */
> +  if (!symbol_mentioned_p (base)
> +      && !symbol_mentioned_p (offset)
> +      && !symbol_mentioned_p (index)
> +      && !symbol_mentioned_p (scale))
> +    {
> +      /* Force "base+offset" into register and construct
> +	 "register+index*scale".  Return the new expression
> +	 only if it's valid.  */
> +      tmp = gen_rtx_PLUS (SImode, base, offset);
> +      base_reg = force_reg (SImode, tmp);
> +      tmp = gen_rtx_fmt_ee (MULT, SImode, index, scale);
> +      new_rtx = gen_rtx_PLUS (SImode, base_reg, tmp);
> +      return new_rtx;
> +    }
> +
> +  return x;
> +}
> +
> +/* Try machine-dependent ways of modifying an illegitimate Thumb2 address
> +   to be legitimate.  If we find one, return the new address.
> +
> +   TODO: legitimize_address for Thumb2.  */
> +static rtx
> +thumb2_legitimize_address (rtx x, rtx orig_x ATTRIBUTE_UNUSED,
> +			   enum machine_mode mode)
> +{
> +  if (GET_CODE (x) == PLUS)
> +    return try_multiplier_address (x, mode);
> +
> +  return x;
> +}
> +
>  /* Try machine-dependent ways of modifying an illegitimate address
>     to be legitimate.  If we find one, return the new, valid address.  */
>  rtx
> @@ -6659,9 +6761,9 @@ arm_legitimize_address (rtx x, rtx orig_x, enum ma
>  {
>    if (!TARGET_ARM)
>      {
> -      /* TODO: legitimize_address for Thumb2.  */
>        if (TARGET_THUMB2)
> -        return x;
> +	return thumb2_legitimize_address (x, orig_x, mode);
> +
>        return thumb_legitimize_address (x, orig_x, mode);
>      }
>  
> @@ -6673,6 +6775,10 @@ arm_legitimize_address (rtx x, rtx orig_x, enum ma
>        rtx xop0 = XEXP (x, 0);
>        rtx xop1 = XEXP (x, 1);
>  
> +      rtx new_rtx = try_multiplier_address (x, mode);
> +      if (new_rtx != x)
> +	return new_rtx;
> +
>        if (CONSTANT_P (xop0) && !symbol_mentioned_p (xop0))
>  	xop0 = force_reg (SImode, xop0);
>  
>
Bin Cheng - Sept. 2, 2013, 7:08 a.m.
> -----Original Message-----
> From: Richard Earnshaw 
> Sent: Thursday, August 29, 2013 9:06 PM
> To: Bin Cheng
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH ARM]Refine scaled address expression on ARM
>
> On 28/08/13 08:00, bin.cheng wrote:
> > Hi,
> > 
> > This patch refines scaled address expression on ARM.  It supports 
> > "base+index*scale" in arm_legitimate_address_outer_p.  It also tries 
> > to legitimize "base + index * scale + offset" with "reg <- base + 
> > offset;  reg
> > + index * scale" by introducing thumb2_legitimize_address.  For now 
> > + function
> > thumb2_legitimize_address is a kind of placeholder and just does the 
> > mentioned transformation by calling to try_multiplier_address.  Hoping 
> > we can improve it in the future.
> > 
> > With this patch:
> > 1) "base+index*scale" is recognized.
>
> That's because (PLUS (REG) (MULT (REG) (CONST))) is not canonical form.
>  So this shouldn't be necessary.  Can you identify where this
non-canoncial form is being generated?
>

Oh, for now ivopt constructs "index*scale" to test whether backend supports
scaled addressing mode, which is not valid on ARM, so I was going to
construct "base + index*scale" instead.  Since "base + index * scale" is not
canonical form, I will construct the canonical form and drop this part of
the patch.

Is rest of this patch OK?

Thanks.
bin

Patch

Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	(revision 200774)
+++ gcc/config/arm/arm.c	(working copy)
@@ -5931,7 +5931,9 @@  arm_legitimate_address_outer_p (enum machine_mode
 		    && arm_legitimate_index_p (mode, xop1, outer, strict_p))
 		   || (!strict_p && will_be_in_index_register (xop1))))
 	      || (arm_address_register_rtx_p (xop1, strict_p)
-		  && arm_legitimate_index_p (mode, xop0, outer, strict_p)));
+		  && arm_legitimate_index_p (mode, xop0, outer, strict_p))
+	      || (arm_address_register_rtx_p (xop0, strict_p)
+		  && arm_legitimate_index_p (mode, xop1, outer, strict_p)));
     }
 
 #if 0
@@ -6652,6 +6654,106 @@  legitimize_tls_address (rtx x, rtx reg)
     }
 }
 
+/* Try to find address expression like base + index * scale + offset
+   in X.  If we find one, force base + offset into register and
+   construct new expression reg + index * scale; return the new
+   address expression if it's valid.  Otherwise return X.  */
+static rtx
+try_multiplier_address (rtx x, enum machine_mode mode ATTRIBUTE_UNUSED)
+{
+  rtx tmp, base_reg, new_rtx;
+  rtx base = NULL_RTX, index = NULL_RTX, scale = NULL_RTX, offset = NULL_RTX;
+
+  gcc_assert (GET_CODE (x) == PLUS);
+
+  /* Try to find and record base/index/scale/offset in X. */
+  if (GET_CODE (XEXP (x, 1)) == MULT)
+    {
+      tmp = XEXP (x, 0);
+      index = XEXP (XEXP (x, 1), 0);
+      scale = XEXP (XEXP (x, 1), 1);
+      if (GET_CODE (tmp) != PLUS)
+	return x;
+
+      base = XEXP (tmp, 0);
+      offset = XEXP (tmp, 1);
+    }
+  else
+    {
+      tmp = XEXP (x, 0);
+      offset = XEXP (x, 1);
+      if (GET_CODE (tmp) != PLUS)
+	return x;
+
+      base = XEXP (tmp, 0);
+      scale = XEXP (tmp, 1);
+      if (GET_CODE (base) == MULT)
+	{
+	  tmp = base;
+	  base = scale;
+	  scale = tmp;
+	}
+      if (GET_CODE (scale) != MULT)
+	return x;
+
+      index = XEXP (scale, 0);
+      scale = XEXP (scale, 1);
+    }
+
+  if (CONST_INT_P (base))
+    {
+      tmp = base;
+      base = offset;
+      offset = tmp;
+    }
+
+  if (CONST_INT_P (index))
+    {
+      tmp = index;
+      index = scale;
+      scale = tmp;
+    }
+
+  /* ARM only supports constant scale in address.  */
+  if (!CONST_INT_P (scale))
+    return x;
+
+  if (GET_MODE (base) != SImode || GET_MODE (index) != SImode)
+    return x;
+
+  /* Only register/constant are allowed in each part.  */
+  if (!symbol_mentioned_p (base)
+      && !symbol_mentioned_p (offset)
+      && !symbol_mentioned_p (index)
+      && !symbol_mentioned_p (scale))
+    {
+      /* Force "base+offset" into register and construct
+	 "register+index*scale".  Return the new expression
+	 only if it's valid.  */
+      tmp = gen_rtx_PLUS (SImode, base, offset);
+      base_reg = force_reg (SImode, tmp);
+      tmp = gen_rtx_fmt_ee (MULT, SImode, index, scale);
+      new_rtx = gen_rtx_PLUS (SImode, base_reg, tmp);
+      return new_rtx;
+    }
+
+  return x;
+}
+
+/* Try machine-dependent ways of modifying an illegitimate Thumb2 address
+   to be legitimate.  If we find one, return the new address.
+
+   TODO: legitimize_address for Thumb2.  */
+static rtx
+thumb2_legitimize_address (rtx x, rtx orig_x ATTRIBUTE_UNUSED,
+			   enum machine_mode mode)
+{
+  if (GET_CODE (x) == PLUS)
+    return try_multiplier_address (x, mode);
+
+  return x;
+}
+
 /* Try machine-dependent ways of modifying an illegitimate address
    to be legitimate.  If we find one, return the new, valid address.  */
 rtx
@@ -6659,9 +6761,9 @@  arm_legitimize_address (rtx x, rtx orig_x, enum ma
 {
   if (!TARGET_ARM)
     {
-      /* TODO: legitimize_address for Thumb2.  */
       if (TARGET_THUMB2)
-        return x;
+	return thumb2_legitimize_address (x, orig_x, mode);
+
       return thumb_legitimize_address (x, orig_x, mode);
     }
 
@@ -6673,6 +6775,10 @@  arm_legitimize_address (rtx x, rtx orig_x, enum ma
       rtx xop0 = XEXP (x, 0);
       rtx xop1 = XEXP (x, 1);
 
+      rtx new_rtx = try_multiplier_address (x, mode);
+      if (new_rtx != x)
+	return new_rtx;
+
       if (CONSTANT_P (xop0) && !symbol_mentioned_p (xop0))
 	xop0 = force_reg (SImode, xop0);