On 03/08/2026 12:30, Richard Biener wrote:
On Thu, Jul 30, 2026 at 4:58 PM Christopher Bazley <[email protected]> wrote:
@@ -14173,7 +14211,33 @@ vect_maybe_update_slp_op_vectype (vec_info *vinfo,
slp_tree op, tree vectype)
&& SLP_TREE_DEF_TYPE (op) == vect_external_def
&& SLP_TREE_LANES (op) > 1)
return false;
- (void) vinfo; /* FORNOW */
+
+ /* When the vectorizer falls back to building vector operands from scalars,
+ it can create SLP trees with external defs that have a number of lanes not
+ divisible by the number of subparts in a vector type naively inferred from
+ the scalar type. Reject such types to avoid ICE when later computing the
+ prologue cost for invariant operands. */
+ if (SLP_TREE_DEF_TYPE (op) == vect_external_def)
+ {
+ poly_uint64 vf = vinfo->vectorization_factor;
+
+ vf *= SLP_TREE_LANES (op);
+
+ if (maybe_lt (TYPE_VECTOR_SUBPARTS (vectype), vf)
+ && !multiple_p (vf, TYPE_VECTOR_SUBPARTS (vectype)))
Coming here from 4/n, the patch to pass vinfo to all callers (the
split patch makes it
a bit awkward to review separately).
The comment suggests that the caller passes, say, vectype V8SI but for a SLP
node with 4 lanes and VF == 1? It would be the callers fault to do so, and we
should reject it there instead. Or is it that we fail to zero-pad the
external defs
for predicated tails?
I think we discussed this during review last year?
The case that motivated this check was a three-lane external-def node
assigned V2DI by vectorizable_conversion, as described in my follow-up
to your v11 review:
https://inbox.sourceware.org/gcc-patches/[email protected]/
Without this check, vect_prologue_cost_for_slp called
vect_get_num_copies for the node, which eventually tried to divide 3 by
2. I also found that vect_create_constant_vectors appeared liable to
fail for the same reason if analysis got that far.
In July, I asked you whether support for this "one and a half vectors"
lanes/vectype combination was something that could be deferred to a
future patch series. Your subsequent commit bb1344be44e3 ("choose VF
independent of externs/constants") implies that the answer is "no, it
cannot be deferred", because your commit allows constants and external
defs to have excess lanes and uses vect_get_num_copies_for_invariant to
handle them. Is that right?
Since your commit also adds the verification in
vect_slp_analyze_node_operations that you noted should make my
vect_maybe_update_slp_op_vectype change unnecessary, I will remove this
check from my patch series when I rebase. I expect that removing the
check will also make patch 4 of my series, which passes vinfo to
vect_maybe_update_slp_op_vectype, unnecessary.
Without now searching for the patch patching vect_get_num_copies, you
edit that to
/* Return the number of vectors in the context of vectorization region VINFO,
needed for a group of statements and a vector type as specified by NODE. */
inline unsigned int
vect_get_num_copies (vec_info *vinfo, slp_tree node)
{
tree vectype = SLP_TREE_VECTYPE (node);
unsigned int group_size = SLP_TREE_LANES (node);
poly_uint64 nunits;
if (loop_vec_info loop_vinfo = dyn_cast<loop_vec_info> (vinfo))
nunits = LOOP_VINFO_VECT_FACTOR (loop_vinfo) * group_size;
else
{
/* The group size is not necessarily an integral multiple of the number
of subparts in the vector type if doing BB SLP vectorization with
predicated tails. */
if (known_ge (TYPE_VECTOR_SUBPARTS (vectype), group_size))
return 1;
nunits = group_size;
}
return vect_get_num_vectors (nunits, vectype);
}
meaning iff there are less SLP lanes than vector lanes we have a single
vector. We discussed SLP splitting constraints here, I don't remember
exactly, but I think this constaint doesn't work on x86 if you consider
a V3SI -> V3DI promotion which gets you a V4SI and two V2DI vectors,
the 2nd with a predicated tail, so vect_get_num_copies would be 2 for
the V3DI node.
My patch patching vect_get_num_copies is "vect: Prepare vectorizable_*
functions for BB SLP with predicated tails":
https://inbox.sourceware.org/gcc-patches/[email protected]/
I suppose that we could instead change vect_get_num_vectors
to use can_div_away_from_zero_p (which is DIV_CEIL?)? That still
fulfills the documented semantics.
I think bb1344be44e3 has superseded this suggestion too, since you
instead added vect_get_num_copies_for_invariant and retained the
exact-division behaviour for the other callers.
I started rebasing my patch series on trunk and found some conflicts
between your changes and mine in vect_create_vectorized_promotion_stmts.
I will try to reconcile them as part of the rebase.
Thanks,
--
Christopher Bazley
Staff Software Engineer, GNU Tools Team.
Arm Ltd, 110 Fulbourn Road, Cambridge, CB1 9NJ, UK.
http://www.arm.com/
