| Message ID | 1416393981-39626-20-git-send-email-dmalcolm@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 11/19/14 03:46, David Malcolm wrote: > This fixes three leaks in IPA seen in jit testcases with valgrind: > > This one: > 96 bytes in 4 blocks are definitely lost in loss record 102 of 205 > at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) > by 0x5D76447: xmalloc (xmalloc.c:147) > by 0x4E35C23: symbol_table::add_cgraph_insertion_hook(void (*)(cgraph_node*, void*), void*) (cgraph.c:383) > by 0x51070C6: ipa_register_cgraph_hooks() (ipa-prop.c:3864) > by 0x5C753D8: ipcp_generate_summary() (ipa-cp.c:3786) > by 0x5223540: execute_ipa_summary_passes(ipa_opt_pass_d*) (passes.c:2102) > by 0x4E49A60: ipa_passes() (cgraphunit.c:2074) > by 0x4E49E2C: symbol_table::compile() (cgraphunit.c:2172) > by 0x4E4A1C2: symbol_table::finalize_compilation_unit() (cgraphunit.c:2325) > by 0x4DC999C: jit_langhook_write_globals() (dummy-frontend.c:216) > by 0x532B3A6: compile_file() (toplev.c:583) > by 0x532E15F: do_compile() (toplev.c:2020) > > appears to be caused by > ipa-prop.c (ipa_register_cgraph_hooks) > unconditionally inserting ipa_add_new_function as > "function_insertion_hook_holder", rather than if the latter is > non-NULL, like the other hooks. > > These two in ipa-reference.c: > 96 bytes in 4 blocks are definitely lost in loss record 103 of 205 > at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) > by 0x5D76447: xmalloc (xmalloc.c:147) > by 0x4E35AA9: symbol_table::add_cgraph_removal_hook(void (*)(cgraph_node*, void*), void*) (cgraph.c:329) > by 0x5CA446E: ipa_init() (ipa-reference.c:435) > by 0x5CA47D1: generate_summary() (ipa-reference.c:551) > by 0x5CA4E70: propagate() (ipa-reference.c:684) > by 0x5CA640E: (anonymous namespace)::pass_ipa_reference::execute(function*) (ipa-reference.c:1178) > by 0x5223CC1: execute_one_pass(opt_pass*) (passes.c:2306) > by 0x5224B9B: execute_ipa_pass_list(opt_pass*) (passes.c:2700) > by 0x4E49B0D: ipa_passes() (cgraphunit.c:2088) > by 0x4E49E2C: symbol_table::compile() (cgraphunit.c:2172) > by 0x4E4A1C2: symbol_table::finalize_compilation_unit() (cgraphunit.c:2325) > > 96 bytes in 4 blocks are definitely lost in loss record 104 of 205 > at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) > by 0x5D76447: xmalloc (xmalloc.c:147) > by 0x4E35E29: symbol_table::add_cgraph_duplication_hook(void (*)(cgraph_node*, cgraph_node*, void*), void*) (cgraph.c:453) > by 0x5CA4493: ipa_init() (ipa-reference.c:437) > by 0x5CA47D1: generate_summary() (ipa-reference.c:551) > by 0x5CA4E70: propagate() (ipa-reference.c:684) > by 0x5CA640E: (anonymous namespace)::pass_ipa_reference::execute(function*) (ipa-reference.c:1178) > by 0x5223CC1: execute_one_pass(opt_pass*) (passes.c:2306) > by 0x5224B9B: execute_ipa_pass_list(opt_pass*) (passes.c:2700) > by 0x4E49B0D: ipa_passes() (cgraphunit.c:2088) > by 0x4E49E2C: symbol_table::compile() (cgraphunit.c:2172) > by 0x4E4A1C2: symbol_table::finalize_compilation_unit() (cgraphunit.c:2325) > > appear to be due to th hooks never being removed. > > My patch hacks in a removal of them into ipa_reference_c_finalize, but > I suspect there's a cleaner place to put this. > > gcc/ChangeLog: > PR jit/63854 > * ipa-prop.c (ipa_register_cgraph_hooks): Guard insertion of > ipa_add_new_function on function_insertion_hook_holder being > non-NULL. > * ipa-reference.c (ipa_reference_c_finalize): Remove > node_removal_hook_holder and node_duplication_hook_holder if > they've been added to symtab. > * toplev.c (toplev::finalize): Call ipa_reference_c_finalize > before cgraph_c_finalize so that the former can access "symtab". I'm going to let Jan own this one. jeff
On Wed, 2014-11-19 at 12:58 -0700, Jeff Law wrote: > On 11/19/14 03:46, David Malcolm wrote: > > This fixes three leaks in IPA seen in jit testcases with valgrind: > > > > This one: > > 96 bytes in 4 blocks are definitely lost in loss record 102 of 205 > > at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) > > by 0x5D76447: xmalloc (xmalloc.c:147) > > by 0x4E35C23: symbol_table::add_cgraph_insertion_hook(void (*)(cgraph_node*, void*), void*) (cgraph.c:383) > > by 0x51070C6: ipa_register_cgraph_hooks() (ipa-prop.c:3864) > > by 0x5C753D8: ipcp_generate_summary() (ipa-cp.c:3786) > > by 0x5223540: execute_ipa_summary_passes(ipa_opt_pass_d*) (passes.c:2102) > > by 0x4E49A60: ipa_passes() (cgraphunit.c:2074) > > by 0x4E49E2C: symbol_table::compile() (cgraphunit.c:2172) > > by 0x4E4A1C2: symbol_table::finalize_compilation_unit() (cgraphunit.c:2325) > > by 0x4DC999C: jit_langhook_write_globals() (dummy-frontend.c:216) > > by 0x532B3A6: compile_file() (toplev.c:583) > > by 0x532E15F: do_compile() (toplev.c:2020) > > > > appears to be caused by > > ipa-prop.c (ipa_register_cgraph_hooks) > > unconditionally inserting ipa_add_new_function as > > "function_insertion_hook_holder", rather than if the latter is > > non-NULL, like the other hooks. > > > > These two in ipa-reference.c: > > 96 bytes in 4 blocks are definitely lost in loss record 103 of 205 > > at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) > > by 0x5D76447: xmalloc (xmalloc.c:147) > > by 0x4E35AA9: symbol_table::add_cgraph_removal_hook(void (*)(cgraph_node*, void*), void*) (cgraph.c:329) > > by 0x5CA446E: ipa_init() (ipa-reference.c:435) > > by 0x5CA47D1: generate_summary() (ipa-reference.c:551) > > by 0x5CA4E70: propagate() (ipa-reference.c:684) > > by 0x5CA640E: (anonymous namespace)::pass_ipa_reference::execute(function*) (ipa-reference.c:1178) > > by 0x5223CC1: execute_one_pass(opt_pass*) (passes.c:2306) > > by 0x5224B9B: execute_ipa_pass_list(opt_pass*) (passes.c:2700) > > by 0x4E49B0D: ipa_passes() (cgraphunit.c:2088) > > by 0x4E49E2C: symbol_table::compile() (cgraphunit.c:2172) > > by 0x4E4A1C2: symbol_table::finalize_compilation_unit() (cgraphunit.c:2325) > > > > 96 bytes in 4 blocks are definitely lost in loss record 104 of 205 > > at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) > > by 0x5D76447: xmalloc (xmalloc.c:147) > > by 0x4E35E29: symbol_table::add_cgraph_duplication_hook(void (*)(cgraph_node*, cgraph_node*, void*), void*) (cgraph.c:453) > > by 0x5CA4493: ipa_init() (ipa-reference.c:437) > > by 0x5CA47D1: generate_summary() (ipa-reference.c:551) > > by 0x5CA4E70: propagate() (ipa-reference.c:684) > > by 0x5CA640E: (anonymous namespace)::pass_ipa_reference::execute(function*) (ipa-reference.c:1178) > > by 0x5223CC1: execute_one_pass(opt_pass*) (passes.c:2306) > > by 0x5224B9B: execute_ipa_pass_list(opt_pass*) (passes.c:2700) > > by 0x4E49B0D: ipa_passes() (cgraphunit.c:2088) > > by 0x4E49E2C: symbol_table::compile() (cgraphunit.c:2172) > > by 0x4E4A1C2: symbol_table::finalize_compilation_unit() (cgraphunit.c:2325) > > > > appear to be due to th hooks never being removed. > > > > My patch hacks in a removal of them into ipa_reference_c_finalize, but > > I suspect there's a cleaner place to put this. > > > > gcc/ChangeLog: > > PR jit/63854 > > * ipa-prop.c (ipa_register_cgraph_hooks): Guard insertion of > > ipa_add_new_function on function_insertion_hook_holder being > > non-NULL. > > * ipa-reference.c (ipa_reference_c_finalize): Remove > > node_removal_hook_holder and node_duplication_hook_holder if > > they've been added to symtab. > > * toplev.c (toplev::finalize): Call ipa_reference_c_finalize > > before cgraph_c_finalize so that the former can access "symtab". > I'm going to let Jan own this one. > > jeff Jan: please can you have a look at this one; patch can be seen at: https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02415.html Thanks Dave
> > > My patch hacks in a removal of them into ipa_reference_c_finalize, but > > > I suspect there's a cleaner place to put this. > > > > > > gcc/ChangeLog: > > > PR jit/63854 > > > * ipa-prop.c (ipa_register_cgraph_hooks): Guard insertion of > > > ipa_add_new_function on function_insertion_hook_holder being > > > non-NULL. > > > * ipa-reference.c (ipa_reference_c_finalize): Remove > > > node_removal_hook_holder and node_duplication_hook_holder if > > > they've been added to symtab. > > > * toplev.c (toplev::finalize): Call ipa_reference_c_finalize > > > before cgraph_c_finalize so that the former can access "symtab". > > I'm going to let Jan own this one. > > > > jeff > > Jan: please can you have a look at this one; patch can be seen at: > https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02415.html Patch is OK, thanks! I hope some of this mess will go away soon with Martin's summary reorg. Honza > > > Thanks > Dave
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c index f87243c..a8238ac 100644 --- a/gcc/ipa-prop.c +++ b/gcc/ipa-prop.c @@ -3552,7 +3552,8 @@ ipa_register_cgraph_hooks (void) if (!node_duplication_hook_holder) node_duplication_hook_holder = symtab->add_cgraph_duplication_hook (&ipa_node_duplication_hook, NULL); - function_insertion_hook_holder = + if (!function_insertion_hook_holder) + function_insertion_hook_holder = symtab->add_cgraph_insertion_hook (&ipa_add_new_function, NULL); } diff --git a/gcc/ipa-reference.c b/gcc/ipa-reference.c index 1ce06d1..b046f9e 100644 --- a/gcc/ipa-reference.c +++ b/gcc/ipa-reference.c @@ -1198,4 +1198,15 @@ ipa_reference_c_finalize (void) bitmap_obstack_release (&optimization_summary_obstack); ipa_init_p = false; } + + if (node_removal_hook_holder) + { + symtab->remove_cgraph_removal_hook (node_removal_hook_holder); + node_removal_hook_holder = NULL; + } + if (node_duplication_hook_holder) + { + symtab->remove_cgraph_duplication_hook (node_duplication_hook_holder); + node_duplication_hook_holder = NULL; + } } diff --git a/gcc/toplev.c b/gcc/toplev.c index c0c418c..96e7af9 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -2162,12 +2162,14 @@ toplev::finalize (void) rtl_initialized = false; this_target_rtl->target_specific_initialized = false; + /* Needs to be called before cgraph_c_finalize since it uses symtab. */ + ipa_reference_c_finalize (); + cgraph_c_finalize (); cgraphunit_c_finalize (); dwarf2out_c_finalize (); gcse_c_finalize (); ipa_cp_c_finalize (); - ipa_reference_c_finalize (); ira_costs_c_finalize (); params_c_finalize ();