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

Message ID bmm.hkh9u5nbgg.gcc.gcc.chris.bazley.191.1.1@forge-stage.sourceware.org
State New
Headers
Series vect: Prepare promotion/demotion for BB SLP with predicated tails |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed

Commit Message

Christopher Bazley via Sourceware Forge July 17, 2026, 1:31 p.m. UTC
  From: Christopher Bazley <chris.bazley@arm.com>

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

Comments

Richard Biener Aug. 3, 2026, 10:56 a.m. UTC | #1
On Fri, Jul 17, 2026 at 3:32 PM Christopher Bazley via Sourceware
Forge <forge-bot+chris.bazley@forge-stage.sourceware.org> wrote:
>
> From: Christopher Bazley <chris.bazley@arm.com>
>
> 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
>
  

Patch

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;
+
       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);
+      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);
+	  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);
   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));
+
       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));
+	}
+
       /* Arguments are ready.  Create the new vector stmts.  */
       if (cvt_type && modifier == NARROW_DST)
 	FOR_EACH_VEC_ELT (vec_oprnds0, i, vop0)