[Bug middle-end/45312] [4.4 Regression] GCC 4.4.4 miscompiles the Linux kernel

[uweigand at gcc dot gnu dot org]

------- 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