On Wed, Sep 2, 2026 at 9:46 PM Christopher Bazley <[email protected]> wrote:
>
> 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?
Yes. I chose to add the _for_invariant variant rather than relaxing the main
variant to constrain where this may happen for the moment.
>
> 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.
Yes, I hoped this would simplify (shorten) your series.
> > 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.
Yes.
> 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.
Richard.
> Thanks,
> --
> Christopher Bazley
> Staff Software Engineer, GNU Tools Team.
> Arm Ltd, 110 Fulbourn Road, Cambridge, CB1 9NJ, UK.
> http://www.arm.com/
