Re: A reload inheritance bug

[Mark Shinwell <shinwell at codesourcery dot com>]

Bernd Schmidt wrote:
> > insn 5301: (set (reg/f:SI 4082) (reg/f:SI 3275))
> >
> > insn 5291 (set (reg:DF 4078])
> >           (mem/s:DF (plus:SI (reg/f:SI 3275) (reg:SI 3812))))
> >   REG_DEAD 3275
> >
> > insn 5314 (set (reg:DF 4096)
> >           (mem/s:DF (plus:SI (reg/f:SI 4082) (reg:SI 4084))))
>
> > After reload we end up with the following.  I've added dividers to show
> > the correspondence with the insns above.
> >
> > insn 5301 (set (mem/f/c:SI (plus:SI (reg/f:SI 13 sp) (const_int 12)))
> > 	         (reg/f:SI 9 r9 [3275]))
> > ---
> > insn 6675 (set (reg:SI 9 r9)
> >                (plus:SI (reg/f:SI 9 r9 [3275]) (reg:SI 10 sl [3812])))
> >
> > insn 5291 (set (reg:DF 75 s12 [4078])
> >                (mem/s:DF (reg:SI 9 r9)))
> > ---
> > insn 6680 (set (reg:SI 1 r1) (const_int 4928))
> >
> > insn 6681 (set (reg:SI 1 r1)
> >                (plus:SI (reg/f:SI 9 r9 [3275]) (reg:SI 1 r1)))
> >
> > insn 5314 (set (reg:DF 75 s12 [4096])
> >                (mem/s:DF (reg:SI 1 r1)))
> >
> > We see here how pseudo 3275 was allocated to r9 and pseudo 4082 was
> > spilled to the stack.  At insn 5291, r9 has been allocated [*] as the
> > reload register since pseudo 3275 dies in that instruction; at insn
> > 5314 we see the then-incorrect use of r9 (in instruction 6681)
> > for the value of pseudo 4082.  Note also how the dump shows that the
> > compiler thinks r9 still holds the value of pseudo 3275 at insn 6681.
>
> Presumably this is one thing that is mildly unusual - R9 being chosen in
> find_reloads already.  This wouldn't happen later, since it's in
> reg_used_in_insn and therefore disallowed.

Sorry for the delay in replying -- hectic week!

As you say, one unusual thing about this situation must be the fact
that the reload register is getting chosen by the code in
push_reload heralded by "If this is an input reload and the operand
contains a register that dies in this insn and is used nowhere else,
see if it is the right class to be used for this reload.  Use it if so."
etc.  I suspect it is the conjunction of this code and the behaviour of
reload in choosing r9 for pseudo 4082 in insn 5314 above that causes the
problem.

I don't entirely follow your example below...

> Still, assume a similar sequence
>
> insn 5301: (set (reg/f:SI 4082) (reg/f:SI 3275))
>
> insn 5291 (set (reg:DF 4078])
>                (unspec:DF (mem/s:DF (plus:SI (reg/f:SI 3275)
>                                              (reg:SI 3812)))
>                           (reg:SI 3275)))
>   REG_DEAD 3275
>
> <some other insn where R9 is used for an input reload>
>
> insn 5314 (set (reg:DF 4096)
>           (mem/s:DF (plus:SI (reg/f:SI 4082) (reg:SI 4084))))
>
> Here, we wouldn't use R9 as reload register in 5291

...since here, as far as I understand it, the clause mentioned above
in push_reload wouldn't select r9 to use as a reload register for
5291.  My gut feeling is that this example will work as a consequence.
(Perhaps you're getting at the fact that the decision procedure leading
us to choose r9 for pseudo 4082 in insn 5314 might be at fault instead?
Even if so, I still suspect it's the reuse of a hard reg in an insn with
a REG_DEAD note for the corresponding pseudo that is the real cause
because it upsets the later code -- that's what my patch was trying to
correct.)

Mark