| Message ID | 565774DC.5020808@suse.cz |
|---|---|
| State | New |
| Headers | show |
On Fri, Nov 27, 2015 at 5:08 AM, Martin Liška <mliska@suse.cz> wrote: > Hi. > > There's one more patch that fixes really of lot memory leaks related to loop > ivopts. > The regression was introduced by r230647. > > Patch was tested in the series with the rest and the compiler bootstraps > successfully. > > Ready for trunk? Hi Martin, Thanks for fixing my issue. The IVO part of patch is OK. Just for me to understand, iv_common_cand is freed via free_ptr_hash, and thus typed_free_remove. So what leaks is the iv_use * vector in struct iv_common_cand, right? I did forget to free that. BTW, how do you monitor memory use in GCC, maybe I can run same test for my future patches. Thanks, bin
On 11/27/2015 04:54 AM, Bin.Cheng wrote: > On Fri, Nov 27, 2015 at 5:08 AM, Martin Liška <mliska@suse.cz> wrote: >> Hi. >> >> There's one more patch that fixes really of lot memory leaks related to loop >> ivopts. >> The regression was introduced by r230647. >> >> Patch was tested in the series with the rest and the compiler bootstraps >> successfully. >> >> Ready for trunk? > > Hi Martin, > Thanks for fixing my issue. The IVO part of patch is OK. > Just for me to understand, iv_common_cand is freed via free_ptr_hash, > and thus typed_free_remove. So what leaks is the iv_use * vector in > struct iv_common_cand, right? I did forget to free that. Hi. You are right, the suggested patch uses delete operator for deallocation of iv_common_cand structure. That eventually calls dtor of auto_vec. > BTW, how do you monitor memory use in GCC, maybe I can run same test > for my future patches. I've been working on removal of memory leaks using valgrind, just configure the compiler with '--enable-valgrind-annotations' and run for instance: valgrind --leak-check=yes --trace-children=yes ./gcc/xgcc -Bgcc ../gcc/testsuite/gcc.dg/tree-ssa/loop-32.c -c -O2 Producing: ... ==13919== 216 bytes in 3 blocks are definitely lost in loss record 679 of 795 ==13919== at 0x4C2A00F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==13919== by 0x107CEDF: xrealloc (xmalloc.c:178) ==13919== by 0xAC46AA: reserve<iv_use*> (vec.h:288) ==13919== by 0xAC46AA: reserve (vec.h:1406) ==13919== by 0xAC46AA: reserve_exact (vec.h:1426) ==13919== by 0xAC46AA: create (vec.h:1441) ==13919== by 0xAC46AA: record_common_cand(ivopts_data*, tree_node*, tree_node*, iv_use*) (tree-ssa-loop-ivopts.c:3133) ==13919== by 0xAC49C5: add_iv_candidate_for_use(ivopts_data*, iv_use*) (tree-ssa-loop-ivopts.c:3220) ==13919== by 0xAC4EA2: add_iv_candidate_for_uses (tree-ssa-loop-ivopts.c:3294) ==13919== by 0xAC4EA2: find_iv_candidates(ivopts_data*) (tree-ssa-loop-ivopts.c:5705) ==13919== by 0xAC839D: tree_ssa_iv_optimize_loop (tree-ssa-loop-ivopts.c:7708) ==13919== by 0xAC839D: tree_ssa_iv_optimize() (tree-ssa-loop-ivopts.c:7758) ==13919== by 0xADE4D0: (anonymous namespace)::pass_iv_optimize::execute(function*) (tree-ssa-loop.c:520) ==13919== by 0x920033: execute_one_pass(opt_pass*) (passes.c:2335) ==13919== by 0x920547: execute_pass_list_1(opt_pass*) [clone .constprop.84] (passes.c:2408) ==13919== by 0x920559: execute_pass_list_1(opt_pass*) [clone .constprop.84] (passes.c:2409) ==13919== by 0x920559: execute_pass_list_1(opt_pass*) [clone .constprop.84] (passes.c:2409) ==13919== by 0x9205A4: execute_pass_list(function*, opt_pass*) (passes.c:2419) ... Martin > > Thanks, > bin >
On Fri, Nov 27, 2015 at 4:29 PM, Martin Liška <mliska@suse.cz> wrote: > On 11/27/2015 04:54 AM, Bin.Cheng wrote: >> On Fri, Nov 27, 2015 at 5:08 AM, Martin Liška <mliska@suse.cz> wrote: >>> Hi. >>> >>> There's one more patch that fixes really of lot memory leaks related to loop >>> ivopts. >>> The regression was introduced by r230647. >>> >>> Patch was tested in the series with the rest and the compiler bootstraps >>> successfully. >>> >>> Ready for trunk? >> >> Hi Martin, >> Thanks for fixing my issue. The IVO part of patch is OK. >> Just for me to understand, iv_common_cand is freed via free_ptr_hash, >> and thus typed_free_remove. So what leaks is the iv_use * vector in >> struct iv_common_cand, right? I did forget to free that. > > Hi. > > You are right, the suggested patch uses delete operator for deallocation of iv_common_cand > structure. That eventually calls dtor of auto_vec. > >> BTW, how do you monitor memory use in GCC, maybe I can run same test >> for my future patches. > > I've been working on removal of memory leaks using valgrind, just configure the compiler with > '--enable-valgrind-annotations' and run for instance: > > valgrind --leak-check=yes --trace-children=yes ./gcc/xgcc -Bgcc ../gcc/testsuite/gcc.dg/tree-ssa/loop-32.c -c -O2 > > Producing: > ... > ==13919== 216 bytes in 3 blocks are definitely lost in loss record 679 of 795 > ==13919== at 0x4C2A00F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) > ==13919== by 0x107CEDF: xrealloc (xmalloc.c:178) > ==13919== by 0xAC46AA: reserve<iv_use*> (vec.h:288) > ==13919== by 0xAC46AA: reserve (vec.h:1406) > ==13919== by 0xAC46AA: reserve_exact (vec.h:1426) > ==13919== by 0xAC46AA: create (vec.h:1441) > ==13919== by 0xAC46AA: record_common_cand(ivopts_data*, tree_node*, tree_node*, iv_use*) (tree-ssa-loop-ivopts.c:3133) > ==13919== by 0xAC49C5: add_iv_candidate_for_use(ivopts_data*, iv_use*) (tree-ssa-loop-ivopts.c:3220) > ==13919== by 0xAC4EA2: add_iv_candidate_for_uses (tree-ssa-loop-ivopts.c:3294) > ==13919== by 0xAC4EA2: find_iv_candidates(ivopts_data*) (tree-ssa-loop-ivopts.c:5705) > ==13919== by 0xAC839D: tree_ssa_iv_optimize_loop (tree-ssa-loop-ivopts.c:7708) > ==13919== by 0xAC839D: tree_ssa_iv_optimize() (tree-ssa-loop-ivopts.c:7758) > ==13919== by 0xADE4D0: (anonymous namespace)::pass_iv_optimize::execute(function*) (tree-ssa-loop.c:520) > ==13919== by 0x920033: execute_one_pass(opt_pass*) (passes.c:2335) > ==13919== by 0x920547: execute_pass_list_1(opt_pass*) [clone .constprop.84] (passes.c:2408) > ==13919== by 0x920559: execute_pass_list_1(opt_pass*) [clone .constprop.84] (passes.c:2409) > ==13919== by 0x920559: execute_pass_list_1(opt_pass*) [clone .constprop.84] (passes.c:2409) > ==13919== by 0x9205A4: execute_pass_list(function*, opt_pass*) (passes.c:2419) > ... Thanks for explanation, I will do that in future. Thanks, bin
From 1f06962c8f126de5aa847882dadba4b95fc89bfc Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Wed, 25 Nov 2015 13:06:07 +0100 Subject: [PATCH 6/6] Fix newly introduced memory leak in tree-ssa-loop-ivopts.c gcc/ChangeLog: 2015-11-25 Martin Liska <mliska@suse.cz> * hash-traits.h (struct typed_delete_remove): New function. (typed_delete_remove ::remove): Likewise. * tree-ssa-loop-ivopts.c (struct iv_common_cand): Replace auto_vec with vec. (record_common_cand): Replace XNEW with operator new. --- gcc/hash-traits.h | 23 +++++++++++++++++++++++ gcc/tree-ssa-loop-ivopts.c | 6 +++--- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/gcc/hash-traits.h b/gcc/hash-traits.h index 450354a..3997ede 100644 --- a/gcc/hash-traits.h +++ b/gcc/hash-traits.h @@ -38,6 +38,23 @@ typed_free_remove <Type>::remove (Type *p) free (p); } +/* Helpful type for removing with delete. */ + +template <typename Type> +struct typed_delete_remove +{ + static inline void remove (Type *p); +}; + + +/* Remove with delete. */ + +template <typename Type> +inline void +typed_delete_remove <Type>::remove (Type *p) +{ + delete p; +} /* Helpful type for a no-op remove. */ @@ -260,6 +277,12 @@ struct nofree_ptr_hash : pointer_hash <T>, typed_noop_remove <T *> {}; template <typename T> struct free_ptr_hash : pointer_hash <T>, typed_free_remove <T> {}; +/* Traits for pointer elements that should be freed via delete operand when an + element is deleted. */ + +template <typename T> +struct delete_ptr_hash : pointer_hash <T>, typed_delete_remove <T> {}; + /* Traits for elements that point to gc memory. The pointed-to data must be kept across collections. */ diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c index 98dc451..d7a0e9e 100644 --- a/gcc/tree-ssa-loop-ivopts.c +++ b/gcc/tree-ssa-loop-ivopts.c @@ -253,13 +253,13 @@ struct iv_common_cand tree base; tree step; /* IV uses from which this common candidate is derived. */ - vec<iv_use *> uses; + auto_vec<iv_use *> uses; hashval_t hash; }; /* Hashtable helpers. */ -struct iv_common_cand_hasher : free_ptr_hash <iv_common_cand> +struct iv_common_cand_hasher : delete_ptr_hash <iv_common_cand> { static inline hashval_t hash (const iv_common_cand *); static inline bool equal (const iv_common_cand *, const iv_common_cand *); @@ -3127,7 +3127,7 @@ record_common_cand (struct ivopts_data *data, tree base, slot = data->iv_common_cand_tab->find_slot (&ent, INSERT); if (*slot == NULL) { - *slot = XNEW (struct iv_common_cand); + *slot = new iv_common_cand (); (*slot)->base = base; (*slot)->step = step; (*slot)->uses.create (8); -- 2.6.3