diff mbox

[ARM,1/2] Make ARM_LEGITIMIZE_RELOAD_ADDRESS be a function

Message ID 4D119D62.3010407@codesourcery.com
State New
Headers show

Commit Message

Jie Zhang Dec. 22, 2010, 6:40 a.m. UTC 
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,

Comments

Jie Zhang Jan. 17, 2011, 2:10 a.m. UTC | #1 
PING

On 12/22/2010 02:40 PM, 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,
Ramana Radhakrishnan Jan. 27, 2011, 1:39 a.m. UTC | #2 
Hi Jie,

On Wed, Dec 22, 2010 at 6:40 AM, Jie Zhang <jie@codesourcery.com>
wrote:Testing is
> going. OK if the result is good?

I can't approve or reject your patch though personally I think this is
a good change. I've eyeballed it once and it looks good. Hopefully
your testing showed no regressions :) ?


Ramana
Jie Zhang Jan. 27, 2011, 2:08 a.m. UTC | #3 
On 01/27/2011 09:39 AM, Ramana Radhakrishnan wrote:
> Hi Jie,
>
> On Wed, Dec 22, 2010 at 6:40 AM, Jie Zhang<jie@codesourcery.com>
> wrote:Testing is
>> going. OK if the result is good?
>
> I can't approve or reject your patch though personally I think this is
> a good change. I've eyeballed it once and it looks good. Hopefully
> your testing showed no regressions :) ?
>
Thanks for your review. I confirm that my testing shows no regressions.

Regards,
Richard Earnshaw Jan. 28, 2011, 2:26 p.m. UTC | #4 
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.

R.
diff mbox

Patch


	* 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.

diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 53923bd..f037a45 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -54,6 +54,8 @@  extern rtx legitimize_pic_address (rtx, enum machine_mode, 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);
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 2aaec8c..95bde4a 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -6388,6 +6388,63 @@  thumb_legitimize_address (rtx x, rtx orig_x, enum machine_mode mode)
   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
+      && REGNO (XEXP (*p, 0)) < FIRST_PSEUDO_REGISTER
+      && REG_MODE_OK_FOR_BASE_P (XEXP (*p, 0), mode)
+      && 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,
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 9951553..6abc832 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -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)