This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[reload-v2] Reverting Jan 12 change, it's unsafe due to assumptions in reload
- From: Jeff Law <law at redhat dot com>
- To: gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 07 Jul 2010 09:30:58 -0600
- Subject: [reload-v2] Reverting Jan 12 change, it's unsafe due to assumptions in reload
Back in January I added some code to reload-v2 which caused it to ignore
conflicts between pseudos with overlapping lifetimes when those pseudos
were known to hold the same value at runtime (via a copy). This was
done in response to seeing cases in which those false conflicts were
resulting in poor allocations. Until now that code has worked
flawlessly, but after this week's merge a latent bug has surfaced.
We have an output reload for this insn:
(insn 225 224 226 21 /home/law/merge/reload-v2a/zlib/deflate.c:1160 (set
(reg/v:QI 294 [orig:66 scan_end ] [66])
(mem:QI (plus:DI (reg:DI 333)
(reg:DI 140 [ D.5169 ])) [0 *D.5172_123+0 S1 A8])) 65
{*movqi_internal} (expr_list:REG_DEAD (reg:DI 333)
(expr_list:REG_DEAD (reg:DI 140 [ D.5169 ])
(nil))))
Which looks something like this after reload substitutes hard regs for
pseudos and does other housekeeping:
(insn 225 224 226 21 /home/law/merge/reload-v2a/zlib/deflate.c:1160 (set
(reg/v:QI 294 [orig:66 scan_end ] [66])
(mem:QI (plus:DI (reg:DI 44 r15 [333])
(reg:DI 0 ax [orig:140 D.5169 ] [140])) [0
*D.5172_123+0 S1 A8])) 65 {*movqi_internal} (expr_list:REG_DEAD (reg:DI
44 r15 [333])
(expr_list:REG_DEAD (reg:DI 0 ax [orig:140 D.5169 ] [140])
(nil))))
Earlier in the insn stream we have:
(insn 307 376 210 19 /home/law/merge/reload-v2a/zlib/deflate.c:1148 (set
(reg/v/f:DI 44 r15 [orig:61 scan ] [61])
(reg:DI 44 r15 [333])) 61 {*movdi_internal_rex64} (nil))
And it's important to know that there are uses of p61 (r44) which are
reachable from insn 225. Note carefully the REG_DEAD note for p333
(r44) attached to insn 225.
So while p333 is dead, its hard reg, r44 is still holding a useful value
after insn 225 (for p61).
combine_reloads ultimately selects r44 as the reload reg for insn 225's
output reload because it thinks r44 is dead after insn 225, which is
clearly is not due to p61 being allocated to r44 and uses of p61 being
reachable from insn 225.
One could argue that the REG_DEAD note is bogus one the pseudo is
replaced with its hard reg. However, to make that determination, we'd
have to re-run note creation after replacing pseudos with hard regs and
the solution would have to be updated if a pseudo's hard register
allocation changes. Ick.
As best as I can tell, reload has gotten away with relying on REG_DEAD
notes in this manner because the allocators don't set up a situation
where two pseudos with overlapping lifetimes are assigned the same hard
reg if the pseudos hold the same value.
The range-splitting code I've got for reload-v2 allowed this kind of
allocation and the latent bug exposed itself after this week's merge.
The attached patch reverts the change which allowed this kind of allocation.
Applied to the branch.
Attachment:
PP
Description: Text document