Re: [PATCH]: PR 11271 Handle reloading of R+R+CONST for any reg

[Ulrich Weigand <weigand at i1 dot informatik dot uni-erlangen dot de>]

Eric Botcazou wrote:

> I recently came accross the very same problem with gen_add2_insn in 
> postreload.c on i386.  And your remark is a bit frightening, because it 
> appears that every single use of gen_add2_insn might be unsafe...

Yes, that's what I think as well.

> > IMO the in the PLUS case in find_reloads_address_part the test for
> > LEGITIMATE_CONSTANT_P / PREFERRED_RELOAD_CLASS is completely bogus.
> > Instead, I think that a PLUS to be reloaded should be treated as
> > an address (which it always is, actually!), and be fixed up via
> > find_reloads_address if necessary, possibly like so:
> >
> > static void
> > find_reloads_address_part (rtx x, rtx *loc, enum reg_class class,
> >                            enum machine_mode mode, int opnum,
> >                            enum reload_type type, int ind_levels)
> > {
> >   if (CONSTANT_P (x)
> >       && (! LEGITIMATE_CONSTANT_P (x)
> >
> >           || PREFERRED_RELOAD_CLASS (x, class) == NO_REGS))
> >
> >     {
> >       rtx tem;
> >
> >       tem = x = force_const_mem (mode, x);
> >       find_reloads_address (mode, &tem, XEXP (tem, 0), &XEXP (tem, 0),
> >                             opnum, type, ind_levels, 0);
> >     }
> >
> >   else if (GET_CODE (x) == PLUS)
> >     {
> >       find_reloads_address (VOIDmode, (rtx *)0, x, loc,
> >                             opnum, type, ind_levels, 0);
> >     }
> >
> >   push_reload (x, NULL_RTX, loc, (rtx*) 0, class,
> >                mode, VOIDmode, 0, 0, opnum, type);
> > }
> 
> I think this would cure the SH4 bug too.

This does indeed fix the bug on s390, and survives bootstrap/
regtest on s390-ibm-linux and s390x-ibm-linux.

Unfortunately it breaks ia64, because on that platform apparently 
there are no two-register addressing modes, and thus the above
call to find_reloads_address comes to this piece of code:

          /* If the sum of two regs is not necessarily valid,
             reload the sum into a base reg.
             That will at least work.  */
          find_reloads_address_part (ad, loc, MODE_BASE_REG_CLASS (mode),
                                     Pmode, opnum, type, ind_levels);

which calls right back into find_reloads_address_part and
infinite recursion occurs.

It would appear that at least in this code path, 
find_reloads_address_part is deliberately called with a PLUS
that is *not* a valid address.  It is unclear to me what exactly
is supposed to be allowed as argument of type PLUS in 
find_reloads_address_part then :-(

I'd appreciate any suggestions how to solve this ...


Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de