Thanks,
On 2019/11/26 18:15, Jan Hubicka wrote:
>> Hi,
>>
>> On 2019/11/26 16:04, Jan Hubicka wrote:
>>>> Summary variables should be deleted at the end of write_summary.
>>>> It's first newed in generate_summary, and second newed in read_summary.
>>>> Therefore, delete the first in write_summary, delete the second in
>>>> execute.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2019-11-26 Luo Xiong Hu <luo...@linux.ibm.com>
>>>>
>>>> * ipa-pure-const.c (pure_const_write_summary): Fix memory leak.
>>>> * ipa-sra.c (ipa_sra_write_summary): Likewise.
>>>
>>> This is not going to work with -ffat-lto-objects because in this case
>>> IPA pass is executed later. So you will need
>>> if (!flag_fat_lto_objects).
>>>
>>> I think most IPA passes just waits for compiler to exit with LTO rather
>>> than free the summary. ipa-fnsummary and ipa-prop allocate optimization
>>> summaries, too. Are those freed?
>>
>> ipa-prop is a bit different, the ipcp_transformation_sum is only created in
>> read_summary, so need only delete once after execute.
>
> It also has ipa_node_params_sum and ipa_edge_args_sum which live from
> ipa-prop analysis until after ipa-inlining.
>>
>> ipa-fnsummary also forgot to free the ipa summaries at the end of
>> write_summary. The pass pass_ipa_free_fn_summary will delete all summaries
>> in execute.
>>
>> For -ffat-lto-objects, I suppose there would be no write_summary and
>> read_summary? so summaries are also newed once and delete once? Thanks.
>>
>> Anyway, lto1 will run after cc1, which means cc1 is finished, the memory is
>> freed theoretically.
>
> With -ffat-lto-object we run full optimization pipieline at compile
> time. This means that we first write summary, then execute the pass
> (which uses the summary and frees it) and proceed by compilation.
> With -fno-fat-lto-object the pipieline is terminated after summary
> writting. By quiting compiler we free all the memory.
>
> We do have a lot of places where memory leaks this way and is freed at
> exit. I see that for jit we added sume explicit free calls, but I think
> jit is not using this path.
>
> I am not against freeing things explicitly since it makes it easier to
> identify real leaks, but we should do it systematically. Thinking of
> this case probably may even lead to some memory savings because after
> summary streaming the global stream is produced which is typically the
> biggest object streamed. So freeing it is OK, but please add check for
> !fat lto objects and also free ipa-fnsummary (there is
> ipa_free_fn_summary, ipa_free_size_summary for that) and ipa-prop
> summaries.
Verified that "if (!flag_fat_lto_objects)" is required for explicitly
deleting the summaries when built with -ffat-lto-objects.
>
> I will also look into your changes for ipa-profile tomorrow (I suppose
> you noticed this while looking into summaries for that patch :)
It's true that I am moving the speculative target summaries from cgraph.h
to ipa-profile.c, code is exploded with hundred of new lines. Simple test
case could work now with write_summary/read_summary/execute, but most spec
test cases ICEd due to ggc_grow() or ggc_collect(), I debugged the code and
found that read_summary is correct streaming in the target summaries, but some
edge's summary was GCed UNEXPECTEDLY by followed ggc_grow() or ggc_collect()
before calling ipa_profile, any clue on this, please?
Great that you take time on this!:)
ipa-profile.c
...
+/* Structure containing speculative target information from profile. */
+
+struct GTY (()) speculative_call_target
+{
+ speculative_call_target (unsigned int id, int prob)
+ : target_id (id), target_probability (prob)
+ {
+ }
+
+ /* Profile_id of target obtained from profile. */
+ unsigned int target_id;
+ /* Probability that call will land in function with target_id. */
+ int target_probability;
+};
+
+class speculative_call_summary
+{
+public:
+ speculative_call_summary () : speculative_call_targets () {}
+
+ vec<speculative_call_target, va_gc> *speculative_call_targets;
+ ~speculative_call_summary ();
+
+ void dump (FILE *f);
+
+ /* Check whether this is a empty summary. */
+ bool is_empty ();
+};
...
Xiong Hu
> Honza
>>
>> Xiong Hu
>>
>>
>>>
>>> Honza
>>>> ---
>>>> gcc/ipa-pure-const.c | 3 +++
>>>> gcc/ipa-sra.c | 5 +++++
>>>> 2 files changed, 8 insertions(+)
>>>>
>>>> diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c
>>>> index a142e0cc8f6..89f334cedac 100644
>>>> --- a/gcc/ipa-pure-const.c
>>>> +++ b/gcc/ipa-pure-const.c
>>>> @@ -1260,6 +1260,9 @@ pure_const_write_summary (void)
>>>> }
>>>> lto_destroy_simple_output_block (ob);
>>>> +
>>>> + delete funct_state_summaries;
>>>> + funct_state_summaries = NULL;
>>>> }
>>>> diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c
>>>> index c6ed0f44b87..bbc5e84edfb 100644
>>>> --- a/gcc/ipa-sra.c
>>>> +++ b/gcc/ipa-sra.c
>>>> @@ -2671,6 +2671,11 @@ ipa_sra_write_summary (void)
>>>> streamer_write_char_stream (ob->main_stream, 0);
>>>> produce_asm (ob, NULL);
>>>> destroy_output_block (ob);
>>>> +
>>>> + ggc_delete (func_sums);
>>>> + func_sums = NULL;
>>>> + delete call_sums;
>>>> + call_sums = NULL;
>>>> }
>>>> /* Read intraprocedural analysis information about E and all of its
>>>> outgoing
>>>> --
>>>> 2.21.0.777.g83232e3864
>>>>
>>
