This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RELOAD_OTHER bug?


DJ Delorie <dj@redhat.com> 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]