Unloop no longer looping loops in loop-ch

Unloop no longer looping loops in loop-ch

[Jan Hubicka]

Hi,
I noticed this after adding sanity check that the upper bound on number
of iterations never drop to -1.  It seems to be relatively common case
(happening few hundred times in testsuite and also during bootstrap)
that loop-ch duplicates enough so the loop itself no longer loops.

This is later detected in loop unrolling but since we test the number
of iterations anyway it seems better to do that earlier.

Bootstrapped/regtested x86_64-linux, OK?
Honza

	* cfgloopmanip.h (unloop_loops): Declare.
	* tree-ssa-loop-ch.cc (ch_base::copy_headers): Unloop loops that
	no longer loop.
	* tree-ssa-loop-ivcanon.cc (unloop_loops): Export; do not release
	vectors passed.
	(canonicalize_induction_variables): Release vectors here.
	(tree_unroll_loops_completely): Release vectors here.

diff --git a/gcc/cfgloopmanip.h b/gcc/cfgloopmanip.h
index c40cfeae0e3..75b2a5e9b75 100644
--- a/gcc/cfgloopmanip.h
+++ b/gcc/cfgloopmanip.h
@@ -43,6 +43,10 @@ extern edge create_empty_if_region_on_edge (edge, tree);
 extern class loop *create_empty_loop_on_edge (edge, tree, tree, tree, tree,
 					       tree *, tree *, class loop *);
 extern void unloop (class loop *, bool *, bitmap);
+extern void unloop_loops (vec<class loop *> &loops_to_unloop,
+			  vec<int> &loops_to_unloop_nunroll,
+			  bitmap loop_closed_ssa_invalidated,
+			  bool *irred_invalidated);
 extern void copy_loop_info (class loop *loop, class loop *target);
 extern class loop * duplicate_loop (class loop *, class loop *,
 				     class loop * = NULL);
diff --git a/gcc/gimple-fold.h b/gcc/gimple-fold.h
index 2fd58db9a2e..05f1dd9b500 100644
--- a/gcc/gimple-fold.h
+++ b/gcc/gimple-fold.h
@@ -64,6 +64,7 @@ extern gimple_seq rewrite_to_defined_overflow (gimple *, bool = false);
 extern void replace_call_with_value (gimple_stmt_iterator *, tree);
 extern tree tree_vec_extract (gimple_stmt_iterator *, tree, tree, tree, tree);
 extern void gsi_replace_with_seq_vops (gimple_stmt_iterator *, gimple_seq);
+extern bool can_refer_decl_in_current_unit_p (tree decl, tree from_decl);
 
 /* gimple_build, functionally matching fold_buildN, outputs stmts
    int the provided sequence, matching and simplifying them on-the-fly.
diff --git a/gcc/tree-ssa-loop-ch.cc b/gcc/tree-ssa-loop-ch.cc
index 2fad2a3d7b6..0dfad0f9bfd 100644
--- a/gcc/tree-ssa-loop-ch.cc
+++ b/gcc/tree-ssa-loop-ch.cc
@@ -396,6 +396,8 @@ ch_base::copy_headers (function *fun)
 
   auto_vec<loop_p> candidates;
   auto_vec<std::pair<edge, loop_p> > copied;
+  auto_vec<class loop *> loops_to_unloop;
+  auto_vec<int> loops_to_unloop_nunroll;
 
   mark_dfs_back_edges ();
   gimple_ranger *ranger = new gimple_ranger;
@@ -408,6 +410,14 @@ ch_base::copy_headers (function *fun)
 		 "Analyzing loop %i\n", loop->num);
 
       header = loop->header;
+      if (!get_max_loop_iterations_int (loop))
+	{
+	  if (dump_file && (dump_flags & TDF_DETAILS))
+	    fprintf (dump_file, "Loop %d never loops.\n", loop->num);
+	  loops_to_unloop.safe_push (loop);
+	  loops_to_unloop_nunroll.safe_push (0);
+	  continue;
+	}
 
       /* If the loop is already a do-while style one (either because it was
 	 written as such, or because jump threading transformed it into one),
@@ -593,13 +603,6 @@ ch_base::copy_headers (function *fun)
       /* We possibly decreased number of itrations by 1.  */
       auto_vec<edge> exits = get_loop_exit_edges (loop);
       bool precise = (nexits == (int) exits.length ());
