This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Cleanup do_per_function, require less push/pop_cfun
- From: Richard Biener <rguenther at suse dot de>
- To: gcc-patches at gcc dot gnu dot org
- Cc: Jan Hubicka <hubicka at ucw dot cz>
- Date: Mon, 28 Apr 2014 13:25:54 +0200 (CEST)
- Subject: Re: [PATCH] Cleanup do_per_function, require less push/pop_cfun
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot LSU dot 2 dot 11 dot 1404231636150 dot 18709 at zhemvz dot fhfr dot qr>
On Wed, 23 Apr 2014, Richard Biener wrote:
>
> This avoids all the complex work on simple things like
> clear_last_verified. It also makes eventually inlining all
> calls (for example the one with the small IPA pass hack)
> less code-duplicating.
>
> I had to remove the asserts in favor of frees of DOM info in
> release_function_body as the old code released DOM info in
> various odd places.
>
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>
> Honza, does this look ok to you?
I've heard nothing back so I assume it's ok and committed it.
Richard.
> Thanks,
> Richard.
>
> 2014-04-23 Richard Biener <rguenther@suse.de>
>
> * tree-pass.h (execute_pass_list): Adjust prototype.
> * passes.c (pass_manager::execute_early_local_passes):
> Adjust.
> (do_per_function): Change callback signature, push all actual
> work to the callbals.
> (do_per_function_toporder): Likewise.
> (execute_function_dump): Adjust.
> (execute_function_todo): Likewise.
> (clear_last_verified): Likewise.
> (verify_curr_properties): Likewise.
> (update_properties_after_pass): Likewise.
> (apply_ipa_transforms): Likewise.
> (execute_pass_list_1): Split out from ...
> (execute_pass_list): ... here. Adjust.
> (execute_ipa_pass_list): Likewise.
> * cgraphunit.c (cgraph_add_new_function): Adjust.
> (analyze_function): Likewise.
> (expand_function): Likewise.
> * cgraph.c (release_function_body): Free dominance info
> here instead of asserting it was magically freed elsewhere.
>
> Index: gcc/tree-pass.h
> ===================================================================
> *** gcc/tree-pass.h.orig 2014-04-23 14:55:25.640624814 +0200
> --- gcc/tree-pass.h 2014-04-23 15:40:56.443436802 +0200
> *************** extern gimple_opt_pass *make_pass_conver
> *** 587,593 ****
> extern opt_pass *current_pass;
>
> extern bool execute_one_pass (opt_pass *);
> ! extern void execute_pass_list (opt_pass *);
> extern void execute_ipa_pass_list (opt_pass *);
> extern void execute_ipa_summary_passes (ipa_opt_pass_d *);
> extern void execute_all_ipa_transforms (void);
> --- 587,593 ----
> extern opt_pass *current_pass;
>
> extern bool execute_one_pass (opt_pass *);
> ! extern void execute_pass_list (function *, opt_pass *);
> extern void execute_ipa_pass_list (opt_pass *);
> extern void execute_ipa_summary_passes (ipa_opt_pass_d *);
> extern void execute_all_ipa_transforms (void);
> *************** extern bool function_called_by_processed
> *** 615,621 ****
> extern bool first_pass_instance;
>
> /* Declare for plugins. */
> ! extern void do_per_function_toporder (void (*) (void *), void *);
>
> extern void disable_pass (const char *);
> extern void enable_pass (const char *);
> --- 615,621 ----
> extern bool first_pass_instance;
>
> /* Declare for plugins. */
> ! extern void do_per_function_toporder (void (*) (function *, void *), void *);
>
> extern void disable_pass (const char *);
> extern void enable_pass (const char *);
> Index: gcc/passes.c
> ===================================================================
> *** gcc/passes.c.orig 2014-04-23 14:55:25.642624814 +0200
> --- gcc/passes.c 2014-04-23 15:41:02.414436391 +0200
> *************** opt_pass::opt_pass (const pass_data &dat
> *** 132,138 ****
> void
> pass_manager::execute_early_local_passes ()
> {
> ! execute_pass_list (pass_early_local_passes_1->sub);
> }
>
> unsigned int
> --- 132,138 ----
> void
> pass_manager::execute_early_local_passes ()
> {
> ! execute_pass_list (cfun, pass_early_local_passes_1->sub);
> }
>
> unsigned int
> *************** pass_manager::pass_manager (context *ctx
> *** 1498,1524 ****
> call CALLBACK on the current function. */
>
> static void
> ! do_per_function (void (*callback) (void *data), void *data)
> {
> if (current_function_decl)
> ! callback (data);
> else
> {
> struct cgraph_node *node;
> FOR_EACH_DEFINED_FUNCTION (node)
> if (node->analyzed && gimple_has_body_p (node->decl)
> && (!node->clone_of || node->decl != node->clone_of->decl))
> ! {
> ! push_cfun (DECL_STRUCT_FUNCTION (node->decl));
> ! callback (data);
> ! if (!flag_wpa)
> ! {
> ! free_dominance_info (CDI_DOMINATORS);
> ! free_dominance_info (CDI_POST_DOMINATORS);
> ! }
> ! pop_cfun ();
> ! ggc_collect ();
> ! }
> }
> }
>
> --- 1498,1514 ----
> call CALLBACK on the current function. */
>
> static void
> ! do_per_function (void (*callback) (function *, void *data), void *data)
> {
> if (current_function_decl)
> ! callback (cfun, data);
> else
> {
> struct cgraph_node *node;
> FOR_EACH_DEFINED_FUNCTION (node)
> if (node->analyzed && gimple_has_body_p (node->decl)
> && (!node->clone_of || node->decl != node->clone_of->decl))
> ! callback (DECL_STRUCT_FUNCTION (node->decl), data);
> }
> }
>
> *************** static GTY ((length ("nnodes"))) cgraph_
> *** 1533,1544 ****
> call CALLBACK on the current function.
> This function is global so that plugins can use it. */
> void
> ! do_per_function_toporder (void (*callback) (void *data), void *data)
> {
> int i;
>
> if (current_function_decl)
> ! callback (data);
> else
> {
> gcc_assert (!order);
> --- 1523,1534 ----
> call CALLBACK on the current function.
> This function is global so that plugins can use it. */
> void
> ! do_per_function_toporder (void (*callback) (function *, void *data), void *data)
> {
> int i;
>
> if (current_function_decl)
> ! callback (cfun, data);
> else
> {
> gcc_assert (!order);
> *************** do_per_function_toporder (void (*callbac
> *** 1554,1568 ****
> order[i] = NULL;
> node->process = 0;
> if (cgraph_function_with_gimple_body_p (node))
> ! {
> ! cgraph_get_body (node);
> ! push_cfun (DECL_STRUCT_FUNCTION (node->decl));
> ! callback (data);
> ! free_dominance_info (CDI_DOMINATORS);
> ! free_dominance_info (CDI_POST_DOMINATORS);
> ! pop_cfun ();
> ! ggc_collect ();
> ! }
> }
> }
> ggc_free (order);
> --- 1544,1550 ----
> order[i] = NULL;
> node->process = 0;
> if (cgraph_function_with_gimple_body_p (node))
> ! callback (DECL_STRUCT_FUNCTION (node->decl), data);
> }
> }
> ggc_free (order);
> *************** do_per_function_toporder (void (*callbac
> *** 1573,1586 ****
> /* Helper function to perform function body dump. */
>
> static void
> ! execute_function_dump (void *data)
> {
> opt_pass *pass = (opt_pass *)data;
>
> ! if (dump_file && current_function_decl)
> {
> ! if (cfun->curr_properties & PROP_trees)
> ! dump_function_to_file (current_function_decl, dump_file, dump_flags);
> else
> print_rtl_with_bb (dump_file, get_insns (), dump_flags);
>
> --- 1555,1570 ----
> /* Helper function to perform function body dump. */
>
> static void
> ! execute_function_dump (function *fn, void *data)
> {
> opt_pass *pass = (opt_pass *)data;
>
> ! if (dump_file)
> {
> ! push_cfun (fn);
> !
> ! if (fn->curr_properties & PROP_trees)
> ! dump_function_to_file (fn->decl, dump_file, dump_flags);
> else
> print_rtl_with_bb (dump_file, get_insns (), dump_flags);
>
> *************** execute_function_dump (void *data)
> *** 1588,1594 ****
> close the file before aborting. */
> fflush (dump_file);
>
> ! if ((cfun->curr_properties & PROP_cfg)
> && (dump_flags & TDF_GRAPH))
> {
> if (!pass->graph_dump_initialized)
> --- 1572,1578 ----
> close the file before aborting. */
> fflush (dump_file);
>
> ! if ((fn->curr_properties & PROP_cfg)
> && (dump_flags & TDF_GRAPH))
> {
> if (!pass->graph_dump_initialized)
> *************** execute_function_dump (void *data)
> *** 1596,1603 ****
> clean_graph_dump_file (dump_file_name);
> pass->graph_dump_initialized = true;
> }
> ! print_graph_cfg (dump_file_name, cfun);
> }
> }
> }
>
> --- 1580,1589 ----
> clean_graph_dump_file (dump_file_name);
> pass->graph_dump_initialized = true;
> }
> ! print_graph_cfg (dump_file_name, fn);
> }
> +
> + pop_cfun ();
> }
> }
>
> *************** pass_manager::dump_profile_report () con
> *** 1728,1740 ****
> /* Perform all TODO actions that ought to be done on each function. */
>
> static void
> ! execute_function_todo (void *data)
> {
> unsigned int flags = (size_t)data;
> ! flags &= ~cfun->last_verified;
> if (!flags)
> return;
>
> /* Always cleanup the CFG before trying to update SSA. */
> if (flags & TODO_cleanup_cfg)
> {
> --- 1714,1728 ----
> /* Perform all TODO actions that ought to be done on each function. */
>
> static void
> ! execute_function_todo (function *fn, void *data)
> {
> unsigned int flags = (size_t)data;
> ! flags &= ~fn->last_verified;
> if (!flags)
> return;
>
> + push_cfun (fn);
> +
> /* Always cleanup the CFG before trying to update SSA. */
> if (flags & TODO_cleanup_cfg)
> {
> *************** execute_function_todo (void *data)
> *** 1774,1780 ****
>
> /* If we've seen errors do not bother running any verifiers. */
> if (seen_error ())
> ! return;
>
> #if defined ENABLE_CHECKING
> if (flags & TODO_verify_ssa
> --- 1762,1771 ----
>
> /* If we've seen errors do not bother running any verifiers. */
> if (seen_error ())
> ! {
> ! pop_cfun ();
> ! return;
> ! }
>
> #if defined ENABLE_CHECKING
> if (flags & TODO_verify_ssa
> *************** execute_function_todo (void *data)
> *** 1793,1799 ****
> verify_rtl_sharing ();
> #endif
>
> ! cfun->last_verified = flags & TODO_verify_all;
> }
>
> /* Perform all TODO actions. */
> --- 1784,1792 ----
> verify_rtl_sharing ();
> #endif
>
> ! fn->last_verified = flags & TODO_verify_all;
> !
> ! pop_cfun ();
> }
>
> /* Perform all TODO actions. */
> *************** verify_interpass_invariants (void)
> *** 1855,1863 ****
> /* Clear the last verified flag. */
>
> static void
> ! clear_last_verified (void *data ATTRIBUTE_UNUSED)
> {
> ! cfun->last_verified = 0;
> }
>
> /* Helper function. Verify that the properties has been turn into the
> --- 1848,1856 ----
> /* Clear the last verified flag. */
>
> static void
> ! clear_last_verified (function *fn, void *data ATTRIBUTE_UNUSED)
> {
> ! fn->last_verified = 0;
> }
>
> /* Helper function. Verify that the properties has been turn into the
> *************** clear_last_verified (void *data ATTRIBUT
> *** 1865,1874 ****
>
> #ifdef ENABLE_CHECKING
> static void
> ! verify_curr_properties (void *data)
> {
> unsigned int props = (size_t)data;
> ! gcc_assert ((cfun->curr_properties & props) == props);
> }
> #endif
>
> --- 1858,1867 ----
>
> #ifdef ENABLE_CHECKING
> static void
> ! verify_curr_properties (function *fn, void *data)
> {
> unsigned int props = (size_t)data;
> ! gcc_assert ((fn->curr_properties & props) == props);
> }
> #endif
>
> *************** pass_fini_dump_file (opt_pass *pass)
> *** 1927,1937 ****
> properties. */
>
> static void
> ! update_properties_after_pass (void *data)
> {
> opt_pass *pass = (opt_pass *) data;
> ! cfun->curr_properties = (cfun->curr_properties | pass->properties_provided)
> ! & ~pass->properties_destroyed;
> }
>
> /* Execute summary generation for all of the passes in IPA_PASS. */
> --- 1920,1930 ----
> properties. */
>
> static void
> ! update_properties_after_pass (function *fn, void *data)
> {
> opt_pass *pass = (opt_pass *) data;
> ! fn->curr_properties = (fn->curr_properties | pass->properties_provided)
> ! & ~pass->properties_destroyed;
> }
>
> /* Execute summary generation for all of the passes in IPA_PASS. */
> *************** execute_all_ipa_transforms (void)
> *** 2042,2055 ****
> /* Callback for do_per_function to apply all IPA transforms. */
>
> static void
> ! apply_ipa_transforms (void *data)
> {
> ! struct cgraph_node *node = cgraph_get_node (current_function_decl);
> if (!node->global.inlined_to && node->ipa_transforms_to_apply.exists ())
> {
> *(bool *)data = true;
> execute_all_ipa_transforms ();
> rebuild_cgraph_edges ();
> }
> }
>
> --- 2035,2053 ----
> /* Callback for do_per_function to apply all IPA transforms. */
>
> static void
> ! apply_ipa_transforms (function *fn, void *data)
> {
> ! struct cgraph_node *node = cgraph_get_node (fn->decl);
> if (!node->global.inlined_to && node->ipa_transforms_to_apply.exists ())
> {
> + push_cfun (fn);
> *(bool *)data = true;
> execute_all_ipa_transforms ();
> rebuild_cgraph_edges ();
> + free_dominance_info (CDI_DOMINATORS);
> + free_dominance_info (CDI_POST_DOMINATORS);
> + pop_cfun ();
> + ggc_collect ();
> }
> }
>
> *************** execute_one_pass (opt_pass *pass)
> *** 2202,2221 ****
> return true;
> }
>
> ! void
> ! execute_pass_list (opt_pass *pass)
> {
> do
> {
> gcc_assert (pass->type == GIMPLE_PASS
> || pass->type == RTL_PASS);
> if (execute_one_pass (pass) && pass->sub)
> ! execute_pass_list (pass->sub);
> pass = pass->next;
> }
> while (pass);
> }
>
> /* Write out all LTO data. */
> static void
> write_lto (void)
> --- 2200,2232 ----
> return true;
> }
>
> ! static void
> ! execute_pass_list_1 (opt_pass *pass)
> {
> do
> {
> gcc_assert (pass->type == GIMPLE_PASS
> || pass->type == RTL_PASS);
> if (execute_one_pass (pass) && pass->sub)
> ! execute_pass_list_1 (pass->sub);
> pass = pass->next;
> }
> while (pass);
> }
>
> + void
> + execute_pass_list (function *fn, opt_pass *pass)
> + {
> + push_cfun (fn);
> + execute_pass_list_1 (pass);
> + if (fn->cfg)
> + {
> + free_dominance_info (CDI_DOMINATORS);
> + free_dominance_info (CDI_POST_DOMINATORS);
> + }
> + pop_cfun ();
> + }
> +
> /* Write out all LTO data. */
> static void
> write_lto (void)
> *************** execute_ipa_pass_list (opt_pass *pass)
> *** 2539,2545 ****
> if (pass->sub->type == GIMPLE_PASS)
> {
> invoke_plugin_callbacks (PLUGIN_EARLY_GIMPLE_PASSES_START, NULL);
> ! do_per_function_toporder ((void (*)(void *))execute_pass_list,
> pass->sub);
> invoke_plugin_callbacks (PLUGIN_EARLY_GIMPLE_PASSES_END, NULL);
> }
> --- 2550,2557 ----
> if (pass->sub->type == GIMPLE_PASS)
> {
> invoke_plugin_callbacks (PLUGIN_EARLY_GIMPLE_PASSES_START, NULL);
> ! do_per_function_toporder ((void (*)(function *, void *))
> ! execute_pass_list,
> pass->sub);
> invoke_plugin_callbacks (PLUGIN_EARLY_GIMPLE_PASSES_END, NULL);
> }
> Index: gcc/cgraphunit.c
> ===================================================================
> *** gcc/cgraphunit.c.orig 2014-04-23 14:56:40.528619658 +0200
> --- gcc/cgraphunit.c 2014-04-23 15:00:28.173603985 +0200
> *************** cgraph_add_new_function (tree fndecl, bo
> *** 520,526 ****
> push_cfun (DECL_STRUCT_FUNCTION (fndecl));
> gimple_register_cfg_hooks ();
> bitmap_obstack_initialize (NULL);
> ! execute_pass_list (passes->all_lowering_passes);
> passes->execute_early_local_passes ();
> bitmap_obstack_release (NULL);
> pop_cfun ();
> --- 520,526 ----
> push_cfun (DECL_STRUCT_FUNCTION (fndecl));
> gimple_register_cfg_hooks ();
> bitmap_obstack_initialize (NULL);
> ! execute_pass_list (cfun, passes->all_lowering_passes);
> passes->execute_early_local_passes ();
> bitmap_obstack_release (NULL);
> pop_cfun ();
> *************** analyze_function (struct cgraph_node *no
> *** 658,664 ****
>
> gimple_register_cfg_hooks ();
> bitmap_obstack_initialize (NULL);
> ! execute_pass_list (g->get_passes ()->all_lowering_passes);
> free_dominance_info (CDI_POST_DOMINATORS);
> free_dominance_info (CDI_DOMINATORS);
> compact_blocks ();
> --- 658,664 ----
>
> gimple_register_cfg_hooks ();
> bitmap_obstack_initialize (NULL);
> ! execute_pass_list (cfun, g->get_passes ()->all_lowering_passes);
> free_dominance_info (CDI_POST_DOMINATORS);
> free_dominance_info (CDI_DOMINATORS);
> compact_blocks ();
> *************** expand_function (struct cgraph_node *nod
> *** 1771,1777 ****
> /* Signal the start of passes. */
> invoke_plugin_callbacks (PLUGIN_ALL_PASSES_START, NULL);
>
> ! execute_pass_list (g->get_passes ()->all_passes);
>
> /* Signal the end of passes. */
> invoke_plugin_callbacks (PLUGIN_ALL_PASSES_END, NULL);
> --- 1771,1777 ----
> /* Signal the start of passes. */
> invoke_plugin_callbacks (PLUGIN_ALL_PASSES_START, NULL);
>
> ! execute_pass_list (cfun, g->get_passes ()->all_passes);
>
> /* Signal the end of passes. */
> invoke_plugin_callbacks (PLUGIN_ALL_PASSES_END, NULL);
> Index: gcc/cgraph.c
> ===================================================================
> *** gcc/cgraph.c.orig 2014-04-23 15:10:15.332563560 +0200
> --- gcc/cgraph.c 2014-04-23 15:10:22.548563063 +0200
> *************** release_function_body (tree decl)
> *** 1696,1703 ****
> }
> if (cfun->cfg)
> {
> ! gcc_assert (dom_computed[0] == DOM_NONE);
> ! gcc_assert (dom_computed[1] == DOM_NONE);
> clear_edges ();
> cfun->cfg = NULL;
> }
> --- 1696,1703 ----
> }
> if (cfun->cfg)
> {
> ! free_dominance_info (CDI_DOMINATORS);
> ! free_dominance_info (CDI_POST_DOMINATORS);
> clear_edges ();
> cfun->cfg = NULL;
> }
>
--
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer