[03/46] Remove unnecessary update of NUM_SLP_USES

Message ID 87k1plrmc1.fsf@arm.com
State New
Headers show
Series
  • Remove vinfo_for_stmt etc.
Related show

Commit Message

Richard Sandiford July 24, 2018, 9:53 a.m.
vect_free_slp_tree had:

  gimple *stmt;
  FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (node), i, stmt)
    /* After transform some stmts are removed and thus their vinfo is gone.  */
    if (vinfo_for_stmt (stmt))
      {
	gcc_assert (STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt)) > 0);
	STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt))--;
      }

But after transform this update is redundant even for statements that do
exist, so it seems better to skip this loop for the final teardown.


2018-07-24  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* tree-vectorizer.h (vect_free_slp_instance): Add a final_p parameter.
	* tree-vect-slp.c (vect_free_slp_tree): Likewise.  Don't update
	STMT_VINFO_NUM_SLP_USES when it's true.
	(vect_free_slp_instance): Add a final_p parameter and pass it to
	vect_free_slp_tree.
	(vect_build_slp_tree_2): Update call to vect_free_slp_instance.
	(vect_analyze_slp_instance): Likewise.
	(vect_slp_analyze_operations): Likewise.
	(vect_slp_analyze_bb_1): Likewise.
	* tree-vectorizer.c (vec_info): Likewise.
	* tree-vect-loop.c (vect_transform_loop): Likewise.

Comments

Richard Biener July 25, 2018, 8:45 a.m. | #1
On Tue, Jul 24, 2018 at 11:53 AM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>

> vect_free_slp_tree had:

>

>   gimple *stmt;

>   FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (node), i, stmt)

>     /* After transform some stmts are removed and thus their vinfo is gone.  */

>     if (vinfo_for_stmt (stmt))

>       {

>         gcc_assert (STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt)) > 0);

>         STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt))--;

>       }

>

> But after transform this update is redundant even for statements that do

> exist, so it seems better to skip this loop for the final teardown.


OK

>

> 2018-07-24  Richard Sandiford  <richard.sandiford@arm.com>

>

> gcc/

>         * tree-vectorizer.h (vect_free_slp_instance): Add a final_p parameter.

>         * tree-vect-slp.c (vect_free_slp_tree): Likewise.  Don't update

>         STMT_VINFO_NUM_SLP_USES when it's true.

>         (vect_free_slp_instance): Add a final_p parameter and pass it to

>         vect_free_slp_tree.

>         (vect_build_slp_tree_2): Update call to vect_free_slp_instance.

>         (vect_analyze_slp_instance): Likewise.

>         (vect_slp_analyze_operations): Likewise.

>         (vect_slp_analyze_bb_1): Likewise.

>         * tree-vectorizer.c (vec_info): Likewise.

>         * tree-vect-loop.c (vect_transform_loop): Likewise.

>

> Index: gcc/tree-vectorizer.h

> ===================================================================

> --- gcc/tree-vectorizer.h       2018-07-03 10:59:30.480481417 +0100

> +++ gcc/tree-vectorizer.h       2018-07-24 10:22:09.237496975 +0100

> @@ -1634,7 +1634,7 @@ extern int vect_get_known_peeling_cost (

>  extern tree cse_and_gimplify_to_preheader (loop_vec_info, tree);

>

>  /* In tree-vect-slp.c.  */

> -extern void vect_free_slp_instance (slp_instance);

> +extern void vect_free_slp_instance (slp_instance, bool);

>  extern bool vect_transform_slp_perm_load (slp_tree, vec<tree> ,

>                                           gimple_stmt_iterator *, poly_uint64,

>                                           slp_instance, bool, unsigned *);

> Index: gcc/tree-vect-slp.c

> ===================================================================

> --- gcc/tree-vect-slp.c 2018-07-23 16:58:06.000000000 +0100

> +++ gcc/tree-vect-slp.c 2018-07-24 10:22:09.237496975 +0100

> @@ -47,25 +47,32 @@ Software Foundation; either version 3, o

>  #include "internal-fn.h"

>

>

> -/* Recursively free the memory allocated for the SLP tree rooted at NODE.  */

> +/* Recursively free the memory allocated for the SLP tree rooted at NODE.

> +   FINAL_P is true if we have vectorized the instance or if we have

> +   made a final decision not to vectorize the statements in any way.  */

>

>  static void

> -vect_free_slp_tree (slp_tree node)

> +vect_free_slp_tree (slp_tree node, bool final_p)

>  {

>    int i;

>    slp_tree child;

>

>    FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (node), i, child)

> -    vect_free_slp_tree (child);

> +    vect_free_slp_tree (child, final_p);

>

> -  gimple *stmt;

> -  FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (node), i, stmt)

