On 29/10/2024 13:41, Richard Biener wrote:
> On Mon, 28 Oct 2024, Alex Coplan wrote:
>
> > From: Tamar Christina <[email protected]>
> >
> > The alignment peeling changes exposed a latent missing dominator update
> > with early break vectorization, specifically when inserting the vector
> > skip edge, since the new edge bypasses the prolog skip block and thus
> > has the potential to subvert its dominance. This patch fixes that.
>
> OK.
Thanks. Regular (-O2) bootstrap on aarch64-linux-gnu showed this
triggers -Wmaybe-uninitialized due to the new use of the prolog variable
(which I think is a false positive, since it's guarded by
prolog_peeling). The attached version works around it by initializing
prolog to NULL. Incrementally, that's:
diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
index 1dcb0b427f0..f98c162fc0a 100644
--- a/gcc/tree-vect-loop-manip.cc
+++ b/gcc/tree-vect-loop-manip.cc
@@ -3198,7 +3198,7 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters,
tree nitersm1,
prob_prolog = prob_epilog = profile_probability::guessed_always ()
.apply_scale (estimated_vf - 1, estimated_vf);
- class loop *prolog, *epilog = NULL;
+ class loop *prolog = NULL, *epilog = NULL;
class loop *first_loop = loop;
bool irred_flag = loop_preheader_edge (loop)->flags & EDGE_IRREDUCIBLE_LOOP;
I guess the incremental change should be obvious, but just to check you're happy
with the workaround, is the updated version OK?
Alex
>
> Thanks,
> Richard.
>
> > gcc/ChangeLog:
> >
> > * tree-vect-loop-manip.cc (vect_do_peeling): Update immediate
> > dominators of nodes that were dominated by the prolog skip block
> > after inserting vector skip edge.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/vect/vect-early-break_6.cc: New test.
> >
> > Co-Authored-By: Alex Coplan <[email protected]>
> > ---
> > .../g++.dg/vect/vect-early-break_6.cc | 25 +++++++++++++++++++
> > gcc/tree-vect-loop-manip.cc | 24 ++++++++++++++++++
> > 2 files changed, 49 insertions(+)
> > create mode 100644 gcc/testsuite/g++.dg/vect/vect-early-break_6.cc
> >
> >
>
> --
> Richard Biener <[email protected]>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
diff --git a/gcc/testsuite/g++.dg/vect/vect-early-break_6.cc
b/gcc/testsuite/g++.dg/vect/vect-early-break_6.cc
new file mode 100644
index 00000000000..fdd9af832a7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/vect/vect-early-break_6.cc
@@ -0,0 +1,25 @@
+// { dg-do compile }
+// ICE in verify_dominators, reduced from charset.cc (libstdc++).
+
+void convert_escape(int *);
+int cpp_interpret_string_1_to, cpp_interpret_string_1_tbuf;
+char *cpp_interpret_string_1_base;
+char cpp_interpret_string_1_limit;
+void cpp_interpret_string_1() {
+ char *p;
+ for (;;) {
+ cpp_interpret_string_1_base = p;
+ while (p < &cpp_interpret_string_1_limit && *p)
+ p++;
+ if (p > cpp_interpret_string_1_base)
+ if (cpp_interpret_string_1_to)
+ goto fail;
+ if (p >= &cpp_interpret_string_1_limit)
+ break;
+ int *tbuf_ptr =
+ cpp_interpret_string_1_to ? &cpp_interpret_string_1_tbuf : __null;
+ convert_escape(tbuf_ptr);
+ }
+fail:
+ ;
+}
diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
index 1dcb0b427f0..f98c162fc0a 100644
--- a/gcc/tree-vect-loop-manip.cc
+++ b/gcc/tree-vect-loop-manip.cc
@@ -3198,7 +3198,7 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters,
tree nitersm1,
prob_prolog = prob_epilog = profile_probability::guessed_always ()
.apply_scale (estimated_vf - 1, estimated_vf);
- class loop *prolog, *epilog = NULL;
+ class loop *prolog = NULL, *epilog = NULL;
class loop *first_loop = loop;
bool irred_flag = loop_preheader_edge (loop)->flags & EDGE_IRREDUCIBLE_LOOP;
@@ -3465,6 +3465,30 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters,
tree nitersm1,
skip_e = guard_e;
e = EDGE_PRED (guard_to, 0);
e = (e != guard_e ? e : EDGE_PRED (guard_to, 1));
+
+ /* Handle any remaining dominator updates needed after
+ inserting the loop skip edge above. */
+ if (LOOP_VINFO_EARLY_BREAKS (loop_vinfo)
+ && prolog_peeling)
+ {
+ /* Adding a skip edge to skip a loop with multiple exits
+ means the dominator of the join blocks for all exits shifts
+ from the prolog skip guard to the loop skip guard. */
+ auto prolog_skip_bb
+ = single_pred (loop_preheader_edge (prolog)->src);
+ auto needs_update
+ = get_dominated_by (CDI_DOMINATORS, prolog_skip_bb);
+
+ /* Update everything except for the immediate children of
+ the prolog skip block (the prolog and vector preheaders).
+ Those should remain dominated by the prolog skip block itself,
+ since the loop guard edge goes to the epilogue. */
+ for (auto bb : needs_update)
+ if (bb != EDGE_SUCC (prolog_skip_bb, 0)->dest
+ && bb != EDGE_SUCC (prolog_skip_bb, 1)->dest)
+ set_immediate_dominator (CDI_DOMINATORS, bb, guard_bb);
+ }
+
slpeel_update_phi_nodes_for_guard1 (first_loop, epilog, guard_e, e);
/* Simply propagate profile info from guard_bb to guard_to which is
