> -----Original Message-----
> From: Richard Biener <rguent...@suse.de>
> Sent: Monday, February 5, 2024 1:22 PM
> To: Tamar Christina <tamar.christ...@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; j...@ventanamicro.com
> Subject: Re: [PATCH]middle-end: fix ICE when moving statements to empty BB
> [PR113731]
>
> On Mon, 5 Feb 2024, Tamar Christina wrote:
>
> > Hi All,
> >
> > We use gsi_move_before (&stmt_gsi, &dest_gsi); to request that the new
> statement
> > be placed before any other statement. Typically this then moves the current
> > pointer to be after the statement we just inserted.
> >
> > However it looks like when the BB is empty, this does not happen and the CUR
> > pointer stays NULL. There's a comment in the source of gsi_insert_before
> > that
> > explains:
> >
> > /* If CUR is NULL, we link at the end of the sequence (this case happens
> >
> > so it adds it to the end instead of start like you asked. This means that
> > in
> > this case there's nothing to move and so we shouldn't move the pointer if
> > we're
> > already at the HEAD.
>
> The issue is that a gsi_end_p () is ambiguous, it could be the start
> or the end. gsi_insert_before treats it as "end" while gsi_insert_after
> treats it as "start" since you can't really insert "after" the "end".
>
> gsi_move_before doesn't update the insertion pointer (using
> GSI_SAME_STMT), so with a gsi_end_p () you get what you ask for.
>
> Btw,
>
> /* Move all stmts that need moving. */
> basic_block dest_bb = LOOP_VINFO_EARLY_BRK_DEST_BB (loop_vinfo);
> gimple_stmt_iterator dest_gsi = gsi_start_bb (dest_bb);
>
> should probably use gsi_after_labels (dest_bb) just in case.
See next patch.
>
> It looks like LOOP_VINFO_EARLY_BRK_STORES is "reverse"? Is that
> why you are doing gsi_move_before + gsi_prev? Why do gsi_prev
> at all?
>
Yes, it stores them reverse because we record them from the latch on up.
So we either have to iterate backwards, insert them to the front or move gsi.
I guess I could remove it by removing the for-each loop and iterating in
reverse. Is that preferred?
Tamar.
> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> >
> > Ok for master?
> >
> > Thanks,
> > Tamar
> >
> > gcc/ChangeLog:
> >
> > PR tree-optimization/113731
> > * tree-vect-loop.cc (move_early_exit_stmts): Conditionally move pointer.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR tree-optimization/113731
> > * gcc.dg/vect/vect-early-break_111-pr113731.c: New test.
> >
> > --- inline copy of patch --
> > diff --git a/gcc/testsuite/gcc.dg/vect/vect-early-break_111-pr113731.c
> b/gcc/testsuite/gcc.dg/vect/vect-early-break_111-pr113731.c
> > new file mode 100644
> > index
> 0000000000000000000000000000000000000000..2d6db91df97625a7f1160
> 9d034e89af0461129b2
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/vect/vect-early-break_111-pr113731.c
> > @@ -0,0 +1,21 @@
> > +/* { dg-do compile } */
> > +/* { dg-add-options vect_early_break } */
> > +/* { dg-require-effective-target vect_early_break } */
> > +/* { dg-require-effective-target vect_int } */
> > +
> > +/* { dg-final { scan-tree-dump "LOOP VECTORIZED" "vect" } } */
> > +
> > +char* inet_net_pton_ipv4_bits;
> > +char inet_net_pton_ipv4_odst;
> > +void __errno_location();
> > +void inet_net_pton_ipv4();
> > +void inet_net_pton() { inet_net_pton_ipv4(); }
> > +void inet_net_pton_ipv4(char *dst, int size) {
> > + while ((inet_net_pton_ipv4_bits > dst) & inet_net_pton_ipv4_odst) {
> > + if (size-- <= 0)
> > + goto emsgsize;
> > + *dst++ = '\0';
> > + }
> > +emsgsize:
> > + __errno_location();
> > +}
> > diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
> > index
> 30b90d99925bea74caf14833d8ab1695607d0fe9..e2587315020a35a7d4ebd3e
> 7a9842caa36bb5d3c 100644
> > --- a/gcc/tree-vect-loop.cc
> > +++ b/gcc/tree-vect-loop.cc
> > @@ -11801,7 +11801,8 @@ move_early_exit_stmts (loop_vec_info loop_vinfo)
> >
> > gimple_stmt_iterator stmt_gsi = gsi_for_stmt (stmt);
> > gsi_move_before (&stmt_gsi, &dest_gsi);
> > - gsi_prev (&dest_gsi);
> > + if (!gsi_end_p (dest_gsi))
> > + gsi_prev (&dest_gsi);
> > }
> >
> > /* Update all the stmts with their new reaching VUSES. */
> >
> >
> >
> >
> >
>
> --
> 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)