> -    /* After transform some stmts are removed and thus their vinfo is gone.  */

> -    if (vinfo_for_stmt (stmt))

> -      {

> -       gcc_assert (STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt)) > 0);

> -       STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt))--;

> -      }

> +  /* Don't update STMT_VINFO_NUM_SLP_USES if it isn't relevant.

> +     Some statements might no longer exist, after having been

> +     removed by vect_transform_stmt.  Updating the remaining

> +     statements would be redundant.  */

> +  if (!final_p)

> +    {

> +      gimple *stmt;

> +      FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (node), i, stmt)

> +       {

> +         gcc_assert (STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt)) > 0);

> +         STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt))--;

> +       }

> +    }

>

>    SLP_TREE_CHILDREN (node).release ();

>    SLP_TREE_SCALAR_STMTS (node).release ();

> @@ -76,12 +83,14 @@ vect_free_slp_tree (slp_tree node)

>  }

>

>

> -/* Free the memory allocated for the SLP instance.  */

> +/* Free the memory allocated for the SLP instance.  FINAL_P is true if we

> +   have vectorized the instance or if we have made a final decision not

> +   to vectorize the statements in any way.  */

>

>  void

> -vect_free_slp_instance (slp_instance instance)

> +vect_free_slp_instance (slp_instance instance, bool final_p)

>  {

> -  vect_free_slp_tree (SLP_INSTANCE_TREE (instance));

> +  vect_free_slp_tree (SLP_INSTANCE_TREE (instance), final_p);

>    SLP_INSTANCE_LOADS (instance).release ();

>    free (instance);

>  }

> @@ -1284,7 +1293,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,

>        if (++this_tree_size > max_tree_size)

>         {

>           FOR_EACH_VEC_ELT (children, j, child)

> -           vect_free_slp_tree (child);

> +           vect_free_slp_tree (child, false);

>           vect_free_oprnd_info (oprnds_info);

>           return NULL;

>         }

> @@ -1315,7 +1324,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,

>                   this_loads.truncate (old_nloads);

>                   this_tree_size = old_tree_size;

>                   FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (child), j, grandchild)

> -                   vect_free_slp_tree (grandchild);

> +                   vect_free_slp_tree (grandchild, false);

>                   SLP_TREE_CHILDREN (child).truncate (0);

>

>                   dump_printf_loc (MSG_NOTE, vect_location,

> @@ -1495,7 +1504,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,

>                       this_loads.truncate (old_nloads);

>                       this_tree_size = old_tree_size;

>                       FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (child), j, grandchild)

> -                       vect_free_slp_tree (grandchild);

> +                       vect_free_slp_tree (grandchild, false);

>                       SLP_TREE_CHILDREN (child).truncate (0);

>

>                       dump_printf_loc (MSG_NOTE, vect_location,

> @@ -1519,7 +1528,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,

>  fail:

>        gcc_assert (child == NULL);

>        FOR_EACH_VEC_ELT (children, j, child)

> -       vect_free_slp_tree (child);

> +       vect_free_slp_tree (child, false);

>        vect_free_oprnd_info (oprnds_info);

>        return NULL;

>      }

> @@ -2036,13 +2045,13 @@ vect_analyze_slp_instance (vec_info *vin

>                                  "Build SLP failed: store group "

>                                  "size not a multiple of the vector size "

>                                  "in basic block SLP\n");

> -             vect_free_slp_tree (node);

> +             vect_free_slp_tree (node, false);

>               loads.release ();

>               return false;

>             }

>           /* Fatal mismatch.  */

>           matches[group_size / const_max_nunits * const_max_nunits] = false;

> -         vect_free_slp_tree (node);

> +         vect_free_slp_tree (node, false);

>           loads.release ();

>         }

>        else

> @@ -2102,7 +2111,7 @@ vect_analyze_slp_instance (vec_info *vin

>                       dump_gimple_stmt (MSG_MISSED_OPTIMIZATION,

>                                         TDF_SLIM, stmt, 0);

>                  }

> -              vect_free_slp_instance (new_instance);

> +             vect_free_slp_instance (new_instance, false);