-      if (!get_max_loop_iterations_int (loop))
-	{
-	  if (dump_file && (dump_flags & TDF_DETAILS))
-	    fprintf (dump_file, "Loop %d no longer loops.\n", loop->num);
-	  /* TODO: We can unloop like in tree-ssa-loop-ivcanon.  */
-	  precise = false;
-	}
       /* Check that loop may not terminate in other way than via
 	 basic blocks we duplicated.  */
       if (precise)
@@ -640,7 +643,15 @@ ch_base::copy_headers (function *fun)
 		 precise = false;
 	   }
 	}
-      if (precise)
+      if (precise
+	  && get_max_loop_iterations_int (loop) == 1)
+	{
+	  if (dump_file && (dump_flags & TDF_DETAILS))
+	    fprintf (dump_file, "Loop %d no longer loops.\n", loop->num);
+	  loops_to_unloop.safe_push (loop);
+	  loops_to_unloop_nunroll.safe_push (0);
+	}
+      else if (precise)
 	{
 	  if (dump_file && (dump_flags & TDF_DETAILS))
 	    fprintf (dump_file,
@@ -688,6 +699,13 @@ ch_base::copy_headers (function *fun)
 	  BITMAP_FREE (exit_bbs);
 	}
     }
+  if (loops_to_unloop.length())
+    {
+      bool irred_invalidated;
+      unloop_loops (loops_to_unloop, loops_to_unloop_nunroll, NULL, &irred_invalidated);
+      changed = true;
+      fprintf (stderr, "Bingo\n");
+    }
   free (bbs);
   free (copied_bbs);
 
diff --git a/gcc/tree-ssa-loop-ivcanon.cc b/gcc/tree-ssa-loop-ivcanon.cc
index 9f72d534b7c..ce9712058d4 100644
--- a/gcc/tree-ssa-loop-ivcanon.cc
+++ b/gcc/tree-ssa-loop-ivcanon.cc
@@ -618,8 +618,10 @@ static bitmap peeled_loops;
    LOOP_CLOSED_SSA_INVALIDATED is used to bookkepp the case
    when we need to go into loop closed SSA form.  */
 
-static void
-unloop_loops (bitmap loop_closed_ssa_invalidated,
+void
+unloop_loops (vec<class loop *> &loops_to_unloop,
+	      vec<int> &loops_to_unloop_nunroll,
+	      bitmap loop_closed_ssa_invalidated,
 	      bool *irred_invalidated)
 {
   while (loops_to_unloop.length ())
@@ -653,8 +655,6 @@ unloop_loops (bitmap loop_closed_ssa_invalidated,
       gsi = gsi_start_bb (latch_edge->dest);
       gsi_insert_after (&gsi, stmt, GSI_NEW_STMT);
     }
-  loops_to_unloop.release ();
-  loops_to_unloop_nunroll.release ();
 
   /* Remove edges in peeled copies.  Given remove_path removes dominated
      regions we need to cope with removal of already removed paths.  */
@@ -1326,7 +1326,10 @@ canonicalize_induction_variables (void)
     }
   gcc_assert (!need_ssa_update_p (cfun));
 
-  unloop_loops (loop_closed_ssa_invalidated, &irred_invalidated);
+  unloop_loops (loops_to_unloop, loops_to_unloop_nunroll,
+		loop_closed_ssa_invalidated, &irred_invalidated);
+  loops_to_unloop.release ();
+  loops_to_unloop_nunroll.release ();
   if (irred_invalidated
       && loops_state_satisfies_p (LOOPS_HAVE_MARKED_IRREDUCIBLE_REGIONS))
     mark_irreducible_loops ();
@@ -1473,7 +1476,12 @@ tree_unroll_loops_completely (bool may_increase_size, bool unroll_outer)
 	{
 	  unsigned i;
 
-          unloop_loops (loop_closed_ssa_invalidated, &irred_invalidated);
+	  unloop_loops (loops_to_unloop,
+			loops_to_unloop_nunroll,
+			loop_closed_ssa_invalidated,
+			&irred_invalidated);
+	  loops_to_unloop.release ();
+	  loops_to_unloop_nunroll.release ();
 
 	  /* We cannot use TODO_update_ssa_no_phi because VOPS gets confused.  */
 	  if (loop_closed_ssa_invalidated

Re: Unloop no longer looping loops in loop-ch

[Bernhard Reutner-Fischer]

On 25 April 2023 17:12:50 CEST, Jan Hubicka via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:

+      fprintf (stderr, "Bingo\n");

You forgot to remove that..
Do we prune Bingo in the testsuite? ;-)

Re: Unloop no longer looping loops in loop-ch

[Jan Hubicka]

> On 25 April 2023 17:12:50 CEST, Jan Hubicka via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> +      fprintf (stderr, "Bingo\n");
> 
> You forgot to remove that..
> Do we prune Bingo in the testsuite? ;-)
Ah, thanks :)
I was curious how much I win with unlooping.
Updated patch is attached...

	* cfgloopmanip.h (unloop_loops): Declare.
	* tree-ssa-loop-ch.cc (ch_base::copy_headers): Unloop loops that
	no longer loop.
	* tree-ssa-loop-ivcanon.cc (unloop_loops): Export; do not release
	vectors passed.
	(canonicalize_induction_variables): Release vectors here.
	(tree_unroll_loops_completely): Release vectors here.

