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

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?

> 
> 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?

> +  /* 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?).

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)
>   	    {