>                return false;

>              }

>          }

> @@ -2133,7 +2142,7 @@ vect_analyze_slp_instance (vec_info *vin

>                 dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,

>                                  "Built SLP cancelled: can use "

>                                  "load/store-lanes\n");

> -             vect_free_slp_instance (new_instance);

> +             vect_free_slp_instance (new_instance, false);

>               return false;

>             }

>         }

> @@ -2668,7 +2677,7 @@ vect_slp_analyze_operations (vec_info *v

>           dump_gimple_stmt (MSG_NOTE, TDF_SLIM,

>                             SLP_TREE_SCALAR_STMTS

>                               (SLP_INSTANCE_TREE (instance))[0], 0);

> -         vect_free_slp_instance (instance);

> +         vect_free_slp_instance (instance, false);

>            vinfo->slp_instances.ordered_remove (i);

>           cost_vec.release ();

>         }

> @@ -2947,7 +2956,7 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera

>           dump_gimple_stmt (MSG_NOTE, TDF_SLIM,

>                             SLP_TREE_SCALAR_STMTS

>                               (SLP_INSTANCE_TREE (instance))[0], 0);

> -         vect_free_slp_instance (instance);

> +         vect_free_slp_instance (instance, false);

>           BB_VINFO_SLP_INSTANCES (bb_vinfo).ordered_remove (i);

>           continue;

>         }

> Index: gcc/tree-vectorizer.c

> ===================================================================

> --- gcc/tree-vectorizer.c       2018-06-27 10:27:09.894649672 +0100

> +++ gcc/tree-vectorizer.c       2018-07-24 10:22:09.237496975 +0100

> @@ -466,7 +466,7 @@ vec_info::~vec_info ()

>    unsigned int i;

>

>    FOR_EACH_VEC_ELT (slp_instances, i, instance)

> -    vect_free_slp_instance (instance);

> +    vect_free_slp_instance (instance, true);

>

>    destroy_cost_data (target_cost_data);

>    free_stmt_vec_infos (&stmt_vec_infos);

> Index: gcc/tree-vect-loop.c

> ===================================================================

> --- gcc/tree-vect-loop.c        2018-07-24 10:22:06.269523330 +0100

> +++ gcc/tree-vect-loop.c        2018-07-24 10:22:09.237496975 +0100

> @@ -2229,7 +2229,7 @@ vect_analyze_loop_2 (loop_vec_info loop_

>    LOOP_VINFO_VECT_FACTOR (loop_vinfo) = saved_vectorization_factor;

>    /* Free the SLP instances.  */

>    FOR_EACH_VEC_ELT (LOOP_VINFO_SLP_INSTANCES (loop_vinfo), j, instance)

> -    vect_free_slp_instance (instance);

> +    vect_free_slp_instance (instance, false);

>    LOOP_VINFO_SLP_INSTANCES (loop_vinfo).release ();

>    /* Reset SLP type to loop_vect on all stmts.  */

>    for (i = 0; i < LOOP_VINFO_LOOP (loop_vinfo)->num_nodes; ++i)

> @@ -8683,7 +8683,7 @@ vect_transform_loop (loop_vec_info loop_

>       won't work.  */

>    slp_instance instance;

>    FOR_EACH_VEC_ELT (LOOP_VINFO_SLP_INSTANCES (loop_vinfo), i, instance)

> -    vect_free_slp_instance (instance);

> +    vect_free_slp_instance (instance, true);

>    LOOP_VINFO_SLP_INSTANCES (loop_vinfo).release ();

>    /* Clear-up safelen field since its value is invalid after vectorization

>       since vectorized loop can have loop-carried dependencies.  */

Patch

Index: gcc/tree-vectorizer.h
===================================================================
--- gcc/tree-vectorizer.h	2018-07-03 10:59:30.480481417 +0100
+++ gcc/tree-vectorizer.h	2018-07-24 10:22:09.237496975 +0100
@@ -1634,7 +1634,7 @@  extern int vect_get_known_peeling_cost (
 extern tree cse_and_gimplify_to_preheader (loop_vec_info, tree);
 
 /* In tree-vect-slp.c.  */
-extern void vect_free_slp_instance (slp_instance);
+extern void vect_free_slp_instance (slp_instance, bool);
 extern bool vect_transform_slp_perm_load (slp_tree, vec<tree> ,
 					  gimple_stmt_iterator *, poly_uint64,
 					  slp_instance, bool, unsigned *);
Index: gcc/tree-vect-slp.c
===================================================================
--- gcc/tree-vect-slp.c	2018-07-23 16:58:06.000000000 +0100
+++ gcc/tree-vect-slp.c	2018-07-24 10:22:09.237496975 +0100
@@ -47,25 +47,32 @@  Software Foundation; either version 3, o
 #include "internal-fn.h"
 
 
-/* Recursively free the memory allocated for the SLP tree rooted at NODE.  */
+/* Recursively free the memory allocated for the SLP tree rooted at NODE.
+   FINAL_P is true if we have vectorized the instance or if we have
+   made a final decision not to vectorize the statements in any way.  */
 
 static void
-vect_free_slp_tree (slp_tree node)
+vect_free_slp_tree (slp_tree node, bool final_p)
 {
   int i;
   slp_tree child;
 
   FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (node), i, child)
-    vect_free_slp_tree (child);
+    vect_free_slp_tree (child, final_p);
 
-  gimple *stmt;
-  FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (node), i, stmt)
-    /* After transform some stmts are removed and thus their vinfo is gone.  */
-    if (vinfo_for_stmt (stmt))
-      {
-	gcc_assert (STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt)) > 0);
-	STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt))--;
-      }
+  /* Don't update STMT_VINFO_NUM_SLP_USES if it isn't relevant.
+     Some statements might no longer exist, after having been
+     removed by vect_transform_stmt.  Updating the remaining
+     statements would be redundant.  */
+  if (!final_p)
+    {
+      gimple *stmt;
+      FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (node), i, stmt)
+	{
+	  gcc_assert (STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt)) > 0);
+	  STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt))--;
+	}
+    }
 
   SLP_TREE_CHILDREN (node).release ();
   SLP_TREE_SCALAR_STMTS (node).release ();
