public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix memory leak due to destroy_loop_vec_info (PR middle-end/56461)
@ 2013-03-01 20:03 Jakub Jelinek
  2013-03-04  9:58 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2013-03-01 20:03 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

Hi!

This fixes leaks like
==31176== 176 bytes in 2 blocks are definitely lost in loss record 432 of 541
==31176==    at 0x4A0881C: malloc (vg_replace_malloc.c:270)
==31176==    by 0x114DF6F: xrealloc (xmalloc.c:177)
==31176==    by 0x85E0AA: void va_heap::reserve<gimple_statement_d*>(vec<gimple_statement_d*, va_heap, vl_embed>*&, unsigned int, bool) (vec.h:300)
==31176==    by 0x85DE23: vec<gimple_statement_d*, va_heap, vl_ptr>::reserve(unsigned int, bool) (vec.h:1468)
==31176==    by 0xA7961C: vec<gimple_statement_d*, va_heap, vl_ptr>::reserve_exact(unsigned int) (vec.h:1482)
==31176==    by 0xA79402: vec<gimple_statement_d*, va_heap, vl_ptr>::create(unsigned int) (vec.h:1497)
==31176==    by 0xC7189C: new_loop_vec_info(loop*) (tree-vect-loop.c:871)
==31176==    by 0xC725EB: vect_analyze_loop_form(loop*) (tree-vect-loop.c:1251)
==31176==    by 0xC71DEE: vect_analyze_loop_1(loop*) (tree-vect-loop.c:1000)
==31176==    by 0xC71F6D: vect_analyze_loop_form(loop*) (tree-vect-loop.c:1098)
==31176==    by 0xC73815: vect_analyze_loop(loop*) (tree-vect-loop.c:1764)
==31176==    by 0xC8F9EA: vectorize_loops() (tree-vectorizer.c:113)
and various others, when vect_analyze_loop_form calls destroy_loop_vec_info
with false as second argument, we leak various fields.  My understanding
is that back when clean_stmts argument has been added, the if (!clean_stmts)
switch got a copy of all the cleanups the other code path did, except for the loop
actually freeing stmt_vec_info, but already shortly after it new fields
have been added and cleanup for those has been added to just one spot instead
of two.

The patch fixes it by doing all the cleanups for !clean_stmts too in the
same code path, except for the stmt_vec_info freeing.  With -O3 -mavx
I didn't see any code differences on all of gcc.dg/vect/*.c.

Also, passing false to the second to last call to destroy_loop_vec_info
where we return NULL looks like a typo to my eyes, so I've changed that too.

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

2013-03-01  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/56461
	* tree-vect-loop.c (destroy_loop_vec_info): For !clean_stmts, just
	set nbbs to 0 instead of having separate code path.
	(vect_analyze_loop_form): Call destroy_loop_vec_info with true
	instead of false as last argument if returning NULL.

