| Message ID | f29954fd1da6458050c2cbfceb20a1dad464e1f4.1478938444.git.segher@kernel.crashing.org |
|---|---|
| State | New |
| Headers | show |
On Sat, Nov 12, 2016 at 9:31 AM, Segher Boessenkool <segher@kernel.crashing.org> wrote: > So far all target implementations of the separate shrink-wrapping hooks > use the DF LIVE info to figure out around which basic blocks the non- > volatile registers need to be saved. This is done by looking at the > IN+GEN+KILL sets of the basic blocks. However, that doesn't work for > registers that DF says are defined in the entry block, or used in the > exit block. > > This patch introduces a shrink_wrap_separate_in_progress variable, and > makes df_get_entry_block_def_set and df_get_exit_block_use_set set the > respective sets to empty if that variable is set to true. It also > changes the rs6000 port to use IN+GEN+KILL for the LR component. > > [ <https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01986.html> is an older > version of this, using a different (much inferior) approach. ] > > Tested on powerpc64-linux {-m32,-m64}. Is this okay for trunk? Globals like this are somewhat gross. There are df_changeable_flags where we seem to have a "related" flag DF_RD_PRUNE_DEAD_DEFS so you could add a flag for this. There's also local_flags (only used by df_chain_add_problem). Richard. > > Segher > > > 2016-11-12 Segher Boessenkool <segher@kernel.crashing.org> > > * config/rs6000/rs6000.c (rs6000_components_for_bb): Mark the LR > component as used also if LR_REGNO is a live input to the bb. > * df-scan.c (df_get_entry_block_def_set): Return immediately after > clearing the set if shrink_wrap_separate_in_progress. > (df_get_exit_block_use_set): Ditto. > * rtl.h (shrink_wrap_separate_in_progress): Declare new variable. > * shrink-wrap.c (shrink_wrap_separate_in_progress): New variable. > (try_shrink_wrapping_separate): Set shrink_wrap_separate_in_progress > and call df_update_entry_block_defs and df_update_exit_block_uses > at the start; clear that variable and call those functions at the end. > > --- > gcc/config/rs6000/rs6000.c | 3 ++- > gcc/df-scan.c | 16 ++++++++++++++++ > gcc/rtl.h | 3 +++ > gcc/shrink-wrap.c | 12 ++++++++++++ > 4 files changed, 33 insertions(+), 1 deletion(-) > > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > index 49985f1..8d6b2d5 100644 > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -27714,7 +27714,8 @@ rs6000_components_for_bb (basic_block bb) > bitmap_set_bit (components, regno); > > /* LR needs to be saved around a bb if it is killed in that bb. */ > - if (bitmap_bit_p (gen, LR_REGNO) > + if (bitmap_bit_p (in, LR_REGNO) > + || bitmap_bit_p (gen, LR_REGNO) > || bitmap_bit_p (kill, LR_REGNO)) > bitmap_set_bit (components, 0); > > diff --git a/gcc/df-scan.c b/gcc/df-scan.c > index 7cfd34b..398842b 100644 > --- a/gcc/df-scan.c > +++ b/gcc/df-scan.c > @@ -3506,6 +3506,14 @@ df_get_entry_block_def_set (bitmap entry_block_defs) > > bitmap_clear (entry_block_defs); > > + /* For separate shrink-wrapping we use LIVE to analyze which basic blocks > + need a prologue for some component to be executed before that block, > + and we do not care about any other registers. Hence, we do not want > + any register for any component defined in the entry block, and we can > + just leave all registers undefined. */ > + if (shrink_wrap_separate_in_progress) > + return; > + > for (i = 0; i < FIRST_PSEUDO_REGISTER; i++) > { > if (global_regs[i]) > @@ -3665,6 +3673,14 @@ df_get_exit_block_use_set (bitmap exit_block_uses) > > bitmap_clear (exit_block_uses); > > + /* For separate shrink-wrapping we use LIVE to analyze which basic blocks > + need an epilogue for some component to be executed after that block, > + and we do not care about any other registers. Hence, we do not want > + any register for any component seen as used in the exit block, and we > + can just say no registers at all are used. */ > + if (shrink_wrap_separate_in_progress) > + return; > + > /* Stack pointer is always live at the exit. */ > bitmap_set_bit (exit_block_uses, STACK_POINTER_REGNUM); > > diff --git a/gcc/rtl.h b/gcc/rtl.h > index 3bb6a22..d1409bc 100644 > --- a/gcc/rtl.h > +++ b/gcc/rtl.h > @@ -3468,6 +3468,9 @@ extern int lra_in_progress; > > #define can_create_pseudo_p() (!reload_in_progress && !reload_completed) > > +/* Set to true during separate shrink-wrapping. */ > +extern bool shrink_wrap_separate_in_progress; > + > #ifdef STACK_REGS > /* Nonzero after end of regstack pass. > Set to 1 or 0 by reg-stack.c. */ > diff --git a/gcc/shrink-wrap.c b/gcc/shrink-wrap.c > index e480d4d..dd0cae1 100644 > --- a/gcc/shrink-wrap.c > +++ b/gcc/shrink-wrap.c > @@ -1764,6 +1764,10 @@ insert_prologue_epilogue_for_components (sbitmap components) > commit_edge_insertions (); > } > > +/* Used by DF: if true, the entry block defines no registers, and the exit > + block uses none. */ > +bool shrink_wrap_separate_in_progress = false; > + > /* The main entry point to this subpass. FIRST_BB is where the prologue > would be normally put. */ > void > @@ -1794,6 +1798,9 @@ try_shrink_wrapping_separate (basic_block first_bb) > return; > > /* We need LIVE info. */ > + shrink_wrap_separate_in_progress = true; > + df_update_entry_block_defs (); > + df_update_exit_block_uses (); > df_live_add_problem (); > df_live_set_all_dirty (); > df_analyze (); > @@ -1859,6 +1866,11 @@ try_shrink_wrapping_separate (basic_block first_bb) > free_dominance_info (CDI_DOMINATORS); > free_dominance_info (CDI_POST_DOMINATORS); > > + /* All done. */ > + shrink_wrap_separate_in_progress = false; > + df_update_entry_block_defs (); > + df_update_exit_block_uses (); > + > if (crtl->shrink_wrapped_separate) > { > df_live_set_all_dirty (); > -- > 1.9.3 >
On Mon, Nov 14, 2016 at 11:09:19AM +0100, Richard Biener wrote: > On Sat, Nov 12, 2016 at 9:31 AM, Segher Boessenkool > <segher@kernel.crashing.org> wrote: > > So far all target implementations of the separate shrink-wrapping hooks > > use the DF LIVE info to figure out around which basic blocks the non- > > volatile registers need to be saved. This is done by looking at the > > IN+GEN+KILL sets of the basic blocks. However, that doesn't work for > > registers that DF says are defined in the entry block, or used in the > > exit block. > > > > This patch introduces a shrink_wrap_separate_in_progress variable, and > > makes df_get_entry_block_def_set and df_get_exit_block_use_set set the > > respective sets to empty if that variable is set to true. It also > > changes the rs6000 port to use IN+GEN+KILL for the LR component. > Globals like this are somewhat gross. This code already uses epilogue_completed and reload_completed for similar purposes ;-) > There are df_changeable_flags > where we seem to have a "related" flag DF_RD_PRUNE_DEAD_DEFS > so you could add a flag for this. There's also local_flags (only used > by df_chain_add_problem). That does sound a bit nicer, I'll try that. Thanks for the hint, Segher
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 49985f1..8d6b2d5 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -27714,7 +27714,8 @@ rs6000_components_for_bb (basic_block bb) bitmap_set_bit (components, regno); /* LR needs to be saved around a bb if it is killed in that bb. */ - if (bitmap_bit_p (gen, LR_REGNO) + if (bitmap_bit_p (in, LR_REGNO) + || bitmap_bit_p (gen, LR_REGNO) || bitmap_bit_p (kill, LR_REGNO)) bitmap_set_bit (components, 0); diff --git a/gcc/df-scan.c b/gcc/df-scan.c index 7cfd34b..398842b 100644 --- a/gcc/df-scan.c +++ b/gcc/df-scan.c @@ -3506,6 +3506,14 @@ df_get_entry_block_def_set (bitmap entry_block_defs) bitmap_clear (entry_block_defs); + /* For separate shrink-wrapping we use LIVE to analyze which basic blocks + need a prologue for some component to be executed before that block, + and we do not care about any other registers. Hence, we do not want + any register for any component defined in the entry block, and we can + just leave all registers undefined. */ + if (shrink_wrap_separate_in_progress) + return; + for (i = 0; i < FIRST_PSEUDO_REGISTER; i++) { if (global_regs[i]) @@ -3665,6 +3673,14 @@ df_get_exit_block_use_set (bitmap exit_block_uses) bitmap_clear (exit_block_uses); + /* For separate shrink-wrapping we use LIVE to analyze which basic blocks + need an epilogue for some component to be executed after that block, + and we do not care about any other registers. Hence, we do not want + any register for any component seen as used in the exit block, and we + can just say no registers at all are used. */ + if (shrink_wrap_separate_in_progress) + return; + /* Stack pointer is always live at the exit. */ bitmap_set_bit (exit_block_uses, STACK_POINTER_REGNUM); diff --git a/gcc/rtl.h b/gcc/rtl.h index 3bb6a22..d1409bc 100644 --- a/gcc/rtl.h +++ b/gcc/rtl.h @@ -3468,6 +3468,9 @@ extern int lra_in_progress; #define can_create_pseudo_p() (!reload_in_progress && !reload_completed) +/* Set to true during separate shrink-wrapping. */ +extern bool shrink_wrap_separate_in_progress; + #ifdef STACK_REGS /* Nonzero after end of regstack pass. Set to 1 or 0 by reg-stack.c. */ diff --git a/gcc/shrink-wrap.c b/gcc/shrink-wrap.c index e480d4d..dd0cae1 100644 --- a/gcc/shrink-wrap.c +++ b/gcc/shrink-wrap.c @@ -1764,6 +1764,10 @@ insert_prologue_epilogue_for_components (sbitmap components) commit_edge_insertions (); } +/* Used by DF: if true, the entry block defines no registers, and the exit + block uses none. */ +bool shrink_wrap_separate_in_progress = false; + /* The main entry point to this subpass. FIRST_BB is where the prologue would be normally put. */ void @@ -1794,6 +1798,9 @@ try_shrink_wrapping_separate (basic_block first_bb) return; /* We need LIVE info. */ + shrink_wrap_separate_in_progress = true; + df_update_entry_block_defs (); + df_update_exit_block_uses (); df_live_add_problem (); df_live_set_all_dirty (); df_analyze (); @@ -1859,6 +1866,11 @@ try_shrink_wrapping_separate (basic_block first_bb) free_dominance_info (CDI_DOMINATORS); free_dominance_info (CDI_POST_DOMINATORS); + /* All done. */ + shrink_wrap_separate_in_progress = false; + df_update_entry_block_defs (); + df_update_exit_block_uses (); + if (crtl->shrink_wrapped_separate) { df_live_set_all_dirty ();
So far all target implementations of the separate shrink-wrapping hooks use the DF LIVE info to figure out around which basic blocks the non- volatile registers need to be saved. This is done by looking at the IN+GEN+KILL sets of the basic blocks. However, that doesn't work for registers that DF says are defined in the entry block, or used in the exit block. This patch introduces a shrink_wrap_separate_in_progress variable, and makes df_get_entry_block_def_set and df_get_exit_block_use_set set the respective sets to empty if that variable is set to true. It also changes the rs6000 port to use IN+GEN+KILL for the LR component. [ <https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01986.html> is an older version of this, using a different (much inferior) approach. ] Tested on powerpc64-linux {-m32,-m64}. Is this okay for trunk? Segher 2016-11-12 Segher Boessenkool <segher@kernel.crashing.org> * config/rs6000/rs6000.c (rs6000_components_for_bb): Mark the LR component as used also if LR_REGNO is a live input to the bb. * df-scan.c (df_get_entry_block_def_set): Return immediately after clearing the set if shrink_wrap_separate_in_progress. (df_get_exit_block_use_set): Ditto. * rtl.h (shrink_wrap_separate_in_progress): Declare new variable. * shrink-wrap.c (shrink_wrap_separate_in_progress): New variable. (try_shrink_wrapping_separate): Set shrink_wrap_separate_in_progress and call df_update_entry_block_defs and df_update_exit_block_uses at the start; clear that variable and call those functions at the end. --- gcc/config/rs6000/rs6000.c | 3 ++- gcc/df-scan.c | 16 ++++++++++++++++ gcc/rtl.h | 3 +++ gcc/shrink-wrap.c | 12 ++++++++++++ 4 files changed, 33 insertions(+), 1 deletion(-)