Re: [PATCH] Don't reapply loops flags if unnecessary in loop_optimizer_init

[Tom de Vries <Tom_deVries at mentor dot com>]

On 23/11/15 11:29, Richard Biener wrote:
> On Mon, 23 Nov 2015, Tom de Vries wrote:
>
> > [ was: Re: [PATCH, 10/16] Add pass_oacc_kernels pass group in passes.def ]
> >
> > On 20/11/15 11:37, Richard Biener wrote:
> > > I'd rather make loop_optimizer_init do nothing
> > > if requested flags are already set and no fixup is needed and
> > > call the above unconditionally.  Thus sth like
> > >
> > > Index: gcc/loop-init.c
> > > ===================================================================
> > > --- gcc/loop-init.c     (revision 230649)
> > > +++ gcc/loop-init.c     (working copy)
> > > @@ -103,7 +103,11 @@ loop_optimizer_init (unsigned flags)
> > >          calculate_dominance_info (CDI_DOMINATORS);
> > >
> > >          if (!needs_fixup)
> > > -       checking_verify_loop_structure ();
> > > +       {
> > > +         checking_verify_loop_structure ();
> > > +         if (loops_state_satisfies_p (flags))
> > > +           goto out;
> > > +       }
> > >
> > >          /* Clear all flags.  */
> > >          if (recorded_exits)
> > > @@ -122,11 +126,12 @@ loop_optimizer_init (unsigned flags)
> > >      /* Apply flags to loops.  */
> > >      apply_loop_flags (flags);
> > >
> > > +  checking_verify_loop_structure ();
> > > +
> > > +out:
> > >      /* Dump loops.  */
> > >      flow_loops_dump (dump_file, NULL, 1);
> > >
> > > -  checking_verify_loop_structure ();
> > > -
> > >      timevar_pop (TV_LOOP_INIT);
> > >    }
> >
> > This patch implements that approach, but the patch is slightly more
> > complicated because of the need to handle LOOPS_MAY_HAVE_MULTIPLE_LATCHES
> > differently than the rest of the flags.
> >
> > Bootstrapped and reg-tested on x86_64.
> >
> > OK for stage3 trunk?
>
> Let's revisit this during stage1 if the scev_initialized () thing
> SLP vectorization uses works, ok?

OK, I'll give that a try.

