reload inheritance failure handling (Was: [PATCH] Fix ACATS failure on i586)

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

When you send a reload patch, it would be useful if the subject referred
to reload.  I just stumbled on this by accident.

> But the reload inheritance mechanism has 2 phases: first it tests whether 
> each reload for an insn can use inheritance and then it re-tests all 
> inherited reloads for global validity.  In our case, the first test succeeds 
> but not the second one so we end up with a regular reload with the reload 
> reg that was scheduled for inheritance (reg 3) instead of (reg 0).

>From my reading of i386.{h,md} , bx would have worked perfectly fine as a
reload register.  AFAICS, it must have been this test at
reload_reg_free_for_value_p that must have failed:

  if (TEST_HARD_REG_BIT (reload_reg_unavailable, regno))
    return 0;

, and failed because the inherited register was not a spill register.
It is perfectly fine to use a non-spill register for a reload, as long
as the reload does not modify the register.
Reloads that have the out field set (N.B. auto-inc reloads also have the
out field set, but not the out_reg field.), and RELOAD_FOR_INSN reloads
modify the register.  For RELOAD_OTHER reloads we have insufficient
information (It would probably be a good idea to add that, that could be
a one-bit flag, which would have to be set if a RELOAD_FOR_INSN reload
is merged, or a RELOAD_OTHER that already has this flag set.)
All other reloads don't modify the register.

> I didn't manage to make an opinion on the failure mode of the inheritance 
> mechanism: should the initial reload reg be restored?

Yes, if the reload is not inherited, but not optional, it should get an
allocated spill register.  AFAICR, before the local spilling code, if
inheritance failed, we allocated the register after finding out that fact.

>                                                        What I found strange 
> is the discrepancy between the first test:
> reload1.c:5637
> 		  if ((regs_used
> 		       && ! free_for_value_p (regno, rld[r].mode,
> 					      rld[r].opnum, rld[r].when_needed,
> 					      rld[r].in, rld[r].out, r, 1))
> 		      || bad_for_class)

The regs_used test is there mainly for speed (and also a bit for historical
reasons).  When we find a register to inherit for an input-only reload,
the only reason why free_for_value_p would legitimately reject it is that
the register is already in use.
Note that search_equiv is only set if in is set but out is not.  That
basically means this this is an input-only reload (except for RELOAD_OTHER,
which is ambigous as described above, and this is probably a bug, but not
the one you are looking for).

> 2004-04-27  Eric Botcazou  <ebotcazou@act-europe.fr>
> 
> 	* reload1.c (choose_reload_regs): When searching for an
> 	equivalent reload register, check that it is both not
> 	used for reload and free for the value being reloaded.

This is plain wrong.  The main point of reload_reg_free_for_value_p
is that it allows you to re-use the same reload register for
multiple reloads if they want the same value.