Patchwork [GOOGLE] Refactor the LIPO fixup

login
register
mail settings
Submitter Dehao Chen
Date March 26, 2014, 10:54 p.m.
Message ID <CAO2gOZUEG=Zo0znpWoKRXhGJf4F-E_YmvoHTuRk1wS7+WqK-NA@mail.gmail.com>
Download mbox | patch
Permalink /patch/334131/
State New
Headers show

Comments

Dehao Chen - March 26, 2014, 10:54 p.m.
Patch updated, passed performance tests.

Dehao

On Tue, Mar 25, 2014 at 4:03 PM, Xinliang David Li <davidxl@google.com> wrote:
> Add comment to the new function. init_node_map is better invoked after
> the link step to avoid creating entries with for dead nodes.
>
> Ok if large perf testing is fine.
>
> David
>
> On Tue, Mar 25, 2014 at 3:38 PM, Dehao Chen <dehao@google.com> wrote:
>> This patch refactors LIPO fixup related code to move it into a
>> standalone function. This makes sure that
>> symtab_remove_unreachable_nodes is called right after the fixup so
>> that there is not dangling cgraph nodes any time.
>>
>> Bootstrapped and regression test on-going.
>>
>> OK for google-4_8?
>>
>> Thanks,
>> Dehao
Xinliang David Li - March 26, 2014, 11:05 p.m.
is cgraph_init_gid_map called after linking?

David

On Wed, Mar 26, 2014 at 3:54 PM, Dehao Chen <dehao@google.com> wrote:
> Patch updated, passed performance tests.
>
> Dehao
>
> On Tue, Mar 25, 2014 at 4:03 PM, Xinliang David Li <davidxl@google.com> wrote:
>> Add comment to the new function. init_node_map is better invoked after
>> the link step to avoid creating entries with for dead nodes.
>>
>> Ok if large perf testing is fine.
>>
>> David
>>
>> On Tue, Mar 25, 2014 at 3:38 PM, Dehao Chen <dehao@google.com> wrote:
>>> This patch refactors LIPO fixup related code to move it into a
>>> standalone function. This makes sure that
>>> symtab_remove_unreachable_nodes is called right after the fixup so
>>> that there is not dangling cgraph nodes any time.
>>>
>>> Bootstrapped and regression test on-going.
>>>
>>> OK for google-4_8?
>>>
>>> Thanks,
>>> Dehao

Patch

