On Tue, Oct 29, 2024 at 9:24 AM Kugan Vivekanandarajah <kvivekana...@nvidia.com> wrote: > > Hi Richard, > Thanks for the review. > > > On 28 Oct 2024, at 9:18 pm, Richard Biener <richard.guent...@gmail.com> > > wrote: > > > > External email: Use caution opening links or attachments > > > > > > On Mon, Oct 28, 2024 at 9:35 AM Kugan Vivekanandarajah > > <kvivekana...@nvidia.com> wrote: > >> > >> Hi, > >> > >> When ifcvt version a loop, it sets dont_vectorize to the scalar loop. If > >> the > >> vector loop is not vectorized and removed, the scalar loop is still left > >> with > >> dont_vectorize. As a result, BB vectorization will not happen. > >> > >> This patch adds a new attribute called dont_loop_vectorize (that is > >> different > >> from general dont_vectorize) specifically for loops versioned. BB > >> vectorization > >> does not need to honour this and still can vectorize. > >> > >> Bootstrapped and regression tested on aarch64-linux-gnu with no new > >> regressions. > >> > >> Is this OK? > > > > I believe if-conversion never versions a loop that has > > ->dont_vectorize set so when > > the vectorizer elides the .LOOP_VECTORIZED test it can simply clear > > the flag again. > > > > I don't like adding a new flag, instead if the above doesn't work, the > > vectorizer > > should be changed how it identifies loop candidates, not relying on this > > flag. > > Here is a version where I am resetting dont_vectorize while folding > ffold_loop_internal_call with false. > > Bootstrapped and regression tested on aarch64-linux-gnu. Is this OK?
Hmm, I'm not sure this way is super reliable given taken_edge->dest could be a preheader block. I see we have many calls to fold_loop_internal_call - one is even suspicious to break with your patch: /* If we are going to vectorize outer loop, prevent vectorization of the inner loop in the scalar loop - either the scalar loop is thrown away, so it is a wasted work, or is used only for a few iterations. */ if (scalar_loop->inner) { gimple *g = vect_loop_vectorized_call (scalar_loop->inner); if (g) { arg = gimple_call_arg (g, 0); get_loop (fun, tree_to_shwi (arg))->dont_vectorize = true; fold_loop_internal_call (g, boolean_false_node); does it work if you just do diff --git a/gcc/tree-vectorizer.cc b/gcc/tree-vectorizer.cc index af112f212fe..16fa0ec1bb7 100644 --- a/gcc/tree-vectorizer.cc +++ b/gcc/tree-vectorizer.cc @@ -1326,6 +1326,7 @@ pass_vectorize::execute (function *fun) if (g) { fold_loop_internal_call (g, boolean_false_node); + loop->dont_vectorize = false; ret |= TODO_cleanup_cfg; g = NULL; } @@ -1335,6 +1336,7 @@ pass_vectorize::execute (function *fun) if (g) { fold_loop_internal_call (g, boolean_false_node); + loop->dont_vectorize = false; ret |= TODO_cleanup_cfg; } } ? > Thanks, > Kugan > > > > > > Richard. > > > >> Thanks, > >> Kugan > >> >