diff --git a/gcc/cfgloopmanip.h b/gcc/cfgloopmanip.h
index c40cfeae0e3..75b2a5e9b75 100644
--- a/gcc/cfgloopmanip.h
+++ b/gcc/cfgloopmanip.h
@@ -43,6 +43,10 @@ extern edge create_empty_if_region_on_edge (edge, tree);
 extern class loop *create_empty_loop_on_edge (edge, tree, tree, tree, tree,
 					       tree *, tree *, class loop *);
 extern void unloop (class loop *, bool *, bitmap);
+extern void unloop_loops (vec<class loop *> &loops_to_unloop,
+			  vec<int> &loops_to_unloop_nunroll,
+			  bitmap loop_closed_ssa_invalidated,
+			  bool *irred_invalidated);
 extern void copy_loop_info (class loop *loop, class loop *target);
 extern class loop * duplicate_loop (class loop *, class loop *,
 				     class loop * = NULL);
diff --git a/gcc/gimple-fold.h b/gcc/gimple-fold.h
index 2fd58db9a2e..05f1dd9b500 100644
--- a/gcc/gimple-fold.h
+++ b/gcc/gimple-fold.h
@@ -64,6 +64,7 @@ extern gimple_seq rewrite_to_defined_overflow (gimple *, bool = false);
 extern void replace_call_with_value (gimple_stmt_iterator *, tree);
 extern tree tree_vec_extract (gimple_stmt_iterator *, tree, tree, tree, tree);
 extern void gsi_replace_with_seq_vops (gimple_stmt_iterator *, gimple_seq);
+extern bool can_refer_decl_in_current_unit_p (tree decl, tree from_decl);
 
 /* gimple_build, functionally matching fold_buildN, outputs stmts
    int the provided sequence, matching and simplifying them on-the-fly.
diff --git a/gcc/tree-ssa-loop-ch.cc b/gcc/tree-ssa-loop-ch.cc
index 2fad2a3d7b6..0dfad0f9bfd 100644
--- a/gcc/tree-ssa-loop-ch.cc
+++ b/gcc/tree-ssa-loop-ch.cc
@@ -396,6 +396,8 @@ ch_base::copy_headers (function *fun)
 
   auto_vec<loop_p> candidates;
   auto_vec<std::pair<edge, loop_p> > copied;
+  auto_vec<class loop *> loops_to_unloop;
+  auto_vec<int> loops_to_unloop_nunroll;
 
   mark_dfs_back_edges ();
   gimple_ranger *ranger = new gimple_ranger;
@@ -408,6 +410,14 @@ ch_base::copy_headers (function *fun)
 		 "Analyzing loop %i\n", loop->num);
 
       header = loop->header;
+      if (!get_max_loop_iterations_int (loop))
+	{
+	  if (dump_file && (dump_flags & TDF_DETAILS))
+	    fprintf (dump_file, "Loop %d never loops.\n", loop->num);
+	  loops_to_unloop.safe_push (loop);
+	  loops_to_unloop_nunroll.safe_push (0);
+	  continue;
+	}
 
       /* If the loop is already a do-while style one (either because it was
 	 written as such, or because jump threading transformed it into one),
@@ -593,13 +603,6 @@ ch_base::copy_headers (function *fun)
       /* We possibly decreased number of itrations by 1.  */
       auto_vec<edge> exits = get_loop_exit_edges (loop);
       bool precise = (nexits == (int) exits.length ());
