diff mbox

Clean up PURE_SLP_STMT handling

Message ID 87a8jkeohz.fsf@e105548-lin.cambridge.arm.com
State New
Headers show

Commit Message

Richard Sandiford May 20, 2016, 3:30 p.m. UTC
The vectorizable_* routines had many instances of:

    slp_node || PURE_SLP_STMT (stmt_info)

which gives the misleading impression that we can have
!slp_node && PURE_SLP_STMT (stmt_info).  In this context
it's really enough to test slp_node on its own.

There are three cases:

  loop vectorisation only:
    vectorizable_foo called only with !slp_node

  pure SLP:
    vectorizable_foo called only with slp_node

  hybrid SLP:
    (e.g. a vector that's used in SLP statements and also in a reduction)
    - vectorizable_foo called once with slp_node for the SLP uses.
    - vectorizable_foo called once with !slp_node for the non-SLP uses.

Hybrid SLP isn't possible for stores, so I added an explicit assert
for that.

I also made vectorizable_comparison static, to make it obvious that
no other callers outside tree-vect-stmts.c could use it with the
!slp && PURE_SLP_STMT combination.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Thanks,
Richard


gcc/
	* tree-vectorizer.h (vectorizable_comparison): Delete.
	* tree-vect-loop.c (vectorizable_reduction): Remove redundant
	PURE_SLP_STMT check.
	* tree-vect-stmts.c (vectorizable_call): Likewise.
	(vectorizable_simd_clone_call): Likewise.
	(vectorizable_conversion): Likewise.
	(vectorizable_assignment): Likewise.
	(vectorizable_shift): Likewise.
	(vectorizable_operation): Likewise.
	(vectorizable_load): Likewise.
	(vectorizable_condition): Likewise.
	(vectorizable_store): Likewise.  Assert that we don't have
	hybrid SLP.
	(vectorizable_comparison): Make static.  Remove redundant
	PURE_SLP_STMT check.
	(vect_transform_stmt): Assert that we always have an slp_node
	if PURE_SLP_STMT.

Comments

Richard Biener May 23, 2016, 8:31 a.m. UTC | #1
On Fri, May 20, 2016 at 5:30 PM, Richard Sandiford
<richard.sandiford@arm.com> wrote:
> The vectorizable_* routines had many instances of:
>
>     slp_node || PURE_SLP_STMT (stmt_info)
>
> which gives the misleading impression that we can have
> !slp_node && PURE_SLP_STMT (stmt_info).  In this context
> it's really enough to test slp_node on its own.

Yeah, that's a cleanup opportunity since I changed the code last year
to always pass down slp_node in all phases.

> There are three cases:
>
>   loop vectorisation only:
>     vectorizable_foo called only with !slp_node
>
>   pure SLP:
>     vectorizable_foo called only with slp_node
>
>   hybrid SLP:
>     (e.g. a vector that's used in SLP statements and also in a reduction)
>     - vectorizable_foo called once with slp_node for the SLP uses.
>     - vectorizable_foo called once with !slp_node for the non-SLP uses.
>
> Hybrid SLP isn't possible for stores, so I added an explicit assert
> for that.
>
> I also made vectorizable_comparison static, to make it obvious that
> no other callers outside tree-vect-stmts.c could use it with the
> !slp && PURE_SLP_STMT combination.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Ok.

Thanks,
Richard.

> Thanks,
> Richard
>
>
> gcc/
>         * tree-vectorizer.h (vectorizable_comparison): Delete.
>         * tree-vect-loop.c (vectorizable_reduction): Remove redundant
>         PURE_SLP_STMT check.
>         * tree-vect-stmts.c (vectorizable_call): Likewise.
>         (vectorizable_simd_clone_call): Likewise.
>         (vectorizable_conversion): Likewise.
>         (vectorizable_assignment): Likewise.
>         (vectorizable_shift): Likewise.
>         (vectorizable_operation): Likewise.
>         (vectorizable_load): Likewise.
>         (vectorizable_condition): Likewise.
>         (vectorizable_store): Likewise.  Assert that we don't have
>         hybrid SLP.
>         (vectorizable_comparison): Make static.  Remove redundant
>         PURE_SLP_STMT check.
>         (vect_transform_stmt): Assert that we always have an slp_node
>         if PURE_SLP_STMT.
>
> Index: gcc/tree-vectorizer.h
> ===================================================================
> --- gcc/tree-vectorizer.h
> +++ gcc/tree-vectorizer.h
> @@ -1004,8 +1004,6 @@ extern void vect_remove_stores (gimple *);
>  extern bool vect_analyze_stmt (gimple *, bool *, slp_tree);
>  extern bool vectorizable_condition (gimple *, gimple_stmt_iterator *,
>                                     gimple **, tree, int, slp_tree);
> -extern bool vectorizable_comparison (gimple *, gimple_stmt_iterator *,
> -                                    gimple **, tree, int, slp_tree);
>  extern void vect_get_load_cost (struct data_reference *, int, bool,
>                                 unsigned int *, unsigned int *,
>                                 stmt_vector_for_cost *,
> Index: gcc/tree-vect-loop.c
> ===================================================================
> --- gcc/tree-vect-loop.c
> +++ gcc/tree-vect-loop.c
> @@ -5594,7 +5594,7 @@ vectorizable_reduction (gimple *stmt, gimple_stmt_iterator *gsi,
>    if (STMT_VINFO_LIVE_P (vinfo_for_stmt (reduc_def_stmt)))
>      return false;
>
> -  if (slp_node || PURE_SLP_STMT (stmt_info))
> +  if (slp_node)
>      ncopies = 1;
>    else
>      ncopies = (LOOP_VINFO_VECT_FACTOR (loop_vinfo)
> Index: gcc/tree-vect-stmts.c
> ===================================================================
> --- gcc/tree-vect-stmts.c
> +++ gcc/tree-vect-stmts.c
> @@ -2342,7 +2342,7 @@ vectorizable_call (gimple *gs, gimple_stmt_iterator *gsi, gimple **vec_stmt,
>         }
>      }
>
> -  if (slp_node || PURE_SLP_STMT (stmt_info))
> +  if (slp_node)
>      ncopies = 1;
>    else if (modifier == NARROW && ifn == IFN_LAST)
>      ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits_out;
> @@ -2792,7 +2792,7 @@ vectorizable_simd_clone_call (gimple *stmt, gimple_stmt_iterator *gsi,
>      return false;
>
>    /* FORNOW */
> -  if (slp_node || PURE_SLP_STMT (stmt_info))
> +  if (slp_node)
>      return false;
>
>    /* Process function arguments.  */
> @@ -3761,7 +3761,7 @@ vectorizable_conversion (gimple *stmt, gimple_stmt_iterator *gsi,
>    /* Multiple types in SLP are handled by creating the appropriate number of
>       vectorized stmts for each SLP node.  Hence, NCOPIES is always 1 in
>       case of SLP.  */
> -  if (slp_node || PURE_SLP_STMT (stmt_info))
> +  if (slp_node)
>      ncopies = 1;
>    else if (modifier == NARROW)
>      ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits_out;
> @@ -4242,7 +4242,7 @@ vectorizable_assignment (gimple *stmt, gimple_stmt_iterator *gsi,
>    /* Multiple types in SLP are handled by creating the appropriate number of
>       vectorized stmts for each SLP node.  Hence, NCOPIES is always 1 in
>       case of SLP.  */
> -  if (slp_node || PURE_SLP_STMT (stmt_info))
> +  if (slp_node)
>      ncopies = 1;
>    else
>      ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits;
> @@ -4502,7 +4502,7 @@ vectorizable_shift (gimple *stmt, gimple_stmt_iterator *gsi,
>    /* Multiple types in SLP are handled by creating the appropriate number of
>       vectorized stmts for each SLP node.  Hence, NCOPIES is always 1 in
>       case of SLP.  */
> -  if (slp_node || PURE_SLP_STMT (stmt_info))
> +  if (slp_node)
>      ncopies = 1;
>    else
>      ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits_in;
> @@ -4933,7 +4933,7 @@ vectorizable_operation (gimple *stmt, gimple_stmt_iterator *gsi,
>    /* Multiple types in SLP are handled by creating the appropriate number of
>       vectorized stmts for each SLP node.  Hence, NCOPIES is always 1 in
>       case of SLP.  */
> -  if (slp_node || PURE_SLP_STMT (stmt_info))
> +  if (slp_node)
>      ncopies = 1;
>    else
>      ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits_in;
> @@ -5250,6 +5250,10 @@ vectorizable_store (gimple *stmt, gimple_stmt_iterator *gsi, gimple **vec_stmt,
>        && TREE_CODE (scalar_dest) != MEM_REF)
>      return false;
>
> +  /* Cannot have hybrid store SLP -- that would mean storing to the
> +     same location twice.  */
> +  gcc_assert (slp == PURE_SLP_STMT (stmt_info));
> +
>    gcc_assert (gimple_assign_single_p (stmt));
>
>    tree vectype = STMT_VINFO_VECTYPE (stmt_info), rhs_vectype = NULL_TREE;
> @@ -5261,7 +5265,7 @@ vectorizable_store (gimple *stmt, gimple_stmt_iterator *gsi, gimple **vec_stmt,
>    /* Multiple types in SLP are handled by creating the appropriate number of
>       vectorized stmts for each SLP node.  Hence, NCOPIES is always 1 in
>       case of SLP.  */
> -  if (slp || PURE_SLP_STMT (stmt_info))
> +  if (slp)
>      ncopies = 1;
>    else
>      ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits;
> @@ -5343,9 +5347,7 @@ vectorizable_store (gimple *stmt, gimple_stmt_iterator *gsi, gimple **vec_stmt,
>        grouped_store = true;
>        first_stmt = GROUP_FIRST_ELEMENT (stmt_info);
>        group_size = GROUP_SIZE (vinfo_for_stmt (first_stmt));
> -      if (!slp
> -         && !PURE_SLP_STMT (stmt_info)
> -         && !STMT_VINFO_STRIDED_P (stmt_info))
> +      if (!slp && !STMT_VINFO_STRIDED_P (stmt_info))
>         {
>           if (vect_store_lanes_supported (vectype, group_size))
>             store_lanes_p = true;
> @@ -5354,7 +5356,7 @@ vectorizable_store (gimple *stmt, gimple_stmt_iterator *gsi, gimple **vec_stmt,
>         }
>
>        if (STMT_VINFO_STRIDED_P (stmt_info)
> -         && (slp || PURE_SLP_STMT (stmt_info))
> +         && slp
>           && (group_size > nunits
>               || nunits % group_size != 0))
>         {
> @@ -6263,7 +6265,7 @@ vectorizable_load (gimple *stmt, gimple_stmt_iterator *gsi, gimple **vec_stmt,
>    /* Multiple types in SLP are handled by creating the appropriate number of
>       vectorized stmts for each SLP node.  Hence, NCOPIES is always 1 in
>       case of SLP.  */
> -  if (slp || PURE_SLP_STMT (stmt_info))
> +  if (slp)
>      ncopies = 1;
>    else
>      ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits;
> @@ -6316,9 +6318,7 @@ vectorizable_load (gimple *stmt, gimple_stmt_iterator *gsi, gimple **vec_stmt,
>        first_stmt = GROUP_FIRST_ELEMENT (stmt_info);
>        group_size = GROUP_SIZE (vinfo_for_stmt (first_stmt));
>
> -      if (!slp
> -         && !PURE_SLP_STMT (stmt_info)
> -         && !STMT_VINFO_STRIDED_P (stmt_info))
> +      if (!slp && !STMT_VINFO_STRIDED_P (stmt_info))
>         {
>           if (vect_load_lanes_supported (vectype, group_size))
>             load_lanes_p = true;
> @@ -6431,8 +6431,8 @@ vectorizable_load (gimple *stmt, gimple_stmt_iterator *gsi, gimple **vec_stmt,
>      }
>    else if (STMT_VINFO_STRIDED_P (stmt_info))
>      {
> -      if ((grouped_load
> -          && (slp || PURE_SLP_STMT (stmt_info)))
> +      if (grouped_load
> +         && slp
>           && (group_size > nunits
>               || nunits % group_size != 0))
>         {
> @@ -7510,7 +7510,7 @@ vectorizable_condition (gimple *stmt, gimple_stmt_iterator *gsi,
>    int nunits = TYPE_VECTOR_SUBPARTS (vectype);
>    tree vectype1 = NULL_TREE, vectype2 = NULL_TREE;
>
> -  if (slp_node || PURE_SLP_STMT (stmt_info))
> +  if (slp_node)
>      ncopies = 1;
>    else
>      ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits;
> @@ -7716,7 +7716,7 @@ vectorizable_condition (gimple *stmt, gimple_stmt_iterator *gsi,
>
>     Return FALSE if not a vectorizable STMT, TRUE otherwise.  */
>
> -bool
> +static bool
>  vectorizable_comparison (gimple *stmt, gimple_stmt_iterator *gsi,
>                          gimple **vec_stmt, tree reduc_def,
>                          slp_tree slp_node)
> @@ -7750,7 +7750,7 @@ vectorizable_comparison (gimple *stmt, gimple_stmt_iterator *gsi,
>    mask_type = vectype;
>    nunits = TYPE_VECTOR_SUBPARTS (vectype);
>
> -  if (slp_node || PURE_SLP_STMT (stmt_info))
> +  if (slp_node)
>      ncopies = 1;
>    else
>      ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits;
> @@ -8160,6 +8160,7 @@ vect_transform_stmt (gimple *stmt, gimple_stmt_iterator *gsi,
>    stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
>    bool done;
>
> +  gcc_assert (slp_node || !PURE_SLP_STMT (stmt_info));
>    gimple *old_vec_stmt = STMT_VINFO_VEC_STMT (stmt_info);
>
>    switch (STMT_VINFO_TYPE (stmt_info))
diff mbox

Patch

Index: gcc/tree-vectorizer.h
===================================================================
--- gcc/tree-vectorizer.h
+++ gcc/tree-vectorizer.h
@@ -1004,8 +1004,6 @@  extern void vect_remove_stores (gimple *);
 extern bool vect_analyze_stmt (gimple *, bool *, slp_tree);
 extern bool vectorizable_condition (gimple *, gimple_stmt_iterator *,
 				    gimple **, tree, int, slp_tree);
-extern bool vectorizable_comparison (gimple *, gimple_stmt_iterator *,
-				     gimple **, tree, int, slp_tree);
 extern void vect_get_load_cost (struct data_reference *, int, bool,
 				unsigned int *, unsigned int *,
 				stmt_vector_for_cost *,
Index: gcc/tree-vect-loop.c
===================================================================
--- gcc/tree-vect-loop.c
+++ gcc/tree-vect-loop.c
@@ -5594,7 +5594,7 @@  vectorizable_reduction (gimple *stmt, gimple_stmt_iterator *gsi,
   if (STMT_VINFO_LIVE_P (vinfo_for_stmt (reduc_def_stmt)))
     return false;
 
-  if (slp_node || PURE_SLP_STMT (stmt_info))
+  if (slp_node)
     ncopies = 1;
   else
     ncopies = (LOOP_VINFO_VECT_FACTOR (loop_vinfo)
Index: gcc/tree-vect-stmts.c
===================================================================
--- gcc/tree-vect-stmts.c
+++ gcc/tree-vect-stmts.c
@@ -2342,7 +2342,7 @@  vectorizable_call (gimple *gs, gimple_stmt_iterator *gsi, gimple **vec_stmt,
 	}
     }
 
-  if (slp_node || PURE_SLP_STMT (stmt_info))
+  if (slp_node)
     ncopies = 1;
   else if (modifier == NARROW && ifn == IFN_LAST)
     ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits_out;
@@ -2792,7 +2792,7 @@  vectorizable_simd_clone_call (gimple *stmt, gimple_stmt_iterator *gsi,
     return false;
 
   /* FORNOW */
-  if (slp_node || PURE_SLP_STMT (stmt_info))
+  if (slp_node)
     return false;
 
   /* Process function arguments.  */
@@ -3761,7 +3761,7 @@  vectorizable_conversion (gimple *stmt, gimple_stmt_iterator *gsi,
   /* Multiple types in SLP are handled by creating the appropriate number of
      vectorized stmts for each SLP node.  Hence, NCOPIES is always 1 in
      case of SLP.  */
-  if (slp_node || PURE_SLP_STMT (stmt_info))
+  if (slp_node)
     ncopies = 1;
   else if (modifier == NARROW)
     ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits_out;
@@ -4242,7 +4242,7 @@  vectorizable_assignment (gimple *stmt, gimple_stmt_iterator *gsi,
   /* Multiple types in SLP are handled by creating the appropriate number of
      vectorized stmts for each SLP node.  Hence, NCOPIES is always 1 in
      case of SLP.  */
-  if (slp_node || PURE_SLP_STMT (stmt_info))
+  if (slp_node)
     ncopies = 1;
   else
     ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits;
@@ -4502,7 +4502,7 @@  vectorizable_shift (gimple *stmt, gimple_stmt_iterator *gsi,
   /* Multiple types in SLP are handled by creating the appropriate number of
      vectorized stmts for each SLP node.  Hence, NCOPIES is always 1 in
      case of SLP.  */
-  if (slp_node || PURE_SLP_STMT (stmt_info))
+  if (slp_node)
     ncopies = 1;
   else
     ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits_in;
@@ -4933,7 +4933,7 @@  vectorizable_operation (gimple *stmt, gimple_stmt_iterator *gsi,
   /* Multiple types in SLP are handled by creating the appropriate number of
      vectorized stmts for each SLP node.  Hence, NCOPIES is always 1 in
      case of SLP.  */
-  if (slp_node || PURE_SLP_STMT (stmt_info))
+  if (slp_node)
     ncopies = 1;
   else
     ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits_in;
@@ -5250,6 +5250,10 @@  vectorizable_store (gimple *stmt, gimple_stmt_iterator *gsi, gimple **vec_stmt,
       && TREE_CODE (scalar_dest) != MEM_REF)
     return false;
 
+  /* Cannot have hybrid store SLP -- that would mean storing to the
+     same location twice.  */
+  gcc_assert (slp == PURE_SLP_STMT (stmt_info));
+
   gcc_assert (gimple_assign_single_p (stmt));
 
   tree vectype = STMT_VINFO_VECTYPE (stmt_info), rhs_vectype = NULL_TREE;
@@ -5261,7 +5265,7 @@  vectorizable_store (gimple *stmt, gimple_stmt_iterator *gsi, gimple **vec_stmt,
   /* Multiple types in SLP are handled by creating the appropriate number of
      vectorized stmts for each SLP node.  Hence, NCOPIES is always 1 in
      case of SLP.  */
-  if (slp || PURE_SLP_STMT (stmt_info))
+  if (slp)
     ncopies = 1;
   else
     ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits;
@@ -5343,9 +5347,7 @@  vectorizable_store (gimple *stmt, gimple_stmt_iterator *gsi, gimple **vec_stmt,
       grouped_store = true;
       first_stmt = GROUP_FIRST_ELEMENT (stmt_info);
       group_size = GROUP_SIZE (vinfo_for_stmt (first_stmt));
-      if (!slp
-	  && !PURE_SLP_STMT (stmt_info)
-	  && !STMT_VINFO_STRIDED_P (stmt_info))
+      if (!slp && !STMT_VINFO_STRIDED_P (stmt_info))
 	{
 	  if (vect_store_lanes_supported (vectype, group_size))
 	    store_lanes_p = true;
@@ -5354,7 +5356,7 @@  vectorizable_store (gimple *stmt, gimple_stmt_iterator *gsi, gimple **vec_stmt,
 	}
 
       if (STMT_VINFO_STRIDED_P (stmt_info)
-	  && (slp || PURE_SLP_STMT (stmt_info))
+	  && slp
 	  && (group_size > nunits
 	      || nunits % group_size != 0))
 	{
@@ -6263,7 +6265,7 @@  vectorizable_load (gimple *stmt, gimple_stmt_iterator *gsi, gimple **vec_stmt,
   /* Multiple types in SLP are handled by creating the appropriate number of
      vectorized stmts for each SLP node.  Hence, NCOPIES is always 1 in
      case of SLP.  */
-  if (slp || PURE_SLP_STMT (stmt_info))
+  if (slp)
     ncopies = 1;
   else
     ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits;
@@ -6316,9 +6318,7 @@  vectorizable_load (gimple *stmt, gimple_stmt_iterator *gsi, gimple **vec_stmt,
       first_stmt = GROUP_FIRST_ELEMENT (stmt_info);
       group_size = GROUP_SIZE (vinfo_for_stmt (first_stmt));
 
-      if (!slp
-	  && !PURE_SLP_STMT (stmt_info)
-	  && !STMT_VINFO_STRIDED_P (stmt_info))
+      if (!slp && !STMT_VINFO_STRIDED_P (stmt_info))
 	{
 	  if (vect_load_lanes_supported (vectype, group_size))
 	    load_lanes_p = true;
@@ -6431,8 +6431,8 @@  vectorizable_load (gimple *stmt, gimple_stmt_iterator *gsi, gimple **vec_stmt,
     }
   else if (STMT_VINFO_STRIDED_P (stmt_info))
     {
-      if ((grouped_load
-	   && (slp || PURE_SLP_STMT (stmt_info)))
+      if (grouped_load
+	  && slp
 	  && (group_size > nunits
 	      || nunits % group_size != 0))
 	{
@@ -7510,7 +7510,7 @@  vectorizable_condition (gimple *stmt, gimple_stmt_iterator *gsi,
   int nunits = TYPE_VECTOR_SUBPARTS (vectype);
   tree vectype1 = NULL_TREE, vectype2 = NULL_TREE;
 
-  if (slp_node || PURE_SLP_STMT (stmt_info))
+  if (slp_node)
     ncopies = 1;
   else
     ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits;
@@ -7716,7 +7716,7 @@  vectorizable_condition (gimple *stmt, gimple_stmt_iterator *gsi,
 
    Return FALSE if not a vectorizable STMT, TRUE otherwise.  */
 
-bool
+static bool
 vectorizable_comparison (gimple *stmt, gimple_stmt_iterator *gsi,
 			 gimple **vec_stmt, tree reduc_def,
 			 slp_tree slp_node)
@@ -7750,7 +7750,7 @@  vectorizable_comparison (gimple *stmt, gimple_stmt_iterator *gsi,
   mask_type = vectype;
   nunits = TYPE_VECTOR_SUBPARTS (vectype);
 
-  if (slp_node || PURE_SLP_STMT (stmt_info))
+  if (slp_node)
     ncopies = 1;
   else
     ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits;
@@ -8160,6 +8160,7 @@  vect_transform_stmt (gimple *stmt, gimple_stmt_iterator *gsi,
   stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
   bool done;
 
+  gcc_assert (slp_node || !PURE_SLP_STMT (stmt_info));
   gimple *old_vec_stmt = STMT_VINFO_VEC_STMT (stmt_info);
 
   switch (STMT_VINFO_TYPE (stmt_info))