Re: [PATCH 14/21]middle-end: Change loop analysis from looking at at number of BB to actual cfg

[Richard Biener <rguenther@suse.de>]

On Mon, 6 Nov 2023, Tamar Christina wrote:

> Hi All,
> 
> The vectorizer at the moment uses a num_bb check to check for control flow.
> This rejects a number of loops with no reason.  Instead this patch changes it
> to check the destination of the exits instead.
> 
> This also allows early break to work by also dropping the single_exit check.
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> 
> Ok for master?

I think this can go in independently, one comment below ...

> Thanks,
> Tamar
> 
> gcc/ChangeLog:
> 
> 	* tree-vect-loop-manip.cc (slpeel_can_duplicate_loop_p):
> 	* tree-vect-loop.cc (vect_analyze_loop_form):
> 
> --- inline copy of patch -- 
> diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
> index 9c1405d79fd8fe8689007df3b7605b7a3d3ecdd7..466cf4c47154099a33dc63e22d74eef42d282444 100644
> --- a/gcc/tree-vect-loop-manip.cc
> +++ b/gcc/tree-vect-loop-manip.cc
> @@ -1937,12 +1937,10 @@ slpeel_can_duplicate_loop_p (const class loop *loop, const_edge exit_e,
>    edge entry_e = loop_preheader_edge (loop);
>    gcond *orig_cond = get_loop_exit_condition (exit_e);
>    gimple_stmt_iterator loop_exit_gsi = gsi_last_bb (exit_e->src);
> -  unsigned int num_bb = loop->inner? 5 : 2;
>  
>    /* All loops have an outer scope; the only case loop->outer is NULL is for
>       the function itself.  */
>    if (!loop_outer (loop)
> -      || loop->num_nodes != num_bb
>        || !empty_block_p (loop->latch)
>        || !exit_e
>        /* Verify that new loop exit condition can be trivially modified.  */
> diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
> index ddb6cad60f2f2cfdc96732f3f256d86e315d7357..27ab6abfa854f14f8a4cf3d9fcb1ac1c203a4198 100644
> --- a/gcc/tree-vect-loop.cc
> +++ b/gcc/tree-vect-loop.cc
> @@ -1727,6 +1727,17 @@ vect_analyze_loop_form (class loop *loop, vect_loop_form_info *info)
>  		       "using as main loop exit: %d -> %d [AUX: %p]\n",
>  		       exit_e->src->index, exit_e->dest->index, exit_e->aux);
>  
> +  /* Check if we have any control flow that doesn't leave the loop.  */
> +  class loop *v_loop = loop->inner ? loop->inner : loop;
> +  basic_block *bbs= get_loop_body (v_loop);
> +  for (unsigned i = 0; i < v_loop->num_nodes; i++)
> +    if (!empty_block_p (bbs[i])
> +	&& !loop_exits_from_bb_p (v_loop, bbs[i])
> +	&& bbs[i]->loop_father == v_loop)

That looks a bit complicated.  Better matching the comment would be

       if (EDGE_COUNT (bbs[i]->succs) != 1
           && (EDGE_COUNT (bbs[i]->succs) != 2
               || !loop_exits_from_bb_p (bb[i]->loop_father, bb[i])))

I'd say OK with that change, and independently if the removed
single_exit test below isn't harmful (I suppose it is).

Btw, for the outer loop case we still have the single_exit tests
but you already said you're not supporting multi-exits there yet.

Thanks,
Richard.

> +      return opt_result::failure_at (vect_location,
> +				     "not vectorized:"
> +				     " unsupported control flow in loop.\n");
> +
>    /* Different restrictions apply when we are considering an inner-most loop,
>       vs. an outer (nested) loop.
>       (FORNOW. May want to relax some of these restrictions in the future).  */
> @@ -1746,11 +1757,6 @@ vect_analyze_loop_form (class loop *loop, vect_loop_form_info *info)
>                             |
>                          (exit-bb)  */
>  
> -      if (loop->num_nodes != 2)
> -	return opt_result::failure_at (vect_location,
> -				       "not vectorized:"
> -				       " control flow in loop.\n");
> -
>        if (empty_block_p (loop->header))
>  	return opt_result::failure_at (vect_location,
>  				       "not vectorized: empty loop.\n");
> @@ -1782,11 +1788,6 @@ vect_analyze_loop_form (class loop *loop, vect_loop_form_info *info)
>  				       "not vectorized:"
>  				       " multiple nested loops.\n");
>  
> -      if (loop->num_nodes != 5)
> -	return opt_result::failure_at (vect_location,
> -				       "not vectorized:"
> -				       " control flow in loop.\n");
> -
>        entryedge = loop_preheader_edge (innerloop);
>        if (entryedge->src != loop->header
>  	  || !single_exit (innerloop)
> @@ -1823,9 +1824,6 @@ vect_analyze_loop_form (class loop *loop, vect_loop_form_info *info)
>        info->inner_loop_cond = inner.conds[0];
>      }
>  
> -  if (!single_exit (loop))
> -    return opt_result::failure_at (vect_location,
> -				   "not vectorized: multiple exits.\n");
>    if (EDGE_COUNT (loop->header->preds) != 2)
>      return opt_result::failure_at (vect_location,
>  				   "not vectorized:"