Patchwork Change edge_var_map_vector into vl_embed vector (PR middle-end/56461)

login
register
mail settings
Submitter Jakub Jelinek
Date Feb. 26, 2013, 9:26 p.m.
Message ID <20130226212634.GE12913@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/223406/
State New
Headers show

Comments

Jakub Jelinek - Feb. 26, 2013, 9:26 p.m.
Hi!

On larger testcases, we leak megabytes of memory since the vec.h changes,
where edge_var_map_vector has been changed from a VEC(edge_var_map, heap) *
into the space efficient vector, but is missing freeing of the pointed
memory (i.e. release ()), and additionally it isn't very space and compile
time efficient to malloc (well, new) vec<edge_var_map> objects, because
those are just pointer sized objects which we can put directly into the
pointer map slot.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2013-02-26  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/56461
	* tree-flow.h (edge_var_map_vector): Change into va_heap, vl_embed
	vector.
	* tree-ssa.c (redirect_edge_var_map_add): Use vec_safe_reserve and
	vec_safe_push, always update *slot.
	(redirect_edge_var_map_clear): Use vec_free.
	(redirect_edge_var_map_dup): Use vec_safe_copy and vec_safe_reserve.
	(free_var_map_entry): Use vec_free.
	* tree-cfgcleanup.c (remove_forwarder_block_with_phi): Use
	FOR_EACH_VEC_SAFE_ELT instead of FOR_EACH_VEC_ELT.


	Jakub
Richard Guenther - Feb. 27, 2013, 9:14 a.m.
On Tue, 26 Feb 2013, Jakub Jelinek wrote:

> Hi!
> 
> On larger testcases, we leak megabytes of memory since the vec.h changes,
> where edge_var_map_vector has been changed from a VEC(edge_var_map, heap) *
> into the space efficient vector, but is missing freeing of the pointed
> memory (i.e. release ()), and additionally it isn't very space and compile
> time efficient to malloc (well, new) vec<edge_var_map> objects, because
> those are just pointer sized objects which we can put directly into the
> pointer map slot.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?

Ok.

Thanks,
Richard.

