PING Re: [PATCH 19/21] PR jit/63854: Fix leak of ipa hooks

2014-12-01 Thread David Malcolm 
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

Re: PING Re: [PATCH 19/21] PR jit/63854: Fix leak of ipa hooks

2014-12-01 Thread Jan Hubicka 
   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

[PATCH 19/21] PR jit/63854: Fix leak of ipa hooks

2014-11-19 Thread David Malcolm 
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.
---
 gcc/ipa-prop.c  |  3 ++-
 gcc/ipa-reference.c | 11 +++
 gcc/toplev.c|  4 +++-
 3 files changed, 16 insertions(+), 2 deletions(-)

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

Re: [PATCH 19/21] PR jit/63854: Fix leak of ipa hooks

2014-11-19 Thread Jeff Law 

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