-      if (!get_max_loop_iterations_int (loop))
-	{
-	  if (dump_file && (dump_flags & TDF_DETAILS))
-	    fprintf (dump_file, "Loop %d no longer loops.\n", loop->num);
-	  /* TODO: We can unloop like in tree-ssa-loop-ivcanon.  */
-	  precise = false;
-	}
       /* Check that loop may not terminate in other way than via
 	 basic blocks we duplicated.  */
       if (precise)
@@ -640,7 +643,15 @@ ch_base::copy_headers (function *fun)
 		 precise = false;
 	   }
 	}
-      if (precise)
+      if (precise
+	  && get_max_loop_iterations_int (loop) == 1)
+	{
+	  if (dump_file && (dump_flags & TDF_DETAILS))
+	    fprintf (dump_file, "Loop %d no longer loops.\n", loop->num);
+	  loops_to_unloop.safe_push (loop);
+	  loops_to_unloop_nunroll.safe_push (0);
+	}
+      else if (precise)
 	{
 	  if (dump_file && (dump_flags & TDF_DETAILS))
 	    fprintf (dump_file,
@@ -688,6 +699,12 @@ ch_base::copy_headers (function *fun)
 	  BITMAP_FREE (exit_bbs);
 	}
     }
+  if (loops_to_unloop.length())
+    {
+      bool irred_invalidated;
+      unloop_loops (loops_to_unloop, loops_to_unloop_nunroll, NULL, &irred_invalidated);
+      changed = true;
+    }
   free (bbs);
   free (copied_bbs);
 
diff --git a/gcc/tree-ssa-loop-ivcanon.cc b/gcc/tree-ssa-loop-ivcanon.cc
index 9f72d534b7c..ce9712058d4 100644
--- a/gcc/tree-ssa-loop-ivcanon.cc
+++ b/gcc/tree-ssa-loop-ivcanon.cc
@@ -618,8 +618,10 @@ static bitmap peeled_loops;
    LOOP_CLOSED_SSA_INVALIDATED is used to bookkepp the case
    when we need to go into loop closed SSA form.  */
 