> 2013-02-26  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/56461
> 	* tree-flow.h (edge_var_map_vector): Change into va_heap, vl_embed
> 	vector.
> 	* tree-ssa.c (redirect_edge_var_map_add): Use vec_safe_reserve and
> 	vec_safe_push, always update *slot.
> 	(redirect_edge_var_map_clear): Use vec_free.
> 	(redirect_edge_var_map_dup): Use vec_safe_copy and vec_safe_reserve.
> 	(free_var_map_entry): Use vec_free.
> 	* tree-cfgcleanup.c (remove_forwarder_block_with_phi): Use
> 	FOR_EACH_VEC_SAFE_ELT instead of FOR_EACH_VEC_ELT.
> 
> --- gcc/tree-flow.h.jj	2013-02-13 21:47:17.000000000 +0100
> +++ gcc/tree-flow.h	2013-02-26 16:19:29.567822673 +0100
> @@ -481,7 +481,7 @@ typedef struct _edge_var_map edge_var_ma
>  
>  
>  /* A vector of var maps.  */
> -typedef vec<edge_var_map> edge_var_map_vector;
> +typedef vec<edge_var_map, va_heap, vl_embed> edge_var_map_vector;
>  
>  extern void init_tree_ssa (struct function *);
>  extern void redirect_edge_var_map_add (edge, tree, tree, source_location);
> --- gcc/tree-ssa.c.jj	2013-02-12 17:27:48.000000000 +0100
> +++ gcc/tree-ssa.c	2013-02-26 16:33:19.345295876 +0100
> @@ -59,16 +59,13 @@ redirect_edge_var_map_add (edge e, tree
>    slot = pointer_map_insert (edge_var_maps, e);
>    head = (edge_var_map_vector *) *slot;
>    if (!head)
> -    {
> -      head = new edge_var_map_vector;
> -      head->create (5);
> -      *slot = head;
> -    }
> +    vec_safe_reserve (head, 5);
>    new_node.def = def;
>    new_node.result = result;
>    new_node.locus = locus;
>  
> -  head->safe_push (new_node);
> +  vec_safe_push (head, new_node);
> +  *slot = head;
>  }
>  
>  
> @@ -88,7 +85,7 @@ redirect_edge_var_map_clear (edge e)
>    if (slot)
>      {
>        head = (edge_var_map_vector *) *slot;
> -      delete head;
> +      vec_free (head);
>        *slot = NULL;
>      }
>  }
> @@ -115,11 +112,11 @@ redirect_edge_var_map_dup (edge newe, ed
>      return;
>    head = (edge_var_map_vector *) *old_slot;
>  
> -  edge_var_map_vector *new_head = new edge_var_map_vector;
> +  edge_var_map_vector *new_head = NULL;
>    if (head)
> -    *new_head = head->copy ();
> +    new_head = vec_safe_copy (head);
>    else
> -    new_head->create (5);
> +    vec_safe_reserve (new_head, 5);
>    *new_slot = new_head;
>  }
>  
> @@ -151,7 +148,7 @@ free_var_map_entry (const void *key ATTR
>  		    void *data ATTRIBUTE_UNUSED)
>  {
>    edge_var_map_vector *head = (edge_var_map_vector *) *value;
> -  delete head;
> +  vec_free (head);
>    return true;
>  }
>  
> --- gcc/tree-cfgcleanup.c.jj	2013-02-12 11:23:37.000000000 +0100
> +++ gcc/tree-cfgcleanup.c	2013-02-26 16:36:17.671315288 +0100
> @@ -822,7 +822,7 @@ remove_forwarder_block_with_phi (basic_b
>  		 redirection, replace it with the PHI argument that used
>  		 to be on E.  */
>  	      head = redirect_edge_var_map_vector (e);
> -	      FOR_EACH_VEC_ELT (*head, i, vm)
> +	      FOR_EACH_VEC_SAFE_ELT (head, i, vm)
>  		{
>  		  tree old_arg = redirect_edge_var_map_result (vm);
>  		  tree new_arg = redirect_edge_var_map_def (vm);
> 
> 	Jakub
> 
>

Patch

--- gcc/tree-flow.h.jj	2013-02-13 21:47:17.000000000 +0100
+++ gcc/tree-flow.h	2013-02-26 16:19:29.567822673 +0100
@@ -481,7 +481,7 @@  typedef struct _edge_var_map edge_var_ma
 
 
 /* A vector of var maps.  */
-typedef vec<edge_var_map> edge_var_map_vector;
+typedef vec<edge_var_map, va_heap, vl_embed> edge_var_map_vector;
 
 extern void init_tree_ssa (struct function *);
 extern void redirect_edge_var_map_add (edge, tree, tree, source_location);
--- gcc/tree-ssa.c.jj	2013-02-12 17:27:48.000000000 +0100
+++ gcc/tree-ssa.c	2013-02-26 16:33:19.345295876 +0100
@@ -59,16 +59,13 @@  redirect_edge_var_map_add (edge e, tree
   slot = pointer_map_insert (edge_var_maps, e);
   head = (edge_var_map_vector *) *slot;
   if (!head)
-    {
-      head = new edge_var_map_vector;
-      head->create (5);
-      *slot = head;
-    }
+    vec_safe_reserve (head, 5);
   new_node.def = def;
   new_node.result = result;
   new_node.locus = locus;
 
-  head->safe_push (new_node);
+  vec_safe_push (head, new_node);
+  *slot = head;
 }
 
 
@@ -88,7 +85,7 @@  redirect_edge_var_map_clear (edge e)
   if (slot)
     {
       head = (edge_var_map_vector *) *slot;
-      delete head;
+      vec_free (head);
       *slot = NULL;
     }
 }
@@ -115,11 +112,11 @@  redirect_edge_var_map_dup (edge newe, ed
     return;
   head = (edge_var_map_vector *) *old_slot;
 
-  edge_var_map_vector *new_head = new edge_var_map_vector;
+  edge_var_map_vector *new_head = NULL;
   if (head)
-    *new_head = head->copy ();
+    new_head = vec_safe_copy (head);
   else
-    new_head->create (5);
+    vec_safe_reserve (new_head, 5);
   *new_slot = new_head;
 }
 
@@ -151,7 +148,7 @@  free_var_map_entry (const void *key ATTR
 		    void *data ATTRIBUTE_UNUSED)
 {
   edge_var_map_vector *head = (edge_var_map_vector *) *value;
-  delete head;
+  vec_free (head);
   return true;
 }
 
--- gcc/tree-cfgcleanup.c.jj	2013-02-12 11:23:37.000000000 +0100
+++ gcc/tree-cfgcleanup.c	2013-02-26 16:36:17.671315288 +0100
@@ -822,7 +822,7 @@  remove_forwarder_block_with_phi (basic_b
 		 redirection, replace it with the PHI argument that used
 		 to be on E.  */
 	      head = redirect_edge_var_map_vector (e);
-	      FOR_EACH_VEC_ELT (*head, i, vm)
+	      FOR_EACH_VEC_SAFE_ELT (head, i, vm)
 		{
 		  tree old_arg = redirect_edge_var_map_result (vm);
 		  tree new_arg = redirect_edge_var_map_def (vm);