Re: [RTL, ColdFire 24/63] Add support for a MODE_INDEX_REG_CLASS macro

[Jeffrey Law <law at redhat dot com>]

On Wed, 2007-02-14 at 09:37 +0000, Richard Sandiford wrote:
> Jeffrey Law <law@redhat.com> writes:
> > On Wed, 2007-01-17 at 05:36 +0000, Richard Sandiford wrote:
> >> The wider issue seems to be: should we either
> >> 
> >>   (a) allow indexed addresses for all modes and use constraints and
> >>       secondary reloads to handle the cases where they aren't allowed or
> >> 
> >>   (b) prevent the use of index registers for modes if the vast majority
> >>       of MEMs in those modes are likely to not be index registers.
> > Fundamentally the PA and m68k really need to be rejecting indexed
> > address modes since those modes are not valid addressing modes
> > according to a strict definition of G_I_L_A.  The two ports lie
> > about the legitimitacy of such address modes and if they continue
> > to do so, then the details of dealing with the lie really belong
> > in the backend (until such time as someone fixes reload and 
> > G_I_L_A to be general enough to handle the kinds of cases that
> > come up on the m68k and PA).
> 
> I don't agree.  You seem to be saying that G_I_L_A has to be independent
> of the addressed machine_mode, so that if we forbid index registers for
> SFmode, we must do it for all modes.  Or, conversely, if we allow
> disp(base,index) addresses for QImode, we must do so for SFmode too.
Somewhat.  It's legitimate to select based on mode, but you must not
confuse GCC's concept of a mode with the register file used on the
actual hardware.

You also have to be careful because of how double_reg_address_ok
is computed.  double_reg_address_ok blindly uses QImode and if
we ever get a valid address in QImode, then reload assumes that it
can use indexing for any mode.

An alternate (and possibly better) approach to fixing this mess
would be to make double_reg_address_ok an array of booleans 
indexed by mode, then reload could check for validity of 
double_reg_address_ok for the desired mode rather than assuming
that if it's valid for QImode, then it must be valid for other 
modes.

> 
> As I've said before, I think ColdFire's G_I_L_A should continue
> rejecting indexed addressing modes for SFmode _not_ for correctness,
> but for _optimisation_.  We want to optimise SFmode code based
> on the assumption that we will be using the FPU to implement it.
> We therefore want to expose the lack of indexed addresses early on,
> so that the pre-reload optimisers and register allocators can see the
> separate address calculations.  Sure, we lose the ability to copy
> SFmode values around using GPRs and indexed addresses, but the
> trade-off is IMO a good one.
I've got absolutely no problems with rejecting addressing modes
in G_I_L_A.


> So, _from an optimisation perspective_, I think it is reasonable to
> accept indexed addresses for SImode and not for SFmode.  I'm still
> not sure whether you disagree with that.
I disagree with that if the port allows integer values in FP regsters
and the hardware can't do FP indexed loads/stores because G_I_L_A
could be passed SImode but generate an FP load/store.


> 
> Now G_I_L_A has a mode argument _precisely_ so that the legitimacy of
> an address can depend on the mode.  However, G_I_L_A is not the only
> backend interface for describing addressing modes; we also have the
> BASE_REG_CLASS and INDEX_REG_CLASS macros.  These latter macros are
> all that reload looks at when legitimizing addresses, so they must
> of course agree with G_I_L_A.
I disagree on this point and as I've stated before I think you're
using this to avoid fixing the backend to deal with a broken G_I_L_A.

> 
> The problem we have at the moment is that, while G_I_L_A and
> BASE_REG_CLASS are parameterised on the mode (and other things),
> INDEX_REG_CLASS isn't.  So at the moment, it isn't possible for
> the *_REG_CLASS macros to fully agree with G_I_L_A.  My patch
> is fixing that, and nothing else.
When did BASE_REG_CLASS get parameterized on the mode?  That seems
awful wrong as well.  I still don't see any valid need for
{BASE,INDEX}_REG_CLASS to be parameterized on the mode of the
memory reference.

Can you point me to the discusion for the BASE_REG_CLASS change?
[ God I hope I didn't approve that patch :-) ]


Jeff