On 29/10/2024 13:41, Richard Biener wrote:
> On Mon, 28 Oct 2024, Alex Coplan wrote:
> 
> > From: Tamar Christina <tamar.christ...@arm.com>
> > 
> > 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 <alex.cop...@arm.com>
> > ---
> >  .../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 <rguent...@suse.de>
> 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

Reply via email to