> -----Original Message-----
> From: Richard Biener <[email protected]>
> Sent: 08 July 2026 12:29
> 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 Wed, 8 Jul 2026, Tamar Christina wrote:
>
> > > -----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'.
>
> Yes, or like asked for in PR126099, or PR126080.
Thanks, hadn't seen those.
>
> > > 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?
>
> SLP discovery is required to reproduce the seed lanes so I can't see how.
>
Sure, I was thinking of something like
{ (cast)a, (cast)b }
Where the seed is the constructor and cast is a truncating expression and b
e.g. V4SI.
> > And we also have ldst where usually the lanes in the root doesn't match the
> > children.
>
> Note the "root" is the entry to the graph, not the leafs.
Sure, I was referring to e.g. https://godbolt.org/z/W8jTv8qWx
Where the seed has two lanes, or at least two scalar statements
note: Final SLP tree for instance 0x17311410:
note: node 0x1736c438 (max_nunits=4, refcnt=1) vector(4) float
note: op template: *_12 = _13;
note: stmt 0 *_12 = _13;
note: stmt 1 *_19 = _20;
note: children 0x1736bd08 0x1736c158 (store-lanes)
note: node 0x1736bd08 (max_nunits=4, refcnt=3) vector(4) float
note: op template: _13 = _4 + _11;
note: stmt 0 _13 = _4 + _11;
note: children 0x1736bdc0 0x1736be78
note: node 0x1736bdc0 (max_nunits=4, refcnt=2) vector(4) float
note: op template: _4 = *_3;
note: stmt 0 _4 = *_3;
note: load permutation { 0 }
note: node 0x1736be78 (max_nunits=4, refcnt=2) vector(4) float
note: op template: _11 = _6 * _10;
note: stmt 0 _11 = _6 * _10;
note: children 0x1736bf30 0x1736bfe8
note: node 0x1736bf30 (max_nunits=4, refcnt=3) vector(4) float
note: op template: _6 = *_5;
note: stmt 0 _6 = *_5;
note: load permutation { 0 }
note: node 0x1736bfe8 (max_nunits=4, refcnt=2) vector(4) float
note: op template: _10 = *_9;
note: stmt 0 _10 = *_9;
note: load permutation { 1 }
note: node 0x1736c158 (max_nunits=4, refcnt=3) vector(4) float
note: op template: _20 = _15 + _18;
note: stmt 0 _20 = _15 + _18;
note: children 0x1736c210 0x1736c2c8
note: node 0x1736c210 (max_nunits=4, refcnt=2) vector(4) float
note: op template: _15 = *_14;
note: stmt 0 _15 = *_14;
note: load permutation { 1 }
note: node 0x1736c2c8 (max_nunits=4, refcnt=2) vector(4) float
note: op template: _18 = _6 * _17;
note: stmt 0 _18 = _6 * _17;
note: children 0x1736bf30 0x1736c380
note: node 0x1736c380 (max_nunits=4, refcnt=2) vector(4) float
note: op template: _17 = *_16;
note: stmt 0 _17 = *_16;
note: load permutation { 0 }
but this may be a case where the PP is now showing accurately.
when matching them they were a bit awkward....
Just to avoid confusion, It wasn't clear to me whether you wanted me
to change the current patch for this aside from the comments update?
Thanks,
Tamar
>
> > >
> > > > - 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)
> >
>
> --
> Richard Biener <[email protected]>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Jochen Jaser, Andrew McDonald; (HRB 36809, AG Nuernberg)
