Re: [PATCH] Fix memory leak in IPA passes

[Martin Liška <mliska at suse dot cz>]

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