------- Comment #14 from uweigand at gcc dot gnu dot org 2010-09-03 18:30
-------
(In reply to comment #12)
> Yes, it would but I think the reload should still generate the right code in
> this particular order of insns. IMHO, fixing the order of insn is not the
> right thing to do because there might be situation of cycle (e.g. value of
> p600
> is inherited from 2 but should be reloaded into 3 and p356 is inherited from 3
> and should be reloaded into 2).
>
> The problem is definitely in reload inheritance. Reg_last_reload_reg is not
> invalidated by insn #1407 which is generated by another reload of insn #675.
Actually, I think this really is a reload insn ordering problem. Note that
reload inheritance tracking via reg_last_reload_reg works on the whole reloaded
insn including all the generated reload insns as a whole. Conflicts between
different reloads of the same original insn should be treated specially, taking
into account reload insn ordering (this is done in free_for_value_p).
In this particular case, what happens is this. After initial reload register
selection, we have this set of reloads:
Reload 0: reload_in (SI) = (reg/v/f:SI 132 [ kpte ])
GENERAL_REGS, RELOAD_FOR_OPERAND_ADDRESS (opnum = 0)
reload_in_reg: (reg/v/f:SI 132 [ kpte ])
reload_reg_rtx: (reg:SI 5 di)
Reload 1: reload_in (SI) = (reg/v/f:SI 132 [ kpte ])
DIREG, RELOAD_FOR_INPUT (opnum = 1)
reload_in_reg: (reg/v/f:SI 132 [ kpte ])
reload_reg_rtx: (reg:SI 5 di)
Reload 2: reload_in (SI) = (reg:SI 588 [ D.29684 ])
BREG, RELOAD_FOR_INPUT (opnum = 2)
reload_in_reg: (reg:SI 588 [ D.29684 ])
reload_reg_rtx: (reg:SI 3 bx)
Reload 3: reload_in (SI) = (reg:SI 356)
CREG, RELOAD_FOR_INPUT (opnum = 3)
reload_in_reg: (reg:SI 356)
reload_reg_rtx: (reg:SI 2 cx)
Reload inheritance notes that reload_in of reload 2 (reg:SI 588) is already
available in CX at this point. While this cannot be used as reload register
due to the BREG class, it is chosen as "override input", i.e. reload_in is set
to (reg:SI 2 cx) instead of of (reg:SI 588).
Code near the end of choose_reload_regs then verifies whether this causes any
problems due to re-use of the CX register, and comes to the (correct) decision
that it does not, since it knows the reload insn for reload 3 (a
RELOAD_FOR_INPUT for operand 3) which clobbers CX will certainly be generated
*after* the override-input reload insn for reload 2 (a RELOAD_FOR_INPUT for
operand 2).
Unfortunately, some time *after* this decision has been made, the when_needed
field of reload 3 is changed from RELOAD_FOR_INPUT to RELOAD_OTHER. This
causes the sequence of generated reload insns to change (RELOAD_OTHER reloads
are emitted before RELOAD_FOR_INPUT reloads) -- this breaks the assumptions the
reload inheritance code made, and causes wrong code to be generated.
Now the question is, why does this change to RELOAD_OTHER happen. This is done
in merge_assigned_reloads, which is called because i386 is a target with
SMALL_REGISTER_CLASSES. This routine notices that reloads 0 and 1 share a
reload register DI, and decides to merge them, making reload 0 a RELOAD_OTHER
reload (and cancelling reload 1). It then goes through all other reloads:
/* If this is now RELOAD_OTHER, look for any reloads that
load parts of this operand and set them to
RELOAD_FOR_OTHER_ADDRESS if they were for inputs,
RELOAD_OTHER for outputs. Note that this test is
equivalent to looking for reloads for this operand
number.
This loop now appears to detect reload 3 as a reload that "loads part of" the
operand 0. This happens because
reg_overlap_mentioned_for_reload_p (rld[j].in,
rld[i].in))
returns true since both reload-in values are stack slots, and
reg_overlap_mentioned_for_reload_p conservatively assumes that all memory
accesses conflict.
Therefore, when_needed for reload 3 is also set to RELOAD_OTHER. This is not
only unnecessary, but in turn causes the breakage.
Now in this particular case, it might be possible to fix the problem by using a
better detection of which additional reloads need to be modified. (The comment
says, "Note that this test is equivalent to looking for reloads for this
operand number." -- which is not true, but could be implemented instead ...)
However, the problem seems to be more fundamental. If *any* change, even
necessary changes, to when_needed flags are made at this point, *any* decision
on reload inheritance or input overrides that was based on reload insn ordering
may now be incorrect.
I guess a conservative fix could be to check whether any reload inheritance was
indeed or input override was indeed performed on this insn, and if so, refuse
to perform the merge ...
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45312
