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]

Re: [Patch] Fix PR 60040


On 04/15/2016 02:52 PM, Senthil Kumar Selvaraj wrote:

For both testcases in the PR, reload fails to take into account that
FP-SP elimination can no longer be performed, and tries to find reload
regs for an rtx generated when FP-SP elimination was valid.

1. reload initializes elim table with FP->SP elimination enabled.
2. alter_reg for a pseudo allocates a stack slot for the pseudo, and sets
    reg_equiv_memory_loc to frame_pointer_rtx plus offset. It also sets
    something_was_spilled to true.
3. The main reload loop starts, and it resets something_was_spilled to false.
4. reload calls eliminate_regs for the pseudo and sets reg_equiv_address to
    (mem(SP + offset)).
5. calculate_needs_all_insns pushes a reload for SP (for the AVR target,
    SP cannot be a pointer reg - it needs to be reloaded into X Y or Z regs).
6. update_eliminables_and_spill calls targetm.frame_pointer_required,
    which returns true. That causes can_eliminate for FP->SP to be reset
    to zero, and FP to be added to bad_spill_regs_global. For the AVR,
    FP is Y, one of the 3 pointer regs. reload also notes that something
    has changed, and that the loop needs to run again.
7. reload still calls select_reload_regs, and find_regs fails to find a
    pointer reg to reload SP, which is unnecessary as FP->SP elimination
    had been disabled anyway in (6).

IOW, reload fails to find pointer regs for an RTL expression that was
created when FP->SP elimination was true, even after it turns out that
the elimination can't be done after all. The patch tries to detect that
- if it knows the loop is going to run again, it silences the failure.

Also note that at a different point in the loop, the reload loop starts
over if something_was_spilled (line 982-986). If set outside the reload
loop by alter_reg, it gets reset at (3) - not sure why. I'd think a
"continue" after update_eliminables_and_spill (line 1019-1022) would
also work - haven't tested it though.

That's what I was going to ask next. I think if that works for you, I think that's an approach that would make more sense.

However, it looks like after this call, and the other one in the same loop, should probably be calling finish_spills. It looks like an oversight in the existing code that this doesn't happen for the existing continue. Please try adding it in both places.


Bernd


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