| Message ID | 20131003162313.00ny1vjalss8sskw-nzlynne@webmail.spamcop.net |
|---|---|
| State | New |
| Headers | show |
On 10/03/13 14:23, Joern Rennecke wrote: > To make sure that reloading is done with up-to-date register eliminations, > I call update_eliminables also in the code path that continues for a frame > size change. Not directly, since I need the code to spill the no longer > eliminated registers; I've factored out this code into a new function > update_eliminables_and_spill. > Also, I move the frame size alignment code before the code that tests for > a frame size change so that this is also covered with the enhanced check. > > In principle, we could inline update_eliminables into > update_eliminables_and_spill now, since the former is only called from the > latter, but I leave this to the bootstrap compiler so as not to break the > connection between init_elim_table and update_eliminables. > > Regression tested on avr atmega128-sim. > > Bootstrapped and regtested on i686-pc-linux-gnu. > > OK to apply? Isn't this going to result in more stack alignments (and thus unused slots on the stack). In particular the current structure will do as much of the pseudo spilling, caller save setup, as possible. Once those two steps stabilize, the stack is aligned and we proceed further into the main reload loop. With your change it seems to me that we do a single round of spilling & caller-save setup, then align the stack, then restart. The net result being we align the stack a lot more often. Jeff
On 11 October 2013 04:53, Jeff Law <law@redhat.com> wrote: > With your change it seems to me that we do a single round of spilling & > caller-save setup, then align the stack, then restart. The net result being > we align the stack a lot more often. Yes, but AFAICT, that should not result in more space being used, because assign_stack_local uses ASLK_RECORD_PAD, so assign_stack_local_1 will record any space added for alignment purposes with add_frame_space, so it can be used for a subsequent spill of suitable size. Also, trying to do register elimination with an unaligned frame size could lead to invalid stack slot addresses; if not ICEs, these could lead to extra reloads, extra spill register needs, and thus ultimately more spills and larger frame size.
On 10/11/13 08:40, Joern Rennecke wrote: > On 11 October 2013 04:53, Jeff Law <law@redhat.com> wrote: > > With your change it seems to me that we do a single round of spilling & >> caller-save setup, then align the stack, then restart. The net result being >> we align the stack a lot more often. > > Yes, but AFAICT, that should not result in more space being used, > because assign_stack_local uses ASLK_RECORD_PAD, so > assign_stack_local_1 will record any space > added for alignment purposes with add_frame_space, so it can be used > for a subsequent spill of suitable size. You're right -- I wasn't aware of Bernd's work to reuse alignment paddings for real objects. It was added a few months after I'd been poking at problems in that loop of reload. Let me look at this stuff again. jeff
On 10/03/13 14:23, Joern Rennecke wrote: > > 2013-10-02 Joern Rennecke<joern.rennecke@embecosm.com> > > PR other/58545 > * reload1.c (update_eliminables_and_spill): New function, broken > out of reload. > (reload): Use it. Check for frame size change after frame > size alignment, and call update_eliminables_and_spill first > if continue-ing. With a testcase included, this is OK. I still think we can get extra alignments, but the impact of them is reduced since we can use those slots for other things. jeff
Index: reload1.c =================================================================== --- reload1.c (revision 203159) +++ reload1.c (working copy) @@ -373,6 +373,7 @@ static void init_eliminable_invariants ( static void init_elim_table (void); static void free_reg_equiv (void); static void update_eliminables (HARD_REG_SET *); +static bool update_eliminables_and_spill (void); static void elimination_costs_in_insn (rtx); static void spill_hard_reg (unsigned int, int); static int finish_spills (int); @@ -913,9 +914,6 @@ reload (rtx first, int global) 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; if (starting_frame_size && crtl->stack_alignment_needed) { /* If we have a stack frame, we must align it now. The @@ -927,8 +925,12 @@ reload (rtx first, int global) STARTING_FRAME_OFFSET not be already aligned to STACK_BOUNDARY. */ assign_stack_local (BLKmode, 0, crtl->stack_alignment_needed); - if (starting_frame_size != get_frame_size ()) - continue; + } + /* If we allocated another stack slot, redo elimination bookkeeping. */ + if (something_was_spilled || starting_frame_size != get_frame_size ()) + { + update_eliminables_and_spill (); + continue; } if (caller_save_needed) @@ -962,30 +964,11 @@ reload (rtx first, int global) else if (!verify_initial_elim_offsets ()) something_changed = 1; - { - HARD_REG_SET to_spill; - CLEAR_HARD_REG_SET (to_spill); - update_eliminables (&to_spill); - AND_COMPL_HARD_REG_SET (used_spill_regs, to_spill); - - for (i = 0; i < FIRST_PSEUDO_REGISTER; i++) - if (TEST_HARD_REG_BIT (to_spill, i)) - { - spill_hard_reg (i, 1); - did_spill = 1; - - /* Regardless of the state of spills, if we previously had - a register that we thought we could eliminate, but now can - not eliminate, we must run another pass. - - Consider pseudos which have an entry in reg_equiv_* which - reference an eliminable register. We must make another pass - to update reg_equiv_* so that we do not substitute in the - old value from when we thought the elimination could be - performed. */ - something_changed = 1; - } - } + if (update_eliminables_and_spill ()) + { + did_spill = 1; + something_changed = 1; + } select_reload_regs (); if (failure) @@ -4031,6 +4014,38 @@ update_eliminables (HARD_REG_SET *pset) SET_HARD_REG_BIT (*pset, HARD_FRAME_POINTER_REGNUM); } +/* Call update_eliminables an spill any registers we can't eliminate anymore. + Return true iff a register was spilled. */ + +static bool +update_eliminables_and_spill (void) +{ + int i; + bool did_spill = false; + HARD_REG_SET to_spill; + CLEAR_HARD_REG_SET (to_spill); + update_eliminables (&to_spill); + AND_COMPL_HARD_REG_SET (used_spill_regs, to_spill); + + for (i = 0; i < FIRST_PSEUDO_REGISTER; i++) + if (TEST_HARD_REG_BIT (to_spill, i)) + { + spill_hard_reg (i, 1); + did_spill = true; + + /* Regardless of the state of spills, if we previously had + a register that we thought we could eliminate, but now can + not eliminate, we must run another pass. + + Consider pseudos which have an entry in reg_equiv_* which + reference an eliminable register. We must make another pass + to update reg_equiv_* so that we do not substitute in the + old value from when we thought the elimination could be + performed. */ + } + return did_spill; +} + /* Return true if X is used as the target register of an elimination. */ bool