Re: [ARM] [1/2] Make ARM_LEGITIMIZE_RELOAD_ADDRESS be a function

[Jie Zhang <jie at codesourcery dot com>]

On 01/28/2011 10:26 PM, Richard Earnshaw wrote:
> On Wed, 2010-12-22 at 14:40 +0800, Jie Zhang wrote:
> > Hi,
> >
> > This patch should have no functionality changes. It just moves most of
> > ARM_LEGITIMIZE_RELOAD_ADDRESS from arm.h to
> > arm_legitimize_reload_address in arm.c. This is needed by the next
> > patch. It also eases debugging. Testing is going. OK if the result is good?
> >
> > Regards,
>
> So I think there's a subtle gotcha in this change that's easy to miss.
>
> +&&  REG_MODE_OK_FOR_BASE_P (XEXP (*p, 0), mode)
>
> is a macro that expands into
>
> #define REG_MODE_OK_FOR_BASE_P(X, MODE)         \
>    (TARGET_THUMB1                                \
>     ? THUMB1_REG_MODE_OK_FOR_BASE_P (X, MODE)    \
>     : ARM_REG_OK_FOR_BASE_P (X))
>
>
> But THUMB1_REG_MODE_OK_FOR_BASE_P and ARM_REG_OK_FOR_BASE_P both have
> two possible definitions, that are picked depending upon the file in
> which the macro is used (a nasty consequence of some attempt to save
> duplication of code a long long time ago in the early days of GCC).
>
> Now IIRC reload.c (which uses legitimize_reload_address) defines
> REG_OK_STRICT which leads to one definition of these macros, but arm.c
> does not (and nor should it), which leads to the other definition.
>
> Overall, I think that means that your patch has quietly changed the
> results that this macro can give.
Good catch! Thanks for review! How about expanding 
REG_MODE_OK_FOR_BASE_P a bit since we know when REG_MODE_OK_FOR_BASE_P 
is used in arm_legitimize_reload_address, REG_OK_STRICT is defined and
TARGET_THUMB1 is false?  In the attached patch, I replace

+      && REGNO (XEXP (*p, 0)) < FIRST_PSEUDO_REGISTER
+      && REG_MODE_OK_FOR_BASE_P (XEXP (*p, 0), mode)

with

+      && ARM_REGNO_OK_FOR_BASE_P (REGNO (XEXP (*p, 0)))


GCC build is OK. But I have not regression tested it yet.


--
Jie Zhang

	* config/arm/arm.c (arm_legitimize_reload_address): New.
	* config/arm/arm.h (ARM_LEGITIMIZE_RELOAD_ADDRESS): Use
	arm_legitimize_reload_address.
	* config/arm/arm-protos.h (arm_legitimize_reload_address):
	Declare.

Index: config/arm/arm.c
===================================================================
--- config/arm/arm.c	(revision 169362)
+++ config/arm/arm.c	(working copy)
@@ -6392,6 +6392,62 @@ thumb_legitimize_address (rtx x, rtx ori
   return x;
 }
 
