> -----Original Message-----
> From: Richard Biener <[email protected]>
> Sent: 08 July 2026 13:13
> 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: 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....
>
> The above is from permute lowering, the store (root) still has
> two lanes but with the iffy representation of store-lanes
> (I have elided a VEC_PERM node that concats/interleaves here IIRC).
>
I see. Makes sense thank you.
> > 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?
>
> I think the patch is fine, it doesn't improve things though, and the
> comment needs updating.
Yeah the patch was just so I don't have to do the refactoring work during the
re-vectorization work.
My intention was to just do the NFC work first and then the next patches use
these and update as needed. I'm still working through replacing the vector lanes
using SLP_TREE_LANES.
Apologies if I didn't make it clear what I was trying to do here.
Thanks,
Tamar
>
> Richard.
>
> > 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)
> >
>
> --
> Richard Biener <[email protected]>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Jochen Jaser, Andrew McDonald; (HRB 36809, AG Nuernberg)
