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

[Eric Botcazou <ebotcazou at act-europe dot fr>]

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

Agreed.

> 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.

IIRC you're right, reload_reg_free_for_value_p returns 0 for this reason.  So 
this test is overly conservative?

> > 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.

So part of the problem lies in the failure mode of the inheritance mechanism?

> > 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.

Ah!  This is a very interesting explanation, that it would be nice to have in 
a comment just above the code.

> > 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.

Indeed, I now see the problem.  So removing regs_used from the test would 
have been correct, although not the ideal fix.

Thanks for taking the time to explain this in depth.

-- 
Eric Botcazou