--- gcc/tree-vect-loop.c.jj	2013-02-27 22:40:24.000000000 +0100
+++ gcc/tree-vect-loop.c	2013-03-01 11:27:53.205162019 +0100
@@ -905,23 +905,9 @@ destroy_loop_vec_info (loop_vec_info loo
   loop = LOOP_VINFO_LOOP (loop_vinfo);
 
   bbs = LOOP_VINFO_BBS (loop_vinfo);
-  nbbs = loop->num_nodes;
+  nbbs = clean_stmts ? loop->num_nodes : 0;
   swapped = LOOP_VINFO_OPERANDS_SWAPPED (loop_vinfo);
 
-  if (!clean_stmts)
-    {
-      free (LOOP_VINFO_BBS (loop_vinfo));
-      free_data_refs (LOOP_VINFO_DATAREFS (loop_vinfo));
-      free_dependence_relations (LOOP_VINFO_DDRS (loop_vinfo));
-      LOOP_VINFO_LOOP_NEST (loop_vinfo).release ();
-      LOOP_VINFO_MAY_MISALIGN_STMTS (loop_vinfo).release ();
-      LOOP_VINFO_MAY_ALIAS_DDRS (loop_vinfo).release ();
-
-      free (loop_vinfo);
-      loop->aux = NULL;
-      return;
-    }
-
   for (j = 0; j < nbbs; j++)
     {
       basic_block bb = bbs[j];
@@ -1244,7 +1230,7 @@ vect_analyze_loop_form (struct loop *loo
 	dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
 			 "not vectorized: number of iterations = 0.");
       if (inner_loop_vinfo)
-        destroy_loop_vec_info (inner_loop_vinfo, false);
+        destroy_loop_vec_info (inner_loop_vinfo, true);
       return NULL;
     }
 

	Jakub

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Fix memory leak due to destroy_loop_vec_info (PR middle-end/56461)
  2013-03-01 20:03 [PATCH] Fix memory leak due to destroy_loop_vec_info (PR middle-end/56461) Jakub Jelinek
@ 2013-03-04  9:58 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2013-03-04  9:58 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On Fri, 1 Mar 2013, Jakub Jelinek wrote:

> Hi!
> 
> This fixes leaks like
> ==31176== 176 bytes in 2 blocks are definitely lost in loss record 432 of 541
> ==31176==    at 0x4A0881C: malloc (vg_replace_malloc.c:270)
> ==31176==    by 0x114DF6F: xrealloc (xmalloc.c:177)
> ==31176==    by 0x85E0AA: void va_heap::reserve<gimple_statement_d*>(vec<gimple_statement_d*, va_heap, vl_embed>*&, unsigned int, bool) (vec.h:300)
> ==31176==    by 0x85DE23: vec<gimple_statement_d*, va_heap, vl_ptr>::reserve(unsigned int, bool) (vec.h:1468)
> ==31176==    by 0xA7961C: vec<gimple_statement_d*, va_heap, vl_ptr>::reserve_exact(unsigned int) (vec.h:1482)
> ==31176==    by 0xA79402: vec<gimple_statement_d*, va_heap, vl_ptr>::create(unsigned int) (vec.h:1497)
> ==31176==    by 0xC7189C: new_loop_vec_info(loop*) (tree-vect-loop.c:871)
> ==31176==    by 0xC725EB: vect_analyze_loop_form(loop*) (tree-vect-loop.c:1251)
> ==31176==    by 0xC71DEE: vect_analyze_loop_1(loop*) (tree-vect-loop.c:1000)
> ==31176==    by 0xC71F6D: vect_analyze_loop_form(loop*) (tree-vect-loop.c:1098)
> ==31176==    by 0xC73815: vect_analyze_loop(loop*) (tree-vect-loop.c:1764)
> ==31176==    by 0xC8F9EA: vectorize_loops() (tree-vectorizer.c:113)
> and various others, when vect_analyze_loop_form calls destroy_loop_vec_info
> with false as second argument, we leak various fields.  My understanding
> is that back when clean_stmts argument has been added, the if (!clean_stmts)
> switch got a copy of all the cleanups the other code path did, except for the loop
> actually freeing stmt_vec_info, but already shortly after it new fields
> have been added and cleanup for those has been added to just one spot instead
> of two.
> 
> The patch fixes it by doing all the cleanups for !clean_stmts too in the
> same code path, except for the stmt_vec_info freeing.  With -O3 -mavx
> I didn't see any code differences on all of gcc.dg/vect/*.c.
> 
> Also, passing false to the second to last call to destroy_loop_vec_info
> where we return NULL looks like a typo to my eyes, so I've changed that too.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2013-03-01  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/56461
> 	* tree-vect-loop.c (destroy_loop_vec_info): For !clean_stmts, just
> 	set nbbs to 0 instead of having separate code path.
> 	(vect_analyze_loop_form): Call destroy_loop_vec_info with true
> 	instead of false as last argument if returning NULL.
> 
> --- gcc/tree-vect-loop.c.jj	2013-02-27 22:40:24.000000000 +0100
> +++ gcc/tree-vect-loop.c	2013-03-01 11:27:53.205162019 +0100
> @@ -905,23 +905,9 @@ destroy_loop_vec_info (loop_vec_info loo
>    loop = LOOP_VINFO_LOOP (loop_vinfo);
>  
>    bbs = LOOP_VINFO_BBS (loop_vinfo);
> -  nbbs = loop->num_nodes;
> +  nbbs = clean_stmts ? loop->num_nodes : 0;
>    swapped = LOOP_VINFO_OPERANDS_SWAPPED (loop_vinfo);
>  
> -  if (!clean_stmts)
> -    {
> -      free (LOOP_VINFO_BBS (loop_vinfo));
> -      free_data_refs (LOOP_VINFO_DATAREFS (loop_vinfo));
> -      free_dependence_relations (LOOP_VINFO_DDRS (loop_vinfo));
> -      LOOP_VINFO_LOOP_NEST (loop_vinfo).release ();
> -      LOOP_VINFO_MAY_MISALIGN_STMTS (loop_vinfo).release ();
> -      LOOP_VINFO_MAY_ALIAS_DDRS (loop_vinfo).release ();
> -
> -      free (loop_vinfo);
> -      loop->aux = NULL;
> -      return;
> -    }
> -
>    for (j = 0; j < nbbs; j++)
>      {
>        basic_block bb = bbs[j];
> @@ -1244,7 +1230,7 @@ vect_analyze_loop_form (struct loop *loo
>  	dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>  			 "not vectorized: number of iterations = 0.");
>        if (inner_loop_vinfo)
> -        destroy_loop_vec_info (inner_loop_vinfo, false);
> +        destroy_loop_vec_info (inner_loop_vinfo, true);
>        return NULL;
>      }
>  
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2013-03-04  9:58 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-01 20:03 [PATCH] Fix memory leak due to destroy_loop_vec_info (PR middle-end/56461) Jakub Jelinek
2013-03-04  9:58 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).