Hi Alfie,
> -----Original Message-----
> From: Alfie Richards <[email protected]>
> Sent: 23 October 2026 13:34
> To: [email protected]
> Cc: Richard Biener <[email protected]>; Tamar Christina
> <[email protected]>; Robin Dapp <[email protected]>; Richard
> Sandiford <[email protected]>; [email protected];
> [email protected]; Alfie Richards <[email protected]>
> Subject: [PATCH 2/2] vect: Fix final iterations counts and latch values for
> peeled early break [PR 127805]
>
> After recent changes to how we accomplish peeling for early break the
> final iv updates and the niters calculations were wrong.
>
> This rewrites and simplifies some of that logic fixing the issue.
>
> gcc/ChangeLog:
>
> PR tree-optimization/127805
>
> * tree-vect-loop-manip.cc (vect_set_loop_controls_directly):
> Remove niter catch adjustment.
> (vect_set_loop_condition_partial_vectors): Update final_iv
> calculation.
> (vect_set_loop_condition_partial_vectors_avx512): Likewise.
> (vect_set_loop_condition_normal): Change final_iv calculation
> to properly take into account the different loop forms for
> peeled early break.
> (vect_gen_vector_loop_niters): Update to account for do-loop
> transformation when peeling for early break.
> (vect_gen_vector_loop_niters_mult_vf): Remove unused early
> break restart.
> (vect_do_peeling): Force skip vectors true for all peeled
> early break cases, and don't overrule final_iv when peeling
> for early break (instead only when uncounted).
> * tree-vect-loop.cc (vect_transform_loop): Update compile time
> known niters case for early break possibility.
>
Sorry for the delay in reviewing. I had to go through the examples to
figure out what this was doing.
I largely agree with the patch, as you mentioned, by moving the header
rotation to the loop controls it now means the loop manip code shouldn't
do it itself anymore.
Which is why you removed the niters_vector_mult_vf adjustment. However
you left the adjustments to vect_update_ivs_after_vectorizer and instead
added a specific fixup using skip_vector, and you then fix it up in the
control code. This is why you still have the odd fixup code here and there.
But if we move the iteration adjustment code to loop control we don't need
any of the special handling for PEELED loops anymore in loop manip.
This means that indeed LOOP_VINFO_NITERS_UNCOUNTED_P stays the only
actual special case. So the entire thing can be much simpler.
I've tested a patch that takes this cleanup started here further and will post
that in a bit since I think we can now avoid treating PEELED special here,
and in turn also cleanup the various loop_condition_* functions.
Thanks,
Tamar
> gcc/testsuite/ChangeLog:
>
> * vect/vect-early-break_147-pr128028.c: New test.
>
> -- >8 --
>
> Hi all,
>
> Sorry for the delay on this fix, it really sent me on a whole journey,
> but I learnt a lot! For instance I learnt my understanding of the
> early break peeling and how it interacts with niters and final_iv was
> completely off :)
>
> I'm pretty sure this is now correct, and cleans up some of the weirdness
> in the code.
>
> Unfortunately we stil have pretty poor test coverage for early break and
> especially early-break peeled. However the gmp test suite serves as a
> pretty excellent test of it in my expereince now!
>
> Bootstrapped and regr-tested on x86_64-linux-gnu and aarch64-linux-gnu.
>
> GMP testsuite run with a bunch of different vectorization settings
> for x86_64 and aarch64.
>
> Okay for master?
> Alfie Richards
>
> ---
> .../vect/vect-early-break_147-pr128028.c | 117 ++++++++++++++++++
> gcc/tree-vect-loop-manip.cc | 88 +++++--------
> gcc/tree-vect-loop.cc | 5 +-
> 3 files changed, 151 insertions(+), 59 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/vect/vect-early-break_147-
> pr128028.c
>
> diff --git a/gcc/testsuite/gcc.dg/vect/vect-early-break_147-pr128028.c
> b/gcc/testsuite/gcc.dg/vect/vect-early-break_147-pr128028.c
> new file mode 100644
> index 00000000000..c7b1c3ad4b2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/vect-early-break_147-pr128028.c
> @@ -0,0 +1,117 @@
> +/* PR tree-optimization/126778 */
> +/* { dg-do run } */
> +/* { dg-add-options vect_early_break } */
> +/* { dg-require-effective-target vect_early_break_hw } */
> +/* { dg-require-effective-target vect_int } */
> +
> +#define N 64
> +
> +/* The counting exit is tested before the early exit, so the loop body runs
> + one time less than the loop header. The vector loop must not process a
> + lane for the iteration that only tests the counter, as that lane reads the
> + element next to the arrays. */
> +
> +__attribute__((optimize("no-tree-vectorize")))
> +__attribute__ ((noipa)) int
> +differs (const long *a, const long *b, unsigned long n)
> +{
> + while (n != 0)
> + {
> + n--;
> + if (a[n] != b[n])
> + return 1;
> + }
> + return 0;
> +}
> +__attribute__((optimize("no-tree-vectorize")))
> +__attribute__ ((noipa)) int
> +differs_signed (const long *a, const long *b, long n)
> +{
> + if (n < 0)
> + return 2;
> + while (--n >= 0)
> + {
> + if (a[n] != b[n])
> + return 1;
> + }
> + return 0;
> +}
> +
> +__attribute__ ((noipa)) int
> +differs_up (const long *a, const long *b, unsigned long n)
> +{
> + unsigned long i = 0;
> + while (i != n)
> + {
> + unsigned long j = i;
> + i++;
> + if (a[j] != b[j])
> + return 1;
> + }
> + return 0;
> +}
> +
> +/* The store before the early exit must not write the extra lane and must run
> + once for every element. */
> +
> +__attribute__((optimize("no-tree-vectorize")))
> +__attribute__ ((noipa)) int
> +bump (const long *restrict a, const long *restrict b, long *restrict c,
> + unsigned long n)
> +{
> + while (n != 0)
> + {
> + n--;
> + c[n] += 1;
> + if (a[n] != b[n])
> + return 1;
> + }
> + return 0;
> +}
> +static long abuf[N + 16] __attribute__ ((aligned (64)));
> +static long bbuf[N + 16] __attribute__ ((aligned (64)));
> +static long cbuf[N + 16] __attribute__ ((aligned (64)));
> +
> +__attribute__((optimize("no-tree-vectorize")))
> +int
> +main (void)
> +{
> +
> + for (unsigned long n = 0; n <= N; n++)
> + for (int off = 0; off < 8; off++)
> + {
> + /* Start the arrays at every alignment of a 64 byte vector and give
> + the elements around them different values. */
> + long *a = abuf + 8 + off;
> + long *b = bbuf + 8 + off;
> + long *c = cbuf + 8 + off;
> + for (unsigned long i = 0; i < n; i++)
> + a[i] = b[i] = c[i] = i * 3 + 1;
> + a[-1] = 1;
> + b[-1] = 2;
> + c[-1] = 5;
> + a[n] = 3;
> + b[n] = 4;
> + c[n] = 6;
> +
> + if (differs (a, b, n) != 0 || differs_signed (a, b, n) != 0
> + || differs_up (a, b, n) != 0)
> + __builtin_abort ();
> +
> + if (bump (a, b, c, n) != 0 || c[-1] != 5 || c[n] != 6)
> + __builtin_abort ();
> + for (unsigned long i = 0; i < n; i++)
> + if (c[i] != i * 3 + 2)
> + __builtin_abort ();
> +
> + if (n > 0)
> + {
> + b[n / 2]++;
> + if (differs (a, b, n) != 1 || differs_signed (a, b, n) != 1
> + || differs_up (a, b, n) != 1)
> + __builtin_abort ();
> + }
> + }
> +
> + return 0;
> +}
> diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
> index 4dfef69de87..1154b6edf50 100644
> --- a/gcc/tree-vect-loop-manip.cc
> +++ b/gcc/tree-vect-loop-manip.cc
> @@ -789,13 +789,6 @@ vect_set_loop_controls_directly (class loop *loop,
> loop_vec_info loop_vinfo,
> bias_tree);
> }
>
> - /* A do-while loop always executes the body once, as such the limit
> - the end counter should be lowered by 1 iteration. */
> - if (LOOP_VINFO_EARLY_BREAKS_VECT_PEELED (loop_vinfo))
> - this_test_limit = gimple_build (preheader_seq, MINUS_EXPR,
> - compare_type, this_test_limit,
> - build_one_cst (compare_type));
> -
> /* Create the initial control. First include all items that
> are within the loop limit. */
> tree init_ctrl = NULL_TREE;
> @@ -1097,17 +1090,7 @@ vect_set_loop_condition_partial_vectors (class
> loop *loop, edge exit_edge,
> if (final_iv)
> {
> gassign *assign;
> - /* If vectorizing an inverted early break loop we have to restart the
> - scalar loop at niters - vf. This matches what we do in
> - vect_gen_vector_loop_niters_mult_vf for non-masked loops. */
> - if (LOOP_VINFO_EARLY_BREAKS_VECT_PEELED (loop_vinfo))
> - {
> - tree ftype = TREE_TYPE (orig_niters);
> - tree vf = build_int_cst (ftype, LOOP_VINFO_VECT_FACTOR
> (loop_vinfo));
> - assign = gimple_build_assign (final_iv, MINUS_EXPR, orig_niters, vf);
> - }
> - else
> - assign = gimple_build_assign (final_iv, orig_niters);
> + assign = gimple_build_assign (final_iv, orig_niters);
> gsi_insert_on_edge_immediate (LOOP_VINFO_MAIN_EXIT (loop_vinfo),
> assign);
> }
>
> @@ -1354,19 +1337,8 @@ vect_set_loop_condition_partial_vectors_avx512
> (class loop *loop,
>
> if (final_iv)
> {
> - gassign *assign;
> - /* If vectorizing an inverted early break loop we have to restart the
> - scalar loop at niters - vf. This matches what we do in
> - vect_gen_vector_loop_niters_mult_vf for non-masked loops. */
> - if (LOOP_VINFO_EARLY_BREAKS_VECT_PEELED (loop_vinfo))
> - {
> - tree ftype = TREE_TYPE (orig_niters);
> - tree vf = build_int_cst (ftype, LOOP_VINFO_VECT_FACTOR
> (loop_vinfo));
> - assign = gimple_build_assign (final_iv, MINUS_EXPR, orig_niters, vf);
> - }
> - else
> - assign = gimple_build_assign (final_iv, orig_niters);
> - gsi_insert_on_edge_immediate (LOOP_VINFO_MAIN_EXIT (loop_vinfo),
> assign);
> + gassign *assign = gimple_build_assign (final_iv, orig_niters);
> + gsi_insert_on_edge_immediate (LOOP_VINFO_MAIN_EXIT (loop_vinfo),
> assign);
> }
>
> return cond_stmt;
> @@ -1499,29 +1471,27 @@ vect_set_loop_condition_normal (loop_vec_info
> loop_vinfo, edge exit_edge,
>
> if (final_iv)
> {
> + /* If we are using step=1 catch=niters/vf then we should be using the
> + logic in vect_gen_vector_loop_niters_mult_vf for the final iv and
> this
> + branch should never be taken. */
> + gcc_assert (!integer_onep (step));
> +
> gassign *assign;
> - tree phi_dest
> - = integer_zerop (init) ? final_iv : copy_ssa_name (indx_after_incr);
> + tree iv_end = LOOP_VINFO_EARLY_BREAKS_VECT_PEELED (loop_vinfo)
> + ? indx_before_incr
> + : indx_after_incr;
> +
> + tree phi_dest = integer_zerop (init) ? final_iv : copy_ssa_name
> (iv_end);
> +
> /* Make sure to maintain LC SSA form here and elide the subtraction
> if the value is zero. */
> gphi *phi = create_phi_node (phi_dest, exit_edge->dest);
> - add_phi_arg (phi, indx_after_incr, exit_edge, UNKNOWN_LOCATION);
> + add_phi_arg (phi, iv_end, exit_edge, UNKNOWN_LOCATION);
> +
> if (!integer_zerop (init))
> {
> - /* If vectorizing an inverted early break loop we have to restart the
> - scalar loop at niters - vf. This matches what we do in
> - vect_gen_vector_loop_niters_mult_vf for non-masked loops. */
> - if (LOOP_VINFO_EARLY_BREAKS_VECT_PEELED (loop_vinfo))
> - {
> - tree ftype = TREE_TYPE (final_iv);
> - tree vf
> - = build_int_cst (ftype, LOOP_VINFO_VECT_FACTOR
> (loop_vinfo));
> - assign = gimple_build_assign (final_iv, MINUS_EXPR, init, vf);
> - }
> - else
> - assign = gimple_build_assign (final_iv, MINUS_EXPR, phi_dest, init);
> - gimple_stmt_iterator gsi
> - = gsi_after_labels (LOOP_VINFO_MAIN_EXIT (loop_vinfo)->dest);
> + gimple_stmt_iterator gsi = gsi_after_labels (LOOP_VINFO_MAIN_EXIT
> (loop_vinfo)->dest);
> + assign = gimple_build_assign (final_iv, MINUS_EXPR, phi_dest, init);
> gsi_insert_before (&gsi, assign, GSI_SAME_STMT);
> }
> }
> @@ -3110,8 +3080,12 @@ vect_gen_vector_loop_niters (loop_vec_info
> loop_vinfo, tree niters,
>
> /* If epilogue loop is required because of data accesses with gaps, we
> subtract one iteration from the total number of iterations here for
> - correct calculation of RATIO. */
> - if (LOOP_VINFO_PEELING_FOR_GAPS (loop_vinfo))
> + correct calculation of RATIO.
> +
> + Alternatively, if we are peeled for early breaks, then loop niters
> + is 1 too large as we convert the loop into a do-loop. */
> + if (LOOP_VINFO_PEELING_FOR_GAPS (loop_vinfo)
> + || LOOP_VINFO_EARLY_BREAKS_VECT_PEELED (loop_vinfo))
> {
> ni_minus_gap = fold_build2 (MINUS_EXPR, type, niters,
> build_one_cst (type));
> @@ -3250,12 +3224,6 @@ vect_gen_vector_loop_niters_mult_vf
> (loop_vec_info loop_vinfo,
> tree niters_vector_mult_vf = fold_build2 (MULT_EXPR, type,
> niters_vector, tree_vf);
>
> - /* If we've peeled a vector iteration then subtract one full vector
> - iteration. */
> - if (LOOP_VINFO_EARLY_BREAKS_VECT_PEELED (loop_vinfo))
> - niters_vector_mult_vf = fold_build2 (MINUS_EXPR, type,
> - niters_vector_mult_vf, tree_vf);
> -
> if (!is_gimple_val (niters_vector_mult_vf))
> {
> tree var = create_tmp_var (type, "niters_vector_mult_vf");
> @@ -3633,7 +3601,8 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree
> niters, tree nitersm1,
> ? maybe_lt (LOOP_VINFO_INT_NITERS (loop_vinfo),
> bound_prolog + bound_epilog)
> : (!LOOP_VINFO_USE_VERSIONING_WITHOUT_PEELING
> (loop_vinfo)
> - || vect_epilogues));
> + || vect_epilogues
> + || LOOP_VINFO_EARLY_BREAKS_VECT_PEELED
> (loop_vinfo)));
>
> /* Epilog loop must be executed if the number of iterations for epilog
> loop is known at compile time, otherwise we need to add a check at
> @@ -4017,7 +3986,10 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree
> niters, tree nitersm1,
> vect_update_ivs_after_vectorizer (loop_vinfo, tmp_niters_vf,
> update_e, true);
> }
> - if (LOOP_VINFO_EARLY_BREAKS_VECT_PEELED (loop_vinfo))
> + /* If the loop is uncounted we do not have a loop control to set
> + the final niters for the main exit. Therefore, we need to use
> + the early break exit niters for all exits. */
> + if (LOOP_VINFO_NITERS_UNCOUNTED_P (loop_vinfo))
> vector_iters_vf = tmp_niters_vf;
>
> LOOP_VINFO_EARLY_BRK_NITERS_VAR (loop_vinfo) = tmp_niters_vf;
> diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
> index 7ed3f58bf23..9feea2d2e9c 100644
> --- a/gcc/tree-vect-loop.cc
> +++ b/gcc/tree-vect-loop.cc
> @@ -11529,9 +11529,12 @@ vect_transform_loop (loop_vec_info
> loop_vinfo, gimple *loop_vectorized_call)
> && !LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo)
> && known_eq (lowest_vf, vf))
> {
> + /* Make sure to use niters here, as if we have peeled for
> + early break then this will be updated to remove the extra
> + iteration for the latch. */
> niters_vector
> = build_int_cst (TREE_TYPE (LOOP_VINFO_NITERS (loop_vinfo)),
> - LOOP_VINFO_INT_NITERS (loop_vinfo) /
> lowest_vf);
> + TREE_INT_CST_LOW (niters) / lowest_vf);
> step_vector = build_one_cst (TREE_TYPE (niters));
> }
> else if (vect_use_loop_mask_for_alignment_p (loop_vinfo))
> --
> 2.55.0
