> -----Original Message-----
> From: Richard Biener <[email protected]>
> Sent: 07 July 2026 12:26
> To: Tamar Christina <[email protected]>
> Cc: [email protected]; nd <[email protected]>; [email protected]
> Subject: Re: [patch v2 2/2][vect]: replace usages of SLP_TREE_SCALAR_SMTS
> where possible
>
> On Mon, 6 Jul 2026, Tamar Christina wrote:
>
> > Continuing the work started in GCC 12 with g:a0dae768c6f78eba
> > this patch replaces uses of SLP_TREE_SCALAR_STMTS with SLP_TREE_LANES
> when used
> > to determine the number of lanes in the SLP tree. This de-couples the need
> to
> > have the same number of scalar statements as lanes in the SLP tree.
> >
> > Bootstrapped Regtested on aarch64-none-linux-gnu,
> > arm-none-linux-gnueabihf, x86_64-pc-linux-gnu
> > -m32, -m64 and no issues.
> >
> > Any feedback?
> >
> > Thanks,
> > Tamar
> >
> > gcc/ChangeLog:
> >
> > * tree-vect-slp-patterns.cc (compatible_complex_nodes_p): Support
> > mismatched group sizes and use group size to iterate.
> > * tree-vect-slp.cc (vect_build_slp_instance,
> > vect_analyze_slp_reduc_chain, vect_analyze_slp_reduction,
> > vect_analyze_slp_reduction_group, vect_analyze_slp_instance,
> > vect_analyze_slp): Compare against lanes instead of .length ().
> > (vect_optimize_slp_pass::get_result_with_layout): replace .length ()
> > with .exist ().
> > (vect_transform_slp_perm_load_1): Use lanes instead of .length ().
> > (vect_remove_slp_scalar_calls): Replace iterators.
> > * tree-vect-stmts.cc (vectorizable_shift): Use group size over ops
> > length.
> >
> > ---
> > diff --git a/gcc/tree-vect-slp-patterns.cc b/gcc/tree-vect-slp-patterns.cc
> > index
> 181281c2154bedfc8e8c6b1bde1226e194ba99e5..3194642ef88e20b60ead
> c44b1d69c0d4f5a59a96 100644
> > --- a/gcc/tree-vect-slp-patterns.cc
> > +++ b/gcc/tree-vect-slp-patterns.cc
> > @@ -762,7 +762,10 @@ compatible_complex_nodes_p
> (slp_compat_nodes_map_t *compat_cache,
> > are externals. */
> > if (SLP_TREE_DEF_TYPE (a) != vect_internal_def)
> > {
> > - for (unsigned i = 0; i < SLP_TREE_SCALAR_OPS (a).length (); i++)
> > + unsigned group_size = SLP_TREE_LANES (a);
> > + gcc_assert (SLP_TREE_SCALAR_OPS (a).length () == group_size
> > + && SLP_TREE_SCALAR_OPS (b).length () == group_size);
> > + for (unsigned i = 0; i < group_size; i++)
>
> OK.
>
> > {
> > tree op1 = SLP_TREE_SCALAR_OPS (a)[pa[i % 2]];
> > tree op2 = SLP_TREE_SCALAR_OPS (b)[pb[i % 2]];
> > diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
> > index
> 514bc0c5c654cc392ab5a47ea6b08b65fe7786fe..e74556b7dc558f931482b
> 9dc1300d90c84b008fd 100644
> > --- a/gcc/tree-vect-slp.cc
> > +++ b/gcc/tree-vect-slp.cc
> > @@ -4285,8 +4285,8 @@ vect_build_slp_instance (vec_info *vinfo,
> > /* ??? We've replaced the old SLP_INSTANCE_GROUP_SIZE with
> > the number of scalar stmts in the root in a few places.
> > Verify that assumption holds. */
>
> The comment and the copies need adjustment with the change. I wonder
> whether at the point this was added SLP_TREE_LANES existed?
>
> That said, group_size in this function is the number of "scalar"
> stmts. I think we want to elide 'group_size' from the
> vect_build_slp_tree API since this is not a "global" invariant
> but the number of lanes could change throughout the SLP graph.
>
Yeah, I guess something like a narrowing of a pair of vectors and combining
them changes the 'group_size'.
> The assertions are still somewhat relevant in that the root nodes
> number of lanes should match the number of lanes from the scalar
> stmt seeds.
Hmm is that always the case? Wouldn't in a 2 V4SI -> V8HI operation the lanes
between the seed and root be possibly mismatched?
And we also have ldst where usually the lanes in the root doesn't match the
children.
>
> > - gcc_assert (SLP_TREE_SCALAR_STMTS (SLP_INSTANCE_TREE
> (new_instance))
> > - .length () == group_size);
> > + gcc_assert (SLP_TREE_LANES (SLP_INSTANCE_TREE (new_instance))
> > + == group_size);
> >
> > if (dump_enabled_p ())
> > {
> > @@ -4720,8 +4720,8 @@ vect_analyze_slp_reduc_chain (loop_vec_info
> vinfo,
> > /* ??? We've replaced the old SLP_INSTANCE_GROUP_SIZE with
> > the number of scalar stmts in the root in a few places.
> > Verify that assumption holds. */
> > - gcc_assert (SLP_TREE_SCALAR_STMTS (SLP_INSTANCE_TREE
> (new_instance))
> > - .length () == group_size);
> > + gcc_assert (SLP_TREE_LANES (SLP_INSTANCE_TREE (new_instance))
> > + == group_size);
> >
> > if (dump_enabled_p ())
> > {
> > @@ -4809,8 +4809,8 @@ vect_analyze_slp_reduction (loop_vec_info vinfo,
> > /* ??? We've replaced the old SLP_INSTANCE_GROUP_SIZE with
> > the number of scalar stmts in the root in a few places.
> > Verify that assumption holds. */
> > - gcc_assert (SLP_TREE_SCALAR_STMTS (SLP_INSTANCE_TREE
> (new_instance))
> > - .length () == group_size);
> > + gcc_assert (SLP_TREE_LANES (SLP_INSTANCE_TREE (new_instance))
> > + == group_size);
> >
> > if (dump_enabled_p ())
> > {
> > @@ -4882,8 +4882,8 @@ vect_analyze_slp_reduction_group
> (loop_vec_info loop_vinfo,
> > /* ??? We've replaced the old SLP_INSTANCE_GROUP_SIZE with
> > the number of scalar stmts in the root in a few places.
> > Verify that assumption holds. */
> > - gcc_assert (SLP_TREE_SCALAR_STMTS (SLP_INSTANCE_TREE
> (new_instance))
> > - .length () == group_size);
> > + gcc_assert (SLP_TREE_LANES (SLP_INSTANCE_TREE (new_instance))
> > + == group_size);
> >
> > if (dump_enabled_p ())
> > {
> > @@ -5138,8 +5138,8 @@ vect_analyze_slp_instance (vec_info *vinfo,
> > /* ??? We've replaced the old SLP_INSTANCE_GROUP_SIZE with
> > the number of scalar stmts in the root in a few places.
> > Verify that assumption holds. */
> > - gcc_assert (SLP_TREE_SCALAR_STMTS (SLP_INSTANCE_TREE
> (new_instance))
> > - .length () == group_size);
> > + gcc_assert (SLP_TREE_LANES (SLP_INSTANCE_TREE (new_instance))
> > + == group_size);
> >
> > if (dump_enabled_p ())
> > {
> > @@ -5385,8 +5385,8 @@ vect_analyze_slp_instance (vec_info *vinfo,
> > /* ??? We've replaced the old SLP_INSTANCE_GROUP_SIZE with
> > the number of scalar stmts in the root in a few places.
> > Verify that assumption holds. */
> > - gcc_assert (SLP_TREE_SCALAR_STMTS (SLP_INSTANCE_TREE
> (new_instance))
> > - .length () == group_size);
> > + gcc_assert (SLP_TREE_LANES (SLP_INSTANCE_TREE (new_instance))
> > + == group_size);
> >
> > if (dump_enabled_p ())
> > {
> > @@ -7906,7 +7906,7 @@ vect_optimize_slp_pass::get_result_with_layout
> (slp_tree node,
> >
> > unsigned int num_lanes = SLP_TREE_LANES (node);
> > result = vect_create_new_slp_node (num_inputs, VEC_PERM_EXPR);
> > - if (SLP_TREE_SCALAR_STMTS (node).length ())
> > + if (SLP_TREE_SCALAR_STMTS (node).exists ())
>
> OK.
>
> > {
> > auto &stmts = SLP_TREE_SCALAR_STMTS (result);
> > stmts.safe_splice (SLP_TREE_SCALAR_STMTS (node));
> > @@ -8819,7 +8819,7 @@ vect_prologue_cost_for_slp (vec_info *vinfo,
> slp_tree node,
> > constants can be implemented as load from the constant pool.
> > When all elements are the same we can use a splat. */
> > tree vectype = SLP_TREE_VECTYPE (node);
> > - unsigned group_size = SLP_TREE_SCALAR_OPS (node).length ();
> > + unsigned group_size = SLP_TREE_LANES (node);
>
> OK.
>
> > unsigned HOST_WIDE_INT const_nunits;
> > unsigned nelt_limit;
> > unsigned nvectors = vect_get_num_copies (vinfo, node);
> > @@ -10987,7 +10987,7 @@ vect_transform_slp_perm_load_1 (vec_info
> *vinfo, slp_tree node,
> > stmt_vec_info stmt_info = SLP_TREE_SCALAR_STMTS (node)[0];
> > int vec_index = 0;
> > tree vectype = SLP_TREE_VECTYPE (node);
> > - unsigned int group_size = SLP_TREE_SCALAR_STMTS (node).length ();
> > + unsigned int group_size = SLP_TREE_LANES (node);
>
> OK.
>
> > unsigned int mask_element;
> > unsigned dr_group_size;
> > machine_mode mode;
> > diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> > index
> 76e24b2c1699b022e4480fc4aac87ff0b6ddf650..f41d49541d570b656eec1
> 0227f7f030051acd0bb 100644
> > --- a/gcc/tree-vect-stmts.cc
> > +++ b/gcc/tree-vect-stmts.cc
> > @@ -6388,15 +6388,18 @@ vectorizable_shift (vec_info *vinfo,
> > /* Now adjust the constant shift amount in place. */
> > if (incompatible_op1_vectype_p
> > && dt[1] == vect_constant_def)
> > - for (unsigned i = 0;
> > - i < SLP_TREE_SCALAR_OPS (slp_op1).length (); ++i)
> > - {
> > - SLP_TREE_SCALAR_OPS (slp_op1)[i]
> > - = fold_convert (TREE_TYPE (vectype),
> > - SLP_TREE_SCALAR_OPS (slp_op1)[i]);
> > - gcc_assert ((TREE_CODE (SLP_TREE_SCALAR_OPS (slp_op1)[i])
> > - == INTEGER_CST));
> > - }
> > + {
> > + unsigned group_size = SLP_TREE_LANES (slp_op1);
> > + gcc_assert (SLP_TREE_SCALAR_OPS (slp_op1).length () ==
> group_size);
>
> This is OK in principle, but how do we handle this when re-vectorizing
> vector by scalar shifts? Have scalar stmts of the vector operand
> { v0, v0, v0, v0 } and the scalar shift { s, s, s, s }, so both
> expanded to be TYPE_VECTOR_SUBPARTS times?
Yeah, my thoughts were that for constant arguments or externals that the
node should be flattened. The lanes would match up but of course not the
number of scalar statements.
This does make it slightly awkward to decompose the tree but otoh it should
just work when vectorizing the arguments, whereas a weird nested operation
would make decompose easier but vectorization harder.
Thanks,
Tamar
>
> > + for (unsigned i = 0; i < group_size; ++i)
> > + {
> > + SLP_TREE_SCALAR_OPS (slp_op1)[i]
> > + = fold_convert (TREE_TYPE (vectype),
> > + SLP_TREE_SCALAR_OPS (slp_op1)[i]);
> > + gcc_assert ((TREE_CODE (SLP_TREE_SCALAR_OPS (slp_op1)[i])
> > + == INTEGER_CST));
> > + }
> > + }
> > SLP_TREE_TYPE (slp_node) = shift_vec_info_type;
> > DUMP_VECT_SCOPE ("vectorizable_shift");
> > vect_model_simple_cost (vinfo, 1, slp_node, cost_vec);
> >
> >
> >
>
> --
> Richard Biener <[email protected]>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Jochen Jaser, Andrew McDonald; (HRB 36809, AG Nuernberg)
