A reload inheritance bug

[Mark Shinwell]

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