-static void
-unloop_loops (bitmap loop_closed_ssa_invalidated,
+void
+unloop_loops (vec<class loop *> &loops_to_unloop,
+	      vec<int> &loops_to_unloop_nunroll,
+	      bitmap loop_closed_ssa_invalidated,
 	      bool *irred_invalidated)
 {
   while (loops_to_unloop.length ())
@@ -653,8 +655,6 @@ unloop_loops (bitmap loop_closed_ssa_invalidated,
       gsi = gsi_start_bb (latch_edge->dest);
       gsi_insert_after (&gsi, stmt, GSI_NEW_STMT);
     }
-  loops_to_unloop.release ();
-  loops_to_unloop_nunroll.release ();
 
   /* Remove edges in peeled copies.  Given remove_path removes dominated
      regions we need to cope with removal of already removed paths.  */
@@ -1326,7 +1326,10 @@ canonicalize_induction_variables (void)
     }
   gcc_assert (!need_ssa_update_p (cfun));
 
-  unloop_loops (loop_closed_ssa_invalidated, &irred_invalidated);
+  unloop_loops (loops_to_unloop, loops_to_unloop_nunroll,
+		loop_closed_ssa_invalidated, &irred_invalidated);
+  loops_to_unloop.release ();
+  loops_to_unloop_nunroll.release ();
   if (irred_invalidated
       && loops_state_satisfies_p (LOOPS_HAVE_MARKED_IRREDUCIBLE_REGIONS))
     mark_irreducible_loops ();
@@ -1473,7 +1476,12 @@ tree_unroll_loops_completely (bool may_increase_size, bool unroll_outer)
 	{
 	  unsigned i;
 
-          unloop_loops (loop_closed_ssa_invalidated, &irred_invalidated);
+	  unloop_loops (loops_to_unloop,
+			loops_to_unloop_nunroll,
+			loop_closed_ssa_invalidated,
+			&irred_invalidated);
+	  loops_to_unloop.release ();
+	  loops_to_unloop_nunroll.release ();
 
 	  /* We cannot use TODO_update_ssa_no_phi because VOPS gets confused.  */
 	  if (loop_closed_ssa_invalidated

Re: Unloop no longer looping loops in loop-ch

[Richard Biener]

On Tue, 25 Apr 2023, Jan Hubicka wrote:

> > On 25 April 2023 17:12:50 CEST, Jan Hubicka via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> > 
> > +      fprintf (stderr, "Bingo\n");
> > 
> > You forgot to remove that..
> > Do we prune Bingo in the testsuite? ;-)
> Ah, thanks :)
> I was curious how much I win with unlooping.
> Updated patch is attached...
> 
> 	* cfgloopmanip.h (unloop_loops): Declare.
> 	* tree-ssa-loop-ch.cc (ch_base::copy_headers): Unloop loops that
> 	no longer loop.
> 	* tree-ssa-loop-ivcanon.cc (unloop_loops): Export; do not release
> 	vectors passed.
> 	(canonicalize_induction_variables): Release vectors here.
> 	(tree_unroll_loops_completely): Release vectors here.
> 
> diff --git a/gcc/cfgloopmanip.h b/gcc/cfgloopmanip.h
> index c40cfeae0e3..75b2a5e9b75 100644
> --- a/gcc/cfgloopmanip.h
> +++ b/gcc/cfgloopmanip.h
> @@ -43,6 +43,10 @@ extern edge create_empty_if_region_on_edge (edge, tree);
>  extern class loop *create_empty_loop_on_edge (edge, tree, tree, tree, tree,
>  					       tree *, tree *, class loop *);
>  extern void unloop (class loop *, bool *, bitmap);
> +extern void unloop_loops (vec<class loop *> &loops_to_unloop,
> +			  vec<int> &loops_to_unloop_nunroll,
> +			  bitmap loop_closed_ssa_invalidated,
> +			  bool *irred_invalidated);
>  extern void copy_loop_info (class loop *loop, class loop *target);
>  extern class loop * duplicate_loop (class loop *, class loop *,
>  				     class loop * = NULL);
> diff --git a/gcc/gimple-fold.h b/gcc/gimple-fold.h
> index 2fd58db9a2e..05f1dd9b500 100644
> --- a/gcc/gimple-fold.h
> +++ b/gcc/gimple-fold.h
> @@ -64,6 +64,7 @@ extern gimple_seq rewrite_to_defined_overflow (gimple *, bool = false);
>  extern void replace_call_with_value (gimple_stmt_iterator *, tree);
>  extern tree tree_vec_extract (gimple_stmt_iterator *, tree, tree, tree, tree);
>  extern void gsi_replace_with_seq_vops (gimple_stmt_iterator *, gimple_seq);
> +extern bool can_refer_decl_in_current_unit_p (tree decl, tree from_decl);
>  
>  /* gimple_build, functionally matching fold_buildN, outputs stmts
>     int the provided sequence, matching and simplifying them on-the-fly.
> diff --git a/gcc/tree-ssa-loop-ch.cc b/gcc/tree-ssa-loop-ch.cc
> index 2fad2a3d7b6..0dfad0f9bfd 100644
> --- a/gcc/tree-ssa-loop-ch.cc
> +++ b/gcc/tree-ssa-loop-ch.cc
> @@ -396,6 +396,8 @@ ch_base::copy_headers (function *fun)
>  
>    auto_vec<loop_p> candidates;
>    auto_vec<std::pair<edge, loop_p> > copied;
> +  auto_vec<class loop *> loops_to_unloop;
> +  auto_vec<int> loops_to_unloop_nunroll;
>  
>    mark_dfs_back_edges ();
>    gimple_ranger *ranger = new gimple_ranger;
> @@ -408,6 +410,14 @@ ch_base::copy_headers (function *fun)
>  		 "Analyzing loop %i\n", loop->num);
>  
>        header = loop->header;
> +      if (!get_max_loop_iterations_int (loop))
> +	{
> +	  if (dump_file && (dump_flags & TDF_DETAILS))
> +	    fprintf (dump_file, "Loop %d never loops.\n", loop->num);
> +	  loops_to_unloop.safe_push (loop);
> +	  loops_to_unloop_nunroll.safe_push (0);
> +	  continue;
> +	}
>  
>        /* If the loop is already a do-while style one (either because it was
>  	 written as such, or because jump threading transformed it into one),
> @@ -593,13 +603,6 @@ ch_base::copy_headers (function *fun)
>        /* We possibly decreased number of itrations by 1.  */
>        auto_vec<edge> exits = get_loop_exit_edges (loop);
>        bool precise = (nexits == (int) exits.length ());
> -      if (!get_max_loop_iterations_int (loop))
> -	{
> -	  if (dump_file && (dump_flags & TDF_DETAILS))
> -	    fprintf (dump_file, "Loop %d no longer loops.\n", loop->num);
> -	  /* TODO: We can unloop like in tree-ssa-loop-ivcanon.  */
> -	  precise = false;
> -	}
>        /* Check that loop may not terminate in other way than via
>  	 basic blocks we duplicated.  */
>        if (precise)
> @@ -640,7 +643,15 @@ ch_base::copy_headers (function *fun)
>  		 precise = false;
>  	   }
>  	}
> -      if (precise)
> +      if (precise
> +	  && get_max_loop_iterations_int (loop) == 1)
> +	{
> +	  if (dump_file && (dump_flags & TDF_DETAILS))
> +	    fprintf (dump_file, "Loop %d no longer loops.\n", loop->num);

but max loop iterations is 1 ...?

> +	  loops_to_unloop.safe_push (loop);
> +	  loops_to_unloop_nunroll.safe_push (0);
> +	}
> +      else if (precise)
>  	{
>  	  if (dump_file && (dump_flags & TDF_DETAILS))
>  	    fprintf (dump_file,
> @@ -688,6 +699,12 @@ ch_base::copy_headers (function *fun)
>  	  BITMAP_FREE (exit_bbs);
>  	}
>      }
> +  if (loops_to_unloop.length())

  !loops_to_unloop.is_empty ()

