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