On Thu, Aug 28, 2025 at 12:17 AM Richard Biener <rguent...@suse.de> wrote:
>
> On Wed, 27 Aug 2025, Tamar Christina wrote:
>
> > > -----Original Message-----
> > > From: Richard Biener <rguent...@suse.de>
> > > Sent: Wednesday, August 27, 2025 2:56 PM
> > > To: gcc-patches@gcc.gnu.org
> > > Cc: RISC-V CI <patchworks...@rivosinc.com>; Tamar Christina
> > > <tamar.christ...@arm.com>
> > > Subject: [PATCH] Pass reduction var to vectorize_fold_left_reduction
> > > directly
> > >
> > > Instead of going via the PHI node def, use the scalar reduction
> > > input from the reduction stmt.
> > >
> > > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> > >
> > > Sorry for the re-post, this is now re-based on pristine trunk,
> > > hopefully making git am happy.
> >
> > AArch64 and Arm are clean, I did notice an ICE on x86_64 though:
> >
> > spawn -ignore SIGHUP /opt/buildAgent/temp/buildTmp/gcc/xgcc
> > -B/opt/buildAgent/temp/buildTmp/gcc/
> > /opt/buildAgent/work/505bfdd4dad8af3d/gcc/testsuite/gcc.dg/vect/vect-cond-reduc-in-order-2-signed-zero.c
> > -m64 -fdiagnostics-plain-output -msse2 -ftree-vectorize
> > -fno-tree-loop-distribute-patterns -fno-vect-cost-model -fno-common -O2
> > -fdump-tree-vect-details -std=gnu99 -fno-fast-math -lm -o
> > ./vect-cond-reduc-in-order-2-signed-zero.exe
> > during GIMPLE pass: vect
> > dump file: ./vect-cond-reduc-in-order-2-signed-zero.c.185t.vect
> > /opt/buildAgent/work/505bfdd4dad8af3d/gcc/testsuite/gcc.dg/vect/vect-cond-reduc-in-order-2-signed-zero.c:
> > In function 'reduc_plus_double':
> > /opt/buildAgent/work/505bfdd4dad8af3d/gcc/testsuite/gcc.dg/vect/vect-cond-reduc-in-order-2-signed-zero.c:13:1:
> > internal compiler error: in compute_live_loop_exits, at
> > tree-ssa-loop-manip.cc:262
> > 0x2e864c7 internal_error(char const*, ...)
> > /opt/buildAgent/work/505bfdd4dad8af3d/gcc/diagnostic-global-context.cc:534
> > 0xf08d9c fancy_abort(char const*, int, char const*)
> > /opt/buildAgent/work/505bfdd4dad8af3d/gcc/diagnostics/context.cc:1687
> > 0x99b97d compute_live_loop_exits
> > /opt/buildAgent/work/505bfdd4dad8af3d/gcc/tree-ssa-loop-manip.cc:262
> > 0x99b97d add_exit_phis_var
> > /opt/buildAgent/work/505bfdd4dad8af3d/gcc/tree-ssa-loop-manip.cc:344
> > 0x99b97d add_exit_phis
> > /opt/buildAgent/work/505bfdd4dad8af3d/gcc/tree-ssa-loop-manip.cc:404
> > 0x19092bf rewrite_into_loop_closed_ssa_1
> > /opt/buildAgent/work/505bfdd4dad8af3d/gcc/tree-ssa-loop-manip.cc:618
> > 0x19092bf rewrite_into_loop_closed_ssa(bitmap_head*, unsigned int)
> > /opt/buildAgent/work/505bfdd4dad8af3d/gcc/tree-ssa-loop-manip.cc:640
> > 0x1ad7855 execute
> > /opt/buildAgent/work/505bfdd4dad8af3d/gcc/tree-vectorizer.cc:1399
> >
> > The compiler was configured with:
> >
> > + /opt/buildAgent/work/505bfdd4dad8af3d/configure
> > --enable-checking=yes,rtl,extra --with-arch=native --enable-multilib
> > --with-multilib-list=m32,m64
>
> -march=native, OK, I can reproduce that. I do wonder whether we want to
> iterate over SSE, AVX2, AVX512 in vect.exp testing ... (and possibly
> drop -flto there).
For aarch64, we should also iterate with -march=armv8-a+sve too.
Dropping LTO might be ok except for the cost model testcases (I can't
remember if there is any with vect.exp).
Thanks,
Andrew
>
> Investigating.
>
> Richard.
>
> > Cheers,
> > Tamar
> >
> > >
> > > * tree-vect-loop.cc (vectorize_fold_left_reduction): Get
> > > reduc_var as argument.
> > > (vect_transform_reduction): Adjust.
> > > ---
> > > gcc/tree-vect-loop.cc | 8 +++-----
> > > 1 file changed, 3 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
> > > index f22613e5a3a..f48a18a593d 100644
> > > --- a/gcc/tree-vect-loop.cc
> > > +++ b/gcc/tree-vect-loop.cc
> > > @@ -6501,7 +6501,7 @@ vectorize_fold_left_reduction (loop_vec_info
> > > loop_vinfo,
> > > stmt_vec_info stmt_info,
> > > gimple_stmt_iterator *gsi,
> > > slp_tree slp_node,
> > > - gimple *reduc_def_stmt,
> > > + tree reduc_var,
> > > code_helper code, internal_fn reduc_fn,
> > > int num_ops, tree vectype_in,
> > > int reduc_index, vec_loop_masks *masks,
> > > @@ -6546,7 +6546,6 @@ vectorize_fold_left_reduction (loop_vec_info
> > > loop_vinfo,
> > > gimple *sdef = vect_orig_stmt (scalar_dest_def_info)->stmt;
> > > tree scalar_dest = gimple_get_lhs (sdef);
> > > tree scalar_type = TREE_TYPE (scalar_dest);
> > > - tree reduc_var = gimple_phi_result (reduc_def_stmt);
> > >
> > > int vec_num = vec_oprnds0.length ();
> > > tree vec_elem_type = TREE_TYPE (vectype_out);
> > > @@ -8016,8 +8015,6 @@ vect_transform_reduction (loop_vec_info loop_vinfo,
> > > The last use is the reduction variable. In case of nested cycle
> > > this
> > > assumption is not true: we use reduc_index to record the index of
> > > the
> > > reduction variable. */
> > > - stmt_vec_info phi_info = STMT_VINFO_REDUC_DEF (vect_orig_stmt
> > > (stmt_info));
> > > - gphi *reduc_def_phi = as_a <gphi *> (phi_info->stmt);
> > > int reduc_index = STMT_VINFO_REDUC_IDX (stmt_info);
> > > tree vectype_in = SLP_TREE_VECTYPE (SLP_TREE_CHILDREN (slp_node)[0]);
> > >
> > > @@ -8059,8 +8056,9 @@ vect_transform_reduction (loop_vec_info loop_vinfo,
> > > {
> > > internal_fn reduc_fn = STMT_VINFO_REDUC_FN (reduc_info);
> > > gcc_assert (code.is_tree_code () || cond_fn_p);
> > > + tree reduc_var = op.ops[reduc_index];
> > > return vectorize_fold_left_reduction
> > > - (loop_vinfo, stmt_info, gsi, slp_node, reduc_def_phi,
> > > + (loop_vinfo, stmt_info, gsi, slp_node, reduc_var,
> > > code, reduc_fn, op.num_ops, vectype_in,
> > > reduc_index, masks, lens);
> > > }
> > > --
> > > 2.43.0
> >
>
> --
> 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)
