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
>
