diff mbox

Fix memory leak in vect_pattern_recog_1

Message ID 20111005180636.GV19412@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Oct. 5, 2011, 6:06 p.m. UTC
Hi!

If vect_recog_func fails (or the other spot where vect_pattern_recog_1
returns early), the vector allocated in the function isn't freed, leading
to memory leak.  But, more importantly, doing a VEC_alloc + VEC_free
num_stmts_in_loop * NUM_PATTERNS times seems to be completely unnecessary,
the following patch allocates just one vector for that purpose in the caller
and only performs VEC_truncate in each call to make it independent from
previous uses of the vector.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2011-10-05  Jakub Jelinek  <jakub@redhat.com>

	* tree-vect-patterns.c (vect_pattern_recog_1): Add stmts_to_replace
	argument, truncate it at the beginning instead of allocating there
	and freeing at the end.
	(vect_pattern_recog): Allocate stmts_to_replace here and free at end,
	pass its address to vect_pattern_recog_1.


	Jakub

Comments

Ira Rosen Oct. 6, 2011, 6:53 a.m. UTC | #1
On 5 October 2011 20:06, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> If vect_recog_func fails (or the other spot where vect_pattern_recog_1
> returns early), the vector allocated in the function isn't freed, leading
> to memory leak.  But, more importantly, doing a VEC_alloc + VEC_free
> num_stmts_in_loop * NUM_PATTERNS times seems to be completely unnecessary,
> the following patch allocates just one vector for that purpose in the caller
> and only performs VEC_truncate in each call to make it independent from
> previous uses of the vector.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Ira

