Re: RFC: reloading sums

[Joern Rennecke <joern dot rennecke at superh dot com>]

Michael Matz wrote:
> Of course, but I felt this was just papering over the problem instead of
> fixing it.  I thought so, because as far as reload knows this optimization
> _is_ valid, if it weren't for the broken gen_reload() function.  Because
> of this I'm not sure if your patch really fixes the problem.  It disables
> the optimization that r13 is chosen as reload reg early.  But noone says
> that nothing else will also chose r13 later.  It could do so, because of
> the reload type, because r13 is dead after the insn, and no other reload
> is needing r13.

After the reload insn, r13 has the value that the reload says it has, and
the lifetime of reloads is already tracked.  r13 is also mentioned in
rld[].in, so It can't be used earlier for another destructive reload, either.
	
> > The sh addsi3 expander does not accept this constant when compiling for
> > -m5-compact.  The constant load is generated by gen_reload.
> 
> Ah.  You mean the case which has the comment
>       ??? At some point, this whole thing needs to be rethought.  */
> ? ;-)

Yes.  Actually, thinking of a new way to do reloads is the easy part.
Implementing it properly and handling all the different ports, optimizations
and corner cases so that we really get something better than what we have now
is the hard part ;-)
	
> I now see that here the cleverness of gen_reload (namely that it tries to
> handle difficult cases) interacts poorly with push_reload.  But I feel
> more that this is a bug gen_reload.

Unfortunately, when you introduce reloads for a new reason, you rock the
foundations of enum reload_class.  Moreover, these new reloads would also
have a new property: on two-address machines like the SH[1-4], they would
be required to share the same reload register with the reload for the PLUS.
Now, as you have two reloads that are required to have the same reload
register, how is that different from a single reload with the combined
lifetime?  And the extra lifetime is interesting for one thing only: if
the register that is used as an input of the PLUS can be reused as the
reload register.

> Hmm, I think if you already determined, that this particular reload can
> not be done in a single insn by gen_reload() it should also be promoted to
> RELOAD_OTHER to ensure that really noone selects its input registers as
> reload regs (except if that is already ensured by some other means?)

It is.  As far as other reloads are concerned there is no difference to
the one-insn add case.  And adding RELOAD_OTHERs willy-nilly can do great
harm to your code quality, and even cause compiler aborts when it runs
out of spill registers.
	
-- 
--------------------------
SuperH (UK) Ltd.
2410 Aztec West / Almondsbury / BRISTOL / BS32 4QX
T:+44 1454 465658