Re: [PATCH v2 3/4] Rename loop_version to clone_loop_to_header_edge.

[Richard Biener <rguenther@suse.de>]

On Wed, 27 Oct 2021, Xionghu Luo wrote:

> Name loop_copy is used in gcc/cfg.c already.

But it's not actually cloning the loop to the header edge but doing
more - adding a condition and re-wiring edges so the cloned loop
is no longer on the header edge.

So no, I don't think this is good to go.  If we can make use
of the part of loop_version that does this we should instead
split out a new API but keep loop_version with its name but
using clone_loop_to_header_edge then.

But we can leave that when we have use of such API.  I originally
thought loop splitting might be able to use such API to make
the transform it does clearer - but whether that's the case is
left as an exercise to the reader.

Thanks,
Richard.

> gcc/ChangeLog:
> 
> 	* cfgloopmanip.c (force_single_succ_latches): Rename
> 	loop_version to clone_loop_to_header_edge.
> 	(lv_adjust_loop_entry_edge): Likewise.
> 	(loop_version): Likewise.
> 	(clone_loop_to_header_edge): Likewise.
> 	* cfgloopmanip.h (class loop): Likewise.
> 	(clone_loop_to_header_edge): Likewise.
> 	* gimple-loop-versioning.cc (loop_versioning::version_loop): Likewise.
> 	* modulo-sched.c (sms_schedule): Likewise.
> 	* tree-if-conv.c (version_loop_for_if_conversion): Likewise.
> 	* tree-loop-distribution.c (version_loop_by_alias_check): Likewise.
> 	* tree-parloops.c (gen_parallel_loop): Likewise.
> 	* tree-ssa-loop-manip.c (tree_transform_and_unroll_loop): Likewise.
> 	* tree-ssa-loop-split.c (split_loop): Likewise.
> 	(get_cond_branch_to_split_loop): Likewise.
> 	(do_split_loop_on_cond): Likewise.
> 	* tree-ssa-loop-unswitch.c (tree_unswitch_loop): Likewise.
> 	* tree-vect-loop-manip.c (vect_loop_versioning): Likewise.
> ---
>  gcc/cfgloopmanip.c            | 22 +++++++++++-----------
>  gcc/cfgloopmanip.h            |  8 ++++----
>  gcc/gimple-loop-versioning.cc | 11 ++++++-----
>  gcc/modulo-sched.c            |  6 +++---
>  gcc/tree-if-conv.c            | 13 +++++++------
>  gcc/tree-loop-distribution.c  |  4 ++--
>  gcc/tree-parloops.c           | 10 +++++-----
>  gcc/tree-ssa-loop-manip.c     |  9 +++++----
>  gcc/tree-ssa-loop-split.c     | 30 +++++++++++++++---------------
>  gcc/tree-ssa-loop-unswitch.c  |  8 +++-----
>  gcc/tree-vect-loop-manip.c    |  5 +++--
>  11 files changed, 64 insertions(+), 62 deletions(-)
> 
> diff --git a/gcc/cfgloopmanip.c b/gcc/cfgloopmanip.c
> index a30ebe1cdb4..066fbddbcfe 100644
> --- a/gcc/cfgloopmanip.c
> +++ b/gcc/cfgloopmanip.c
> @@ -1535,11 +1535,10 @@ force_single_succ_latches (void)
>    loops_state_set (LOOPS_HAVE_SIMPLE_LATCHES);
>  }
>  
> -/* This function is called from loop_version.  It splits the entry edge
> -   of the loop we want to version, adds the versioning condition, and
> -   adjust the edges to the two versions of the loop appropriately.
> -   e is an incoming edge. Returns the basic block containing the
> -   condition.
> +/* This function is called from clone_loop_to_header_edge.  It splits the entry
> +  edge of the loop we want to version, adds the versioning condition, and adjust
> +  the edges to the two versions of the loop appropriately. e is an incoming
> +  edge. Returns the basic block containing the condition.
>  
>     --- edge e ---- > [second_head]
>  
> @@ -1588,7 +1587,7 @@ lv_adjust_loop_entry_edge (basic_block first_head, basic_block second_head,
>    return new_head;
>  }
>  
> -/* Main entry point for Loop Versioning transformation.
> +/* Main entry point for Loop copy transformation.
>  
>     This transformation given a condition and a loop, creates
>     -if (condition) { loop_copy1 } else { loop_copy2 },
> @@ -1609,11 +1608,12 @@ lv_adjust_loop_entry_edge (basic_block first_head, basic_block second_head,
>     instruction stream, otherwise it is placed before LOOP.  */
>  
>  class loop *
> -loop_version (class loop *loop,
> -	      void *cond_expr, basic_block *condition_bb,
> -	      profile_probability then_prob, profile_probability else_prob,
> -	      profile_probability then_scale, profile_probability else_scale,
> -	      bool place_after)
> +clone_loop_to_header_edge (class loop *loop, void *cond_expr,
> +			   basic_block *condition_bb,
> +			   profile_probability then_prob,
> +			   profile_probability else_prob,
> +			   profile_probability then_scale,
> +			   profile_probability else_scale, bool place_after)
>  {
>    basic_block first_head, second_head;
>    edge entry, latch_edge;
> diff --git a/gcc/cfgloopmanip.h b/gcc/cfgloopmanip.h
> index 312a3b48d05..eac09518702 100644
> --- a/gcc/cfgloopmanip.h
> +++ b/gcc/cfgloopmanip.h
> @@ -55,9 +55,9 @@ extern bool mfb_keep_just (edge);
>  basic_block create_preheader (class loop *, int);
>  extern void create_preheaders (int);
>  extern void force_single_succ_latches (void);
> -class loop * loop_version (class loop *, void *,
> -			    basic_block *,
> -			    profile_probability, profile_probability,
> -			    profile_probability, profile_probability, bool);
> +class loop *
> +clone_loop_to_header_edge (class loop *, void *, basic_block *,
> +			   profile_probability, profile_probability,
> +			   profile_probability, profile_probability, bool);
>  
>  #endif /* GCC_CFGLOOPMANIP_H */
> diff --git a/gcc/gimple-loop-versioning.cc b/gcc/gimple-loop-versioning.cc
> index 15e0803dc29..c0fa60b91f8 100644
> --- a/gcc/gimple-loop-versioning.cc
> +++ b/gcc/gimple-loop-versioning.cc
> @@ -1686,11 +1686,12 @@ loop_versioning::version_loop (class loop *loop)
>    /* Version the loop.  */
>    initialize_original_copy_tables ();
>    basic_block cond_bb;
> -  li.optimized_loop = loop_version (loop, cond, &cond_bb,
> -				    profile_probability::unlikely (),
> -				    profile_probability::likely (),
> -				    profile_probability::unlikely (),
> -				    profile_probability::likely (), true);
> +  li.optimized_loop
> +    = clone_loop_to_header_edge (loop, cond, &cond_bb,
> +				 profile_probability::unlikely (),
> +				 profile_probability::likely (),
> +				 profile_probability::unlikely (),
> +				 profile_probability::likely (), true);
>    free_original_copy_tables ();
>    if (!li.optimized_loop)
>      {
> diff --git a/gcc/modulo-sched.c b/gcc/modulo-sched.c
> index 1c1b459d34f..26cd76a279c 100644
> --- a/gcc/modulo-sched.c
> +++ b/gcc/modulo-sched.c
> @@ -1737,9 +1737,9 @@ sms_schedule (void)
>  	      profile_probability prob = profile_probability::guessed_always ()
>  				.apply_scale (PROB_SMS_ENOUGH_ITERATIONS, 100);
>  
> -	      loop_version (loop, comp_rtx, &condition_bb,
> -	  		    prob, prob.invert (),
> -			    prob, prob.invert (), true);
> +	      clone_loop_to_header_edge (loop, comp_rtx, &condition_bb, prob,
> +					 prob.invert (), prob, prob.invert (),
> +					 true);
>  	    }
>  
>  	  /* Now apply the scheduled kernel to the RTL of the loop.  */
> diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c
> index d7b7b309309..893231a2d16 100644
> --- a/gcc/tree-if-conv.c
> +++ b/gcc/tree-if-conv.c
> @@ -2871,7 +2871,8 @@ version_loop_for_if_conversion (class loop *loop, vec<gimple *> *preds)
>  				  integer_zero_node);
>    gimple_call_set_lhs (g, cond);
>  
> -  /* Save BB->aux around loop_version as that uses the same field.  */
> +  /* Save BB->aux around clone_loop_to_header_edge as that uses the same field.
> +   */
>    save_length = loop->inner ? loop->inner->num_nodes : loop->num_nodes;
>    void **saved_preds = XALLOCAVEC (void *, save_length);
>    for (unsigned i = 0; i < save_length; i++)
> @@ -2880,11 +2881,11 @@ version_loop_for_if_conversion (class loop *loop, vec<gimple *> *preds)
>    initialize_original_copy_tables ();
>    /* At this point we invalidate porfile confistency until IFN_LOOP_VECTORIZED
>       is re-merged in the vectorizer.  */
> -  new_loop = loop_version (loop, cond, &cond_bb,
> -			   profile_probability::always (),
> -			   profile_probability::always (),
> -			   profile_probability::always (),
> -			   profile_probability::always (), true);
> +  new_loop = clone_loop_to_header_edge (loop, cond, &cond_bb,
> +					profile_probability::always (),
> +					profile_probability::always (),
> +					profile_probability::always (),
> +					profile_probability::always (), true);
>    free_original_copy_tables ();
>  
>    for (unsigned i = 0; i < save_length; i++)
> diff --git a/gcc/tree-loop-distribution.c b/gcc/tree-loop-distribution.c
> index 2df762c8aa8..ae543fa022d 100644
> --- a/gcc/tree-loop-distribution.c
> +++ b/gcc/tree-loop-distribution.c
> @@ -2705,8 +2705,8 @@ version_loop_by_alias_check (vec<struct partition *> *partitions,
>  
>    prob = profile_probability::guessed_always ().apply_scale (9, 10);
>    initialize_original_copy_tables ();
> -  nloop = loop_version (loop, lhs, &cond_bb, prob, prob.invert (),
> -			prob, prob.invert (), true);
> +  nloop = clone_loop_to_header_edge (loop, lhs, &cond_bb, prob, prob.invert (),
> +				     prob, prob.invert (), true);
>    free_original_copy_tables ();
>    /* Record the original loop number in newly generated loops.  In case of
>       distribution, the original loop will be distributed and the new loop
> diff --git a/gcc/tree-parloops.c b/gcc/tree-parloops.c
> index 5e64d5ed7a3..e19fc52f38b 100644
> --- a/gcc/tree-parloops.c
> +++ b/gcc/tree-parloops.c
> @@ -3083,11 +3083,11 @@ gen_parallel_loop (class loop *loop,
>        initialize_original_copy_tables ();
>  
>        /* We assume that the loop usually iterates a lot.  */
> -      loop_version (loop, many_iterations_cond, NULL,
> -		    profile_probability::likely (),
> -		    profile_probability::unlikely (),
> -		    profile_probability::likely (),
> -		    profile_probability::unlikely (), true);
> +      clone_loop_to_header_edge (loop, many_iterations_cond, NULL,
> +				 profile_probability::likely (),
> +				 profile_probability::unlikely (),
> +				 profile_probability::likely (),
> +				 profile_probability::unlikely (), true);
>        update_ssa (TODO_update_ssa);
>        free_original_copy_tables ();
>      }
> diff --git a/gcc/tree-ssa-loop-manip.c b/gcc/tree-ssa-loop-manip.c
> index c7a2f67b129..350e25bb8d2 100644
> --- a/gcc/tree-ssa-loop-manip.c
> +++ b/gcc/tree-ssa-loop-manip.c
> @@ -1282,10 +1282,11 @@ tree_transform_and_unroll_loop (class loop *loop, unsigned factor,
>  	 frequencies in its body because of this change (scale the frequencies
>  	 of blocks before and after the exit by appropriate factors).  */
>        profile_probability scale_unrolled = prob_entry;
> -      new_loop = loop_version (loop, enter_main_cond, NULL, prob_entry,
> -			       prob_entry.invert (), scale_unrolled,
> -			       profile_probability::guessed_always (),
> -			       true);
> +      new_loop
> +	= clone_loop_to_header_edge (loop, enter_main_cond, NULL, prob_entry,
> +				     prob_entry.invert (), scale_unrolled,
> +				     profile_probability::guessed_always (),
> +				     true);
>        gcc_assert (new_loop != NULL);
>        update_ssa (TODO_update_ssa);
>  
> diff --git a/gcc/tree-ssa-loop-split.c b/gcc/tree-ssa-loop-split.c
> index d30782888f3..5bc9c0f6443 100644
> --- a/gcc/tree-ssa-loop-split.c
> +++ b/gcc/tree-ssa-loop-split.c
> @@ -586,12 +586,12 @@ split_loop (class loop *loop1)
>  	initialize_original_copy_tables ();
>  	basic_block cond_bb;
>  
> -	class loop *loop2 = loop_version (loop1, cond, &cond_bb,
> -					   true_edge->probability,
> -					   true_edge->probability.invert (),
> -					   true_edge->probability,
> -					   true_edge->probability.invert (),
> -					   true);
> +	class loop *loop2
> +	  = clone_loop_to_header_edge (loop1, cond, &cond_bb,
> +				       true_edge->probability,
> +				       true_edge->probability.invert (),
> +				       true_edge->probability,
> +				       true_edge->probability.invert (), true);
>  	gcc_assert (loop2);
>  
>  	edge new_e = connect_loops (loop1, loop2);
> @@ -1465,9 +1465,9 @@ get_cond_branch_to_split_loop (struct loop *loop, gcond *cond)
>                               exits
>  
>     In the graph, loop1 represents the part derived from original one, and
> -   loop2 is duplicated using loop_version (), which corresponds to the part
> -   of original one being splitted out.  In original latch edge of loop1, we
> -   insert a new conditional statement duplicated from the semi-invariant cond,
> +   loop2 is duplicated using clone_loop_to_header_edge (), which corresponds to
> +   the part of original one being splitted out.  In original latch edge of loop1,
> +   we insert a new conditional statement duplicated from the semi-invariant cond,
>     and one of its branch goes back to loop1 header as a latch edge, and the
>     other branch goes to loop2 pre-header as an entry edge.  And also in loop2,
>     we abandon the variant branch of the conditional statement by setting a
> @@ -1489,12 +1489,12 @@ do_split_loop_on_cond (struct loop *loop1, edge invar_branch)
>  
>    initialize_original_copy_tables ();
>  
> -  struct loop *loop2 = loop_version (loop1, boolean_true_node, NULL,
> -				     invar_branch->probability.invert (),
> -				     invar_branch->probability,
> -				     invar_branch->probability.invert (),
> -				     invar_branch->probability,
> -				     true);
> +  struct loop *loop2
> +    = clone_loop_to_header_edge (loop1, boolean_true_node, NULL,
> +				 invar_branch->probability.invert (),
> +				 invar_branch->probability,
> +				 invar_branch->probability.invert (),
> +				 invar_branch->probability, true);
>    if (!loop2)
>      {
>        free_original_copy_tables ();
> diff --git a/gcc/tree-ssa-loop-unswitch.c b/gcc/tree-ssa-loop-unswitch.c
> index fe4dacc0833..6beebe368a7 100644
> --- a/gcc/tree-ssa-loop-unswitch.c
> +++ b/gcc/tree-ssa-loop-unswitch.c
> @@ -488,11 +488,9 @@ tree_unswitch_loop (class loop *loop,
>  
>    extract_true_false_edges_from_block (unswitch_on, &edge_true, &edge_false);
>    prob_true = edge_true->probability;
> -  return loop_version (loop, unshare_expr (cond),
> -		       NULL, prob_true,
> -		       prob_true.invert (),
> -		       prob_true, prob_true.invert (),
> -		       false);
> +  return clone_loop_to_header_edge (loop, unshare_expr (cond), NULL, prob_true,
> +				    prob_true.invert (), prob_true,
> +				    prob_true.invert (), false);
>  }
>  
>  /* Unswitch outer loops by hoisting invariant guard on
> diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c
> index 4988c93fdb6..ebc013f195e 100644
> --- a/gcc/tree-vect-loop-manip.c
> +++ b/gcc/tree-vect-loop-manip.c
> @@ -3558,8 +3558,9 @@ vect_loop_versioning (loop_vec_info loop_vinfo,
>  			 loop_to_version->num);
>  
>        initialize_original_copy_tables ();
> -      nloop = loop_version (loop_to_version, cond_expr, &condition_bb,
> -			    prob, prob.invert (), prob, prob.invert (), true);
> +      nloop = clone_loop_to_header_edge (loop_to_version, cond_expr,
> +					 &condition_bb, prob, prob.invert (),
> +					 prob, prob.invert (), true);
>        gcc_assert (nloop);
>        nloop = get_loop_copy (loop);
>  
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)