> +    {
> +      bool irred_invalidated;
> +      unloop_loops (loops_to_unloop, loops_to_unloop_nunroll, NULL, &irred_invalidated);
> +      changed = true;
> +    }
>    free (bbs);
>    free (copied_bbs);


Since we run VN on the header copies I wonder if, since you remove
edges, we need to run CFG cleanup before this and updating SSA form?
For safety we usually let CFG cleanup do the actual CFG manipulation
and just change cond jumps to if (0) or if (1)?

> diff --git a/gcc/tree-ssa-loop-ivcanon.cc b/gcc/tree-ssa-loop-ivcanon.cc
> index 9f72d534b7c..ce9712058d4 100644
> --- a/gcc/tree-ssa-loop-ivcanon.cc
> +++ b/gcc/tree-ssa-loop-ivcanon.cc
> @@ -618,8 +618,10 @@ static bitmap peeled_loops;
>     LOOP_CLOSED_SSA_INVALIDATED is used to bookkepp the case
>     when we need to go into loop closed SSA form.  */
>  
> -static void
> -unloop_loops (bitmap loop_closed_ssa_invalidated,
> +void
> +unloop_loops (vec<class loop *> &loops_to_unloop,
> +	      vec<int> &loops_to_unloop_nunroll,
> +	      bitmap loop_closed_ssa_invalidated,
>  	      bool *irred_invalidated)
>  {
>    while (loops_to_unloop.length ())
> @@ -653,8 +655,6 @@ unloop_loops (bitmap loop_closed_ssa_invalidated,
>        gsi = gsi_start_bb (latch_edge->dest);
>        gsi_insert_after (&gsi, stmt, GSI_NEW_STMT);
>      }
> -  loops_to_unloop.release ();
> -  loops_to_unloop_nunroll.release ();
>  
>    /* Remove edges in peeled copies.  Given remove_path removes dominated
>       regions we need to cope with removal of already removed paths.  */
> @@ -1326,7 +1326,10 @@ canonicalize_induction_variables (void)
>      }
>    gcc_assert (!need_ssa_update_p (cfun));
>  
> -  unloop_loops (loop_closed_ssa_invalidated, &irred_invalidated);
> +  unloop_loops (loops_to_unloop, loops_to_unloop_nunroll,
> +		loop_closed_ssa_invalidated, &irred_invalidated);
> +  loops_to_unloop.release ();
> +  loops_to_unloop_nunroll.release ();
>    if (irred_invalidated
>        && loops_state_satisfies_p (LOOPS_HAVE_MARKED_IRREDUCIBLE_REGIONS))
>      mark_irreducible_loops ();
> @@ -1473,7 +1476,12 @@ tree_unroll_loops_completely (bool may_increase_size, bool unroll_outer)
>  	{
>  	  unsigned i;
>  
> -          unloop_loops (loop_closed_ssa_invalidated, &irred_invalidated);
> +	  unloop_loops (loops_to_unloop,
> +			loops_to_unloop_nunroll,
> +			loop_closed_ssa_invalidated,
> +			&irred_invalidated);
> +	  loops_to_unloop.release ();
> +	  loops_to_unloop_nunroll.release ();
>  
>  	  /* We cannot use TODO_update_ssa_no_phi because VOPS gets confused.  */
>  	  if (loop_closed_ssa_invalidated
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)

Re: Unloop no longer looping loops in loop-ch

[Jan Hubicka]

> > -      if (precise)
> > +      if (precise
> > +	  && get_max_loop_iterations_int (loop) == 1)
> > +	{
> > +	  if (dump_file && (dump_flags & TDF_DETAILS))
> > +	    fprintf (dump_file, "Loop %d no longer loops.\n", loop->num);
> 
> but max loop iterations is 1 ...?

I first check for loops with 0 iterations, push them to unlooping list
and avoid any header copying (it is useless).
At this patch we already did header duplication and verified that the
maximal number of iterations will drop by 1 since there is no way loop
can terminate except for the header tests we peeled out.

So 1 would turn to 0 in the loop info update and it seems useless to do
it.
> 
> > +	  loops_to_unloop.safe_push (loop);
> > +	  loops_to_unloop_nunroll.safe_push (0);
> > +	}
> > +      else if (precise)
> >  	{
> >  	  if (dump_file && (dump_flags & TDF_DETAILS))
> >  	    fprintf (dump_file,
> > @@ -688,6 +699,12 @@ ch_base::copy_headers (function *fun)
> >  	  BITMAP_FREE (exit_bbs);
> >  	}
> >      }
> > +  if (loops_to_unloop.length())
> 
>   !loops_to_unloop.is_empty ()
I updated that in my copy of the patch.
> 
> > +    {
> > +      bool irred_invalidated;
> > +      unloop_loops (loops_to_unloop, loops_to_unloop_nunroll, NULL, &irred_invalidated);
> > +      changed = true;
> > +    }
> >    free (bbs);
> >    free (copied_bbs);
> 
> 
> Since we run VN on the header copies I wonder if, since you remove
> edges, we need to run CFG cleanup before this and updating SSA form?
> For safety we usually let CFG cleanup do the actual CFG manipulation
> and just change cond jumps to if (0) or if (1)?

I do unlooping only after the VN so I think I am safe here.

Honza

Re: Unloop no longer looping loops in loop-ch

[Richard Biener]

On Wed, 26 Apr 2023, Jan Hubicka wrote:

> > > -      if (precise)
> > > +      if (precise
> > > +	  && get_max_loop_iterations_int (loop) == 1)
> > > +	{
> > > +	  if (dump_file && (dump_flags & TDF_DETAILS))
> > > +	    fprintf (dump_file, "Loop %d no longer loops.\n", loop->num);
> > 
> > but max loop iterations is 1 ...?
> 
> I first check for loops with 0 iterations, push them to unlooping list
> and avoid any header copying (it is useless).
> At this patch we already did header duplication and verified that the
> maximal number of iterations will drop by 1 since there is no way loop
> can terminate except for the header tests we peeled out.
> 
> So 1 would turn to 0 in the loop info update and it seems useless to do
> it.
> > 
> > > +	  loops_to_unloop.safe_push (loop);
> > > +	  loops_to_unloop_nunroll.safe_push (0);
> > > +	}
> > > +      else if (precise)
> > >  	{
> > >  	  if (dump_file && (dump_flags & TDF_DETAILS))
> > >  	    fprintf (dump_file,
> > > @@ -688,6 +699,12 @@ ch_base::copy_headers (function *fun)
> > >  	  BITMAP_FREE (exit_bbs);
> > >  	}
> > >      }
> > > +  if (loops_to_unloop.length())
> > 
> >   !loops_to_unloop.is_empty ()
> I updated that in my copy of the patch.
> > 
> > > +    {
> > > +      bool irred_invalidated;
> > > +      unloop_loops (loops_to_unloop, loops_to_unloop_nunroll, NULL, &irred_invalidated);
> > > +      changed = true;
> > > +    }
> > >    free (bbs);
> > >    free (copied_bbs);
> > 
> > 
> > Since we run VN on the header copies I wonder if, since you remove
> > edges, we need to run CFG cleanup before this and updating SSA form?
> > For safety we usually let CFG cleanup do the actual CFG manipulation
> > and just change cond jumps to if (0) or if (1)?
> 
> I do unlooping only after the VN so I think I am safe here.

Ah OK.

The patch is OK then.

Thanks,
Richard.