Re: [PATCH v1 1/1] vect: Prepare promotion/demotion for BB SLP with predicated tails

On Fri, Jul 17, 2026 at 3:32 PM Christopher Bazley via Sourceware
Forge <[email protected]> wrote:
>
> From: Christopher Bazley <[email protected]>
>
> vect_create_vectorized_promotion_stmts no longer pushes
> more stmts than implied by vect_get_num_copies because it could
> previously overrun the number of slots allocated for an SLP node
> (based on its number of lanes and type). e.g., four defs were
> pushed for a promotion of V8HI to V2DI (8/2=4) even if only two
> lanes of the V8HI were active. Allowing it later caused ICE in
> vectorizable_operation for a parent node, because binary ops
> require both operands to be the same length.
>
> Since promotion no longer produces redundant definitions,
> vectorizable_conversion also had to be modified so that demotion no
> longer relies on an even number of defs being produced. If
> necessary, it now pushes a single constant zero def.
>
> gcc/ChangeLog:
>
>         * tree-vect-stmts.cc (vect_create_vectorized_promotion_stmts):
>         Require an SLP tree node to be passed by the caller, for use
>         by vect_get_num_copies.  Stop pushing more stmts than implied
>         by vect_get_num_copies.
>         (vectorizable_conversion): Pass SLP tree node to
>         vect_create_vectorized_promotion_stmts.  Demotion no longer
>         relies on an even number of definitions being produced by
>         promotion. If necessary, push a single constant zero
>         definition.
> ---
>  gcc/tree-vect-stmts.cc | 61 ++++++++++++++++++++++++++----------------
>  1 file changed, 38 insertions(+), 23 deletions(-)
>
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index 700b3e7e0686..5ce9db48e286 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -5167,7 +5167,7 @@ vect_create_vectorized_demotion_stmts (vec_info *vinfo, vec<tree> *vec_oprnds,
>     call the function recursively.  */
>
>  static void
> -vect_create_vectorized_promotion_stmts (vec_info *vinfo,
> +vect_create_vectorized_promotion_stmts (vec_info *vinfo, slp_tree slp_node,
>                                         vec<tree> *vec_oprnds0,
>                                         vec<tree> *vec_oprnds1,
>                                         stmt_vec_info stmt_info, tree vec_dest,
> @@ -5180,37 +5180,39 @@ vect_create_vectorized_promotion_stmts (vec_info *vinfo,
>    gimple *new_stmt1, *new_stmt2;
>    vec<tree> vec_tmp = vNULL;
>
> -  vec_tmp.create (vec_oprnds0->length () * 2);
> +  const unsigned ncopies = vect_get_num_copies (vinfo, slp_node);
> +  vec_tmp.create (ncopies);
> +  gcc_assert (vec_oprnds0->length () <= ncopies);
>    FOR_EACH_VEC_ELT (*vec_oprnds0, i, vop0)
>      {
> +      if (vec_tmp.length () >= ncopies)
> +       break;

Can this ever happen?  We'd have a completely unused vector operand.
I understand in your case we have the upper part of a vector unused.
I think this warrants a comment, possibly before the loop, to document
the input/output cases we support - formerly it was two times the number
of output vectors due to the "same vector size" constraint.  I wonder
why you do not need to adjust the callers modifier (NONE/WIDEN/NARROW)
handling?

> +
>        if (op_type == binary_op)
>         vop1 = (*vec_oprnds1)[i];
>        else
>         vop1 = NULL_TREE;
>
>        /* Generate the two halves of promotion operation.  */
> -      new_stmt1 = vect_gen_widened_results_half (vinfo, ch1, vop0, vop1,
> -                                                op_type, vec_dest, gsi,
> -                                                stmt_info);
> -      new_stmt2 = vect_gen_widened_results_half (vinfo, ch2, vop0, vop1,
> -                                                op_type, vec_dest, gsi,
> -                                                stmt_info);
> -      if (is_gimple_call (new_stmt1))
> -       {
> -         new_tmp1 = gimple_call_lhs (new_stmt1);
> -         new_tmp2 = gimple_call_lhs (new_stmt2);
> -       }
> -      else
> +      new_stmt1
> +       = vect_gen_widened_results_half (vinfo, ch1, vop0, vop1, op_type,
> +                                        vec_dest, gsi, stmt_info);
> +      new_tmp1 = is_gimple_call (new_stmt1) ? gimple_call_lhs (new_stmt1)
> +                                           : gimple_assign_lhs (new_stmt1);

gimple_get_lhs (new_stmt1);

> +      vec_tmp.quick_push (new_tmp1);
> +
> +      if (vec_tmp.length () < ncopies)
>         {
> -         new_tmp1 = gimple_assign_lhs (new_stmt1);
> -         new_tmp2 = gimple_assign_lhs (new_stmt2);
> +         new_stmt2
> +           = vect_gen_widened_results_half (vinfo, ch2, vop0, vop1, op_type,
> +                                            vec_dest, gsi, stmt_info);
> +         new_tmp2 = is_gimple_call (new_stmt2) ? gimple_call_lhs (new_stmt2)
> +                                               : gimple_assign_lhs (new_stmt2);

likewise.

> +         vec_tmp.quick_push (new_tmp2);
>         }
> -
> -      /* Store the results for the next step.  */
> -      vec_tmp.quick_push (new_tmp1);
> -      vec_tmp.quick_push (new_tmp2);
>      }
>
> +  gcc_assert (vec_tmp.length () <= ncopies);

No need for this, quick_push will ICE already.

>    vec_oprnds0->release ();
>    *vec_oprnds0 = vec_tmp;
>  }
> @@ -5832,12 +5834,15 @@ vectorizable_conversion (vec_info *vinfo,
>                                              stmt_info, this_dest, gsi, c1,
>                                              op_type);
>           else
> -           vect_create_vectorized_promotion_stmts (vinfo, &vec_oprnds0,
> -                                                   &vec_oprnds1, stmt_info,
> -                                                   this_dest, gsi,
> +           vect_create_vectorized_promotion_stmts (vinfo, slp_node,
> +                                                   &vec_oprnds0, &vec_oprnds1,
> +                                                   stmt_info, this_dest, gsi,
>                                                     c1, c2, op_type);
>         }
>
> +      gcc_assert (vec_oprnds0.length ()
> +                 == vect_get_num_copies (vinfo, slp_node));

No need for such asserts IMO.

> +
>        FOR_EACH_VEC_ELT (vec_oprnds0, i, vop0)
>         {
>           gimple *new_stmt;
> @@ -5861,6 +5866,16 @@ vectorizable_conversion (vec_info *vinfo,
>          generate more than one vector stmt - i.e - we need to "unroll"
>          the vector stmt by a factor VF/nunits.  */
>        vect_get_vec_defs (vinfo, slp_node, op0, &vec_oprnds0);
> +
> +      /* Promotion no longer produces redundant defs (since support was
> +       added for length/mask-predicated BB SLP of awkward-sized groups),
> +       therefore demotion now has to handle that case too.  */
> +      if (vec_oprnds0.length () % 2 != 0)
> +       {
> +         tree vectype = TREE_TYPE (vec_oprnds0[0]);
> +         vec_oprnds0.safe_push (build_zero_cst (vectype));

Ah, so it's for those zeros?

> +       }
> +
>        /* Arguments are ready.  Create the new vector stmts.  */
>        if (cvt_type && modifier == NARROW_DST)
>         FOR_EACH_VEC_ELT (vec_oprnds0, i, vop0)
> --
> 2.54.0
>