This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [ARM] [1/2] Make ARM_LEGITIMIZE_RELOAD_ADDRESS be a function
- From: Jie Zhang <jie at codesourcery dot com>
- To: Richard Earnshaw <rearnsha at arm dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Sat, 29 Jan 2011 00:46:52 +0800
- Subject: Re: [ARM] [1/2] Make ARM_LEGITIMIZE_RELOAD_ADDRESS be a function
- References: <4D119D62.3010407@codesourcery.com> <1296224795.9738.39.camel@e102346-lin.cambridge.arm.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);