@@ -76,12 +83,14 @@  vect_free_slp_tree (slp_tree node)
 }
 
 
-/* Free the memory allocated for the SLP instance.  */
+/* Free the memory allocated for the SLP instance.  FINAL_P is true if we
+   have vectorized the instance or if we have made a final decision not
+   to vectorize the statements in any way.  */
 
 void
-vect_free_slp_instance (slp_instance instance)
+vect_free_slp_instance (slp_instance instance, bool final_p)
 {
-  vect_free_slp_tree (SLP_INSTANCE_TREE (instance));
+  vect_free_slp_tree (SLP_INSTANCE_TREE (instance), final_p);
   SLP_INSTANCE_LOADS (instance).release ();
   free (instance);
 }
@@ -1284,7 +1293,7 @@  vect_build_slp_tree_2 (vec_info *vinfo,
       if (++this_tree_size > max_tree_size)
 	{
 	  FOR_EACH_VEC_ELT (children, j, child)
-	    vect_free_slp_tree (child);
+	    vect_free_slp_tree (child, false);
 	  vect_free_oprnd_info (oprnds_info);
 	  return NULL;
 	}
@@ -1315,7 +1324,7 @@  vect_build_slp_tree_2 (vec_info *vinfo,
 		  this_loads.truncate (old_nloads);
 		  this_tree_size = old_tree_size;
 		  FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (child), j, grandchild)
-		    vect_free_slp_tree (grandchild);
+		    vect_free_slp_tree (grandchild, false);
 		  SLP_TREE_CHILDREN (child).truncate (0);
 
 		  dump_printf_loc (MSG_NOTE, vect_location,
@@ -1495,7 +1504,7 @@  vect_build_slp_tree_2 (vec_info *vinfo,
 		      this_loads.truncate (old_nloads);
 		      this_tree_size = old_tree_size;
 		      FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (child), j, grandchild)
-			vect_free_slp_tree (grandchild);
+			vect_free_slp_tree (grandchild, false);
 		      SLP_TREE_CHILDREN (child).truncate (0);
 
 		      dump_printf_loc (MSG_NOTE, vect_location,
@@ -1519,7 +1528,7 @@  vect_build_slp_tree_2 (vec_info *vinfo,
 fail:
       gcc_assert (child == NULL);
       FOR_EACH_VEC_ELT (children, j, child)
-	vect_free_slp_tree (child);
+	vect_free_slp_tree (child, false);
       vect_free_oprnd_info (oprnds_info);
       return NULL;
     }
@@ -2036,13 +2045,13 @@  vect_analyze_slp_instance (vec_info *vin
 				 "Build SLP failed: store group "
 				 "size not a multiple of the vector size "
 				 "in basic block SLP\n");
-	      vect_free_slp_tree (node);
+	      vect_free_slp_tree (node, false);
 	      loads.release ();
 	      return false;
 	    }
 	  /* Fatal mismatch.  */
 	  matches[group_size / const_max_nunits * const_max_nunits] = false;
-	  vect_free_slp_tree (node);
+	  vect_free_slp_tree (node, false);
 	  loads.release ();
 	}
       else
@@ -2102,7 +2111,7 @@  vect_analyze_slp_instance (vec_info *vin
 		      dump_gimple_stmt (MSG_MISSED_OPTIMIZATION,
 					TDF_SLIM, stmt, 0);
                 }
-              vect_free_slp_instance (new_instance);
+	      vect_free_slp_instance (new_instance, false);
               return false;
             }
         }
