Re: reload inheritance bug & patch

["Eric Botcazou" <ebotcazou at libertysurf dot fr>]

> The root cause is a bug in reload inheritance.  We have rtl like this:
>
>    A:  (set (reg:QI R2)
>             (subreg:QI (reg:HI R1)))
>
>    B:  (set (reg:DI R3)
>             (ashift:DI (subreg:DI (reg:QI R2) 0)
>                        (const_int 58)))
>
>    ... later insns which use R1 ...
>
> in which R1 is allocated to a general register (G1) and R2 is spilled
> to the stack.  R2's stack slot naturally satisfies the destination
> constraints of insn A so there are no reloads of R2 for insn B to inherit.
> choose_reload_regs() therefore tries to inherit using the find_equiv_reg()
> route instead.
>
> find_equiv_reg() finds insn A and rightly says that (reg:QI R2) is
> equivalent to (subreg:QI (reg:HI G1) 0).  As expected, the right hand
> side gets simplified to (reg:QI G1):
>
>   else if (GET_CODE (equiv) == SUBREG)
>     {
>       /* This must be a SUBREG of a hard register.
> Make a new REG since this might be used in an
> address and not all machines support SUBREGs
> there.  */
>       regno = subreg_regno (equiv);
>       equiv = gen_rtx_REG (rld[r].mode, regno);
>     }
>
> and then there are various checks to see whether (reg:QI G1) can be
> inherited here.  All these checks say it can, so an inheritance is
> duly recorded:
>
>       /* If we found an equivalent reg, say no code need be generated
> to load it, and use it as our reload reg.  */
>       if (equiv != 0
>   && (regno != HARD_FRAME_POINTER_REGNUM
>       || !frame_pointer_needed))
> {
>                   ...
>   rld[r].reg_rtx = equiv;
>   reload_inherited[r] = 1;
>                   ...
>                 }
>
> However, the inheritance gets rejected by the next loop:
>
>   if (! free_for_value_p (true_regnum (check_reg), rld[r].mode,
>   rld[r].opnum, rld[r].when_needed, rld[r].in,
>   (reload_inherited[r]
>    ? rld[r].out : const0_rtx),
>   r, 1))
>     {
>       if (pass)
> continue;
>       reload_inherited[r] = 0;
>       reload_override_in[r] = 0;
>     }
>
> because reload_reg_free_for_value_p() checks:
>
>   if (TEST_HARD_REG_BIT (reload_reg_unavailable, regno))
>     return 0;
>
> and because G1 is included in reload_reg_unavailable (R1/G1 being live
> throughout insn B).

Same bug as http://gcc.gnu.org/ml/gcc-patches/2004-04/msg01779.html.
Fariborz also ran into it on PPC (PR 16028).  See Joern's take on it:
http://gcc.gnu.org/ml/gcc-patches/2004-06/msg00061.html

> On the other hand, calling free_for_value_p() unconditionally might be
> going too far the other way: it would check lower-priority reloads whose
> reg_rtxes haven't been finalised yet.  (They might change later due to
> inheritance.)

That's my final patch:
   http://gcc.gnu.org/ml/gcc-patches/2004-06/msg01647.html
Fariborz regtested it against SPEC on a 2GHz G5.

With now 3 instances of the same problem, we'll hopefully converge to a
solution.

--
Eric Botcazou