On Wed, 19 Aug 2026, Alfie Richards wrote:
> On 19/08/2026 15:17, Richard Biener wrote:
> > The following decides on the VF based on assigned SLP_TREE_VECTYPE
> > rather than on the tracked max_nunits in the SLP graph which also
> > factors in external and constant nodes. Those get their vector
> > type assigned in vectorizable_* via vect_maybe_update_slp_op_vectype
> > and there's freedom to vectorizable_* to pick a suitable one.
> >
> > Specifically conversions of externs/constants can cause a higher
> > than necessary VF which would be a missed optimization.
> >
> > Changing the VF as outlined will run into both vect_prologue_cost_for_slp
> > and vect_create_constant_vectors ICEing when computing
> > vect_get_num_copies since that asserts it can exact_div the
> > number of lanes as in the unrolled loop by the number of lanes in the
> > (out of loop) vector type. But vectorizable_* can opt to just
> > use the lowpart of such vectors but require it in full due to target
> > constraints. So this RFC patch introduces vect_get_num_copies_
> > which performs a ceil_div instead (I think we can change the global
> > copy to do that). This shifts the ICE to vectorizable_conversion
> > which is mightly confused by "too large" vector types. IMO rather
> > than picking a vector type randomly when not already assigned, it
> > should chose it based on target constraints. Or, even better,
> > emit the conversion in scalar. But I expect similar issues to
> > appear in multi-operand widening/narrowing operations.
> >
> > The fundamental issue is that we eventually break the same-size
> > constraint for loop vectorization here (which we want!), and some
> > vectorizable_* are not prepared.
> >
> > On x86_64 I see the following (unsure if related):
> >
> > FAIL: gcc.target/i386/pr108938-3.c scan-assembler-times bswap[\\t ]+ 3
> >
> > Otherwise bootstrapped/tested on x86_64-unknown-linux-gnu and
> > aarch64-linux-gnu.
> >
> > I suspect coverage for constant/extern operands isn't great, of course
> > and the vectorizable_conversion hack needs to be properly fixed.
> > Plus, I wonder when can_div_away_from_zero_p can possibly fail ...
> >
> > Comments?
>
> Hi Richi,
>
> This seems like a good change to me. If I understand correctly it seems
> like it this removes all the uses for max_nunits. So will you remove the
> member and updates in a future patch?
Yes, of course.
> >
> > Thanks,
> > Richard.
> >
> > * tree-vect-slp.cc (vect_update_slp_vf_for_node): Compute
> > VF based on SLP_TREE_VECTYPE only.
> > (vect_get_num_copies_): New.
> > (vect_prologue_cost_for_slp): Use vect_get_num_copies_.
> > (vect_create_constant_vectors): Likewise.
> > * tree-vect-stmts.cc (vectorizable_conversion): Chose
> > constant/extern vectype based on legacy constraints.
> > Add workaround for missed handling.
> > ---
> > gcc/tree-vect-slp.cc | 35 +++++++++++++++++++++++++++++++----
> > gcc/tree-vect-stmts.cc | 13 ++++++++++++-
> > 2 files changed, 43 insertions(+), 5 deletions(-)
> >
> > diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
> > index 30f70453b2b..fcd9329f176 100644
> > --- a/gcc/tree-vect-slp.cc
> > +++ b/gcc/tree-vect-slp.cc
> > @@ -8766,8 +8766,13 @@ vect_update_slp_vf_for_node (slp_tree node,
> > @@ poly_uint64 &vf,
> > vectorizable_* should honor the vectorization factor when trying to
> > assign vector types to constants and externals and cause iteration
> > to a higher vectorization factor when required. */
> > + tree vectype = SLP_TREE_VECTYPE (node);
> > + if (!vectype)
> > + /* OMP SIMD calls w/o LHS have no SLP_TREE_VECTYPE set. */
> > + return;
> > poly_uint64 node_vf
> > - = calculate_unrolling_factor (node->max_nunits, SLP_TREE_LANES (node));
> > + = calculate_unrolling_factor (TYPE_VECTOR_SUBPARTS (vectype),
> > + SLP_TREE_LANES (node));
> > vf = force_common_multiple (vf, node_vf);
> >
> > /* For permute nodes that are fed from externs or constants we have to
> > @@ -8777,7 +8782,7 @@ vect_update_slp_vf_for_node (slp_tree node,
> > @@ poly_uint64 &vf,
> > if (SLP_TREE_DEF_TYPE (child) != vect_internal_def)
> > {
> > poly_uint64 child_vf
> > - = calculate_unrolling_factor (node->max_nunits,
> > + = calculate_unrolling_factor (TYPE_VECTOR_SUBPARTS (vectype),
> > SLP_TREE_LANES (child));
> > vf = force_common_multiple (vf, child_vf);
> > }
> > @@ -9074,6 +9079,28 @@ vect_scalar_ops_slice_hash::equal (const value_type
> > @@ &s1,
> > return true;
> > }
> >
> > +static unsigned
> > +vect_get_num_copies_ (vec_info *vinfo, slp_tree node)
> > +{
> > + unsigned nvectors; // = vect_get_num_copies (vinfo, node);
>
> I assume this is a left over comment from development?
Yes, I also plan to do this change to the global vect_get_num_copies.
> > + /* Inline copy of vect_get_num_copies with ceil_div instead of exact_div.
> > */
> > + {
> > + poly_uint64 vf;
> > +
> > + if (loop_vec_info loop_vinfo = dyn_cast <loop_vec_info> (vinfo))
> > + vf = LOOP_VINFO_VECT_FACTOR (loop_vinfo);
> > + else
> > + vf = 1;
> > +
> > + vf *= SLP_TREE_LANES (node);
> > + tree vectype = SLP_TREE_VECTYPE (node);
> > + bool res = can_div_away_from_zero_p (vf, TYPE_VECTOR_SUBPARTS
> > (vectype),
> > + &nvectors);
>
> This also drops the assertion that nvectors is a constant poly_int, which I
> think is worth preserving (though I assume this function is not in its final
> state rn?).
Hmm, nvectors is unsigned int, and the comment of the function reads
/* Return true if there is some constant Q and polynomial r such that:
(1) a = b * Q + r
(2) |a| <= |b * Q|
(3) |r| < |b|
Store the value Q in *QUOTIENT if so. */
template<unsigned int N, typename Ca, typename Cb, typename Cq>
inline typename if_nonpoly<Cq, bool>::type
can_div_away_from_zero_p (const poly_int<N, Ca> &a, const poly_int<N, Cb>
&b,
Cq *quotient)
so it should be always constant. What we still might lack is
a guard against !res upthread - here in vectorizable_comparison.
I bet fuzzers will get us test coverage when the patch lands.
Richard.
> Thanks,
> Alfie
>
> > + gcc_assert (res);
> > + }
> > + return nvectors;
> > +}
> > +
> > /* Compute the prologue cost for invariant or constant operands
> > represented
> > by NODE. */
> >
> > @@ -9092,7 +9119,7 @@ vect_prologue_cost_for_slp (vec_info *vinfo, slp_tree
> > @@ node,
> > unsigned group_size = SLP_TREE_LANES (node);
> > unsigned HOST_WIDE_INT const_nunits;
> > unsigned nelt_limit;
> > - unsigned nvectors = vect_get_num_copies (vinfo, node);
> > + unsigned nvectors = vect_get_num_copies_ (vinfo, node);
> > auto ops = &SLP_TREE_SCALAR_OPS (node);
> > auto_vec<unsigned int> starts (nvectors);
> > if (TYPE_VECTOR_SUBPARTS (vectype).is_constant (&const_nunits)
> > @@ -11022,7 +11049,7 @@ vect_create_constant_vectors (vec_info *vinfo,
> > @@ slp_tree op_node)
> > /* We always want SLP_TREE_VECTYPE (op_node) here correctly set. */
> > vector_type = SLP_TREE_VECTYPE (op_node);
> > - unsigned int number_of_vectors = vect_get_num_copies (vinfo, op_node);
> > + unsigned int number_of_vectors = vect_get_num_copies_ (vinfo, op_node);
> > SLP_TREE_VEC_DEFS (op_node).create (number_of_vectors);
> > auto_vec<tree> voprnds (number_of_vectors);
> > diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> > index 344aeddfb5a..802b0de40de 100644
> > --- a/gcc/tree-vect-stmts.cc
> > +++ b/gcc/tree-vect-stmts.cc
> > @@ -5442,7 +5442,14 @@ vectorizable_conversion (vec_info *vinfo,
> > if (!cost_vec)
> > gcc_assert (vectype_in);
> > if (!vectype_in)
> > - vectype_in = get_vectype_for_scalar_type (vinfo, rhs_type, slp_node);
> > + {
> > + if (loop_vinfo)
> > + vectype_in = get_same_sized_vectype (rhs_type, vectype_out);
> > + else
> > + vectype_in = get_related_vectype_for_scalar_type
> > + (vinfo->vector_mode, rhs_type,
> > + TYPE_VECTOR_SUBPARTS (vectype_out));
> > + }
> > if (!vectype_in)
> > {
> > if (dump_enabled_p ())
> > @@ -5875,6 +5882,10 @@ vectorizable_conversion (vec_info *vinfo,
> >
> > FOR_EACH_VEC_ELT (vec_oprnds0, i, vop0)
> > {
> > + /* ??? FIXME - NARROW/WIDEN assumes all lanes are used
> > + (gcc.dg/vect/O3-vect-pr32243.c). */
> > + if (!slp_node->vec_defs.space (1))
> > + break;
> > gimple *new_stmt;
> > if (cvt_type)
> > {
>
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)
