Re: [PATCH] df: Change defs in entry and uses in exit block during separate shrink-wrapping

2016-11-14 Thread Segher Boessenkool
On Mon, Nov 14, 2016 at 11:09:19AM +0100, Richard Biener wrote:
> On Sat, Nov 12, 2016 at 9:31 AM, Segher Boessenkool
>  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


Re: [PATCH] df: Change defs in entry and uses in exit block during separate shrink-wrapping

2016-11-14 Thread Richard Biener
On Sat, Nov 12, 2016 at 9:31 AM, Segher Boessenkool
 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.
>
> [  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  
>
> * 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 

[PATCH] df: Change defs in entry and uses in exit block during separate shrink-wrapping

2016-11-12 Thread Segher Boessenkool
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.

[  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  

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