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

[Jeffrey Law]

On Tue, 2007-02-27 at 16:50 +0000, Richard Sandiford wrote:
> Jeffrey Law <law@redhat.com> writes:
> > On Thu, 2007-02-22 at 19:08 -0800, Mark Mitchell wrote:
> >> I'm not quite sure if I know exactly whose court the ball is in at this
> >> point.  If I understand correctly, Jeff is still unhappy with the patch,
> >> and Richard still thinks it's the right thing.
> > It's in my court.  Current todo is to look at what Bernd did and why
> > and ponder how it plays with what Richard wants to do.
> >
> > I'd also like to see (and this is a new request) some code which
> > triggers these problems.  I have a couple theories that I'd like to
> > poke at, but without triggering code that's going to be tough to do.
> 
> OK.  I can't remember off-hand which testcases failed without this,
> so here's a testcase specifically for it:
> 
> --------------------------------------------------------------------
> void foo (float *x);
> float bar (void)
> {
>   float x[0x5000];
>   foo (x);
>   return x[0x4000] + x[0];
> }
> --------------------------------------------------------------------
> 
> Compile with -O2 -fomit-frame-pointer -mcpu=5485.
So after poking at this for a few minutes with the debugger, I'm really
interested in why you think fixing double_reg_address_ok to be indexed
by the mode of the memory reference is insufficient to fix this problem.


Basically we have this after lreg:

(insn 24 19 30 2 (set (reg/i:SF 0 %d0 [ <result> ])
        (plus:SF (mem/s:SF (plus:SI (reg/f:SI 14 %a6)
                    (const_int -16384 [0xffffc000])) [3 x+65536 S4 A16])
            (mem/s:SF (reg/f:SI 31) [3 x+0 S4 A16]))) 157 {addsf3_cf}
(nil)
    (expr_list:REG_DEAD (reg/f:SI 31)
        (nil)))

Which is a valid insn.  Register elimination changes the memory 
reference to something like:

(mem:SF (plus:SI (%sp) (const_int 65540))

Which is invalid because the offset is too big.

find_reloads_address comes along and realizes the address is 
invalid and reloads the invalid displacement into an index register
creating the (invalid) reg+reg addressing mode via this code:


      if (double_reg_address_ok)
        {
          /* Unshare the sum as well.  */
          *loc = ad = copy_rtx (ad);

          /* Reload the displacement into an index reg.
             We assume the frame pointer or arg pointer is a base reg.
*/
          find_reloads_address_part (XEXP (ad, 1), &XEXP (ad, 1),
                                     INDEX_REG_CLASS, GET_MODE (ad),
opnum,
                                     type, ind_levels);
          return 0;
        }


The problem, ISTM, is that reload assumes that if reg+reg addressing
is valid for QImode (per the double_reg_address_ok computation) then
reg+reg addressing must be valid for all modes which is clearly 
incorrect for coldfire (as well as the PA).    If we twiddle reload
to made double_reg_address_ok be mode dependent, then instead of
reloading the displacement into a register and creating a bogus 
addressing mode, we'll instead reload the entire sum into a register
and use register indirect which is clearly valid.  Note you'll still
get full use of indexed addressing in cases where your G_I_L_A 
allows it.

I haven't really tested the attached patch except to verify that 
the provided testcase no longer aborts and that the reloads we
generate for the problem insn look correct.  However, I feel its a
much more correct solution than what you're trying to do.

Thoughts/Comments?










-------------- next part --------------
A non-text attachment was scrubbed...
Name: DIFFS
Type: text/x-patch
Size: 4494 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20070227/3e80f119/attachment.bin>