@@ -2133,7 +2142,7 @@  vect_analyze_slp_instance (vec_info *vin
 		dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
 				 "Built SLP cancelled: can use "
 				 "load/store-lanes\n");
-	      vect_free_slp_instance (new_instance);
+	      vect_free_slp_instance (new_instance, false);
 	      return false;
 	    }
 	}
@@ -2668,7 +2677,7 @@  vect_slp_analyze_operations (vec_info *v
 	  dump_gimple_stmt (MSG_NOTE, TDF_SLIM,
 			    SLP_TREE_SCALAR_STMTS
 			      (SLP_INSTANCE_TREE (instance))[0], 0);
-	  vect_free_slp_instance (instance);
+	  vect_free_slp_instance (instance, false);
           vinfo->slp_instances.ordered_remove (i);
 	  cost_vec.release ();
 	}
@@ -2947,7 +2956,7 @@  vect_slp_analyze_bb_1 (gimple_stmt_itera
 	  dump_gimple_stmt (MSG_NOTE, TDF_SLIM,
 			    SLP_TREE_SCALAR_STMTS
 			      (SLP_INSTANCE_TREE (instance))[0], 0);
-	  vect_free_slp_instance (instance);
+	  vect_free_slp_instance (instance, false);
 	  BB_VINFO_SLP_INSTANCES (bb_vinfo).ordered_remove (i);
 	  continue;
 	}
Index: gcc/tree-vectorizer.c
===================================================================
--- gcc/tree-vectorizer.c	2018-06-27 10:27:09.894649672 +0100
+++ gcc/tree-vectorizer.c	2018-07-24 10:22:09.237496975 +0100
@@ -466,7 +466,7 @@  vec_info::~vec_info ()
   unsigned int i;
 
   FOR_EACH_VEC_ELT (slp_instances, i, instance)
-    vect_free_slp_instance (instance);
+    vect_free_slp_instance (instance, true);
 
   destroy_cost_data (target_cost_data);
   free_stmt_vec_infos (&stmt_vec_infos);
Index: gcc/tree-vect-loop.c
===================================================================
--- gcc/tree-vect-loop.c	2018-07-24 10:22:06.269523330 +0100
+++ gcc/tree-vect-loop.c	2018-07-24 10:22:09.237496975 +0100
@@ -2229,7 +2229,7 @@  vect_analyze_loop_2 (loop_vec_info loop_
   LOOP_VINFO_VECT_FACTOR (loop_vinfo) = saved_vectorization_factor;
   /* Free the SLP instances.  */
   FOR_EACH_VEC_ELT (LOOP_VINFO_SLP_INSTANCES (loop_vinfo), j, instance)
-    vect_free_slp_instance (instance);
+    vect_free_slp_instance (instance, false);
   LOOP_VINFO_SLP_INSTANCES (loop_vinfo).release ();
   /* Reset SLP type to loop_vect on all stmts.  */
   for (i = 0; i < LOOP_VINFO_LOOP (loop_vinfo)->num_nodes; ++i)
@@ -8683,7 +8683,7 @@  vect_transform_loop (loop_vec_info loop_
      won't work.  */
   slp_instance instance;
   FOR_EACH_VEC_ELT (LOOP_VINFO_SLP_INSTANCES (loop_vinfo), i, instance)
-    vect_free_slp_instance (instance);
+    vect_free_slp_instance (instance, true);
   LOOP_VINFO_SLP_INSTANCES (loop_vinfo).release ();
   /* Clear-up safelen field since its value is invalid after vectorization
      since vectorized loop can have loop-carried dependencies.  */