Re: [PATCH][RFC] Remove max_nunits, choose VF independent of externs/constants

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)