Re: [PATCH] Fix memory leaks and use a pool_allocator

[Richard Biener <richard dot guenther at gmail dot com>]

On Mon, Nov 9, 2015 at 2:26 PM, Martin LiÅka <mliska@suse.cz> wrote:
> On 11/09/2015 01:11 PM, Richard Biener wrote:
>> On Mon, Nov 9, 2015 at 12:22 PM, Martin LiÅka <mliska@suse.cz> wrote:
>>> Hi.
>>>
>>> This is follow-up of changes that Richi started on Friday.
>>>
>>> Patch can bootstrap on x86_64-linux-pc and regression tests are running.
>>>
>>> Ready for trunk?
>>
>>         * tree-ssa-dom.c (free_edge_info): Make the function extern.
>> ...
>>         * tree-ssa.h (free_edge_info): Declare function extern.
>>
>> declare this in tree-ssa-threadupdate.h instead and renaming it to
>> sth less "public", like free_dom_edge_info.
>>
>> diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
>> index fff62de..eb6b7df 100644
>> --- a/gcc/ifcvt.c
>> +++ b/gcc/ifcvt.c
>> @@ -3161,6 +3161,8 @@ noce_convert_multiple_sets (struct noce_if_info *if_info)
>>        set_used_flags (targets[i]);
>>      }
>>
>> +  temporaries.release ();
>> +
>>    set_used_flags (cond);
>>    set_used_flags (x);
>>    set_used_flags (y);
>> @@ -3194,6 +3196,7 @@ noce_convert_multiple_sets (struct noce_if_info *if_info)
>>      }
>>
>>    num_updated_if_blocks++;
>> +  targets.release ();
>>    return TRUE;
>>
>> suspiciously look like candidates for an auto_vec<> (didn't check).
>>
>> @@ -1240,6 +1240,7 @@ lra_create_live_ranges_1 (bool all_p, bool dead_insn_p)
>>    dead_set = sparseset_alloc (max_regno);
>>    unused_set = sparseset_alloc (max_regno);
>>    curr_point = 0;
>> +  point_freq_vec.release ();
>>    point_freq_vec.create (get_max_uid () * 2);
>>
>> a truncate (0) instead of a release () should be cheaper, avoiding the
>> re-allocation.
>>
>> @@ -674,6 +674,10 @@ sra_deinitialize (void)
>>    assign_link_pool.release ();
>>    obstack_free (&name_obstack, NULL);
>>
>> +  for (hash_map<tree, auto_vec<access_p> >::iterator it =
>> +       base_access_vec->begin (); it != base_access_vec->end (); ++it)
>> +    (*it).second.release ();
>> +
>>    delete base_access_vec;
>>
>> I wonder if the better fix is to provide a proper free method for the hash_map?
>> A hash_map with 'auto_vec' looks suspicous - eventually a proper release
>> was intented here via default_hash_map_traits <>?
>>
>> Anyway, most of the things above can be improved as followup of course.
>>
>> Thanks,
>> Richard.
>>
>>> Thanks,
>>> Martin
>
> Hi.
>
> All suggested changes were applied, sending v2 and waiting for bootstrap and
> regression tests.

Ok.

Thanks,
Richard.

> Thanks,
> Martin