> -----Original Message----- > From: Richard Biener <rguent...@suse.de> > Sent: Tuesday, November 7, 2023 3:04 PM > To: Tamar Christina <tamar.christ...@arm.com> > Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; j...@ventanamicro.com > Subject: Re: [PATCH 5/21]middle-end: update vectorizer's control update to > support picking an exit other than loop latch > > On Mon, 6 Nov 2023, Tamar Christina wrote: > > > Hi All, > > > > As requested, the vectorizer is now free to pick it's own exit which > > can be different than what the loop CFG infrastucture uses. The > > vectorizer makes use of this to vectorize loops that it previously could > > not. > > > > But this means that loop control must be materialized in the block > > that needs it less we corrupt the SSA chain. This makes it so we use > > the vectorizer's main IV block instead of the loop infra. > > > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > > > > Ok for master? > > > > Thanks, > > Tamar > > > > gcc/ChangeLog: > > > > * tree-ssa-loop-manip.cc (standard_iv_increment_position): > Conditionally > > take dest BB. > > * tree-ssa-loop-manip.h (standard_iv_increment_position): Likewise. > > * tree-vect-loop-manip.cc (vect_set_loop_controls_directly): Use it. > > (vect_set_loop_condition_partial_vectors_avx512): Likewise. > > (vect_set_loop_condition_normal): Likewise. > > > > --- inline copy of patch -- > > diff --git a/gcc/tree-ssa-loop-manip.h b/gcc/tree-ssa-loop-manip.h > > index > > > bda09f51d5619420331c513a9906831c779fd2b4..5938588c8882d842b00 > 301423df1 > > 11cbe7bf7ba8 100644 > > --- a/gcc/tree-ssa-loop-manip.h > > +++ b/gcc/tree-ssa-loop-manip.h > > @@ -38,7 +38,8 @@ extern basic_block split_loop_exit_edge (edge, bool > > = false); extern basic_block ip_end_pos (class loop *); extern > > basic_block ip_normal_pos (class loop *); extern void > > standard_iv_increment_position (class loop *, > > - gimple_stmt_iterator *, bool *); > > + gimple_stmt_iterator *, bool *, > > + basic_block = NULL); > > extern bool > > gimple_duplicate_loop_body_to_header_edge (class loop *, edge, unsigned > int, > > sbitmap, edge, vec<edge> *, int); > > diff > --git > > a/gcc/tree-ssa-loop-manip.cc b/gcc/tree-ssa-loop-manip.cc index > > > e7436915e01297e7af2a3bcf1afd01e014de6f32..bdc7a3d74a788f450ca5d > de6c294 > > 92ce4d4e4550 100644 > > --- a/gcc/tree-ssa-loop-manip.cc > > +++ b/gcc/tree-ssa-loop-manip.cc > > @@ -792,14 +792,19 @@ ip_normal_pos (class loop *loop) > > > > /* Stores the standard position for induction variable increment in LOOP > > (just before the exit condition if it is available and latch block is > > empty, > > - end of the latch block otherwise) to BSI. INSERT_AFTER is set to true > > if > > - the increment should be inserted after *BSI. */ > > + end of the latch block otherwise) to BSI. If DEST_BB is specified then > > that > > + basic block is used as the destination instead of the loop latch source > > + block. INSERT_AFTER is set to true if the increment should be inserted > after > > + *BSI. */ > > > > void > > standard_iv_increment_position (class loop *loop, gimple_stmt_iterator > *bsi, > > - bool *insert_after) > > + bool *insert_after, basic_block dest_bb) > > { > > - basic_block bb = ip_normal_pos (loop), latch = ip_end_pos (loop); > > + basic_block bb = dest_bb; > > + if (!bb) > > + bb = ip_normal_pos (loop); > > + basic_block latch = ip_end_pos (loop); > > I don't think that's a good API extension. Given that we don't support an > early > exit after the main IV exit doesn't this code already work fine as-is? It > chooses > the last exit. The position is also not semantically relevant, we just try > to keep > the latch empty here (that is, it's a bit of a "bad" API). > > So, do you really need this change?
I'll double check. I remember needing it to fix an ICE before, but also re-did the way the alternative main exits were handled later. At the end of the series as I was writing the cover letter this change also seemed... off to me, I should have checked it again before submitting it. > > Maybe we're really using standard_iv_increment_position wrong here, the > result is supposed to _only_ feed the PHI latch argument. Could be, this was needed before I changed the way I handled the IV updates for the alternate exit loops. I'll double check and drop If not needed. Thanks, Tamar > > Richard. > > > gimple *last = last_nondebug_stmt (latch); > > > > if (!bb > > diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc > > index > > > 6fbb5b80986fd657814b48eb009b52b094f331e6..3d59119787d6afdc5a64 > 65a547d1 > > ea2d3d940373 100644 > > --- a/gcc/tree-vect-loop-manip.cc > > +++ b/gcc/tree-vect-loop-manip.cc > > @@ -531,7 +531,8 @@ vect_set_loop_controls_directly (class loop *loop, > loop_vec_info loop_vinfo, > > tree index_before_incr, index_after_incr; > > gimple_stmt_iterator incr_gsi; > > bool insert_after; > > - standard_iv_increment_position (loop, &incr_gsi, &insert_after); > > + edge exit_e = LOOP_VINFO_IV_EXIT (loop_vinfo); > > + standard_iv_increment_position (loop, &incr_gsi, &insert_after, > > + exit_e->src); > > if (LOOP_VINFO_USING_DECREMENTING_IV_P (loop_vinfo)) > > { > > /* Create an IV that counts down from niters_total and whose > > step @@ -1017,7 +1018,8 @@ > vect_set_loop_condition_partial_vectors_avx512 (class loop *loop, > > tree index_before_incr, index_after_incr; > > gimple_stmt_iterator incr_gsi; > > bool insert_after; > > - standard_iv_increment_position (loop, &incr_gsi, &insert_after); > > + standard_iv_increment_position (loop, &incr_gsi, &insert_after, > > + exit_edge->src); > > create_iv (niters_adj, MINUS_EXPR, iv_step, NULL_TREE, loop, > > &incr_gsi, insert_after, &index_before_incr, > > &index_after_incr); > > @@ -1185,7 +1187,7 @@ vect_set_loop_condition_partial_vectors_avx512 > (class loop *loop, > > loop handles exactly VF scalars per iteration. */ > > > > static gcond * > > -vect_set_loop_condition_normal (loop_vec_info /* loop_vinfo */, edge > > exit_edge, > > +vect_set_loop_condition_normal (loop_vec_info loop_vinfo, edge > > +exit_edge, > > class loop *loop, tree niters, tree step, > > tree final_iv, bool niters_maybe_zero, > > gimple_stmt_iterator loop_cond_gsi) @@ - > 1278,7 +1280,8 @@ > > vect_set_loop_condition_normal (loop_vec_info /* loop_vinfo */, edge > exit_edge, > > } > > } > > > > - standard_iv_increment_position (loop, &incr_gsi, &insert_after); > > + standard_iv_increment_position (loop, &incr_gsi, &insert_after, > > + exit_edge->src); > > create_iv (init, PLUS_EXPR, step, NULL_TREE, loop, > > &incr_gsi, insert_after, &indx_before_incr, &indx_after_incr); > > indx_after_incr = force_gimple_operand_gsi (&loop_cond_gsi, > > indx_after_incr, > > > > > > > > > > > > -- > Richard Biener <rguent...@suse.de> > SUSE Software Solutions Germany GmbH, > Frankenstrasse 146, 90461 Nuernberg, Germany; > GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG > Nuernberg)