DJ Delorie <[EMAIL PROTECTED]> writes:
> > It does seem like reload 0 should be RELOAD_FOR_ something _ADDR.
> > What set it to RELOAD_OTHER?
>
> Ok, we start with a RELOAD_OTHER for the zero_extendhisi.
I'm not clear on why that happens. Most reloads start out as
RELOAD_FOR_INPUT or RELOAD_FOR_OUTPUT. What does the insn pattern
look like in the MD file? It kind of sounds like the insn pattern
might use a "+" constraint, but it shouldn't need to do that.
> The MEM's
> address eventually gets RELOAD_FOR_OTHER_ADDRESS. Part of that
> address is split out into another RELOAD_FOR_OTHER_ADDRESS.
> Separately, we have an RELOAD_FOR_OUTPUT_ADDRESS that matches it. The
> OUTPUT_ADDRESS and OTHER_ADDRESS are merged into a RELOAD_OTHER at the
> munged patch snippet below, where I added a check for OTHER_ADDRESS to
> avoid regressing the priority. It seems to work, but I'm worried that
> there's no logic to guarantee the order of output, other than the
> sequence number, which is commented as part of the sort "so that qsort
> is deterministic". Seems to be a recipe for future disaster.
There is already code which relies on the sequence number; see these
comments in reload.c:
/* This relies on the fact that emit_reload_insns outputs the
instructions for input reloads of type RELOAD_OTHER in the same
order as the reloads. Thus if the outer reload is also of type
RELOAD_OTHER, we are guaranteed that this inner reload will be
output before the outer reload. */
Your patch is a bit troubling because of the following code which
tries to set other reloads to RELOAD_FOR_OTHER_ADDRESS. But the
current code seems clearly wrong: changing RELOAD_FOR_OTHER_ADDRESS
into RELOAD_OTHER can't be right. And the reload ordering should keep
things in the right order. I hope.
I'll approve your patch if it bootstraps and tests on x86. But check
that zero_extend insn also to see why it is RELOAD_OTHER.
Ian