FTR, attached two patches are an attempt at a cleaner solution for 
pass_slp_vectorize::execute (in combination with patch "Don't reapply 
loops flags if unnecessary in loop_optimizer_init").

The first patch introduces a property PROP_scev, set for the duration of 
the loop pipeline. It allows us to call scev_initialize and 
scev_finalize unconditionally. Outside the loop pipeline calling the 
functions has the usual effect. Inside the loop pipeline, calling the 
functions has no effect.

The second patch introduces a property PROP_loops_normal_re_lcssa, set 
for the duration of the loop pipeline. It allows us (in combination with 
"Don't reapply loops flags if unnecessary in loop_optimizer_init") to 
call loop_optimizer_init and loop_optimizer_finalize unconditionally.
Outside the loop pipeline, calling the functions has the usual effect. 
Inside the loop pipeline, calling loop_optimizer_finalize has no effect, 
and calling loop_optimizer_initialize has no effect unless a fixup or a 
new loop property is needed.

Thanks,
- Tom

Add PROP_scev

---
 gcc/tree-pass.h             |  1 +
 gcc/tree-scalar-evolution.c | 13 +++++++++++++
 gcc/tree-ssa-loop.c         |  3 ++-
 gcc/tree-vectorizer.c       |  4 ++--
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
index 004db77..4e66b2c 100644
--- a/gcc/tree-pass.h
+++ b/gcc/tree-pass.h
@@ -227,6 +227,7 @@ protected:
 						   of math functions; the
 						   current choices have
 						   been optimized.  */
+#define PROP_scev		(1 << 16)	/* preserve scev info.  */
 
 #define PROP_trees \
   (PROP_gimple_any | PROP_gimple_lcf | PROP_gimple_leh | PROP_gimple_lomp)
diff --git a/gcc/tree-scalar-evolution.c b/gcc/tree-scalar-evolution.c
index 9b33693..5d5e354 100644
--- a/gcc/tree-scalar-evolution.c
+++ b/gcc/tree-scalar-evolution.c
@@ -280,6 +280,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "params.h"
 #include "tree-ssa-propagate.h"
 #include "gimple-fold.h"
+#include "tree-pass.h"
 
 static tree analyze_scalar_evolution_1 (struct loop *, tree, tree);
 static tree analyze_scalar_evolution_for_address_of (struct loop *loop,
@@ -3168,6 +3169,12 @@ scev_initialize (void)
 {
   struct loop *loop;
 
+  if (cfun->curr_properties & PROP_scev)
+    {
+      gcc_assert (scev_initialized_p ());
+      return;
+    }
+
   scalar_evolution_info = hash_table<scev_info_hasher>::create_ggc (100);
 
   initialize_scalar_evolutions_analyzer ();
@@ -3367,6 +3374,12 @@ simple_iv (struct loop *wrto_loop, struct loop *use_loop, tree op,
 void
 scev_finalize (void)
 {
+  if (cfun->curr_properties & PROP_scev)
+    {
+      gcc_assert (scev_initialized_p ());
+      return;
+    }
+
   if (!scalar_evolution_info)
     return;
   scalar_evolution_info->empty ();
diff --git a/gcc/tree-ssa-loop.c b/gcc/tree-ssa-loop.c
index d30e3c8..739fda7 100644
--- a/gcc/tree-ssa-loop.c
+++ b/gcc/tree-ssa-loop.c
@@ -290,7 +290,7 @@ const pass_data pass_data_tree_loop_init =
   OPTGROUP_LOOP, /* optinfo_flags */
   TV_NONE, /* tv_id */
   PROP_cfg, /* properties_required */
-  0, /* properties_provided */
+  PROP_scev, /* properties_provided */
   0, /* properties_destroyed */
   0, /* todo_flags_start */
   0, /* todo_flags_finish */
@@ -524,6 +524,7 @@ make_pass_iv_optimize (gcc::context *ctxt)
 static unsigned int
 tree_ssa_loop_done (void)
 {
+  cfun->curr_properties &= ~PROP_scev;
   free_numbers_of_iterations_estimates (cfun);
   scev_finalize ();
   loop_optimizer_finalize ();
diff --git a/gcc/tree-vectorizer.c b/gcc/tree-vectorizer.c
index b721c56..b06433d 100644
--- a/gcc/tree-vectorizer.c
+++ b/gcc/tree-vectorizer.c
@@ -731,8 +731,8 @@ pass_slp_vectorize::execute (function *fun)
   if (!in_loop_pipeline)
     {
       loop_optimizer_init (LOOPS_NORMAL);
-      scev_initialize ();
     }
+  scev_initialize ();
 
   /* Mark all stmts as not belonging to the current region and unvisited.  */
   FOR_EACH_BB_FN (bb, fun)
@@ -757,9 +757,9 @@ pass_slp_vectorize::execute (function *fun)
 
   free_stmt_vec_info_vec ();
 
+  scev_finalize ();
   if (!in_loop_pipeline)
     {
-      scev_finalize ();
       loop_optimizer_finalize ();
     }
 

Add PROP_loops_normal_re_lcssa

---
 gcc/loop-init.c       | 13 +++++++++++++
 gcc/tree-pass.h       |  3 +++
 gcc/tree-ssa-loop.c   |  4 ++--
 gcc/tree-vectorizer.c | 11 ++---------
 4 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/gcc/loop-init.c b/gcc/loop-init.c
index 4b72cab..9ce3e9e 100644
--- a/gcc/loop-init.c
+++ b/gcc/loop-init.c
@@ -100,6 +100,10 @@ loop_optimizer_init (unsigned flags)
       bool needs_fixup = loops_state_satisfies_p (LOOPS_NEED_FIXUP);
 
       gcc_assert (cfun->curr_properties & PROP_loops);
+      if (cfun->curr_properties & PROP_loops_normal_re_lcssa)
+	gcc_assert (loops_state_satisfies_p (LOOPS_NORMAL
+					     | LOOPS_HAVE_RECORDED_EXITS
+					     | LOOP_CLOSED_SSA));
 
       /* Ensure that the dominators are computed, like flow_loops_find does.  */
       calculate_dominance_info (CDI_DOMINATORS);
@@ -151,6 +155,15 @@ loop_optimizer_finalize (struct function *fn)
   struct loop *loop;
   basic_block bb;
 
+  if (fn->curr_properties & PROP_loops_normal_re_lcssa)
+    {
+      gcc_assert (loops_state_satisfies_p (fn, LOOPS_NORMAL
+					   | LOOPS_HAVE_RECORDED_EXITS
+					   | LOOP_CLOSED_SSA));
+      gcc_assert (!loops_state_satisfies_p (fn, LOOPS_NEED_FIXUP));
+      return;
+    }
+
   timevar_push (TV_LOOP_FINI);
 
   if (loops_state_satisfies_p (fn, LOOPS_HAVE_RECORDED_EXITS))
diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
index 4e66b2c..c43a5f3 100644
--- a/gcc/tree-pass.h
+++ b/gcc/tree-pass.h
@@ -228,6 +228,9 @@ protected:
 						   current choices have
 						   been optimized.  */
 #define PROP_scev		(1 << 16)	/* preserve scev info.  */
+/* preserve loop structures in LOOPS_NORMAL with recorded exits, and in loop
+   closed ssa.  */
+#define PROP_loops_normal_re_lcssa	(1 << 17)
 
 #define PROP_trees \
   (PROP_gimple_any | PROP_gimple_lcf | PROP_gimple_leh | PROP_gimple_lomp)
diff --git a/gcc/tree-ssa-loop.c b/gcc/tree-ssa-loop.c
index 739fda7..73fbb43 100644
--- a/gcc/tree-ssa-loop.c
+++ b/gcc/tree-ssa-loop.c
@@ -290,7 +290,7 @@ const pass_data pass_data_tree_loop_init =
   OPTGROUP_LOOP, /* optinfo_flags */
   TV_NONE, /* tv_id */
   PROP_cfg, /* properties_required */
-  PROP_scev, /* properties_provided */
+  PROP_loops_normal_re_lcssa | PROP_scev, /* properties_provided */
   0, /* properties_destroyed */
   0, /* todo_flags_start */
   0, /* todo_flags_finish */
@@ -524,7 +524,7 @@ make_pass_iv_optimize (gcc::context *ctxt)
 static unsigned int
 tree_ssa_loop_done (void)
 {
-  cfun->curr_properties &= ~PROP_scev;
+  cfun->curr_properties &= ~(PROP_loops_normal_re_lcssa | PROP_scev);
   free_numbers_of_iterations_estimates (cfun);
   scev_finalize ();
   loop_optimizer_finalize ();
diff --git a/gcc/tree-vectorizer.c b/gcc/tree-vectorizer.c
index b06433d..503f227 100644
--- a/gcc/tree-vectorizer.c
+++ b/gcc/tree-vectorizer.c
@@ -727,11 +727,7 @@ pass_slp_vectorize::execute (function *fun)
 {
   basic_block bb;
 
-  bool in_loop_pipeline = scev_initialized_p ();
-  if (!in_loop_pipeline)
-    {
-      loop_optimizer_init (LOOPS_NORMAL);
-    }
+  loop_optimizer_init (LOOPS_NORMAL);
   scev_initialize ();
 
   /* Mark all stmts as not belonging to the current region and unvisited.  */
@@ -758,10 +754,7 @@ pass_slp_vectorize::execute (function *fun)
   free_stmt_vec_info_vec ();
 
   scev_finalize ();
-  if (!in_loop_pipeline)
-    {
-      loop_optimizer_finalize ();
-    }
+  loop_optimizer_finalize ();
 
   return 0;
 }