Comment 13 for bug 1112499

Revision history for this message
In , Denisc-9 (denisc-9) wrote :

Sorry, wrong button was pressed.

(In reply to comment #8)
> I'm not going to be able to look at it anytime soon, but just some general
> thoughts:

I think that I'm ready to explain the bug.

> 1. Disabling caller-saves probably isn't appropriate. Just looking at
> codesize isn't the way to evaluate caller-saves either as caller-saves is
> tasked with improving performance, possibly at the expense of codesize.

I'm agree. I don't want to disable caller-saves.

>
> 2. The first thing someone needs to do is provide information as to why that
> insn needs reloads. I don't know enough about the AVR to hazard as guess why
> that insn needs reloads.
>
> 3. Find out where insn 172 comes from. There are restrictions on the insns
> created by caller-save. So if caller-save creates a bogus insn, then that
> needs to be investigated.

Generally, caller-save generate right insn.

  1. AVR port have a specific dependency between frame_pointer_needed and
get_frame_size().
avr_frame_pointer_required_p (void)
{
  return (cfun->calls_alloca
          || cfun->calls_setjmp
          || cfun->has_nonlocal_label
          || crtl->args.info.nregs == 0
          || get_frame_size () > 0);
}

  2. reload calls the `setup_save_areas ()' and after that get_frame_size () equal to 2, but frame_pointer_needed is 0.
It's wrong for AVR port (If dependency between frame_pointer_needed and get_frame_size() is permitted by GCC core).

  3. After that caller-save generate right save/restore insns for save to frame slot and restore from it. Like this (r28 is a frame pointer):
(insn 162 77 163 10 (set (reg:QI 18 r18)
        (mem/c:QI (plus:HI (reg/f:HI 28 r28)
                (const_int 1 [0x1])) [8 S1 A8])) /mnt/d/avr-work/tst/nl.c:120 20 {movqi_insn}
     (nil))

  4. After that the following code was executed:
   /* If needed, eliminate any eliminable registers. */
   if (num_eliminable || num_eliminable_invariants)
     did_elimination = eliminate_regs_in_insn (insn, 0);
And right insns converted to wrong (__SP_L__ can not be used as a pointer):
(insn 162 77 163 10 (set (reg:QI 18 r18)
        (mem/c:QI (plus:HI (reg/f:HI 32 __SP_L__)
                (const_int 1 [0x1])) [8 S1 A8])) /mnt/d/avr-work/tst/nl.c:120 20 {movqi_insn}
     (nil))
Here we have a wrong elimination FP->SP because frame_pointer_needed was not recalculated earlier.

  5. relod have the following fragment:
      if (caller_save_needed)
 setup_save_areas ();

      /* If we allocated another stack slot, redo elimination bookkeeping. */
      if (something_was_spilled || starting_frame_size != get_frame_size ())
 continue;
-------------------------------
But it's not resolve the problem. frame_pointer_needed isn't recalculated.
Call to `update_eliminables ()' seems as a right solution.