+bool
+arm_legitimize_reload_address (rtx *p,
+			       enum machine_mode mode,
+			       int opnum, int type,
+			       int ind_levels ATTRIBUTE_UNUSED)
+{
+  if (GET_CODE (*p) == PLUS
+      && GET_CODE (XEXP (*p, 0)) == REG
+      && ARM_REGNO_OK_FOR_BASE_P (REGNO (XEXP (*p, 0)))
+      && GET_CODE (XEXP (*p, 1)) == CONST_INT)
+    {
+      HOST_WIDE_INT val = INTVAL (XEXP (*p, 1));
+      HOST_WIDE_INT low, high;
+
+      if (mode == DImode || (mode == DFmode && TARGET_SOFT_FLOAT))
+	low = ((val & 0xf) ^ 0x8) - 0x8;
+      else if (TARGET_MAVERICK && TARGET_HARD_FLOAT)
+	/* Need to be careful, -256 is not a valid offset.  */
+	low = val >= 0 ? (val & 0xff) : -((-val) & 0xff);
+      else if (mode == SImode
+	       || (mode == SFmode && TARGET_SOFT_FLOAT)
+	       || ((mode == HImode || mode == QImode) && ! arm_arch4))
+	/* Need to be careful, -4096 is not a valid offset.  */
+	low = val >= 0 ? (val & 0xfff) : -((-val) & 0xfff);
+      else if ((mode == HImode || mode == QImode) && arm_arch4)
+	/* Need to be careful, -256 is not a valid offset.  */
+	low = val >= 0 ? (val & 0xff) : -((-val) & 0xff);
+      else if (GET_MODE_CLASS (mode) == MODE_FLOAT
+	       && TARGET_HARD_FLOAT && TARGET_FPA)
+	/* Need to be careful, -1024 is not a valid offset.  */
+	low = val >= 0 ? (val & 0x3ff) : -((-val) & 0x3ff);
+      else
+	return false;
+
+      high = ((((val - low) & (unsigned HOST_WIDE_INT) 0xffffffff)
+	       ^ (unsigned HOST_WIDE_INT) 0x80000000)
+	      - (unsigned HOST_WIDE_INT) 0x80000000);
+      /* Check for overflow or zero */
+      if (low == 0 || high == 0 || (high + low != val))
+	return false;
+
+      /* Reload the high part into a base reg; leave the low part
+	 in the mem.  */
+      *p = gen_rtx_PLUS (GET_MODE (*p),
+			 gen_rtx_PLUS (GET_MODE (*p), XEXP (*p, 0),
+				       GEN_INT (high)),
+			 GEN_INT (low));
+      push_reload (XEXP (*p, 0), NULL_RTX, &XEXP (*p, 0), NULL,
+		   MODE_BASE_REG_CLASS (mode), GET_MODE (*p),
+		   VOIDmode, 0, 0, opnum, (enum reload_type) type);
+      return true;
+    }
+
+  return false;
+}
+
 rtx
 thumb_legitimize_reload_address (rtx *x_p,
 				 enum machine_mode mode,
Index: config/arm/arm.h
===================================================================
--- config/arm/arm.h	(revision 169362)
+++ config/arm/arm.h	(working copy)
@@ -1273,53 +1273,8 @@ enum reg_class
 #define ARM_LEGITIMIZE_RELOAD_ADDRESS(X, MODE, OPNUM, TYPE, IND, WIN)	   \
   do									   \
     {									   \
-      if (GET_CODE (X) == PLUS						   \
-	  && GET_CODE (XEXP (X, 0)) == REG				   \
-	  && REGNO (XEXP (X, 0)) < FIRST_PSEUDO_REGISTER		   \
-	  && REG_MODE_OK_FOR_BASE_P (XEXP (X, 0), MODE)			   \
-	  && GET_CODE (XEXP (X, 1)) == CONST_INT)			   \
-	{								   \
-	  HOST_WIDE_INT val = INTVAL (XEXP (X, 1));			   \
-	  HOST_WIDE_INT low, high;					   \
-									   \
-	  if (MODE == DImode || (MODE == DFmode && TARGET_SOFT_FLOAT))	   \
-	    low = ((val & 0xf) ^ 0x8) - 0x8;				   \
-	  else if (TARGET_MAVERICK && TARGET_HARD_FLOAT)		   \
-	    /* Need to be careful, -256 is not a valid offset.  */	   \
-	    low = val >= 0 ? (val & 0xff) : -((-val) & 0xff);		   \
-	  else if (MODE == SImode					   \
-		   || (MODE == SFmode && TARGET_SOFT_FLOAT)		   \
-		   || ((MODE == HImode || MODE == QImode) && ! arm_arch4)) \
-	    /* Need to be careful, -4096 is not a valid offset.  */	   \
-	    low = val >= 0 ? (val & 0xfff) : -((-val) & 0xfff);		   \
-	  else if ((MODE == HImode || MODE == QImode) && arm_arch4)	   \
-	    /* Need to be careful, -256 is not a valid offset.  */	   \
-	    low = val >= 0 ? (val & 0xff) : -((-val) & 0xff);		   \
-	  else if (GET_MODE_CLASS (MODE) == MODE_FLOAT			   \
-		   && TARGET_HARD_FLOAT && TARGET_FPA)			   \
-	    /* Need to be careful, -1024 is not a valid offset.  */	   \
-	    low = val >= 0 ? (val & 0x3ff) : -((-val) & 0x3ff);		   \
-	  else								   \
-	    break;							   \
-									   \
-	  high = ((((val - low) & (unsigned HOST_WIDE_INT) 0xffffffff)	   \
-		   ^ (unsigned HOST_WIDE_INT) 0x80000000)		   \
-		  - (unsigned HOST_WIDE_INT) 0x80000000);		   \
-	  /* Check for overflow or zero */				   \
-	  if (low == 0 || high == 0 || (high + low != val))		   \
-	    break;							   \
-									   \
-	  /* Reload the high part into a base reg; leave the low part	   \
-	     in the mem.  */						   \
-	  X = gen_rtx_PLUS (GET_MODE (X),				   \
-			    gen_rtx_PLUS (GET_MODE (X), XEXP (X, 0),	   \
-					  GEN_INT (high)),		   \
-			    GEN_INT (low));				   \
-	  push_reload (XEXP (X, 0), NULL_RTX, &XEXP (X, 0), NULL,	   \
-		       MODE_BASE_REG_CLASS (MODE), GET_MODE (X), 	   \
-		       VOIDmode, 0, 0, OPNUM, TYPE);			   \
-	  goto WIN;							   \
-	}								   \
+      if (arm_legitimize_reload_address (&X, MODE, OPNUM, TYPE, IND))	   \
+	goto WIN;							   \
     }									   \
   while (0)
 
Index: config/arm/arm-protos.h
===================================================================
--- config/arm/arm-protos.h	(revision 169362)
+++ config/arm/arm-protos.h	(working copy)
@@ -54,6 +54,8 @@ extern rtx legitimize_pic_address (rtx,
 extern rtx legitimize_tls_address (rtx, rtx);
 extern int arm_legitimate_address_outer_p (enum machine_mode, rtx, RTX_CODE, int);
 extern int thumb_legitimate_offset_p (enum machine_mode, HOST_WIDE_INT);
+extern bool arm_legitimize_reload_address (rtx *, enum machine_mode, int, int,
+					   int);
 extern rtx thumb_legitimize_reload_address (rtx *, enum machine_mode, int, int,
 					    int);
 extern int arm_const_double_rtx (rtx);