>
> 2011-10-05  Jakub Jelinek  <jakub@redhat.com>
>
>        * tree-vect-patterns.c (vect_pattern_recog_1): Add stmts_to_replace
>        argument, truncate it at the beginning instead of allocating there
>        and freeing at the end.
>        (vect_pattern_recog): Allocate stmts_to_replace here and free at end,
>        pass its address to vect_pattern_recog_1.
>
> --- gcc/tree-vect-patterns.c.jj 2011-09-26 14:06:52.000000000 +0200
> +++ gcc/tree-vect-patterns.c    2011-10-05 15:57:38.000000000 +0200
> @@ -1281,7 +1281,8 @@ vect_mark_pattern_stmts (gimple orig_stm
>  static void
>  vect_pattern_recog_1 (
>        gimple (* vect_recog_func) (VEC (gimple, heap) **, tree *, tree *),
> -       gimple_stmt_iterator si)
> +       gimple_stmt_iterator si,
> +       VEC (gimple, heap) **stmts_to_replace)
>  {
>   gimple stmt = gsi_stmt (si), pattern_stmt;
>   stmt_vec_info stmt_info;
> @@ -1291,14 +1292,14 @@ vect_pattern_recog_1 (
>   enum tree_code code;
>   int i;
>   gimple next;
> -  VEC (gimple, heap) *stmts_to_replace = VEC_alloc (gimple, heap, 1);
>
> -  VEC_quick_push (gimple, stmts_to_replace, stmt);
> -  pattern_stmt = (* vect_recog_func) (&stmts_to_replace, &type_in, &type_out);
> +  VEC_truncate (gimple, *stmts_to_replace, 0);
> +  VEC_quick_push (gimple, *stmts_to_replace, stmt);
> +  pattern_stmt = (* vect_recog_func) (stmts_to_replace, &type_in, &type_out);
>   if (!pattern_stmt)
>     return;
>
> -  stmt = VEC_last (gimple, stmts_to_replace);
> +  stmt = VEC_last (gimple, *stmts_to_replace);
>   stmt_info = vinfo_for_stmt (stmt);
>   loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info);
>
> @@ -1363,8 +1364,8 @@ vect_pattern_recog_1 (
>   /* It is possible that additional pattern stmts are created and inserted in
>      STMTS_TO_REPLACE.  We create a stmt_info for each of them, and mark the
>      relevant statements.  */
> -  for (i = 0; VEC_iterate (gimple, stmts_to_replace, i, stmt)
> -              && (unsigned) i < (VEC_length (gimple, stmts_to_replace) - 1);
> +  for (i = 0; VEC_iterate (gimple, *stmts_to_replace, i, stmt)
> +             && (unsigned) i < (VEC_length (gimple, *stmts_to_replace) - 1);
>        i++)
>     {
>       stmt_info = vinfo_for_stmt (stmt);
> @@ -1377,8 +1378,6 @@ vect_pattern_recog_1 (
>
>       vect_mark_pattern_stmts (stmt, pattern_stmt, NULL_TREE);
>     }
> -
> -  VEC_free (gimple, heap, stmts_to_replace);
>  }
>
>
> @@ -1468,6 +1467,7 @@ vect_pattern_recog (loop_vec_info loop_v
>   gimple_stmt_iterator si;
>   unsigned int i, j;
>   gimple (* vect_recog_func_ptr) (VEC (gimple, heap) **, tree *, tree *);
> +  VEC (gimple, heap) *stmts_to_replace = VEC_alloc (gimple, heap, 1);
>
>   if (vect_print_dump_info (REPORT_DETAILS))
>     fprintf (vect_dump, "=== vect_pattern_recog ===");
> @@ -1483,8 +1483,11 @@ vect_pattern_recog (loop_vec_info loop_v
>           for (j = 0; j < NUM_PATTERNS; j++)
>             {
>               vect_recog_func_ptr = vect_vect_recog_func_ptrs[j];
> -              vect_pattern_recog_1 (vect_recog_func_ptr, si);
> +             vect_pattern_recog_1 (vect_recog_func_ptr, si,
> +                                   &stmts_to_replace);
>             }
>         }
>     }
> +
> +  VEC_free (gimple, heap, stmts_to_replace);
>  }
>
>        Jakub
>
diff mbox

Patch

--- gcc/tree-vect-patterns.c.jj	2011-09-26 14:06:52.000000000 +0200
+++ gcc/tree-vect-patterns.c	2011-10-05 15:57:38.000000000 +0200
@@ -1281,7 +1281,8 @@  vect_mark_pattern_stmts (gimple orig_stm
 static void
 vect_pattern_recog_1 (
 	gimple (* vect_recog_func) (VEC (gimple, heap) **, tree *, tree *),
-	gimple_stmt_iterator si)
+	gimple_stmt_iterator si,
+	VEC (gimple, heap) **stmts_to_replace)
 {
   gimple stmt = gsi_stmt (si), pattern_stmt;
   stmt_vec_info stmt_info;
@@ -1291,14 +1292,14 @@  vect_pattern_recog_1 (
   enum tree_code code;
   int i;
   gimple next;
-  VEC (gimple, heap) *stmts_to_replace = VEC_alloc (gimple, heap, 1);
 
-  VEC_quick_push (gimple, stmts_to_replace, stmt);
-  pattern_stmt = (* vect_recog_func) (&stmts_to_replace, &type_in, &type_out);
+  VEC_truncate (gimple, *stmts_to_replace, 0);
+  VEC_quick_push (gimple, *stmts_to_replace, stmt);
+  pattern_stmt = (* vect_recog_func) (stmts_to_replace, &type_in, &type_out);
   if (!pattern_stmt)
     return;
 
-  stmt = VEC_last (gimple, stmts_to_replace);
+  stmt = VEC_last (gimple, *stmts_to_replace);
   stmt_info = vinfo_for_stmt (stmt);
   loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info);
  
@@ -1363,8 +1364,8 @@  vect_pattern_recog_1 (
   /* It is possible that additional pattern stmts are created and inserted in
      STMTS_TO_REPLACE.  We create a stmt_info for each of them, and mark the
      relevant statements.  */
-  for (i = 0; VEC_iterate (gimple, stmts_to_replace, i, stmt)
-              && (unsigned) i < (VEC_length (gimple, stmts_to_replace) - 1);
+  for (i = 0; VEC_iterate (gimple, *stmts_to_replace, i, stmt)
+	      && (unsigned) i < (VEC_length (gimple, *stmts_to_replace) - 1);
        i++)
     {
       stmt_info = vinfo_for_stmt (stmt);
@@ -1377,8 +1378,6 @@  vect_pattern_recog_1 (
 
       vect_mark_pattern_stmts (stmt, pattern_stmt, NULL_TREE);
     }
-
-  VEC_free (gimple, heap, stmts_to_replace);
 }
 
 
@@ -1468,6 +1467,7 @@  vect_pattern_recog (loop_vec_info loop_v
   gimple_stmt_iterator si;
   unsigned int i, j;
   gimple (* vect_recog_func_ptr) (VEC (gimple, heap) **, tree *, tree *);
+  VEC (gimple, heap) *stmts_to_replace = VEC_alloc (gimple, heap, 1);
 
   if (vect_print_dump_info (REPORT_DETAILS))
     fprintf (vect_dump, "=== vect_pattern_recog ===");
@@ -1483,8 +1483,11 @@  vect_pattern_recog (loop_vec_info loop_v
           for (j = 0; j < NUM_PATTERNS; j++)
             {
               vect_recog_func_ptr = vect_vect_recog_func_ptrs[j];
-              vect_pattern_recog_1 (vect_recog_func_ptr, si);
+	      vect_pattern_recog_1 (vect_recog_func_ptr, si,
+				    &stmts_to_replace);
             }
         }
     }
+
+  VEC_free (gimple, heap, stmts_to_replace);
 }