Bernd Schmidt <ber...@codesourcery.com> writes:
> On 10/09/11 10:01, Richard Sandiford wrote:
>> Reload 0: GR_REGS, RELOAD_FOR_OUTPUT_ADDRESS (opnum = 0), can't combine,
>> secondary_reload_p
>> reload_reg_rtx: (reg:SI 5 $5)
>> Reload 1: reload_out (SI) = (reg:SI 32 $f0 [1655])
>> MD1_REG, RELOAD_FOR_OUTPUT (opnum = 0)
>> reload_out_reg: (reg:SI 32 $f0 [1655])
>> reload_reg_rtx: (reg:SI 65 lo)
>> secondary_out_reload = 0
>>
>> Reload 2: reload_out (SI) = (reg:SI 1656)
>> GR_REGS, RELOAD_FOR_OUTPUT (opnum = 3)
>> reload_out_reg: (reg:SI 1656)
>> reload_reg_rtx: (reg:SI 5 $5)
>>
>> So $5 is first stored in 1656 (operand 3), then $5 is used a secondary
>> reload in copying LO to $f0 (operand 0, reg 1655). The next and final
>> use of 1655 ends up inheriting this second reload of $5, so we try to
>> delete the original output copy. The problem is that we delete the
>> wrong one: we delete the store of $5 to 1656 rather than the copy of
>> $5 to 1655/$f0.
>
> So, reload 1 inherited from somewhere else rather than using reg $5 from
> its secondary reload? Where do we try to delete the insn, and what's the
> state of the spill_reg_store data at that point?
No, reload 1 is inherited by a later instruction. And it's inherited
correctly, in terms of the register contents being what we expect.
(Reload 1 is the one that survives to the end of the instruction's
reload sequence. Reload 2, in contrast, is clobbered by reload 1,
so could not be inherited. So when we record inheritance information
in emit_reload_insns, reload_reg_reaches_end_p correctly stops us
from recording reload 2 but allows us to record reload 1.)
The problem is that we record the wrong instruction for reload 1.
We say that reload 1 is performed by the instruction that performs
reload 2. So spill_reg_store[] contains the instruction for reload 2
rather than the instruction for reload 1. We delete it in
delete_output_reload at the point of inheritance.
>> The fix I went for is to clear new_spill_reg_store[] for all reloads
>> as a separate pass (rather than in the main do_{input,output}_reload
>> loop), then only allow new_spill_store_reg[] to be set if the associated
>> reload register reaches the end of the reload sequence.
>
> In this case, reload 0 is emitted after reload 2, so it reaches the end.
> Correct?
Right. And this is a defined order: output reloads are emitted in reverse
operand order (operand N-1, ..., operand 0). reload_reg_reaches_end_p
and reload_reg_free_p both rely on this property. In this case, reload 2
is associated with operand 3, while reload 1 is associated with operand 0,
so reload 2 has to come first.
> What would happen if the 0/1 pair and 2 were swapped?
If only the reload indices changed (so that reload 2 became 0) then the
reloads would still be emitted in the current order (because the order
is tied to the operand number rather than the reload number). And the
patch would also record the same instruction in spill_reg_store[].
I think current trunk would also behave correctly in this case,
because the reload numbers happen to match the insn order.
If the operand numbers were swapped, then the order of the output
reloads would be too. This register selection would then be invalid.
We would have a secondary reload that uses $5 being emitted while the $5
output from the main instruction is still live. But reload_reg_free_p
knows to avoid this situation.
Richard
