> 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.

The purpose of the assert was to make sure that the dominance trees
are not maintained for whole program at once. release_function_body is
never called for a function that is "current" and thus the dominators should
be always freed.

At beggining the datastructure was global and thus it was not possible
to hold two. I think that is long fixed, but it seems bit wasteful to
keep all the dominance trees around.  Do you know how dominators leak
there?

Honza
> 
> Richard.
> 
> > Thanks,
> > Richard.
> > 
> > 2014-04-23  Richard Biener  <rguent...@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 <rguent...@suse.de>
> SUSE / SUSE Labs
> SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer

Reply via email to