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

[Richard Sandiford <richard at codesourcery dot com>]

Jeffrey Law <law@redhat.com> writes:
> On Wed, 2007-01-10 at 21:27 +0000, Richard Sandiford wrote:
>> Right, that describes the ISA restrictions.  But this patch isn't
>> really about the ISA restrictions per se.  The m68k port makes a pragmatic
>> decision to forbid indexed addressing modes for _all_ SFmode and DFmode
>> MEMs if TARGET_COLDFIRE_FPU, on the basis that the vast majority of
>> memory accesses will be in FPU instructions.  We then expose the
>> necessary addressing code to the pre-reload optimisers and register
>> allocators, rather than leaving reload to find scratch address registers
>> from somewhere.  Although I didn't make that decision (it's what current
>> mainline does), I can well imagine that it leads to better code.
>> 
>> In other words, this is entirely an internal gcc thing.  The current
>> definition of INDEX_REG_CLASS tricks reload into thinking that indexed
>> addressing modes are acceptable SFmode and DFmode memory_operands,
>> which they aren't.  It will then use such memories in copy-in and
>> copy-out reload instructions.
> The fundamental problem is that the m68k port is lying and you're
> hacking up reload to deal with the m68k version of the lie.

I disagree that this patch is a hack.  I think the only real liar here
is INDEX_REG_CLASS, which says that something is a valid index register
even when GO_IF_LEGITIMATE_ADDRESS forbids it.  I think the situation
after the patch is self-consistent and the correct approach to take
(in terms of what addresses are and aren't acceptable).

> FWIW, the PA port lies too, but all the hair of the lie is buried in
> the backend.  You might review how the PA handles this situation since
> they are quite similar (basically reversed -- the PA has full indexed
> addressing modes for loads/stores using FP registers, but not for
> integer registers.

To recap, the original problem was that reload creates copy-in reloads
from (or copy-out reloads to) invalid indexed addresses.  This can
happen when a spill slot MEM has an out-of-range offset, for example.

If I've understood correctly, the PA port only avoids this problem
for SImode MEMs because it defines LEGITIMIZE_RELOAD_ADDRESS.
L_R_A deals with out-of-range offsets by adding the high part of the
offset to the base and then adding the left-over offset.  I'm sure that
this definition was added purely for the sake of optimisation, but it has
the side-effect of stopping reload from loading out-of-range offsets into
index registers.  Would everything still work without L_R_A?  If not,
that seems a little suspect.  I'd always been told that L_R_A should
only be used for optimisation, and that it was wrong to rely on it for
correctness.

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.

(a) is what PA does and (b) is what m68k does.  I think (b) is the
right choice for the reason outlined in the quote above: if we allow
indexed MEMs before reload, it's up to reload to convert indexed MEMs
into legitimate addresses.  To do that it needs some scratch register,
which it will pick in an ad-hoc way.  It's better to expose the scratch
register before reload if possible, and that's what (b) does.
Note that the effect is likely to be more obvious on m68k than PA
because the m68k port sometimes has only 5 allocatable address registers
(after you've taken away the stack pointer, frame pointer and
PIC register).

Note that the m68k port already deals with SImode indexed MEMs in the
same way as the PA.  It accepts indexed addresses and use constraints
to restrict the choice.

Richard