This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]