Index: gcc/l-ipo.h
===================================================================
--- gcc/l-ipo.h	(revision 208826)
+++ gcc/l-ipo.h	(working copy)
@@ -60,7 +60,7 @@  void add_decl_to_current_module_scope (tree decl,
 int lipo_cmp_type (tree t1, tree t2);
 tree get_type_or_decl_name (tree);
 int equivalent_struct_types_for_tbaa (const_tree t1, const_tree t2);
-void lipo_fixup_cgraph_edge_call_target (gimple);
+void lipo_link_and_fixup (void);
 extern void copy_defined_module_set (tree, tree);
 extern bool is_parsing_done_p (void);
 extern const char* get_module_name (unsigned int);
Index: gcc/auto-profile.c
===================================================================
--- gcc/auto-profile.c	(revision 208826)
+++ gcc/auto-profile.c	(working copy)
@@ -1533,16 +1533,10 @@  auto_profile (void)
   if (cgraph_state == CGRAPH_STATE_FINISHED)
     return 0;
 
-  init_node_map ();
   profile_info = autofdo::afdo_profile_info;
+  lipo_link_and_fixup ();
+  init_node_map ();
 
-  cgraph_pre_profiling_inlining_done = true;
-  cgraph_process_module_scope_statics ();
-  /* Now perform link to allow cross module inlining.  */
-  cgraph_do_link ();
-  varpool_do_link ();
-  cgraph_unify_type_alias_sets ();
-
   FOR_EACH_FUNCTION (node)
     {
       if (!gimple_has_body_p (node->symbol.decl))
@@ -1554,35 +1548,6 @@  auto_profile (void)
 
       push_cfun (DECL_STRUCT_FUNCTION (node->symbol.decl));
 
-      if (L_IPO_COMP_MODE)
-	{
-	  basic_block bb;
-	  FOR_EACH_BB (bb)
-	    {
-	      gimple_stmt_iterator gsi;
-	      for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-		{
-		  gimple stmt = gsi_stmt (gsi);
-		  if (is_gimple_call (stmt))
-		    lipo_fixup_cgraph_edge_call_target (stmt);
-		}
-	    }
-	}
-      rebuild_cgraph_edges ();
-      pop_cfun ();
-    }
-
-  FOR_EACH_FUNCTION (node)
-    {
-      if (!gimple_has_body_p (node->symbol.decl))
-	continue;
-
-      /* Don't profile functions produced for builtin stuff.  */
-      if (DECL_SOURCE_LOCATION (node->symbol.decl) == BUILTINS_LOCATION)
-	continue;
-
-      push_cfun (DECL_STRUCT_FUNCTION (node->symbol.decl));
-
       /* First do indirect call promotion and early inline to make the
 	 IR match the profiled binary before actual annotation.
 
Index: gcc/value-prof.c
===================================================================
--- gcc/value-prof.c	(revision 208826)
+++ gcc/value-prof.c	(working copy)
@@ -1286,6 +1286,15 @@  init_gid_map (void)
           entp->node = n;
           entp->gid = ent.gid;
         }
+      else if (cgraph_pre_profiling_inlining_done)
+	{
+	  (*slot)->node = cgraph_lipo_get_resolved_node (n->symbol.decl);
+	  (*slot)->gid = ent.gid;
+	}
+      else
+	{
+	  gcc_assert ((*slot)->gid == ent.gid && (*slot)->node == n);
+	}
     }
 }
 
Index: gcc/cgraphbuild.c
===================================================================
--- gcc/cgraphbuild.c	(revision 208826)
+++ gcc/cgraphbuild.c	(working copy)
@@ -244,9 +244,6 @@  add_fake_indirect_call_edges (struct cgraph_node *
   if (!L_IPO_COMP_MODE)
     return;
 
-  if (cgraph_pre_profiling_inlining_done)
-    return;
-
   ic_counts
       = get_coverage_counts_no_warn (DECL_STRUCT_FUNCTION (node->symbol.decl),
                                      GCOV_COUNTER_ICALL_TOPNV, &n_counts);
@@ -599,7 +596,7 @@  record_references_in_initializer (tree decl, bool
    needs to be set to the resolved node so that ipa-inline
    sees the definitions.  */
 #include "gimple-pretty-print.h"
-void
+static void
 lipo_fixup_cgraph_edge_call_target (gimple stmt)
 {
   tree decl;
@@ -625,6 +622,57 @@  lipo_fixup_cgraph_edge_call_target (gimple stmt)
     }
 }
 
+/* Link the cgraph nodes, varpool nodes and fixup the call target to
+   the correct decl. Remove dead functions.  */
+
+
+void
+lipo_link_and_fixup ()
+{
+  struct cgraph_node *node;
+
+  cgraph_pre_profiling_inlining_done = true;
+  cgraph_process_module_scope_statics ();
+  /* Now perform link to allow cross module inlining.  */
+  cgraph_do_link ();
+  varpool_do_link ();
+  cgraph_unify_type_alias_sets ();
+ 
+  FOR_EACH_DEFINED_FUNCTION (node)
+    {
+      if (!gimple_has_body_p (node->symbol.decl))
+	continue;
+
+      /* Don't profile functions produced for builtin stuff.  */
+      if (DECL_SOURCE_LOCATION (node->symbol.decl) == BUILTINS_LOCATION)
+	continue;
+
+      push_cfun (DECL_STRUCT_FUNCTION (node->symbol.decl));
+
+      if (L_IPO_COMP_MODE)
+	{
+	  basic_block bb;
+	  FOR_EACH_BB (bb)
+	    {
+	      gimple_stmt_iterator gsi;
+	      for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+		{
+		  gimple stmt = gsi_stmt (gsi);
+		  if (is_gimple_call (stmt))
+		    lipo_fixup_cgraph_edge_call_target (stmt);
+		}
+	    }
+	  update_ssa (TODO_update_ssa);
+	}
+      rebuild_cgraph_edges ();
+      pop_cfun ();
+    }
+
+  cgraph_add_fake_indirect_call_edges ();
+  symtab_remove_unreachable_nodes (true, dump_file);
+}
+
+
 /* Rebuild cgraph edges for current function node.  This needs to be run after
    passes that don't update the cgraph.  */
 
@@ -677,7 +725,8 @@  rebuild_cgraph_edges (void)
 				       mark_load, mark_store, mark_address);
     }
 
-  add_fake_indirect_call_edges (node);
+  if (!cgraph_pre_profiling_inlining_done)
+    add_fake_indirect_call_edges (node);
   record_eh_tables (node, cfun);
   gcc_assert (!node->global.inlined_to);
 
Index: gcc/tree-profile.c
===================================================================
--- gcc/tree-profile.c	(revision 208826)
+++ gcc/tree-profile.c	(working copy)
@@ -1118,19 +1118,11 @@  tree_profiling (void)
   /* This is a small-ipa pass that gets called only once, from
      cgraphunit.c:ipa_passes().  */
   gcc_assert (cgraph_state == CGRAPH_STATE_IPA_SSA);
-
   /* After value profile transformation, artificial edges (that keep
      function body from being deleted) won't be needed.  */
+  lipo_link_and_fixup ();
+  init_node_map ();
 
-  cgraph_pre_profiling_inlining_done = true;
-  cgraph_process_module_scope_statics ();
-  /* Now perform link to allow cross module inlining.  */
-  cgraph_do_link ();
-  varpool_do_link ();
-  cgraph_unify_type_alias_sets ();
-
-  init_node_map();
-
   FOR_EACH_DEFINED_FUNCTION (node)
     {
       if (!gimple_has_body_p (node->symbol.decl))
@@ -1142,23 +1134,6 @@  tree_profiling (void)
 
       push_cfun (DECL_STRUCT_FUNCTION (node->symbol.decl));
 
-      if (L_IPO_COMP_MODE)
-        {
-          basic_block bb;
-          FOR_EACH_BB (bb)
-            {
-              gimple_stmt_iterator gsi;
-              for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-                {
-                  gimple stmt = gsi_stmt (gsi);
-                  if (is_gimple_call (stmt))
-                    lipo_fixup_cgraph_edge_call_target (stmt);
-                }
-	    }
-          update_ssa (TODO_update_ssa);
-	}
-
-
       /* Local pure-const may imply need to fixup the cfg.  */
       if (execute_fixup_cfg () & TODO_cleanup_cfg)
 	cleanup_tree_cfg ();