Richard,
I appreciate the extra input.
I agree with what you say. The target should not be doing middle-end
stuff .
The inc/dec and (Rxx) != (frame pointer) parts just reload using
pointer class
which is a one extra register than base pointers but the extra reg
cannot take offset.
However, I dont know what reload can - or cannot do - (but I'm learning
that every day)
I do not know fully what was intended by all of AVR L_R_A and how
this might now be redundant, useful or wrong.
I already have patch with maintainer that takes out a chunk of this to
fix another (spill) bug.
I have copied Anatoly for comment, and I promise to revisit this again
after reviewing reload capabilities.
Andy
-----Original Message-----
From: Richard Sandiford <[EMAIL PROTECTED]>
To: Andy H <[EMAIL PROTECTED]>
Cc: GCC Development <gcc@gcc.gnu.org>; [EMAIL PROTECTED]
Sent: Wed, 28 May 2008 3:00 pm
Subject: Re: Help with reload and naked constant sum causing ICE
Andy H <[EMAIL PROTECTED]> writes:
If L_R_A does nothing with it,
the normal reload handling will first try:
(const:HI (plus:HI (symbol_ref:HI ("chk_fail_buf") (const_int 2))))
This worked just as your described after I added test of
reg_equiv_constant[] inside L_R_A .
So I guess that looks like the fix for bug I posted.
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34641
To summarize
LEGITIMIZE_RELOAD_ADDRESS should now always check reg_equiv_constant
before it trying to do any push_reload of register.
TBH, I still think AVR is doing far too much in L_R_A. To quote
the current version:
#define LEGITIMIZE_RELOAD_ADDRESS(X, MODE, OPNUM, TYPE, IND_LEVELS,
WIN) \
do { \
if (1&&(GET_CODE (X) == POST_INC || GET_CODE (X) == PRE_DEC)) \
{ \
push_reload (XEXP (X,0), XEXP (X,0), &XEXP (X,0), &XEXP (X,0),
\
POINTER_REGS, GET_MODE (X),GET_MODE (X) , 0, 0, \
OPNUM, RELOAD_OTHER); \
goto WIN; \
} \
Why does AVR need different POST_INC and PRE_DEC handling from
other auto-inc targets? This at least deserves a comment.
But really, you should just let reload handle this case.
Does reload currently lack some support you need?
if (GET_CODE (X) == PLUS \
&& REG_P (XEXP (X, 0)) \
&& GET_CODE (XEXP (X, 1)) == CONST_INT \
&& INTVAL (XEXP (X, 1)) >= 1) \
{ \
int fit = INTVAL (XEXP (X, 1)) <= (64 - GET_MODE_SIZE (MODE));
\
if (fit) \
{ \
if (reg_equiv_address[REGNO (XEXP (X, 0))] != 0) \
{ \
int regno = REGNO (XEXP (X, 0)); \
rtx mem = make_memloc (X, regno); \
push_reload (XEXP (mem,0), NULL, &XEXP (mem,0), NULL,
\
POINTER_REGS, Pmode, VOIDmode, 0, 0, \
1, ADDR_TYPE (TYPE)); \
push_reload (mem, NULL_RTX, &XEXP (X, 0), NULL, \
BASE_POINTER_REGS, GET_MODE (X), VOIDmode, 0, 0, \
OPNUM, TYPE); \
goto WIN; \
} \
push_reload (XEXP (X, 0), NULL_RTX, &XEXP (X, 0), NULL, \
BASE_POINTER_REGS, GET_MODE (X), VOIDmode, 0, 0, \
OPNUM, TYPE); \
goto WIN; \
} \
else if (! (frame_pointer_needed && XEXP (X,0) ==
frame_pointer_rtx)) \
{ \
push_reload (X, NULL_RTX, &X, NULL, \
POINTER_REGS, GET_MODE (X), VOIDmode, 0, 0, \
OPNUM, TYPE); \
goto WIN; \
} \
} \
} while(0)
These too don't make much immediate sense to me. What are they trying
to do that reload wouldn't do otherwise?
Rather than duplicating more of reload's checks in this macro, I think
it would be better to strip it down as far as possible.
There's a bit of self-interest here. It makes it very hard to work on
reload if ports try to duplicate so much of the logic in target-specific
files.
Richard
