* [PATCH] Fix memory leak in IPA passes
@ 2015-04-14 8:14 Martin Liška
2015-04-14 8:18 ` Jakub Jelinek
2015-04-14 13:45 ` Jan Hubicka
0 siblings, 2 replies; 6+ messages in thread
From: Martin Liška @ 2015-04-14 8:14 UTC (permalink / raw)
To: GCC Patches; +Cc: Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 332 bytes --]
Hello.
As originally reported by Andi Kleen, following patch fix memory leaks that can
be seen in IPA ICF and IPA CP.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Moreover, with patch applied, we can build Firefox and Linux kernel on ppc64le-linux-gnu.
Ready for both trunk and 5 branch?
Thanks,
Martin
[-- Attachment #2: 0001-Fix-IPA-memory-leaks.patch --]
[-- Type: text/x-patch, Size: 3933 bytes --]
From 5258b6a07d01c632976c3a5c3c5e47da3a9965e3 Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Mon, 13 Apr 2015 13:15:59 +0200
Subject: [PATCH] Fix IPA memory leaks.
gcc/ChangeLog:
2015-04-13 Martin Liska <mliska@suse.cz>
* ipa-cp.c (ipcp_driver): Release prev_edge_clone.
* ipa-icf.c (sem_item_optimizer::subdivide_classes_by_sensitive_refs):
Release symbol_compare_collection.
* ipa-reference.c: Add TODO that a vector should be released.
---
gcc/ipa-cp.c | 1 +
gcc/ipa-icf.c | 26 ++++++++++++++++++++------
gcc/ipa-reference.c | 1 +
3 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index bfe4821..3824029 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -4493,6 +4493,7 @@ ipcp_driver (void)
/* Free all IPCP structures. */
free_toporder_info (&topo);
next_edge_clone.release ();
+ prev_edge_clone.release ();
symtab->remove_edge_removal_hook (edge_removal_hook_holder);
symtab->remove_edge_duplication_hook (edge_duplication_hook_holder);
ipa_free_all_structures_after_ipa_cp ();
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index b902373..a72ac2e 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -2712,6 +2712,9 @@ sem_item_optimizer::subdivide_classes_by_equality (bool in_wpa)
unsigned
sem_item_optimizer::subdivide_classes_by_sensitive_refs ()
{
+ typedef hash_map <symbol_compare_collection *, vec <sem_item *>,
+ symbol_compare_hashmap_traits> subdivide_hash_map;
+
unsigned newly_created_classes = 0;
for (hash_table <congruence_class_group_hash>::iterator it = m_classes.begin ();
@@ -2726,8 +2729,7 @@ sem_item_optimizer::subdivide_classes_by_sensitive_refs ()
if (c->members.length() > 1)
{
- hash_map <symbol_compare_collection *, vec <sem_item *>,
- symbol_compare_hashmap_traits> split_map;
+ subdivide_hash_map split_map;
for (unsigned j = 0; j < c->members.length (); j++)
{
@@ -2735,10 +2737,15 @@ sem_item_optimizer::subdivide_classes_by_sensitive_refs ()
symbol_compare_collection *collection = new symbol_compare_collection (source_node->node);
- vec <sem_item *> *slot = &split_map.get_or_insert (collection);
+ bool existed;
+ vec <sem_item *> *slot = &split_map.get_or_insert (collection,
+ &existed);
gcc_checking_assert (slot);
slot->safe_push (source_node);
+
+ if (existed)
+ delete collection;
}
/* If the map contains more than one key, we have to split the map
@@ -2747,9 +2754,8 @@ sem_item_optimizer::subdivide_classes_by_sensitive_refs ()
{
bool first_class = true;
- hash_map <symbol_compare_collection *, vec <sem_item *>,
- symbol_compare_hashmap_traits>::iterator it2 = split_map.begin ();
- for (; it2 != split_map.end (); ++it2)
+ for (subdivide_hash_map::iterator it2 = split_map.begin ();
+ it2 != split_map.end (); ++it2)
{
congruence_class *new_cls;
new_cls = new congruence_class (class_id++);
@@ -2772,6 +2778,14 @@ sem_item_optimizer::subdivide_classes_by_sensitive_refs ()
}
}
}
+
+ /* Release memory. */
+ for (subdivide_hash_map::iterator it2 = split_map.begin ();
+ it2 != split_map.end (); ++it2)
+ {
+ delete (*it2).first;
+ (*it2).second.release ();
+ }
}
}
diff --git a/gcc/ipa-reference.c b/gcc/ipa-reference.c
index 219a9b3..a420cb2 100644
--- a/gcc/ipa-reference.c
+++ b/gcc/ipa-reference.c
@@ -150,6 +150,7 @@ static struct cgraph_node_hook_list *node_removal_hook_holder;
Indexed by UID of call graph nodes. */
static vec<ipa_reference_vars_info_t> ipa_reference_vars_vector;
+/* TODO: find a place where we should release the vector. */
static vec<ipa_reference_optimization_summary_t> ipa_reference_opt_sum_vector;
/* Return the ipa_reference_vars structure starting from the cgraph NODE. */
--
2.1.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix memory leak in IPA passes
2015-04-14 8:14 [PATCH] Fix memory leak in IPA passes Martin Liška
@ 2015-04-14 8:18 ` Jakub Jelinek
2015-04-14 8:23 ` Martin Liška
2015-04-14 13:45 ` Jan Hubicka
1 sibling, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2015-04-14 8:18 UTC (permalink / raw)
To: Martin Liška; +Cc: GCC Patches, Jan Hubicka
On Tue, Apr 14, 2015 at 10:14:06AM +0200, Martin Liška wrote:
> As originally reported by Andi Kleen, following patch fix memory leaks that can
> be seen in IPA ICF and IPA CP.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> Moreover, with patch applied, we can build Firefox and Linux kernel on ppc64le-linux-gnu.
>
> Ready for both trunk and 5 branch?
If Honza acks this, I'd prefer if it went to 5 branch only after 5.1 is
released.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix memory leak in IPA passes
2015-04-14 8:18 ` Jakub Jelinek
@ 2015-04-14 8:23 ` Martin Liška
0 siblings, 0 replies; 6+ messages in thread
From: Martin Liška @ 2015-04-14 8:23 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: GCC Patches, Jan Hubicka
On 04/14/2015 10:18 AM, Jakub Jelinek wrote:
> On Tue, Apr 14, 2015 at 10:14:06AM +0200, Martin Liška wrote:
>> As originally reported by Andi Kleen, following patch fix memory leaks that can
>> be seen in IPA ICF and IPA CP.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>> Moreover, with patch applied, we can build Firefox and Linux kernel on ppc64le-linux-gnu.
>>
>> Ready for both trunk and 5 branch?
>
> If Honza acks this, I'd prefer if it went to 5 branch only after 5.1 is
> released.
>
> Jakub
>
Hello
I have no problem with that, memory leaks are quite small I guess.
Martin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix memory leak in IPA passes
2015-04-14 8:14 [PATCH] Fix memory leak in IPA passes Martin Liška
2015-04-14 8:18 ` Jakub Jelinek
@ 2015-04-14 13:45 ` Jan Hubicka
2015-04-14 14:15 ` Martin Liška
1 sibling, 1 reply; 6+ messages in thread
From: Jan Hubicka @ 2015-04-14 13:45 UTC (permalink / raw)
To: Martin Liška; +Cc: GCC Patches, Jan Hubicka
>
> 2015-04-13 Martin Liska <mliska@suse.cz>
>
> * ipa-cp.c (ipcp_driver): Release prev_edge_clone.
> * ipa-icf.c (sem_item_optimizer::subdivide_classes_by_sensitive_refs):
> Release symbol_compare_collection.
> * ipa-reference.c: Add TODO that a vector should be released.
> ---
> gcc/ipa-cp.c | 1 +
> gcc/ipa-icf.c | 26 ++++++++++++++++++++------
> gcc/ipa-reference.c | 1 +
> 3 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index bfe4821..3824029 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -4493,6 +4493,7 @@ ipcp_driver (void)
> /* Free all IPCP structures. */
> free_toporder_info (&topo);
> next_edge_clone.release ();
> + prev_edge_clone.release ();
> symtab->remove_edge_removal_hook (edge_removal_hook_holder);
> symtab->remove_edge_duplication_hook (edge_duplication_hook_holder);
> ipa_free_all_structures_after_ipa_cp ();
I already fixed this one on mainline.
> diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
> index b902373..a72ac2e 100644
> --- a/gcc/ipa-icf.c
> +++ b/gcc/ipa-icf.c
> @@ -2712,6 +2712,9 @@ sem_item_optimizer::subdivide_classes_by_equality (bool in_wpa)
> unsigned
> sem_item_optimizer::subdivide_classes_by_sensitive_refs ()
> {
> + typedef hash_map <symbol_compare_collection *, vec <sem_item *>,
> + symbol_compare_hashmap_traits> subdivide_hash_map;
> +
> unsigned newly_created_classes = 0;
>
> for (hash_table <congruence_class_group_hash>::iterator it = m_classes.begin ();
> @@ -2726,8 +2729,7 @@ sem_item_optimizer::subdivide_classes_by_sensitive_refs ()
>
> if (c->members.length() > 1)
> {
> - hash_map <symbol_compare_collection *, vec <sem_item *>,
> - symbol_compare_hashmap_traits> split_map;
> + subdivide_hash_map split_map;
>
> for (unsigned j = 0; j < c->members.length (); j++)
> {
> @@ -2735,10 +2737,15 @@ sem_item_optimizer::subdivide_classes_by_sensitive_refs ()
>
> symbol_compare_collection *collection = new symbol_compare_collection (source_node->node);
>
> - vec <sem_item *> *slot = &split_map.get_or_insert (collection);
> + bool existed;
> + vec <sem_item *> *slot = &split_map.get_or_insert (collection,
> + &existed);
> gcc_checking_assert (slot);
>
> slot->safe_push (source_node);
> +
> + if (existed)
> + delete collection;
> }
>
> /* If the map contains more than one key, we have to split the map
> @@ -2747,9 +2754,8 @@ sem_item_optimizer::subdivide_classes_by_sensitive_refs ()
> {
> bool first_class = true;
>
> - hash_map <symbol_compare_collection *, vec <sem_item *>,
> - symbol_compare_hashmap_traits>::iterator it2 = split_map.begin ();
> - for (; it2 != split_map.end (); ++it2)
> + for (subdivide_hash_map::iterator it2 = split_map.begin ();
> + it2 != split_map.end (); ++it2)
> {
> congruence_class *new_cls;
> new_cls = new congruence_class (class_id++);
> @@ -2772,6 +2778,14 @@ sem_item_optimizer::subdivide_classes_by_sensitive_refs ()
> }
> }
> }
> +
> + /* Release memory. */
> + for (subdivide_hash_map::iterator it2 = split_map.begin ();
> + it2 != split_map.end (); ++it2)
> + {
> + delete (*it2).first;
> + (*it2).second.release ();
> + }
> }
> }
>
This is OK.
> diff --git a/gcc/ipa-reference.c b/gcc/ipa-reference.c
> index 219a9b3..a420cb2 100644
> --- a/gcc/ipa-reference.c
> +++ b/gcc/ipa-reference.c
> @@ -150,6 +150,7 @@ static struct cgraph_node_hook_list *node_removal_hook_holder;
> Indexed by UID of call graph nodes. */
> static vec<ipa_reference_vars_info_t> ipa_reference_vars_vector;
>
> +/* TODO: find a place where we should release the vector. */
> static vec<ipa_reference_optimization_summary_t> ipa_reference_opt_sum_vector;
>
> /* Return the ipa_reference_vars structure starting from the cgraph NODE. */
Hmm, I guess this only makes sense for jitting, because the vector is used thorough
whole late complation (and should be turned into the summary template)
OK
Honza
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix memory leak in IPA passes
2015-04-14 13:45 ` Jan Hubicka
@ 2015-04-14 14:15 ` Martin Liška
2015-04-14 15:21 ` Jan Hubicka
0 siblings, 1 reply; 6+ messages in thread
From: Martin Liška @ 2015-04-14 14:15 UTC (permalink / raw)
To: Jan Hubicka; +Cc: GCC Patches
On 04/14/2015 03:45 PM, Jan Hubicka wrote:
>>
>> 2015-04-13 Martin Liska <mliska@suse.cz>
>>
>> * ipa-cp.c (ipcp_driver): Release prev_edge_clone.
>> * ipa-icf.c (sem_item_optimizer::subdivide_classes_by_sensitive_refs):
>> Release symbol_compare_collection.
>> * ipa-reference.c: Add TODO that a vector should be released.
>> ---
>> gcc/ipa-cp.c | 1 +
>> gcc/ipa-icf.c | 26 ++++++++++++++++++++------
>> gcc/ipa-reference.c | 1 +
>> 3 files changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
>> index bfe4821..3824029 100644
>> --- a/gcc/ipa-cp.c
>> +++ b/gcc/ipa-cp.c
>> @@ -4493,6 +4493,7 @@ ipcp_driver (void)
>> /* Free all IPCP structures. */
>> free_toporder_info (&topo);
>> next_edge_clone.release ();
>> + prev_edge_clone.release ();
>> symtab->remove_edge_removal_hook (edge_removal_hook_holder);
>> symtab->remove_edge_duplication_hook (edge_duplication_hook_holder);
>> ipa_free_all_structures_after_ipa_cp ();
>
> I already fixed this one on mainline.
Ah, thanks.
>
>> diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
>> index b902373..a72ac2e 100644
>> --- a/gcc/ipa-icf.c
>> +++ b/gcc/ipa-icf.c
>> @@ -2712,6 +2712,9 @@ sem_item_optimizer::subdivide_classes_by_equality (bool in_wpa)
>> unsigned
>> sem_item_optimizer::subdivide_classes_by_sensitive_refs ()
>> {
>> + typedef hash_map <symbol_compare_collection *, vec <sem_item *>,
>> + symbol_compare_hashmap_traits> subdivide_hash_map;
>> +
>> unsigned newly_created_classes = 0;
>>
>> for (hash_table <congruence_class_group_hash>::iterator it = m_classes.begin ();
>> @@ -2726,8 +2729,7 @@ sem_item_optimizer::subdivide_classes_by_sensitive_refs ()
>>
>> if (c->members.length() > 1)
>> {
>> - hash_map <symbol_compare_collection *, vec <sem_item *>,
>> - symbol_compare_hashmap_traits> split_map;
>> + subdivide_hash_map split_map;
>>
>> for (unsigned j = 0; j < c->members.length (); j++)
>> {
>> @@ -2735,10 +2737,15 @@ sem_item_optimizer::subdivide_classes_by_sensitive_refs ()
>>
>> symbol_compare_collection *collection = new symbol_compare_collection (source_node->node);
>>
>> - vec <sem_item *> *slot = &split_map.get_or_insert (collection);
>> + bool existed;
>> + vec <sem_item *> *slot = &split_map.get_or_insert (collection,
>> + &existed);
>> gcc_checking_assert (slot);
>>
>> slot->safe_push (source_node);
>> +
>> + if (existed)
>> + delete collection;
>> }
>>
>> /* If the map contains more than one key, we have to split the map
>> @@ -2747,9 +2754,8 @@ sem_item_optimizer::subdivide_classes_by_sensitive_refs ()
>> {
>> bool first_class = true;
>>
>> - hash_map <symbol_compare_collection *, vec <sem_item *>,
>> - symbol_compare_hashmap_traits>::iterator it2 = split_map.begin ();
>> - for (; it2 != split_map.end (); ++it2)
>> + for (subdivide_hash_map::iterator it2 = split_map.begin ();
>> + it2 != split_map.end (); ++it2)
>> {
>> congruence_class *new_cls;
>> new_cls = new congruence_class (class_id++);
>> @@ -2772,6 +2778,14 @@ sem_item_optimizer::subdivide_classes_by_sensitive_refs ()
>> }
>> }
>> }
>> +
>> + /* Release memory. */
>> + for (subdivide_hash_map::iterator it2 = split_map.begin ();
>> + it2 != split_map.end (); ++it2)
>> + {
>> + delete (*it2).first;
>> + (*it2).second.release ();
>> + }
>> }
>> }
>>
>
> This is OK.
OK, I'm going to commit it to trunk.
>
>> diff --git a/gcc/ipa-reference.c b/gcc/ipa-reference.c
>> index 219a9b3..a420cb2 100644
>> --- a/gcc/ipa-reference.c
>> +++ b/gcc/ipa-reference.c
>> @@ -150,6 +150,7 @@ static struct cgraph_node_hook_list *node_removal_hook_holder;
>> Indexed by UID of call graph nodes. */
>> static vec<ipa_reference_vars_info_t> ipa_reference_vars_vector;
>>
>> +/* TODO: find a place where we should release the vector. */
>> static vec<ipa_reference_optimization_summary_t> ipa_reference_opt_sum_vector;
>>
>> /* Return the ipa_reference_vars structure starting from the cgraph NODE. */
>
> Hmm, I guess this only makes sense for jitting, because the vector is used thorough
> whole late complation (and should be turned into the summary template)
Yes, I'll port it to new symbol_summary infrastructure.
Martin
>
> OK
> Honza
>> --
>> 2.1.4
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix memory leak in IPA passes
2015-04-14 14:15 ` Martin Liška
@ 2015-04-14 15:21 ` Jan Hubicka
0 siblings, 0 replies; 6+ messages in thread
From: Jan Hubicka @ 2015-04-14 15:21 UTC (permalink / raw)
To: Martin Liška; +Cc: Jan Hubicka, GCC Patches
>
> OK, I'm going to commit it to trunk.
Thanks, and forgot to comment. I do not consider this critical for 5.1.
We may want to backport for 5.2.
Honza
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-04-14 15:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-14 8:14 [PATCH] Fix memory leak in IPA passes Martin Liška
2015-04-14 8:18 ` Jakub Jelinek
2015-04-14 8:23 ` Martin Liška
2015-04-14 13:45 ` Jan Hubicka
2015-04-14 14:15 ` Martin Liška
2015-04-14 15:21 ` Jan Hubicka
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).