Again thank you and Denis for your comment.
Here is what I deduce from code and Denis comments - I am sure he (and
others) will correct me if wrong :-)
The main issue is that we have one pointer register that cannot take
offset and two base pointers with limited offset (0-63).
Reload provides no means to pick the right pointer register. To do so
would require a "preffered base pointer" mechanism.
AVR L_R_A sole purpose is substituting POINTER class where there are
oppertunities to do so.
BUT as L_R_A is in middle of find_reloads_address, we have a problem
in that some normal reload solutions have already been applied and some
have not.
This may mean that 1) We never get here - so miss POINTER oppertunity
or 2) Miss even better solutions that are applied latter.
3)Have code vulnerable to reload changes.
I have added comment below to explain the parts.
I'll probably need to add some debug hooks to check what parts still
contribute.
best regards
Andy
#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; \
} \
The intent is to use POINTER class registers for INC/DEC so we have one
more register available than BASE_POINTER
Has INC/DEC been handled already making this part redundant?
Are there any other solutions that this code skips?
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; \
}
Im struggling with this part. however, the POINTER class is an
advantageous in the first reloading the address from memory
- where we would not want to waste BASE POINTER and also create overlap
problem.
The implication of using this code is that without it we cannot catch
the inner reload POINTER oppertunity if we leave reload to decompose
BASE+offset. I think this is correct :-(
\
push_reload (XEXP (X, 0), NULL_RTX, &XEXP (X, 0), NULL, \
BASE_POINTER_REGS, GET_MODE (X), VOIDmode, 0, 0, \
OPNUM, TYPE); \
goto WIN;
This part is duplicating reload and should not be here (I have patch
pending with maintainer that removes this - for different bug)
\
} \
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; \
} \
} \
This case is where offset is out of range - so we allow POINTER class
on the basis that this is no worse than BASE POINTER. However, we leave
frame_pointer to reload.
} while(0)
-----Original Message-----
From: Denis Chertykov <[EMAIL PROTECTED]>
To: Jeff Law <[EMAIL PROTECTED]>
Cc: Andy H <[EMAIL PROTECTED]>; GCC Development <gcc@gcc.gnu.org>;
[EMAIL PROTECTED]
Sent: Thu, 29 May 2008 3:23 am
Subject: Re: Help with reload and naked constant sum causing ICE
2008/5/29 Jeff Law <[EMAIL PROTECTED]>:
Richard Sandiford wrote:
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:
[...]
Not only should there be some comments, those comments should clearly
explain how L_R_A is improving the generated code. That is LRA's
job, to
implement target specific reload strategies which improve the
generated
code. If the AVR port is using L_R_A for *correctness*, then the AVR
port
is broken.
I wrote L_R_A for *correctness*, so in your terms AVR port is broken.
It's because AVR have only 3 pointer registers (X,Y,Z) and only 2 of
them Y and Z are base pointers.
Y is a frame pointer, so only one general base pointer - Z.
Offset also very limited -63 to +63.
In L_R_A I play with POINTER_REGS (X,Y,Z) and BASE_POINTER_REGS (Y,Z).
My experiments with reload show me that reload can't handle all
difficult situations.
Also, Jeff I know that you think that reload can.
GCC havn't something like (define_address ...) and reload can't handle
POINTER_REGS and
BASE_POINTER_REGS differently. In my version of L_R_A I tried to do it.
Denis.
