Re: [PATCH ARM]Refine scaled address expression on ARM

[Richard Earnshaw <rearnsha at arm dot com>]

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);
>  
>