On Thu, 8 Feb 2024, Tamar Christina wrote:
> Hi All,
>
> There's a bug in vectorizable_live_operation that restart_loop is defined
> outside the loop.
>
> This variable is supposed to indicate whether we are doing a first or last
> index reduction. The problem is that by defining it outside the loop it
> becomes
> dependent on the order we visit the USE/DEFs.
>
> In the given example, the loop isn't PEELED, but we visit the early exit uses
> first. This then sets the boolean to true and it can't get to false again.
>
> So when we visit the main exit we still treat it as an early exit for that
> SSA name.
>
> This cleans it up and renames the variables to something that's hopefully
> clearer to their intention.
>
> Bootstrapped Regtested on aarch64-none-linux-gnu and
> x86_64-pc-linux-gnu no issues.
>
> Ok for master?
OK.
Thanks,
Richard.
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
> PR tree-optimization/113808
> * tree-vect-loop.cc (vectorizable_live_operation): Don't cache the
> value cross iterations.
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/113808
> * gfortran.dg/vect/vect-early-break_1-PR113808.f90: New test.
>
> --- inline copy of patch --
> diff --git a/gcc/testsuite/gfortran.dg/vect/vect-early-break_1-PR113808.f90
> b/gcc/testsuite/gfortran.dg/vect/vect-early-break_1-PR113808.f90
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..5c339fa7a348fac5527bbbf456a535da96b5c1ed
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/vect/vect-early-break_1-PR113808.f90
> @@ -0,0 +1,21 @@
> +! { dg-add-options vect_early_break }
> +! { dg-require-effective-target vect_early_break }
> +! { dg-require-effective-target vect_long_long }
> +! { dg-additional-options "-fopenmp-simd" }
> +
> +! { dg-final { scan-tree-dump "LOOP VECTORIZED" "vect" } }
> +
> +program main
> + integer :: n, i,k
> + n = 11
> + do i = 1, n,2
> + !$omp simd lastprivate(k)
> + do k = 1, i + 41
> + if (k > 11 + 41 .or. k < 1) error stop
> + end do
> + end do
> + if (k /= 53) then
> + print *, k, 53
> + error stop
> + endif
> +end
> diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
> index
> 190df9ec7741fd05aa0b9abe150baf06b2ca9a57..eed2268e9bae7e7ad36d13da03e0b54eab26ef6f
> 100644
> --- a/gcc/tree-vect-loop.cc
> +++ b/gcc/tree-vect-loop.cc
> @@ -10950,7 +10950,7 @@ vectorizable_live_operation (vec_info *vinfo,
> stmt_vec_info stmt_info,
> did. For the live values we want the value at the start of the
> iteration
> rather than at the end. */
> edge main_e = LOOP_VINFO_IV_EXIT (loop_vinfo);
> - bool restart_loop = LOOP_VINFO_EARLY_BREAKS_VECT_PEELED (loop_vinfo);
> + bool all_exits_as_early_p = LOOP_VINFO_EARLY_BREAKS_VECT_PEELED
> (loop_vinfo);
> FOR_EACH_IMM_USE_STMT (use_stmt, imm_iter, lhs)
> if (!is_gimple_debug (use_stmt)
> && !flow_bb_inside_loop_p (loop, gimple_bb (use_stmt)))
> @@ -10966,8 +10966,7 @@ vectorizable_live_operation (vec_info *vinfo,
> stmt_vec_info stmt_info,
> /* For early exit where the exit is not in the BB that leads
> to the latch then we're restarting the iteration in the
> scalar loop. So get the first live value. */
> - restart_loop = restart_loop || !main_exit_edge;
> - if (restart_loop
> + if ((all_exits_as_early_p || !main_exit_edge)
> && STMT_VINFO_DEF_TYPE (stmt_info) == vect_induction_def)
> {
> tmp_vec_lhs = vec_lhs0;
>
>
>
>
>
--
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)
