* Free more of CFG in release_function_body @ 2020-11-24 15:16 Jan Hubicka 2020-11-24 15:50 ` Richard Biener 0 siblings, 1 reply; 9+ messages in thread From: Jan Hubicka @ 2020-11-24 15:16 UTC (permalink / raw) To: gcc-patches, rguenther Hi, at the end of processing function body we loop over basic blocks and free all edges while we do not free the rest. I think this is leftover from time eges was not garbage collected and we was not using ggc_free. It makes more sense to free all associated structures (which is importnat for WPA memory footprint). Bootstrapped/regtested x86_64-linux, OK? Honza * cfg.c (free_block): New function. (clear_edges): Rename to .... (free_cfg): ... this one; also free BBs and vectors. (expunge_block): Update comment. * cfg.h (clear_edges): Rename to ... (free_cfg): ... this one. * cgraph.c (release_function_body): Use free_cfg. diff --git a/gcc/cfg.c b/gcc/cfg.c index de0e71db850..fc78e48d6e1 100644 --- a/gcc/cfg.c +++ b/gcc/cfg.c @@ -25,7 +25,7 @@ along with GCC; see the file COPYING3. If not see Available functionality: - Initialization/deallocation - init_flow, clear_edges + init_flow, free_cfg - Low level basic block manipulation alloc_block, expunge_block - Edge manipulation @@ -83,7 +83,7 @@ init_flow (struct function *the_fun) the_fun->cfg->bb_flags_allocated = BB_ALL_FLAGS; } \f -/* Helper function for remove_edge and clear_edges. Frees edge structure +/* Helper function for remove_edge and free_cffg. Frees edge structure without actually removing it from the pred/succ arrays. */ static void @@ -93,29 +93,41 @@ free_edge (function *fn, edge e) ggc_free (e); } -/* Free the memory associated with the edge structures. */ +/* Free basic block BB. */ + +static void +free_block (basic_block bb) +{ + vec_free (bb->succs); + vec_free (bb->preds); + ggc_free (bb); +} + +/* Free the memory associated with the CFG in FN. */ void -clear_edges (struct function *fn) +free_cfg (struct function *fn) { - basic_block bb; edge e; edge_iterator ei; + basic_block next; - FOR_EACH_BB_FN (bb, fn) + for (basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (fn); bb; bb = next) { + next = bb->next_bb; FOR_EACH_EDGE (e, ei, bb->succs) free_edge (fn, e); - vec_safe_truncate (bb->succs, 0); - vec_safe_truncate (bb->preds, 0); + free_block (bb); } - FOR_EACH_EDGE (e, ei, ENTRY_BLOCK_PTR_FOR_FN (fn)->succs) - free_edge (fn, e); - vec_safe_truncate (EXIT_BLOCK_PTR_FOR_FN (fn)->preds, 0); - vec_safe_truncate (ENTRY_BLOCK_PTR_FOR_FN (fn)->succs, 0); - gcc_assert (!n_edges_for_fn (fn)); + /* Sanity check that dominance tree is freed. */ + gcc_assert (!fn->cfg->x_dom_computed[0] && !fn->cfg->x_dom_computed[1]); + + vec_free (fn->cfg->x_label_to_block_map); + vec_free (basic_block_info_for_fn (fn)); + ggc_free (fn->cfg); + fn->cfg = NULL; } \f /* Allocate memory for basic_block. */ @@ -190,8 +202,8 @@ expunge_block (basic_block b) /* We should be able to ggc_free here, but we are not. The dead SSA_NAMES are left pointing to dead statements that are pointing to dead basic blocks making garbage collector to die. - We should be able to release all dead SSA_NAMES and at the same time we should - clear out BB pointer of dead statements consistently. */ + We should be able to release all dead SSA_NAMES and at the same time we + should clear out BB pointer of dead statements consistently. */ } \f /* Connect E to E->src. */ diff --git a/gcc/cfg.h b/gcc/cfg.h index 93fde6df2bf..a9c8300f173 100644 --- a/gcc/cfg.h +++ b/gcc/cfg.h @@ -82,7 +82,7 @@ struct GTY(()) control_flow_graph { extern void init_flow (function *); -extern void clear_edges (function *); +extern void free_cfg (function *); extern basic_block alloc_block (void); extern void link_block (basic_block, basic_block); extern void unlink_block (basic_block); diff --git a/gcc/cgraph.c b/gcc/cgraph.c index 19dfe2be23b..5c48a1bb92b 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -1811,7 +1811,7 @@ release_function_body (tree decl) gcc_assert (!dom_info_available_p (fn, CDI_DOMINATORS)); gcc_assert (!dom_info_available_p (fn, CDI_POST_DOMINATORS)); delete_tree_cfg_annotations (fn); - clear_edges (fn); + free_cfg (fn); fn->cfg = NULL; } if (fn->value_histograms) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Free more of CFG in release_function_body 2020-11-24 15:16 Free more of CFG in release_function_body Jan Hubicka @ 2020-11-24 15:50 ` Richard Biener 2020-11-25 14:08 ` Jan Hubicka 0 siblings, 1 reply; 9+ messages in thread From: Richard Biener @ 2020-11-24 15:50 UTC (permalink / raw) To: Jan Hubicka; +Cc: gcc-patches On Tue, 24 Nov 2020, Jan Hubicka wrote: > Hi, > at the end of processing function body we loop over basic blocks and > free all edges while we do not free the rest. I think this is leftover > from time eges was not garbage collected and we was not using ggc_free. > It makes more sense to free all associated structures (which is > importnat for WPA memory footprint). > > Bootstrapped/regtested x86_64-linux, OK? OK. > Honza > > * cfg.c (free_block): New function. > (clear_edges): Rename to .... > (free_cfg): ... this one; also free BBs and vectors. > (expunge_block): Update comment. > * cfg.h (clear_edges): Rename to ... > (free_cfg): ... this one. > * cgraph.c (release_function_body): Use free_cfg. > diff --git a/gcc/cfg.c b/gcc/cfg.c > index de0e71db850..fc78e48d6e1 100644 > --- a/gcc/cfg.c > +++ b/gcc/cfg.c > @@ -25,7 +25,7 @@ along with GCC; see the file COPYING3. If not see > > Available functionality: > - Initialization/deallocation > - init_flow, clear_edges > + init_flow, free_cfg > - Low level basic block manipulation > alloc_block, expunge_block > - Edge manipulation > @@ -83,7 +83,7 @@ init_flow (struct function *the_fun) > the_fun->cfg->bb_flags_allocated = BB_ALL_FLAGS; > } > \f > -/* Helper function for remove_edge and clear_edges. Frees edge structure > +/* Helper function for remove_edge and free_cffg. Frees edge structure > without actually removing it from the pred/succ arrays. */ > > static void > @@ -93,29 +93,41 @@ free_edge (function *fn, edge e) > ggc_free (e); > } > > -/* Free the memory associated with the edge structures. */ > +/* Free basic block BB. */ > + > +static void > +free_block (basic_block bb) > +{ > + vec_free (bb->succs); > + vec_free (bb->preds); > + ggc_free (bb); > +} > + > +/* Free the memory associated with the CFG in FN. */ > > void > -clear_edges (struct function *fn) > +free_cfg (struct function *fn) > { > - basic_block bb; > edge e; > edge_iterator ei; > + basic_block next; > > - FOR_EACH_BB_FN (bb, fn) > + for (basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (fn); bb; bb = next) > { > + next = bb->next_bb; > FOR_EACH_EDGE (e, ei, bb->succs) > free_edge (fn, e); > - vec_safe_truncate (bb->succs, 0); > - vec_safe_truncate (bb->preds, 0); > + free_block (bb); > } > > - FOR_EACH_EDGE (e, ei, ENTRY_BLOCK_PTR_FOR_FN (fn)->succs) > - free_edge (fn, e); > - vec_safe_truncate (EXIT_BLOCK_PTR_FOR_FN (fn)->preds, 0); > - vec_safe_truncate (ENTRY_BLOCK_PTR_FOR_FN (fn)->succs, 0); > - > gcc_assert (!n_edges_for_fn (fn)); > + /* Sanity check that dominance tree is freed. */ > + gcc_assert (!fn->cfg->x_dom_computed[0] && !fn->cfg->x_dom_computed[1]); > + > + vec_free (fn->cfg->x_label_to_block_map); > + vec_free (basic_block_info_for_fn (fn)); > + ggc_free (fn->cfg); > + fn->cfg = NULL; > } > \f > /* Allocate memory for basic_block. */ > @@ -190,8 +202,8 @@ expunge_block (basic_block b) > /* We should be able to ggc_free here, but we are not. > The dead SSA_NAMES are left pointing to dead statements that are pointing > to dead basic blocks making garbage collector to die. > - We should be able to release all dead SSA_NAMES and at the same time we should > - clear out BB pointer of dead statements consistently. */ > + We should be able to release all dead SSA_NAMES and at the same time we > + should clear out BB pointer of dead statements consistently. */ > } > \f > /* Connect E to E->src. */ > diff --git a/gcc/cfg.h b/gcc/cfg.h > index 93fde6df2bf..a9c8300f173 100644 > --- a/gcc/cfg.h > +++ b/gcc/cfg.h > @@ -82,7 +82,7 @@ struct GTY(()) control_flow_graph { > > > extern void init_flow (function *); > -extern void clear_edges (function *); > +extern void free_cfg (function *); > extern basic_block alloc_block (void); > extern void link_block (basic_block, basic_block); > extern void unlink_block (basic_block); > diff --git a/gcc/cgraph.c b/gcc/cgraph.c > index 19dfe2be23b..5c48a1bb92b 100644 > --- a/gcc/cgraph.c > +++ b/gcc/cgraph.c > @@ -1811,7 +1811,7 @@ release_function_body (tree decl) > gcc_assert (!dom_info_available_p (fn, CDI_DOMINATORS)); > gcc_assert (!dom_info_available_p (fn, CDI_POST_DOMINATORS)); > delete_tree_cfg_annotations (fn); > - clear_edges (fn); > + free_cfg (fn); > fn->cfg = NULL; > } > if (fn->value_histograms) > -- Richard Biener <rguenther@suse.de> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imend ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Free more of CFG in release_function_body 2020-11-24 15:50 ` Richard Biener @ 2020-11-25 14:08 ` Jan Hubicka 2020-11-25 14:28 ` Richard Biener 0 siblings, 1 reply; 9+ messages in thread From: Jan Hubicka @ 2020-11-25 14:08 UTC (permalink / raw) To: Richard Biener; +Cc: gcc-patches > On Tue, 24 Nov 2020, Jan Hubicka wrote: > > > Hi, > > at the end of processing function body we loop over basic blocks and > > free all edges while we do not free the rest. I think this is leftover > > from time eges was not garbage collected and we was not using ggc_free. > > It makes more sense to free all associated structures (which is > > importnat for WPA memory footprint). > > > > Bootstrapped/regtested x86_64-linux, OK? > > OK. Unforutnately the patch does not surive LTO bootstrap. The problem is that we keep DECL_INITIAL that points to blocks and blocks points to var_decls and these points to SSA_NAMES that points to statements and those points to basic blocks. I wonder with early debug if we sitll need all the logic about keeping DECL_INITIAL. I have commited version that frees everything but the BB themselves and will look into cleaning the pointers in decl_initial. gcc/ChangeLog: 2020-11-25 Jan Hubicka <hubicka@ucw.cz> * cfg.c (free_block): New function. (clear_edges): Rename to .... (free_cfg): ... this one; also free BBs and vectors. (expunge_block): Update comment. * cfg.h (clear_edges): Rename to ... (free_cfg): ... this one. * cgraph.c (release_function_body): Use free_cfg. diff --git a/gcc/cfg.c b/gcc/cfg.c index de0e71db850..529b6ed2105 100644 --- a/gcc/cfg.c +++ b/gcc/cfg.c @@ -25,7 +25,7 @@ along with GCC; see the file COPYING3. If not see Available functionality: - Initialization/deallocation - init_flow, clear_edges + init_flow, free_cfg - Low level basic block manipulation alloc_block, expunge_block - Edge manipulation @@ -83,7 +83,7 @@ init_flow (struct function *the_fun) the_fun->cfg->bb_flags_allocated = BB_ALL_FLAGS; } \f -/* Helper function for remove_edge and clear_edges. Frees edge structure +/* Helper function for remove_edge and free_cffg. Frees edge structure without actually removing it from the pred/succ arrays. */ static void @@ -93,29 +93,44 @@ free_edge (function *fn, edge e) ggc_free (e); } -/* Free the memory associated with the edge structures. */ +/* Free basic block BB. */ + +static void +free_block (basic_block bb) +{ + vec_free (bb->succs); + bb->succs = NULL; + vec_free (bb->preds); + bb->preds = NULL; + /* Do not free BB itself yet since we leak pointers to dead statements + that points to dead basic blocks. */ +} + +/* Free the memory associated with the CFG in FN. */ void -clear_edges (struct function *fn) +free_cfg (struct function *fn) { - basic_block bb; edge e; edge_iterator ei; + basic_block next; - FOR_EACH_BB_FN (bb, fn) + for (basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (fn); bb; bb = next) { + next = bb->next_bb; FOR_EACH_EDGE (e, ei, bb->succs) free_edge (fn, e); - vec_safe_truncate (bb->succs, 0); - vec_safe_truncate (bb->preds, 0); + free_block (bb); } - FOR_EACH_EDGE (e, ei, ENTRY_BLOCK_PTR_FOR_FN (fn)->succs) - free_edge (fn, e); - vec_safe_truncate (EXIT_BLOCK_PTR_FOR_FN (fn)->preds, 0); - vec_safe_truncate (ENTRY_BLOCK_PTR_FOR_FN (fn)->succs, 0); - gcc_assert (!n_edges_for_fn (fn)); + /* Sanity check that dominance tree is freed. */ + gcc_assert (!fn->cfg->x_dom_computed[0] && !fn->cfg->x_dom_computed[1]); + + vec_free (fn->cfg->x_label_to_block_map); + vec_free (basic_block_info_for_fn (fn)); + ggc_free (fn->cfg); + fn->cfg = NULL; } \f /* Allocate memory for basic_block. */ @@ -190,8 +205,8 @@ expunge_block (basic_block b) /* We should be able to ggc_free here, but we are not. The dead SSA_NAMES are left pointing to dead statements that are pointing to dead basic blocks making garbage collector to die. - We should be able to release all dead SSA_NAMES and at the same time we should - clear out BB pointer of dead statements consistently. */ + We should be able to release all dead SSA_NAMES and at the same time we + should clear out BB pointer of dead statements consistently. */ } \f /* Connect E to E->src. */ diff --git a/gcc/cfg.h b/gcc/cfg.h index 93fde6df2bf..a9c8300f173 100644 --- a/gcc/cfg.h +++ b/gcc/cfg.h @@ -82,7 +82,7 @@ struct GTY(()) control_flow_graph { extern void init_flow (function *); -extern void clear_edges (function *); +extern void free_cfg (function *); extern basic_block alloc_block (void); extern void link_block (basic_block, basic_block); extern void unlink_block (basic_block); diff --git a/gcc/cgraph.c b/gcc/cgraph.c index 19dfe2be23b..dbde8aaaba1 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -1811,7 +1811,7 @@ release_function_body (tree decl) gcc_assert (!dom_info_available_p (fn, CDI_DOMINATORS)); gcc_assert (!dom_info_available_p (fn, CDI_POST_DOMINATORS)); delete_tree_cfg_annotations (fn); - clear_edges (fn); + free_cfg (fn); fn->cfg = NULL; } if (fn->value_histograms) diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c index a4832b75436..5f84f7d467f 100644 --- a/gcc/tree-ssa-structalias.c +++ b/gcc/tree-ssa-structalias.c @@ -4253,12 +4253,20 @@ handle_pure_call (gcall *stmt, vec<ce_s> *results) for (i = 0; i < gimple_call_num_args (stmt); ++i) { tree arg = gimple_call_arg (stmt, i); + int flags = gimple_call_arg_flags (stmt, i); + + if (flags & EAF_UNUSED) + continue; + if (!uses) - { - uses = get_call_use_vi (stmt); - make_any_offset_constraints (uses); - make_transitive_closure_constraints (uses); - } + uses = get_call_use_vi (stmt); + varinfo_t tem = new_var_info (NULL_TREE, "callarg", true); + tem->is_reg_var = true; + make_constraint_to (tem->id, arg); + make_any_offset_constraints (tem); + if (!(flags & EAF_DIRECT)) + make_transitive_closure_constraints (tem); + make_copy_constraint (uses, tem->id); make_constraint_to (uses->id, arg); } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Free more of CFG in release_function_body 2020-11-25 14:08 ` Jan Hubicka @ 2020-11-25 14:28 ` Richard Biener 2020-11-25 14:30 ` Jan Hubicka 0 siblings, 1 reply; 9+ messages in thread From: Richard Biener @ 2020-11-25 14:28 UTC (permalink / raw) To: Jan Hubicka; +Cc: Richard Biener, GCC Patches On Wed, Nov 25, 2020 at 3:11 PM Jan Hubicka <hubicka@ucw.cz> wrote: > > > On Tue, 24 Nov 2020, Jan Hubicka wrote: > > > > > Hi, > > > at the end of processing function body we loop over basic blocks and > > > free all edges while we do not free the rest. I think this is leftover > > > from time eges was not garbage collected and we was not using ggc_free. > > > It makes more sense to free all associated structures (which is > > > importnat for WPA memory footprint). > > > > > > Bootstrapped/regtested x86_64-linux, OK? > > > > OK. > > Unforutnately the patch does not surive LTO bootstrap. The problem is > that we keep DECL_INITIAL that points to blocks and blocks points to > var_decls and these points to SSA_NAMES that points to statements and > those points to basic blocks. VAR_DECLs point to SSA_NAMEs? It's the other way around. We for sure free SSA_NAMEs (well, maybe not explicitely with ggc_free). Richard. > > I wonder with early debug if we sitll need all the logic about keeping > DECL_INITIAL. > > I have commited version that frees everything but the BB themselves and > will look into cleaning the pointers in decl_initial. > > gcc/ChangeLog: > > 2020-11-25 Jan Hubicka <hubicka@ucw.cz> > > * cfg.c (free_block): New function. > (clear_edges): Rename to .... > (free_cfg): ... this one; also free BBs and vectors. > (expunge_block): Update comment. > * cfg.h (clear_edges): Rename to ... > (free_cfg): ... this one. > * cgraph.c (release_function_body): Use free_cfg. > > diff --git a/gcc/cfg.c b/gcc/cfg.c > index de0e71db850..529b6ed2105 100644 > --- a/gcc/cfg.c > +++ b/gcc/cfg.c > @@ -25,7 +25,7 @@ along with GCC; see the file COPYING3. If not see > > Available functionality: > - Initialization/deallocation > - init_flow, clear_edges > + init_flow, free_cfg > - Low level basic block manipulation > alloc_block, expunge_block > - Edge manipulation > @@ -83,7 +83,7 @@ init_flow (struct function *the_fun) > the_fun->cfg->bb_flags_allocated = BB_ALL_FLAGS; > } > > -/* Helper function for remove_edge and clear_edges. Frees edge structure > +/* Helper function for remove_edge and free_cffg. Frees edge structure > without actually removing it from the pred/succ arrays. */ > > static void > @@ -93,29 +93,44 @@ free_edge (function *fn, edge e) > ggc_free (e); > } > > -/* Free the memory associated with the edge structures. */ > +/* Free basic block BB. */ > + > +static void > +free_block (basic_block bb) > +{ > + vec_free (bb->succs); > + bb->succs = NULL; > + vec_free (bb->preds); > + bb->preds = NULL; > + /* Do not free BB itself yet since we leak pointers to dead statements > + that points to dead basic blocks. */ > +} > + > +/* Free the memory associated with the CFG in FN. */ > > void > -clear_edges (struct function *fn) > +free_cfg (struct function *fn) > { > - basic_block bb; > edge e; > edge_iterator ei; > + basic_block next; > > - FOR_EACH_BB_FN (bb, fn) > + for (basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (fn); bb; bb = next) > { > + next = bb->next_bb; > FOR_EACH_EDGE (e, ei, bb->succs) > free_edge (fn, e); > - vec_safe_truncate (bb->succs, 0); > - vec_safe_truncate (bb->preds, 0); > + free_block (bb); > } > > - FOR_EACH_EDGE (e, ei, ENTRY_BLOCK_PTR_FOR_FN (fn)->succs) > - free_edge (fn, e); > - vec_safe_truncate (EXIT_BLOCK_PTR_FOR_FN (fn)->preds, 0); > - vec_safe_truncate (ENTRY_BLOCK_PTR_FOR_FN (fn)->succs, 0); > - > gcc_assert (!n_edges_for_fn (fn)); > + /* Sanity check that dominance tree is freed. */ > + gcc_assert (!fn->cfg->x_dom_computed[0] && !fn->cfg->x_dom_computed[1]); > + > + vec_free (fn->cfg->x_label_to_block_map); > + vec_free (basic_block_info_for_fn (fn)); > + ggc_free (fn->cfg); > + fn->cfg = NULL; > } > > /* Allocate memory for basic_block. */ > @@ -190,8 +205,8 @@ expunge_block (basic_block b) > /* We should be able to ggc_free here, but we are not. > The dead SSA_NAMES are left pointing to dead statements that are pointing > to dead basic blocks making garbage collector to die. > - We should be able to release all dead SSA_NAMES and at the same time we should > - clear out BB pointer of dead statements consistently. */ > + We should be able to release all dead SSA_NAMES and at the same time we > + should clear out BB pointer of dead statements consistently. */ > } > > /* Connect E to E->src. */ > diff --git a/gcc/cfg.h b/gcc/cfg.h > index 93fde6df2bf..a9c8300f173 100644 > --- a/gcc/cfg.h > +++ b/gcc/cfg.h > @@ -82,7 +82,7 @@ struct GTY(()) control_flow_graph { > > > extern void init_flow (function *); > -extern void clear_edges (function *); > +extern void free_cfg (function *); > extern basic_block alloc_block (void); > extern void link_block (basic_block, basic_block); > extern void unlink_block (basic_block); > diff --git a/gcc/cgraph.c b/gcc/cgraph.c > index 19dfe2be23b..dbde8aaaba1 100644 > --- a/gcc/cgraph.c > +++ b/gcc/cgraph.c > @@ -1811,7 +1811,7 @@ release_function_body (tree decl) > gcc_assert (!dom_info_available_p (fn, CDI_DOMINATORS)); > gcc_assert (!dom_info_available_p (fn, CDI_POST_DOMINATORS)); > delete_tree_cfg_annotations (fn); > - clear_edges (fn); > + free_cfg (fn); > fn->cfg = NULL; > } > if (fn->value_histograms) > diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c > index a4832b75436..5f84f7d467f 100644 > --- a/gcc/tree-ssa-structalias.c > +++ b/gcc/tree-ssa-structalias.c > @@ -4253,12 +4253,20 @@ handle_pure_call (gcall *stmt, vec<ce_s> *results) > for (i = 0; i < gimple_call_num_args (stmt); ++i) > { > tree arg = gimple_call_arg (stmt, i); > + int flags = gimple_call_arg_flags (stmt, i); > + > + if (flags & EAF_UNUSED) > + continue; > + > if (!uses) > - { > - uses = get_call_use_vi (stmt); > - make_any_offset_constraints (uses); > - make_transitive_closure_constraints (uses); > - } > + uses = get_call_use_vi (stmt); > + varinfo_t tem = new_var_info (NULL_TREE, "callarg", true); > + tem->is_reg_var = true; > + make_constraint_to (tem->id, arg); > + make_any_offset_constraints (tem); > + if (!(flags & EAF_DIRECT)) > + make_transitive_closure_constraints (tem); > + make_copy_constraint (uses, tem->id); > make_constraint_to (uses->id, arg); > } > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Free more of CFG in release_function_body 2020-11-25 14:28 ` Richard Biener @ 2020-11-25 14:30 ` Jan Hubicka 2020-11-25 14:32 ` Richard Biener 2020-11-27 13:26 ` [C++ patch] " Jan Hubicka 0 siblings, 2 replies; 9+ messages in thread From: Jan Hubicka @ 2020-11-25 14:30 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches, Richard Biener > On Wed, Nov 25, 2020 at 3:11 PM Jan Hubicka <hubicka@ucw.cz> wrote: > > > > > On Tue, 24 Nov 2020, Jan Hubicka wrote: > > > > > > > Hi, > > > > at the end of processing function body we loop over basic blocks and > > > > free all edges while we do not free the rest. I think this is leftover > > > > from time eges was not garbage collected and we was not using ggc_free. > > > > It makes more sense to free all associated structures (which is > > > > importnat for WPA memory footprint). > > > > > > > > Bootstrapped/regtested x86_64-linux, OK? > > > > > > OK. > > > > Unforutnately the patch does not surive LTO bootstrap. The problem is > > that we keep DECL_INITIAL that points to blocks and blocks points to > > var_decls and these points to SSA_NAMES that points to statements and > > those points to basic blocks. > > VAR_DECLs point to SSA_NAMEs? It's the other way around. We for sure > free SSA_NAMEs (well, maybe not explicitely with ggc_free). I am going to debug this more carefully now. I think it was VAR_DECL with variadic type pointing to SSA_NAME. Should be easy to reduct with gcac compiler. Honza ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Free more of CFG in release_function_body 2020-11-25 14:30 ` Jan Hubicka @ 2020-11-25 14:32 ` Richard Biener 2020-11-27 13:26 ` [C++ patch] " Jan Hubicka 1 sibling, 0 replies; 9+ messages in thread From: Richard Biener @ 2020-11-25 14:32 UTC (permalink / raw) To: Jan Hubicka; +Cc: Richard Biener, GCC Patches On Wed, 25 Nov 2020, Jan Hubicka wrote: > > On Wed, Nov 25, 2020 at 3:11 PM Jan Hubicka <hubicka@ucw.cz> wrote: > > > > > > > On Tue, 24 Nov 2020, Jan Hubicka wrote: > > > > > > > > > Hi, > > > > > at the end of processing function body we loop over basic blocks and > > > > > free all edges while we do not free the rest. I think this is leftover > > > > > from time eges was not garbage collected and we was not using ggc_free. > > > > > It makes more sense to free all associated structures (which is > > > > > importnat for WPA memory footprint). > > > > > > > > > > Bootstrapped/regtested x86_64-linux, OK? > > > > > > > > OK. > > > > > > Unforutnately the patch does not surive LTO bootstrap. The problem is > > > that we keep DECL_INITIAL that points to blocks and blocks points to > > > var_decls and these points to SSA_NAMES that points to statements and > > > those points to basic blocks. > > > > VAR_DECLs point to SSA_NAMEs? It's the other way around. We for sure > > free SSA_NAMEs (well, maybe not explicitely with ggc_free). > > I am going to debug this more carefully now. I think it was VAR_DECL > with variadic type pointing to SSA_NAME. Should be easy to reduct with > gcac compiler. Possibly another case of a missing DECL_EXPR then. In those cases SSA names leak into TYPE_MIN/MAX_VALUE or TYPE/DECL_SIZE during gimplification. But those are frontend bugs (there are plenty reported). Richard. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [C++ patch] Re: Free more of CFG in release_function_body 2020-11-25 14:30 ` Jan Hubicka 2020-11-25 14:32 ` Richard Biener @ 2020-11-27 13:26 ` Jan Hubicka 2020-11-30 1:24 ` Jeff Law 2020-11-30 22:28 ` Jason Merrill 1 sibling, 2 replies; 9+ messages in thread From: Jan Hubicka @ 2020-11-27 13:26 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches, Richard Biener, jason > > On Wed, Nov 25, 2020 at 3:11 PM Jan Hubicka <hubicka@ucw.cz> wrote: > > > > > > > On Tue, 24 Nov 2020, Jan Hubicka wrote: > > > > > > > > > Hi, > > > > > at the end of processing function body we loop over basic blocks and > > > > > free all edges while we do not free the rest. I think this is leftover > > > > > from time eges was not garbage collected and we was not using ggc_free. > > > > > It makes more sense to free all associated structures (which is > > > > > importnat for WPA memory footprint). > > > > > > > > > > Bootstrapped/regtested x86_64-linux, OK? > > > > > > > > OK. > > > > > > Unforutnately the patch does not surive LTO bootstrap. The problem is > > > that we keep DECL_INITIAL that points to blocks and blocks points to > > > var_decls and these points to SSA_NAMES that points to statements and > > > those points to basic blocks. > > > > VAR_DECLs point to SSA_NAMEs? It's the other way around. We for sure > > free SSA_NAMEs (well, maybe not explicitely with ggc_free). > > I am going to debug this more carefully now. I think it was VAR_DECL > with variadic type pointing to SSA_NAME. Should be easy to reduct with > gcac compiler. Hi, it turns out that the pointers to statements leaks through saved scopes in C++ FE. Scopes seems to point to internal blocks of functions even after we finish their compiling. This patch adds code to free pointers. I tried to clear saved_blocks but it breaks since C++ finalization uses them, but it does not look into previous class levels. Patch lto-bootstraps/regtestes x86_64-linux with all languages. OK? Honza * cfg.c (free_block): Call ggc_free on BB itself. * cp-tre.eh (cp_tree_c_finish_parsing): Declare. * semantics.c (finish_translation_unit): Call finish_parsing * tree.c (cp_tree_c_finish_parsing): New function. diff --git a/gcc/cfg.c b/gcc/cfg.c index 529b6ed2105..e8bd1456c9f 100644 --- a/gcc/cfg.c +++ b/gcc/cfg.c @@ -102,8 +102,7 @@ free_block (basic_block bb) bb->succs = NULL; vec_free (bb->preds); bb->preds = NULL; - /* Do not free BB itself yet since we leak pointers to dead statements - that points to dead basic blocks. */ + ggc_free (bb); } /* Free the memory associated with the CFG in FN. */ diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 021de76e142..665d171d9b0 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7986,6 +7986,8 @@ struct uid_sensitive_constexpr_evaluation_checker bool evaluation_restricted_p () const; }; +void cp_tree_c_finish_parsing (); + /* In cp-ubsan.c */ extern void cp_ubsan_maybe_instrument_member_call (tree); extern void cp_ubsan_instrument_member_accesses (tree *); diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 5ff70ff4844..e9d17c21985 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -3094,6 +3094,7 @@ finish_translation_unit (void) "%<#pragma omp end declare target%>"); scope_chain->omp_declare_target_attribute = 0; } + cp_tree_c_finish_parsing (); } /* Finish a template type parameter, specified as AGGR IDENTIFIER. diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c index 28e591086b3..e63d383c0a3 100644 --- a/gcc/cp/tree.c +++ b/gcc/cp/tree.c @@ -5844,6 +5844,19 @@ maybe_warn_zero_as_null_pointer_constant (tree expr, location_t loc) return false; } \f +/* Release memory we no longer need after parsing. */ +void +cp_tree_c_finish_parsing () +{ + saved_scope *chain = scope_chain; + while (chain) + { + chain->x_previous_class_level = NULL; + chain = chain->prev; + } + deleted_copy_types = NULL; +} +\f #if defined ENABLE_TREE_CHECKING && (GCC_VERSION >= 2007) /* Complain that some language-specific thing hanging off a tree node has been accessed improperly. */ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [C++ patch] Re: Free more of CFG in release_function_body 2020-11-27 13:26 ` [C++ patch] " Jan Hubicka @ 2020-11-30 1:24 ` Jeff Law 2020-11-30 22:28 ` Jason Merrill 1 sibling, 0 replies; 9+ messages in thread From: Jeff Law @ 2020-11-30 1:24 UTC (permalink / raw) To: Jan Hubicka, Richard Biener; +Cc: Richard Biener, GCC Patches On 11/27/20 6:26 AM, Jan Hubicka wrote: >>> On Wed, Nov 25, 2020 at 3:11 PM Jan Hubicka <hubicka@ucw.cz> wrote: >>>>> On Tue, 24 Nov 2020, Jan Hubicka wrote: >>>>> >>>>>> Hi, >>>>>> at the end of processing function body we loop over basic blocks and >>>>>> free all edges while we do not free the rest. I think this is leftover >>>>>> from time eges was not garbage collected and we was not using ggc_free. >>>>>> It makes more sense to free all associated structures (which is >>>>>> importnat for WPA memory footprint). >>>>>> >>>>>> Bootstrapped/regtested x86_64-linux, OK? >>>>> OK. >>>> Unforutnately the patch does not surive LTO bootstrap. The problem is >>>> that we keep DECL_INITIAL that points to blocks and blocks points to >>>> var_decls and these points to SSA_NAMES that points to statements and >>>> those points to basic blocks. >>> VAR_DECLs point to SSA_NAMEs? It's the other way around. We for sure >>> free SSA_NAMEs (well, maybe not explicitely with ggc_free). >> I am going to debug this more carefully now. I think it was VAR_DECL >> with variadic type pointing to SSA_NAME. Should be easy to reduct with >> gcac compiler. > Hi, > it turns out that the pointers to statements leaks through saved scopes > in C++ FE. Scopes seems to point to internal blocks of functions even > after we finish their compiling. > > This patch adds code to free pointers. I tried to clear saved_blocks > but it breaks since C++ finalization uses them, but it does not look > into previous class levels. > > Patch lto-bootstraps/regtestes x86_64-linux with all languages. OK? > > Honza > > * cfg.c (free_block): Call ggc_free on BB itself. I'll ACK this bit. The C++ team should own the rest, while it looks reasonable to me, there may be some reason I'm not aware of that they're keeping that stuff around. Jeff ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [C++ patch] Re: Free more of CFG in release_function_body 2020-11-27 13:26 ` [C++ patch] " Jan Hubicka 2020-11-30 1:24 ` Jeff Law @ 2020-11-30 22:28 ` Jason Merrill 1 sibling, 0 replies; 9+ messages in thread From: Jason Merrill @ 2020-11-30 22:28 UTC (permalink / raw) To: Jan Hubicka, Richard Biener; +Cc: GCC Patches, Richard Biener On 11/27/20 8:26 AM, Jan Hubicka wrote: >>> On Wed, Nov 25, 2020 at 3:11 PM Jan Hubicka <hubicka@ucw.cz> wrote: >>>> >>>>> On Tue, 24 Nov 2020, Jan Hubicka wrote: >>>>> >>>>>> Hi, >>>>>> at the end of processing function body we loop over basic blocks and >>>>>> free all edges while we do not free the rest. I think this is leftover >>>>>> from time eges was not garbage collected and we was not using ggc_free. >>>>>> It makes more sense to free all associated structures (which is >>>>>> importnat for WPA memory footprint). >>>>>> >>>>>> Bootstrapped/regtested x86_64-linux, OK? >>>>> >>>>> OK. >>>> >>>> Unforutnately the patch does not surive LTO bootstrap. The problem is >>>> that we keep DECL_INITIAL that points to blocks and blocks points to >>>> var_decls and these points to SSA_NAMES that points to statements and >>>> those points to basic blocks. >>> >>> VAR_DECLs point to SSA_NAMEs? It's the other way around. We for sure >>> free SSA_NAMEs (well, maybe not explicitely with ggc_free). >> >> I am going to debug this more carefully now. I think it was VAR_DECL >> with variadic type pointing to SSA_NAME. Should be easy to reduct with >> gcac compiler. > > Hi, > it turns out that the pointers to statements leaks through saved scopes > in C++ FE. Scopes seems to point to internal blocks of functions even > after we finish their compiling. > > This patch adds code to free pointers. I tried to clear saved_blocks > but it breaks since C++ finalization uses them, but it does not look > into previous class levels. > > Patch lto-bootstraps/regtestes x86_64-linux with all languages. OK? > > Honza > > * cfg.c (free_block): Call ggc_free on BB itself. > > * cp-tre.eh (cp_tree_c_finish_parsing): Declare. > * semantics.c (finish_translation_unit): Call finish_parsing > * tree.c (cp_tree_c_finish_parsing): New function. > diff --git a/gcc/cfg.c b/gcc/cfg.c > index 529b6ed2105..e8bd1456c9f 100644 > --- a/gcc/cfg.c > +++ b/gcc/cfg.c > @@ -102,8 +102,7 @@ free_block (basic_block bb) > bb->succs = NULL; > vec_free (bb->preds); > bb->preds = NULL; > - /* Do not free BB itself yet since we leak pointers to dead statements > - that points to dead basic blocks. */ > + ggc_free (bb); > } > > /* Free the memory associated with the CFG in FN. */ > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index 021de76e142..665d171d9b0 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -7986,6 +7986,8 @@ struct uid_sensitive_constexpr_evaluation_checker > bool evaluation_restricted_p () const; > }; > > +void cp_tree_c_finish_parsing (); > + > /* In cp-ubsan.c */ > extern void cp_ubsan_maybe_instrument_member_call (tree); > extern void cp_ubsan_instrument_member_accesses (tree *); > diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c > index 5ff70ff4844..e9d17c21985 100644 > --- a/gcc/cp/semantics.c > +++ b/gcc/cp/semantics.c > @@ -3094,6 +3094,7 @@ finish_translation_unit (void) > "%<#pragma omp end declare target%>"); > scope_chain->omp_declare_target_attribute = 0; > } > + cp_tree_c_finish_parsing (); This is too soon for this call; it should be from c_parse_final_cleanups by the call to fini_constexpr, so that it follows template instantiation at EOF. > } > > /* Finish a template type parameter, specified as AGGR IDENTIFIER. > diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c > index 28e591086b3..e63d383c0a3 100644 > --- a/gcc/cp/tree.c > +++ b/gcc/cp/tree.c > @@ -5844,6 +5844,19 @@ maybe_warn_zero_as_null_pointer_constant (tree expr, location_t loc) > return false; > } > \f > +/* Release memory we no longer need after parsing. */ > +void > +cp_tree_c_finish_parsing () > +{ > + saved_scope *chain = scope_chain; > + while (chain) > + { > + chain->x_previous_class_level = NULL; > + chain = chain->prev; > + } This can just be invalidate_class_lookup_cache (). scope_chain->prev will always be NULL at this point. OK with those changes. > + deleted_copy_types = NULL; > +} > +\f > #if defined ENABLE_TREE_CHECKING && (GCC_VERSION >= 2007) > /* Complain that some language-specific thing hanging off a tree > node has been accessed improperly. */ ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-11-30 22:28 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-24 15:16 Free more of CFG in release_function_body Jan Hubicka 2020-11-24 15:50 ` Richard Biener 2020-11-25 14:08 ` Jan Hubicka 2020-11-25 14:28 ` Richard Biener 2020-11-25 14:30 ` Jan Hubicka 2020-11-25 14:32 ` Richard Biener 2020-11-27 13:26 ` [C++ patch] " Jan Hubicka 2020-11-30 1:24 ` Jeff Law 2020-11-30 22:28 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).