[google][4.6]Make option -freorder-functions= invoke function reordering linker plugin

[Sriraman, 2012-03-14 20:56:32]

This patch overloads GCC option -freorder-functions with -freoder-functions=*
which will invoke the linker plugin libfunction_reordering_plugin.so. For now,
the only accepted option is -freoder-functions=cgedge, where the functions which
are connected by hot callgraph edges are placed closer.  When
-freorder-functions is specified without the '=', the original behaviour is
maintained.  This patch also removes the -fcallgraph-profiles-sections option.

For context, the function_reordering_plugin is only available in gcc-4_6 branch.
I am working on porting this to trunk and will send out a patch soon.


	* cgraphbuild.c (remove_cgraph_callee_edges): Replace
	flag_callgraph_profiles_sections with flag_reorder_functions.
	* final.c (rest_of_handle_final): Ditto.
	* configure: Regenerate.
	* config.in: undef FRPLUGINSONAME.
	* congifure.ac: Define FRPLUGINSONAME.
	* config.host: Set host_function_reordering_plugin_soname.
	* gcc.c: Invoke function_reordering linker plugin with
	-freorder-functions=* option.
	(set_func_reorder_linker_plugin_spec): New function.
	(main): Call set_func_reorder_linker_plugin_spec when
	-freorder-functions=* option is seen.
	* common.opt (fcallgraph-profiles-sections):  Remove.
	(freorder-functions=): New option.

	* function_reordering_plugin.c (register_claim_file_hook): New function
	pointer.
	(register_all_symbols_read_hook): Ditto.
	(no_op): New global.
	(out_file): Make static.
	(is_api_exist): Make static.
	(process_option): New function.
	(onload): Return if no_op is set. Do not register handlers until
	necessary.


Index: function_reordering_plugin/function_reordering_plugin.c
===================================================================
--- function_reordering_plugin/function_reordering_plugin.c	(revision 185372)
+++ function_reordering_plugin/function_reordering_plugin.c	(working copy)
@@ -64,6 +64,9 @@ enum ld_plugin_status claim_file_hook (const struc
                                        int *claimed);
 enum ld_plugin_status all_symbols_read_hook ();
 
+static ld_plugin_register_claim_file register_claim_file_hook = NULL;
+static ld_plugin_register_all_symbols_read
+  register_all_symbols_read_hook = NULL;
 static ld_plugin_get_input_section_count get_input_section_count = NULL;
 static ld_plugin_get_input_section_type get_input_section_type = NULL;
 static ld_plugin_get_input_section_name get_input_section_name = NULL;
@@ -73,12 +76,16 @@ static ld_plugin_allow_section_ordering allow_sect
 
 /* The file where the final function order will be stored.
    It is "./final_layout.txt".  It can be changed by passing
-   new name to --plugin-opt  */
+   new name to --plugin-opt  as --plugin-opt "file=<name>".
+   To dump to stderr, say --plugin-opt "file=stderr".  */
 
-char *out_file = "./final_layout.txt";
+static char *out_file = "./final_layout.txt";
 
-int is_api_exist = 0;
+static int is_api_exist = 0;
 
+/* The plugin does nothing when no-op is 1.  */
+static int no_op = 0;
+
 /* Copies new output file name out_file  */
 void get_filename (const char *name)
 {
@@ -91,6 +98,39 @@ void get_filename (const char *name)
   strcpy (out_file, name);
 }
 
+/* Process options to plugin.  Options with prefix "group=" are special.
+   They specify the type of grouping. The option "group=none" makes the
+   plugin do nothing.   Options with prefix "file=" set the output file
+   where the final function order must be stored.  */
+void
+process_option (const char *name)
+{
+  const char *option_group = "group=";
+  const char *option_file = "file=";
+
+  /* Check if option is "group="  */
+  if (strncmp (name, option_group, strlen (option_group)) == 0)
+    {
+      if (strcmp (name + strlen (option_group), "none") == 0)
+	no_op = 1;
+      else
+	no_op = 0;
+      return;
+    }
+
+  /* Check if option is "file=" */
+  if (strncmp (name, option_file, strlen (option_file)) == 0)
+    {
+      get_filename (name + strlen (option_file));
+      return;
+    }
+
+  /* Unknown option, set no_op to 1.  */
+  no_op = 1;
+  fprintf (stderr, "Unknown option to function reordering plugin :%s\n",
+	   name);
+}
+
 /* Plugin entry point.  */
 enum ld_plugin_status
 onload (struct ld_plugin_tv *tv)
@@ -105,14 +145,16 @@ onload (struct ld_plugin_tv *tv)
         case LDPT_GOLD_VERSION:
           break;
         case LDPT_OPTION:
-	  get_filename (entry->tv_u.tv_string);
+	  process_option (entry->tv_u.tv_string);
+	  /* If no_op is set, do not do anything else.  */
+	  if (no_op) return LDPS_OK;
 	  break;
         case LDPT_REGISTER_CLAIM_FILE_HOOK:
-          assert ((*entry->tv_u.tv_register_claim_file) (claim_file_hook) ==
LDPS_OK);
+	  register_claim_file_hook = *entry->tv_u.tv_register_claim_file;
           break;
 	case LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK:
-          assert ((*entry->tv_u.tv_register_all_symbols_read)
(all_symbols_read_hook)
-		   == LDPS_OK);
+	  register_all_symbols_read_hook
+	    = *entry->tv_u.tv_register_all_symbols_read;
           break;
         case LDPT_GET_INPUT_SECTION_COUNT:
           get_input_section_count = *entry->tv_u.tv_get_input_section_count;
@@ -137,14 +179,25 @@ onload (struct ld_plugin_tv *tv)
         }
     }
 
-  if (get_input_section_count != NULL
+  assert (!no_op);
+
+  if (register_all_symbols_read_hook != NULL
+      && register_claim_file_hook != NULL
+      && get_input_section_count != NULL
       && get_input_section_type != NULL
       && get_input_section_name != NULL
       && get_input_section_contents != NULL
       && update_section_order != NULL
       && allow_section_ordering != NULL)
     is_api_exist = 1;
+  else
+    return LDPS_OK;
 
+  /* Register handlers.  */
+  assert ((*register_all_symbols_read_hook) (all_symbols_read_hook)
+	   == LDPS_OK);
+  assert ((*register_claim_file_hook) (claim_file_hook)
+	  == LDPS_OK);
   return LDPS_OK;
 }
 
@@ -161,9 +214,8 @@ claim_file_hook (const struct ld_plugin_input_file
 
   (void) claimed;
 
-  /* Silently return if the plugin APIs are not supported.  */
-  if (!is_api_exist)
-    return LDPS_OK;
+  /* Plugin APIs are supported if this is called.  */
+  assert (is_api_exist);
 
   if (is_ordering_specified == 0)
     {
@@ -221,9 +273,8 @@ all_symbols_read_hook (void)
   unsigned int *shndx;
   FILE *fp;
 
-  /* Silently return if the plugin APIs are not supported.  */
-  if (!is_api_exist)
-    return LDPS_OK;
+  /* Plugin APIs are supported if this is called.  */
+  assert (is_api_exist);
 
   if (is_callgraph_empty ())
     return LDPS_OK;
Index: function_reordering_plugin/ChangeLog.google-4_6
===================================================================
--- function_reordering_plugin/ChangeLog.google-4_6	(revision 185372)
+++ function_reordering_plugin/ChangeLog.google-4_6	(working copy)
@@ -1,3 +1,15 @@
+2012-03-14  Sriraman Tallam  <tmsriram@google.com>
+
+	* function_reordering_plugin.c (register_claim_file_hook): New function
+	pointer.
+	(register_all_symbols_read_hook): Ditto.
+	(no_op): New global.
+	(out_file): Make static.
+	(is_api_exist): Make static.
+	(process_option): New function.
+	(onload): Return if no_op is set. Do not register handlers until
+	necessary. 	
+
 2012-02-25  Sriraman Tallam  <tmsriram@google.com>
 
 	* function_reordering_plugin.c: Check for presence of elf.h.
Index: gcc/cgraphbuild.c
===================================================================
--- gcc/cgraphbuild.c	(revision 185372)
+++ gcc/cgraphbuild.c	(working copy)
@@ -703,10 +703,10 @@ extern bool cgraph_callee_edges_final_cleanup;
 static unsigned int
 remove_cgraph_callee_edges (void)
 {
-  /* The -fcallgraph-profiles-sections flag needs the call-graph preserved
+  /* The -freorder-functions=* flag needs the call-graph preserved
      till pass_final.  */
   if (cgraph_callee_edges_final_cleanup
-      && flag_callgraph_profiles_sections)
+      && flag_reorder_functions > 1)
       return 0;
 
   cgraph_node_remove_callees (cgraph_node (current_function_decl));
Index: gcc/configure
===================================================================
--- gcc/configure	(revision 185372)
+++ gcc/configure	(working copy)
@@ -4861,7 +4861,7 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $acx_cv_cc_gcc_supports_ada"
>&5
 $as_echo "$acx_cv_cc_gcc_supports_ada" >&6; }
 
-if test x$GNATBIND != xno && test x$GNATMAKE != xno && test
x$acx_cv_cc_gcc_supports_ada != xno; then
+if test "x$GNATBIND" != xno && test "x$GNATMAKE" != xno && test
x$acx_cv_cc_gcc_supports_ada != xno; then
   have_gnat=yes
 else
   have_gnat=no
@@ -11480,6 +11480,12 @@ esac
 
 
 cat >>confdefs.h <<_ACEOF
+#define FRPLUGINSONAME "${host_function_reordering_plugin_soname}"
+_ACEOF
+
+
+
+cat >>confdefs.h <<_ACEOF
 #define LTOPLUGINSONAME "${host_lto_plugin_soname}"
 _ACEOF
 
@@ -17572,7 +17578,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 17575 "configure"
+#line 17581 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -17678,7 +17684,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 17681 "configure"
+#line 17687 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
Index: gcc/final.c
===================================================================
--- gcc/final.c	(revision 185372)
+++ gcc/final.c	(working copy)
@@ -4421,9 +4421,9 @@ rest_of_handle_final (void)
 				decl_fini_priority_lookup
 				  (current_function_decl));
 
-  /* With -fcallgraph-profiles-sections, add ".gnu.callgraph.text" section
+  /* With -freorder-functions=, add ".gnu.callgraph.text" section
      for storing profiling information. */
-  if (flag_callgraph_profiles_sections
+  if (flag_reorder_functions > 1
       && flag_profile_use
       && cgraph_node (current_function_decl) != NULL
       && (cgraph_node (current_function_decl))->callees != NULL)
Index: gcc/gcc.c
===================================================================
--- gcc/gcc.c	(revision 185372)
+++ gcc/gcc.c	(working copy)
@@ -663,6 +663,9 @@ proper position among the other output files.  */
     -plugin-opt=-fresolution=%u.res \
     %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}}
\
     }"PLUGIN_COND_CLOSE" \
+    %{freorder-functions=*: \
+    -plugin %(func_reorder_linker_plugin_file) \
+    -plugin-opt=%(func_reorder_linker_plugin_opt)} \
     %{flto|flto=*:%<fcompare-debug*} \
     %{flto} %{flto=*} %l " LINK_PIE_SPEC \
    "%{fuse-ld=gold:%{fuse-ld=bfd:%e-fuse-ld=gold and -fuse-ld=bfd may not be
used together}} \
@@ -717,6 +720,8 @@ static const char *endfile_spec = ENDFILE_SPEC;
 static const char *startfile_spec = STARTFILE_SPEC;
 static const char *linker_name_spec = LINKER_NAME;
 static const char *linker_plugin_file_spec = "";
+static const char *func_reorder_linker_plugin_file_spec = "";
+static const char *func_reorder_linker_plugin_opt = "";
 static const char *lto_wrapper_spec = "";
 static const char *lto_gcc_spec = "";
 static const char *link_command_spec = LINK_COMMAND_SPEC;
@@ -1206,6 +1211,10 @@ static struct spec_list static_specs[] =
   INIT_STATIC_SPEC ("multilib_options",		&multilib_options),
   INIT_STATIC_SPEC ("linker",			&linker_name_spec),
   INIT_STATIC_SPEC ("linker_plugin_file",	&linker_plugin_file_spec),
+  INIT_STATIC_SPEC ("func_reorder_linker_plugin_file",
+		    &func_reorder_linker_plugin_file_spec),
+  INIT_STATIC_SPEC ("func_reorder_linker_plugin_opt",
+	  	    &func_reorder_linker_plugin_opt),
   INIT_STATIC_SPEC ("lto_wrapper",		&lto_wrapper_spec),
   INIT_STATIC_SPEC ("lto_gcc",			&lto_gcc_spec),
   INIT_STATIC_SPEC ("link_libgcc",		&link_libgcc_spec),
@@ -6108,6 +6117,51 @@ compare_files (char *cmpfile[])
   return ret;
 }
 
+/* Set func_reorder_linker_plugin_file_spec and func_reorder_linker_plugin_opt
+   here.  This is the linker plugin to do global function reordering and is
+   enabled with -freorder-functions=*. */
+
+static void
+set_func_reorder_linker_plugin_spec (void)
+{
+  int i;
+  const char *plugin_opt_none = "group=none";
+  const char *plugin_opt_cgedge = "group=cgedge";
+  
+  /* Find the linker plugin that does function ordering.  */
+  func_reorder_linker_plugin_file_spec = find_a_file (&exec_prefixes,
+					    FRPLUGINSONAME, R_OK, false);
+
+  if (!func_reorder_linker_plugin_file_spec)
+      fatal_error ("-freorder-functions=*, but "
+		   FRPLUGINSONAME " file not found");
+
+  func_reorder_linker_plugin_opt = plugin_opt_none;
+
+  /* Set linker plugin options here.  Option ordering is also checked here.
+     -fno-reorder-functions or -freorder-functions should disable any
+     previous -freorder-functions=*. */
+  for (i = 0; (int) i < n_switches; i++)
+    {
+      /* Check for match with "-freorder-functions=cgedge".  */
+      if (func_reorder_linker_plugin_opt != plugin_opt_cgedge
+	  && !strcmp (switches[i].part1, "freorder-functions=cgedge"))
+	{
+	  func_reorder_linker_plugin_opt = plugin_opt_cgedge;
+	  continue;
+	}
+      /* Set option to none if it matches -fno-reorder-functions
+	 or -freorder-functions  */
+      if (func_reorder_linker_plugin_opt != plugin_opt_none
+	  && (!strcmp (switches[i].part1, "fno-reorder-functions")
+	      || !strcmp (switches[i].part1, "freorder-functions")))
+	{
+	  func_reorder_linker_plugin_opt = plugin_opt_none;
+	  continue;
+	}
+    }
+}
+
 extern int main (int, char **);
 
 int
@@ -6843,6 +6897,8 @@ warranty; not even for MERCHANTABILITY or FITNESS
       const char *fuse_linker_plugin = "fuse-linker-plugin";
 #endif
 
+      const char *freorder_functions_ = "freorder-functions=";
+
       /* We'll use ld if we can't find collect2.  */
       if (! strcmp (linker_name_spec, "collect2"))
 	{
@@ -6866,6 +6922,12 @@ warranty; not even for MERCHANTABILITY or FITNESS
 	    fatal_error ("-fuse-linker-plugin, but " LTOPLUGINSONAME " not found");
 	}
       lto_gcc_spec = argv[0];
+  
+      /* The function reordering linker plugin will be loaded if the option
+	 -freorder-functions= is present in the command-line.  */ 
+      if (switch_matches (freorder_functions_,
+			  freorder_functions_ + strlen (freorder_functions_), 1))
+	set_func_reorder_linker_plugin_spec ();
 
       /* Rebuild the COMPILER_PATH and LIBRARY_PATH environment variables
 	 for collect.  */
Index: gcc/config.in
===================================================================
--- gcc/config.in	(revision 185372)
+++ gcc/config.in	(working copy)
@@ -1583,13 +1583,18 @@
 #endif
 
 
+/* Define to the name of the function reordering plugin DSO that must be
+   passed to the linker's -plugin=LIB option. */
+#ifndef USED_FOR_TARGET
+#undef FRPLUGINSONAME
+#endif
+
 /* Define to the name of the LTO plugin DSO that must be passed to the
    linker's -plugin=LIB option. */
 #ifndef USED_FOR_TARGET
 #undef LTOPLUGINSONAME
 #endif
 
-
 /* Define to the sub-directory in which libtool stores uninstalled libraries.
    */
 #ifndef USED_FOR_TARGET
Index: gcc/configure.ac
===================================================================
--- gcc/configure.ac	(revision 185372)
+++ gcc/configure.ac	(working copy)
@@ -1858,6 +1858,10 @@ case $use_collect2 in
     ;;
 esac
 
+AC_DEFINE_UNQUOTED(FRPLUGINSONAME,"${host_function_reordering_plugin_soname}",
+[Define to the name of the function reordering plugin DSO that must be
+  passed to the linker's -plugin=LIB option.])
+
 AC_DEFINE_UNQUOTED(LTOPLUGINSONAME,"${host_lto_plugin_soname}",
 [Define to the name of the LTO plugin DSO that must be
   passed to the linker's -plugin=LIB option.])
Index: gcc/ChangeLog.google-4_6
===================================================================
--- gcc/ChangeLog.google-4_6	(revision 185372)
+++ gcc/ChangeLog.google-4_6	(working copy)
@@ -1,3 +1,21 @@
+2012-03-14  Sriraman Tallam  <tmsriram@google.com>
+
+	* cgraphbuild.c (remove_cgraph_callee_edges): Replace
+	flag_callgraph_profiles_sections with flag_reorder_functions.
+	* final.c (rest_of_handle_final): Ditto.
+	* configure: Regenerate.
+	* config.in: undef FRPLUGINSONAME.
+	* congifure.ac: Define FRPLUGINSONAME.
+	* config.host: Set host_function_reordering_plugin_soname.
+	* gcc.c: Invoke function_reordering linker plugin with
+	-freorder-functions=* option.
+	(set_func_reorder_linker_plugin_spec): New function.
+	(main): Call set_func_reorder_linker_plugin_spec when
+	-freorder-functions=* option is seen.
+	* common.opt: Add new option -freorder-functions=.  Remove
+	-fcallgraph-profiles-sections.
+	
+
 2012-03-12  Sterling Augustine  <saugustine@google.com>
 	    Cary Coutant  <ccoutant@google.com>
 
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 185372)
+++ gcc/common.opt	(working copy)
@@ -920,10 +920,6 @@ fcaller-saves
 Common Report Var(flag_caller_saves) Optimization
 Save registers around function calls
 
-fcallgraph-profiles-sections
-Common Report Var(flag_callgraph_profiles_sections) Init(0)
-Generate .note.callgraph.text sections listing callees and edge counts.
-
 fcheck-data-deps
 Common Report Var(flag_check_data_deps)
 Compare the results of several data dependence analyzers.
@@ -1758,9 +1754,19 @@ Common Report Var(flag_reorder_blocks_and_partitio
 Reorder basic blocks and partition into hot and cold sections
 
 freorder-functions
-Common Report Var(flag_reorder_functions) Optimization
+Common Report Var(flag_reorder_functions,1) Init(0) Optimization
 Reorder functions to improve code placement
 
+freorder-functions=
+Common Joined RejectNegative Enum(function_reorder) Var(flag_reorder_functions)
Init(0) Optimization
+-freorder-functions=[cgedge]	Select the scheme for function reordering. This
invokes a linker plugin.  Generate .note.callgraph.text sections listing callees
and edge counts.
+
+Enum
+Name(function_reorder) Type(int) UnknownError(unrecognized function reorder
value %qs)
+ 
+EnumValue
+Enum(function_reorder) String(cgedge) Value(2)
+
 frerun-cse-after-loop
 Common Report Var(flag_rerun_cse_after_loop) Optimization
 Add a common subexpression elimination pass after loop optimizations
Index: gcc/config.host
===================================================================
--- gcc/config.host	(revision 185372)
+++ gcc/config.host	(working copy)
@@ -76,6 +76,7 @@ out_host_hook_obj=host-default.o
 host_can_use_collect2=yes
 use_long_long_for_widest_fast_int=no
 host_lto_plugin_soname=liblto_plugin.so
+host_function_reordering_plugin_soname=libfunction_reordering_plugin.so
 
 # Unsupported hosts list.  Generally, only include hosts known to fail here,
 # since we allow hosts not listed to be supported generically.

--
This patch is available for review at http://codereview.appspot.com/5825054

[Sriraman, 2012-03-14 20:57:39]

On Wed, Mar 14, 2012 at 1:56 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> This patch overloads GCC option -freorder-functions with -freoder-functions=*
which will invoke the linker plugin libfunction_reordering_plugin.so. For now,
the only accepted option is -freoder-functions=cgedge, where the functions which
are connected by hot callgraph edges are placed closer.  When
-freorder-functions is specified without the '=', the original behaviour is
maintained.  This patch also removes the -fcallgraph-profiles-sections option.
>
> For context, the function_reordering_plugin is only available in gcc-4_6
branch. I am working on porting this to trunk and will send out a patch soon.
>
>
>        * cgraphbuild.c (remove_cgraph_callee_edges): Replace
>        flag_callgraph_profiles_sections with flag_reorder_functions.
>        * final.c (rest_of_handle_final): Ditto.
>        * configure: Regenerate.
>        * config.in: undef FRPLUGINSONAME.
>        * congifure.ac: Define FRPLUGINSONAME.
>        * config.host: Set host_function_reordering_plugin_soname.
>        * gcc.c: Invoke function_reordering linker plugin with
>        -freorder-functions=* option.
>        (set_func_reorder_linker_plugin_spec): New function.
>        (main): Call set_func_reorder_linker_plugin_spec when
>        -freorder-functions=* option is seen.
>        * common.opt (fcallgraph-profiles-sections):  Remove.
>        (freorder-functions=): New option.
>
>        * function_reordering_plugin.c (register_claim_file_hook): New function
>        pointer.
>        (register_all_symbols_read_hook): Ditto.
>        (no_op): New global.
>        (out_file): Make static.
>        (is_api_exist): Make static.
>        (process_option): New function.
>        (onload): Return if no_op is set. Do not register handlers until
>        necessary.
>
>
> Index: function_reordering_plugin/function_reordering_plugin.c
> ===================================================================
> --- function_reordering_plugin/function_reordering_plugin.c     (revision
185372)
> +++ function_reordering_plugin/function_reordering_plugin.c     (working copy)
> @@ -64,6 +64,9 @@ enum ld_plugin_status claim_file_hook (const struc
>                                        int *claimed);
>  enum ld_plugin_status all_symbols_read_hook ();
>
> +static ld_plugin_register_claim_file register_claim_file_hook = NULL;
> +static ld_plugin_register_all_symbols_read
> +  register_all_symbols_read_hook = NULL;
>  static ld_plugin_get_input_section_count get_input_section_count = NULL;
>  static ld_plugin_get_input_section_type get_input_section_type = NULL;
>  static ld_plugin_get_input_section_name get_input_section_name = NULL;
> @@ -73,12 +76,16 @@ static ld_plugin_allow_section_ordering allow_sect
>
>  /* The file where the final function order will be stored.
>    It is "./final_layout.txt".  It can be changed by passing
> -   new name to --plugin-opt  */
> +   new name to --plugin-opt  as --plugin-opt "file=<name>".
> +   To dump to stderr, say --plugin-opt "file=stderr".  */
>
> -char *out_file = "./final_layout.txt";
> +static char *out_file = "./final_layout.txt";
>
> -int is_api_exist = 0;
> +static int is_api_exist = 0;
>
> +/* The plugin does nothing when no-op is 1.  */
> +static int no_op = 0;
> +
>  /* Copies new output file name out_file  */
>  void get_filename (const char *name)
>  {
> @@ -91,6 +98,39 @@ void get_filename (const char *name)
>   strcpy (out_file, name);
>  }
>
> +/* Process options to plugin.  Options with prefix "group=" are special.
> +   They specify the type of grouping. The option "group=none" makes the
> +   plugin do nothing.   Options with prefix "file=" set the output file
> +   where the final function order must be stored.  */
> +void
> +process_option (const char *name)
> +{
> +  const char *option_group = "group=";
> +  const char *option_file = "file=";
> +
> +  /* Check if option is "group="  */
> +  if (strncmp (name, option_group, strlen (option_group)) == 0)
> +    {
> +      if (strcmp (name + strlen (option_group), "none") == 0)
> +       no_op = 1;
> +      else
> +       no_op = 0;
> +      return;
> +    }
> +
> +  /* Check if option is "file=" */
> +  if (strncmp (name, option_file, strlen (option_file)) == 0)
> +    {
> +      get_filename (name + strlen (option_file));
> +      return;
> +    }
> +
> +  /* Unknown option, set no_op to 1.  */
> +  no_op = 1;
> +  fprintf (stderr, "Unknown option to function reordering plugin :%s\n",
> +          name);
> +}
> +
>  /* Plugin entry point.  */
>  enum ld_plugin_status
>  onload (struct ld_plugin_tv *tv)
> @@ -105,14 +145,16 @@ onload (struct ld_plugin_tv *tv)
>         case LDPT_GOLD_VERSION:
>           break;
>         case LDPT_OPTION:
> -         get_filename (entry->tv_u.tv_string);
> +         process_option (entry->tv_u.tv_string);
> +         /* If no_op is set, do not do anything else.  */
> +         if (no_op) return LDPS_OK;
>          break;
>         case LDPT_REGISTER_CLAIM_FILE_HOOK:
> -          assert ((*entry->tv_u.tv_register_claim_file) (claim_file_hook) ==
LDPS_OK);
> +         register_claim_file_hook = *entry->tv_u.tv_register_claim_file;
>           break;
>        case LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK:
> -          assert ((*entry->tv_u.tv_register_all_symbols_read)
(all_symbols_read_hook)
> -                  == LDPS_OK);
> +         register_all_symbols_read_hook
> +           = *entry->tv_u.tv_register_all_symbols_read;
>           break;
>         case LDPT_GET_INPUT_SECTION_COUNT:
>           get_input_section_count = *entry->tv_u.tv_get_input_section_count;
> @@ -137,14 +179,25 @@ onload (struct ld_plugin_tv *tv)
>         }
>     }
>
> -  if (get_input_section_count != NULL
> +  assert (!no_op);
> +
> +  if (register_all_symbols_read_hook != NULL
> +      && register_claim_file_hook != NULL
> +      && get_input_section_count != NULL
>       && get_input_section_type != NULL
>       && get_input_section_name != NULL
>       && get_input_section_contents != NULL
>       && update_section_order != NULL
>       && allow_section_ordering != NULL)
>     is_api_exist = 1;
> +  else
> +    return LDPS_OK;
>
> +  /* Register handlers.  */
> +  assert ((*register_all_symbols_read_hook) (all_symbols_read_hook)
> +          == LDPS_OK);
> +  assert ((*register_claim_file_hook) (claim_file_hook)
> +         == LDPS_OK);
>   return LDPS_OK;
>  }
>
> @@ -161,9 +214,8 @@ claim_file_hook (const struct ld_plugin_input_file
>
>   (void) claimed;
>
> -  /* Silently return if the plugin APIs are not supported.  */
> -  if (!is_api_exist)
> -    return LDPS_OK;
> +  /* Plugin APIs are supported if this is called.  */
> +  assert (is_api_exist);
>
>   if (is_ordering_specified == 0)
>     {
> @@ -221,9 +273,8 @@ all_symbols_read_hook (void)
>   unsigned int *shndx;
>   FILE *fp;
>
> -  /* Silently return if the plugin APIs are not supported.  */
> -  if (!is_api_exist)
> -    return LDPS_OK;
> +  /* Plugin APIs are supported if this is called.  */
> +  assert (is_api_exist);
>
>   if (is_callgraph_empty ())
>     return LDPS_OK;
> Index: function_reordering_plugin/ChangeLog.google-4_6
> ===================================================================
> --- function_reordering_plugin/ChangeLog.google-4_6     (revision 185372)
> +++ function_reordering_plugin/ChangeLog.google-4_6     (working copy)
> @@ -1,3 +1,15 @@
> +2012-03-14  Sriraman Tallam  <tmsriram@google.com>
> +
> +       * function_reordering_plugin.c (register_claim_file_hook): New
function
> +       pointer.
> +       (register_all_symbols_read_hook): Ditto.
> +       (no_op): New global.
> +       (out_file): Make static.
> +       (is_api_exist): Make static.
> +       (process_option): New function.
> +       (onload): Return if no_op is set. Do not register handlers until
> +       necessary.
> +
>  2012-02-25  Sriraman Tallam  <tmsriram@google.com>
>
>        * function_reordering_plugin.c: Check for presence of elf.h.
> Index: gcc/cgraphbuild.c
> ===================================================================
> --- gcc/cgraphbuild.c   (revision 185372)
> +++ gcc/cgraphbuild.c   (working copy)
> @@ -703,10 +703,10 @@ extern bool cgraph_callee_edges_final_cleanup;
>  static unsigned int
>  remove_cgraph_callee_edges (void)
>  {
> -  /* The -fcallgraph-profiles-sections flag needs the call-graph preserved
> +  /* The -freorder-functions=* flag needs the call-graph preserved
>      till pass_final.  */
>   if (cgraph_callee_edges_final_cleanup
> -      && flag_callgraph_profiles_sections)
> +      && flag_reorder_functions > 1)
>       return 0;
>
>   cgraph_node_remove_callees (cgraph_node (current_function_decl));
> Index: gcc/configure
> ===================================================================
> --- gcc/configure       (revision 185372)
> +++ gcc/configure       (working copy)
> @@ -4861,7 +4861,7 @@ fi
>  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $acx_cv_cc_gcc_supports_ada"
>&5
>  $as_echo "$acx_cv_cc_gcc_supports_ada" >&6; }
>
> -if test x$GNATBIND != xno && test x$GNATMAKE != xno && test
x$acx_cv_cc_gcc_supports_ada != xno; then
> +if test "x$GNATBIND" != xno && test "x$GNATMAKE" != xno && test
x$acx_cv_cc_gcc_supports_ada != xno; then
>   have_gnat=yes
>  else
>   have_gnat=no
> @@ -11480,6 +11480,12 @@ esac
>
>
>  cat >>confdefs.h <<_ACEOF
> +#define FRPLUGINSONAME "${host_function_reordering_plugin_soname}"
> +_ACEOF
> +
> +
> +
> +cat >>confdefs.h <<_ACEOF
>  #define LTOPLUGINSONAME "${host_lto_plugin_soname}"
>  _ACEOF
>
> @@ -17572,7 +17578,7 @@ else
>   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>   lt_status=$lt_dlunknown
>   cat > conftest.$ac_ext <<_LT_EOF
> -#line 17575 "configure"
> +#line 17581 "configure"
>  #include "confdefs.h"
>
>  #if HAVE_DLFCN_H
> @@ -17678,7 +17684,7 @@ else
>   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>   lt_status=$lt_dlunknown
>   cat > conftest.$ac_ext <<_LT_EOF
> -#line 17681 "configure"
> +#line 17687 "configure"
>  #include "confdefs.h"
>
>  #if HAVE_DLFCN_H
> Index: gcc/final.c
> ===================================================================
> --- gcc/final.c (revision 185372)
> +++ gcc/final.c (working copy)
> @@ -4421,9 +4421,9 @@ rest_of_handle_final (void)
>                                decl_fini_priority_lookup
>                                  (current_function_decl));
>
> -  /* With -fcallgraph-profiles-sections, add ".gnu.callgraph.text" section
> +  /* With -freorder-functions=, add ".gnu.callgraph.text" section
>      for storing profiling information. */
> -  if (flag_callgraph_profiles_sections
> +  if (flag_reorder_functions > 1
>       && flag_profile_use
>       && cgraph_node (current_function_decl) != NULL
>       && (cgraph_node (current_function_decl))->callees != NULL)
> Index: gcc/gcc.c
> ===================================================================
> --- gcc/gcc.c   (revision 185372)
> +++ gcc/gcc.c   (working copy)
> @@ -663,6 +663,9 @@ proper position among the other output files.  */
>     -plugin-opt=-fresolution=%u.res \
>     %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}}
\
>     }"PLUGIN_COND_CLOSE" \
> +    %{freorder-functions=*: \
> +    -plugin %(func_reorder_linker_plugin_file) \
> +    -plugin-opt=%(func_reorder_linker_plugin_opt)} \
>     %{flto|flto=*:%<fcompare-debug*} \
>     %{flto} %{flto=*} %l " LINK_PIE_SPEC \
>    "%{fuse-ld=gold:%{fuse-ld=bfd:%e-fuse-ld=gold and -fuse-ld=bfd may not be
used together}} \
> @@ -717,6 +720,8 @@ static const char *endfile_spec = ENDFILE_SPEC;
>  static const char *startfile_spec = STARTFILE_SPEC;
>  static const char *linker_name_spec = LINKER_NAME;
>  static const char *linker_plugin_file_spec = "";
> +static const char *func_reorder_linker_plugin_file_spec = "";
> +static const char *func_reorder_linker_plugin_opt = "";
>  static const char *lto_wrapper_spec = "";
>  static const char *lto_gcc_spec = "";
>  static const char *link_command_spec = LINK_COMMAND_SPEC;
> @@ -1206,6 +1211,10 @@ static struct spec_list static_specs[] =
>   INIT_STATIC_SPEC ("multilib_options",                &multilib_options),
>   INIT_STATIC_SPEC ("linker",                  &linker_name_spec),
>   INIT_STATIC_SPEC ("linker_plugin_file",      &linker_plugin_file_spec),
> +  INIT_STATIC_SPEC ("func_reorder_linker_plugin_file",
> +                   &func_reorder_linker_plugin_file_spec),
> +  INIT_STATIC_SPEC ("func_reorder_linker_plugin_opt",
> +                   &func_reorder_linker_plugin_opt),
>   INIT_STATIC_SPEC ("lto_wrapper",             &lto_wrapper_spec),
>   INIT_STATIC_SPEC ("lto_gcc",                 &lto_gcc_spec),
>   INIT_STATIC_SPEC ("link_libgcc",             &link_libgcc_spec),
> @@ -6108,6 +6117,51 @@ compare_files (char *cmpfile[])
>   return ret;
>  }
>
> +/* Set func_reorder_linker_plugin_file_spec and
func_reorder_linker_plugin_opt
> +   here.  This is the linker plugin to do global function reordering and is
> +   enabled with -freorder-functions=*. */
> +
> +static void
> +set_func_reorder_linker_plugin_spec (void)
> +{
> +  int i;
> +  const char *plugin_opt_none = "group=none";
> +  const char *plugin_opt_cgedge = "group=cgedge";
> +
> +  /* Find the linker plugin that does function ordering.  */
> +  func_reorder_linker_plugin_file_spec = find_a_file (&exec_prefixes,
> +                                           FRPLUGINSONAME, R_OK, false);
> +
> +  if (!func_reorder_linker_plugin_file_spec)
> +      fatal_error ("-freorder-functions=*, but "
> +                  FRPLUGINSONAME " file not found");
> +
> +  func_reorder_linker_plugin_opt = plugin_opt_none;
> +
> +  /* Set linker plugin options here.  Option ordering is also checked here.
> +     -fno-reorder-functions or -freorder-functions should disable any
> +     previous -freorder-functions=*. */
> +  for (i = 0; (int) i < n_switches; i++)
> +    {
> +      /* Check for match with "-freorder-functions=cgedge".  */
> +      if (func_reorder_linker_plugin_opt != plugin_opt_cgedge
> +         && !strcmp (switches[i].part1, "freorder-functions=cgedge"))
> +       {
> +         func_reorder_linker_plugin_opt = plugin_opt_cgedge;
> +         continue;
> +       }
> +      /* Set option to none if it matches -fno-reorder-functions
> +        or -freorder-functions  */
> +      if (func_reorder_linker_plugin_opt != plugin_opt_none
> +         && (!strcmp (switches[i].part1, "fno-reorder-functions")
> +             || !strcmp (switches[i].part1, "freorder-functions")))
> +       {
> +         func_reorder_linker_plugin_opt = plugin_opt_none;
> +         continue;
> +       }
> +    }
> +}
> +
>  extern int main (int, char **);
>
>  int
> @@ -6843,6 +6897,8 @@ warranty; not even for MERCHANTABILITY or FITNESS
>       const char *fuse_linker_plugin = "fuse-linker-plugin";
>  #endif
>
> +      const char *freorder_functions_ = "freorder-functions=";
> +
>       /* We'll use ld if we can't find collect2.  */
>       if (! strcmp (linker_name_spec, "collect2"))
>        {
> @@ -6866,6 +6922,12 @@ warranty; not even for MERCHANTABILITY or FITNESS
>            fatal_error ("-fuse-linker-plugin, but " LTOPLUGINSONAME " not
found");
>        }
>       lto_gcc_spec = argv[0];
> +
> +      /* The function reordering linker plugin will be loaded if the option
> +        -freorder-functions= is present in the command-line.  */
> +      if (switch_matches (freorder_functions_,
> +                         freorder_functions_ + strlen (freorder_functions_),
1))
> +       set_func_reorder_linker_plugin_spec ();
>
>       /* Rebuild the COMPILER_PATH and LIBRARY_PATH environment variables
>         for collect.  */
> Index: gcc/config.in
> ===================================================================
> --- gcc/config.in       (revision 185372)
> +++ gcc/config.in       (working copy)
> @@ -1583,13 +1583,18 @@
>  #endif
>
>
> +/* Define to the name of the function reordering plugin DSO that must be
> +   passed to the linker's -plugin=LIB option. */
> +#ifndef USED_FOR_TARGET
> +#undef FRPLUGINSONAME
> +#endif
> +
>  /* Define to the name of the LTO plugin DSO that must be passed to the
>    linker's -plugin=LIB option. */
>  #ifndef USED_FOR_TARGET
>  #undef LTOPLUGINSONAME
>  #endif
>
> -
>  /* Define to the sub-directory in which libtool stores uninstalled libraries.
>    */
>  #ifndef USED_FOR_TARGET
> Index: gcc/configure.ac
> ===================================================================
> --- gcc/configure.ac    (revision 185372)
> +++ gcc/configure.ac    (working copy)
> @@ -1858,6 +1858,10 @@ case $use_collect2 in
>     ;;
>  esac
>
>
+AC_DEFINE_UNQUOTED(FRPLUGINSONAME,"${host_function_reordering_plugin_soname}",
> +[Define to the name of the function reordering plugin DSO that must be
> +  passed to the linker's -plugin=LIB option.])
> +
>  AC_DEFINE_UNQUOTED(LTOPLUGINSONAME,"${host_lto_plugin_soname}",
>  [Define to the name of the LTO plugin DSO that must be
>   passed to the linker's -plugin=LIB option.])
> Index: gcc/ChangeLog.google-4_6
> ===================================================================
> --- gcc/ChangeLog.google-4_6    (revision 185372)
> +++ gcc/ChangeLog.google-4_6    (working copy)
> @@ -1,3 +1,21 @@
> +2012-03-14  Sriraman Tallam  <tmsriram@google.com>
> +
> +       * cgraphbuild.c (remove_cgraph_callee_edges): Replace
> +       flag_callgraph_profiles_sections with flag_reorder_functions.
> +       * final.c (rest_of_handle_final): Ditto.
> +       * configure: Regenerate.
> +       * config.in: undef FRPLUGINSONAME.
> +       * congifure.ac: Define FRPLUGINSONAME.
> +       * config.host: Set host_function_reordering_plugin_soname.
> +       * gcc.c: Invoke function_reordering linker plugin with
> +       -freorder-functions=* option.
> +       (set_func_reorder_linker_plugin_spec): New function.
> +       (main): Call set_func_reorder_linker_plugin_spec when
> +       -freorder-functions=* option is seen.
> +       * common.opt: Add new option -freorder-functions=.  Remove
> +       -fcallgraph-profiles-sections.
> +
> +
>  2012-03-12  Sterling Augustine  <saugustine@google.com>
>            Cary Coutant  <ccoutant@google.com>
>
> Index: gcc/common.opt
> ===================================================================
> --- gcc/common.opt      (revision 185372)
> +++ gcc/common.opt      (working copy)
> @@ -920,10 +920,6 @@ fcaller-saves
>  Common Report Var(flag_caller_saves) Optimization
>  Save registers around function calls
>
> -fcallgraph-profiles-sections
> -Common Report Var(flag_callgraph_profiles_sections) Init(0)
> -Generate .note.callgraph.text sections listing callees and edge counts.
> -
>  fcheck-data-deps
>  Common Report Var(flag_check_data_deps)
>  Compare the results of several data dependence analyzers.
> @@ -1758,9 +1754,19 @@ Common Report Var(flag_reorder_blocks_and_partitio
>  Reorder basic blocks and partition into hot and cold sections
>
>  freorder-functions
> -Common Report Var(flag_reorder_functions) Optimization
> +Common Report Var(flag_reorder_functions,1) Init(0) Optimization
>  Reorder functions to improve code placement
>
> +freorder-functions=
> +Common Joined RejectNegative Enum(function_reorder)
Var(flag_reorder_functions) Init(0) Optimization
> +-freorder-functions=[cgedge]   Select the scheme for function reordering.
This invokes a linker plugin.  Generate .note.callgraph.text sections listing
callees and edge counts.
> +
> +Enum
> +Name(function_reorder) Type(int) UnknownError(unrecognized function reorder
value %qs)
> +
> +EnumValue
> +Enum(function_reorder) String(cgedge) Value(2)
> +
>  frerun-cse-after-loop
>  Common Report Var(flag_rerun_cse_after_loop) Optimization
>  Add a common subexpression elimination pass after loop optimizations
> Index: gcc/config.host
> ===================================================================
> --- gcc/config.host     (revision 185372)
> +++ gcc/config.host     (working copy)
> @@ -76,6 +76,7 @@ out_host_hook_obj=host-default.o
>  host_can_use_collect2=yes
>  use_long_long_for_widest_fast_int=no
>  host_lto_plugin_soname=liblto_plugin.so
> +host_function_reordering_plugin_soname=libfunction_reordering_plugin.so
>
>  # Unsupported hosts list.  Generally, only include hosts known to fail here,
>  # since we allow hosts not listed to be supported generically.
>
> --
> This patch is available for review at http://codereview.appspot.com/5825054

[davidxl, 2012-03-14 22:01:26]

thanks. This greatly improves the usability of the plugin based
function reordering feature and is in a shape that can be pushed to
trunk.

Regarding the sub-options, cgedge does not seem user friendly. How
about just -freorder-functions=callgraph or
-freorder-function=clustering?

What is the interaction with -ffunction-sections? Should it be turned
on when this flavor of reordering is on? Or at least a warning is
given ?

Thanks,

David

On Wed, Mar 14, 2012 at 1:56 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> This patch overloads GCC option -freorder-functions with -freoder-functions=*
which will invoke the linker plugin libfunction_reordering_plugin.so. For now,
the only accepted option is -freoder-functions=cgedge, where the functions which
are connected by hot callgraph edges are placed closer.  When
-freorder-functions is specified without the '=', the original behaviour is
maintained.  This patch also removes the -fcallgraph-profiles-sections option.
>
> For context, the function_reordering_plugin is only available in gcc-4_6
branch. I am working on porting this to trunk and will send out a patch soon.
>
>
>        * cgraphbuild.c (remove_cgraph_callee_edges): Replace
>        flag_callgraph_profiles_sections with flag_reorder_functions.
>        * final.c (rest_of_handle_final): Ditto.
>        * configure: Regenerate.
>        * config.in: undef FRPLUGINSONAME.
>        * congifure.ac: Define FRPLUGINSONAME.
>        * config.host: Set host_function_reordering_plugin_soname.
>        * gcc.c: Invoke function_reordering linker plugin with
>        -freorder-functions=* option.
>        (set_func_reorder_linker_plugin_spec): New function.
>        (main): Call set_func_reorder_linker_plugin_spec when
>        -freorder-functions=* option is seen.
>        * common.opt (fcallgraph-profiles-sections):  Remove.
>        (freorder-functions=): New option.
>
>        * function_reordering_plugin.c (register_claim_file_hook): New function
>        pointer.
>        (register_all_symbols_read_hook): Ditto.
>        (no_op): New global.
>        (out_file): Make static.
>        (is_api_exist): Make static.
>        (process_option): New function.
>        (onload): Return if no_op is set. Do not register handlers until
>        necessary.
>
>
> Index: function_reordering_plugin/function_reordering_plugin.c
> ===================================================================
> --- function_reordering_plugin/function_reordering_plugin.c     (revision
185372)
> +++ function_reordering_plugin/function_reordering_plugin.c     (working copy)
> @@ -64,6 +64,9 @@ enum ld_plugin_status claim_file_hook (const struc
>                                        int *claimed);
>  enum ld_plugin_status all_symbols_read_hook ();
>
> +static ld_plugin_register_claim_file register_claim_file_hook = NULL;
> +static ld_plugin_register_all_symbols_read
> +  register_all_symbols_read_hook = NULL;
>  static ld_plugin_get_input_section_count get_input_section_count = NULL;
>  static ld_plugin_get_input_section_type get_input_section_type = NULL;
>  static ld_plugin_get_input_section_name get_input_section_name = NULL;
> @@ -73,12 +76,16 @@ static ld_plugin_allow_section_ordering allow_sect
>
>  /* The file where the final function order will be stored.
>    It is "./final_layout.txt".  It can be changed by passing
> -   new name to --plugin-opt  */
> +   new name to --plugin-opt  as --plugin-opt "file=<name>".
> +   To dump to stderr, say --plugin-opt "file=stderr".  */
>
> -char *out_file = "./final_layout.txt";
> +static char *out_file = "./final_layout.txt";
>
> -int is_api_exist = 0;
> +static int is_api_exist = 0;
>
> +/* The plugin does nothing when no-op is 1.  */
> +static int no_op = 0;
> +
>  /* Copies new output file name out_file  */
>  void get_filename (const char *name)
>  {
> @@ -91,6 +98,39 @@ void get_filename (const char *name)
>   strcpy (out_file, name);
>  }
>
> +/* Process options to plugin.  Options with prefix "group=" are special.
> +   They specify the type of grouping. The option "group=none" makes the
> +   plugin do nothing.   Options with prefix "file=" set the output file
> +   where the final function order must be stored.  */
> +void
> +process_option (const char *name)
> +{
> +  const char *option_group = "group=";
> +  const char *option_file = "file=";
> +
> +  /* Check if option is "group="  */
> +  if (strncmp (name, option_group, strlen (option_group)) == 0)
> +    {
> +      if (strcmp (name + strlen (option_group), "none") == 0)
> +       no_op = 1;
> +      else
> +       no_op = 0;
> +      return;
> +    }
> +
> +  /* Check if option is "file=" */
> +  if (strncmp (name, option_file, strlen (option_file)) == 0)
> +    {
> +      get_filename (name + strlen (option_file));
> +      return;
> +    }
> +
> +  /* Unknown option, set no_op to 1.  */
> +  no_op = 1;
> +  fprintf (stderr, "Unknown option to function reordering plugin :%s\n",
> +          name);
> +}
> +
>  /* Plugin entry point.  */
>  enum ld_plugin_status
>  onload (struct ld_plugin_tv *tv)
> @@ -105,14 +145,16 @@ onload (struct ld_plugin_tv *tv)
>         case LDPT_GOLD_VERSION:
>           break;
>         case LDPT_OPTION:
> -         get_filename (entry->tv_u.tv_string);
> +         process_option (entry->tv_u.tv_string);
> +         /* If no_op is set, do not do anything else.  */
> +         if (no_op) return LDPS_OK;
>          break;
>         case LDPT_REGISTER_CLAIM_FILE_HOOK:
> -          assert ((*entry->tv_u.tv_register_claim_file) (claim_file_hook) ==
LDPS_OK);
> +         register_claim_file_hook = *entry->tv_u.tv_register_claim_file;
>           break;
>        case LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK:
> -          assert ((*entry->tv_u.tv_register_all_symbols_read)
(all_symbols_read_hook)
> -                  == LDPS_OK);
> +         register_all_symbols_read_hook
> +           = *entry->tv_u.tv_register_all_symbols_read;
>           break;
>         case LDPT_GET_INPUT_SECTION_COUNT:
>           get_input_section_count = *entry->tv_u.tv_get_input_section_count;
> @@ -137,14 +179,25 @@ onload (struct ld_plugin_tv *tv)
>         }
>     }
>
> -  if (get_input_section_count != NULL
> +  assert (!no_op);
> +
> +  if (register_all_symbols_read_hook != NULL
> +      && register_claim_file_hook != NULL
> +      && get_input_section_count != NULL
>       && get_input_section_type != NULL
>       && get_input_section_name != NULL
>       && get_input_section_contents != NULL
>       && update_section_order != NULL
>       && allow_section_ordering != NULL)
>     is_api_exist = 1;
> +  else
> +    return LDPS_OK;
>
> +  /* Register handlers.  */
> +  assert ((*register_all_symbols_read_hook) (all_symbols_read_hook)
> +          == LDPS_OK);
> +  assert ((*register_claim_file_hook) (claim_file_hook)
> +         == LDPS_OK);
>   return LDPS_OK;
>  }
>
> @@ -161,9 +214,8 @@ claim_file_hook (const struct ld_plugin_input_file
>
>   (void) claimed;
>
> -  /* Silently return if the plugin APIs are not supported.  */
> -  if (!is_api_exist)
> -    return LDPS_OK;
> +  /* Plugin APIs are supported if this is called.  */
> +  assert (is_api_exist);
>
>   if (is_ordering_specified == 0)
>     {
> @@ -221,9 +273,8 @@ all_symbols_read_hook (void)
>   unsigned int *shndx;
>   FILE *fp;
>
> -  /* Silently return if the plugin APIs are not supported.  */
> -  if (!is_api_exist)
> -    return LDPS_OK;
> +  /* Plugin APIs are supported if this is called.  */
> +  assert (is_api_exist);
>
>   if (is_callgraph_empty ())
>     return LDPS_OK;
> Index: function_reordering_plugin/ChangeLog.google-4_6
> ===================================================================
> --- function_reordering_plugin/ChangeLog.google-4_6     (revision 185372)
> +++ function_reordering_plugin/ChangeLog.google-4_6     (working copy)
> @@ -1,3 +1,15 @@
> +2012-03-14  Sriraman Tallam  <tmsriram@google.com>
> +
> +       * function_reordering_plugin.c (register_claim_file_hook): New
function
> +       pointer.
> +       (register_all_symbols_read_hook): Ditto.
> +       (no_op): New global.
> +       (out_file): Make static.
> +       (is_api_exist): Make static.
> +       (process_option): New function.
> +       (onload): Return if no_op is set. Do not register handlers until
> +       necessary.
> +
>  2012-02-25  Sriraman Tallam  <tmsriram@google.com>
>
>        * function_reordering_plugin.c: Check for presence of elf.h.
> Index: gcc/cgraphbuild.c
> ===================================================================
> --- gcc/cgraphbuild.c   (revision 185372)
> +++ gcc/cgraphbuild.c   (working copy)
> @@ -703,10 +703,10 @@ extern bool cgraph_callee_edges_final_cleanup;
>  static unsigned int
>  remove_cgraph_callee_edges (void)
>  {
> -  /* The -fcallgraph-profiles-sections flag needs the call-graph preserved
> +  /* The -freorder-functions=* flag needs the call-graph preserved
>      till pass_final.  */
>   if (cgraph_callee_edges_final_cleanup
> -      && flag_callgraph_profiles_sections)
> +      && flag_reorder_functions > 1)
>       return 0;
>
>   cgraph_node_remove_callees (cgraph_node (current_function_decl));
> Index: gcc/configure
> ===================================================================
> --- gcc/configure       (revision 185372)
> +++ gcc/configure       (working copy)
> @@ -4861,7 +4861,7 @@ fi
>  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $acx_cv_cc_gcc_supports_ada"
>&5
>  $as_echo "$acx_cv_cc_gcc_supports_ada" >&6; }
>
> -if test x$GNATBIND != xno && test x$GNATMAKE != xno && test
x$acx_cv_cc_gcc_supports_ada != xno; then
> +if test "x$GNATBIND" != xno && test "x$GNATMAKE" != xno && test
x$acx_cv_cc_gcc_supports_ada != xno; then
>   have_gnat=yes
>  else
>   have_gnat=no
> @@ -11480,6 +11480,12 @@ esac
>
>
>  cat >>confdefs.h <<_ACEOF
> +#define FRPLUGINSONAME "${host_function_reordering_plugin_soname}"
> +_ACEOF
> +
> +
> +
> +cat >>confdefs.h <<_ACEOF
>  #define LTOPLUGINSONAME "${host_lto_plugin_soname}"
>  _ACEOF
>
> @@ -17572,7 +17578,7 @@ else
>   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>   lt_status=$lt_dlunknown
>   cat > conftest.$ac_ext <<_LT_EOF
> -#line 17575 "configure"
> +#line 17581 "configure"
>  #include "confdefs.h"
>
>  #if HAVE_DLFCN_H
> @@ -17678,7 +17684,7 @@ else
>   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>   lt_status=$lt_dlunknown
>   cat > conftest.$ac_ext <<_LT_EOF
> -#line 17681 "configure"
> +#line 17687 "configure"
>  #include "confdefs.h"
>
>  #if HAVE_DLFCN_H
> Index: gcc/final.c
> ===================================================================
> --- gcc/final.c (revision 185372)
> +++ gcc/final.c (working copy)
> @@ -4421,9 +4421,9 @@ rest_of_handle_final (void)
>                                decl_fini_priority_lookup
>                                  (current_function_decl));
>
> -  /* With -fcallgraph-profiles-sections, add ".gnu.callgraph.text" section
> +  /* With -freorder-functions=, add ".gnu.callgraph.text" section
>      for storing profiling information. */
> -  if (flag_callgraph_profiles_sections
> +  if (flag_reorder_functions > 1
>       && flag_profile_use
>       && cgraph_node (current_function_decl) != NULL
>       && (cgraph_node (current_function_decl))->callees != NULL)
> Index: gcc/gcc.c
> ===================================================================
> --- gcc/gcc.c   (revision 185372)
> +++ gcc/gcc.c   (working copy)
> @@ -663,6 +663,9 @@ proper position among the other output files.  */
>     -plugin-opt=-fresolution=%u.res \
>     %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}}
\
>     }"PLUGIN_COND_CLOSE" \
> +    %{freorder-functions=*: \
> +    -plugin %(func_reorder_linker_plugin_file) \
> +    -plugin-opt=%(func_reorder_linker_plugin_opt)} \
>     %{flto|flto=*:%<fcompare-debug*} \
>     %{flto} %{flto=*} %l " LINK_PIE_SPEC \
>    "%{fuse-ld=gold:%{fuse-ld=bfd:%e-fuse-ld=gold and -fuse-ld=bfd may not be
used together}} \
> @@ -717,6 +720,8 @@ static const char *endfile_spec = ENDFILE_SPEC;
>  static const char *startfile_spec = STARTFILE_SPEC;
>  static const char *linker_name_spec = LINKER_NAME;
>  static const char *linker_plugin_file_spec = "";
> +static const char *func_reorder_linker_plugin_file_spec = "";
> +static const char *func_reorder_linker_plugin_opt = "";
>  static const char *lto_wrapper_spec = "";
>  static const char *lto_gcc_spec = "";
>  static const char *link_command_spec = LINK_COMMAND_SPEC;
> @@ -1206,6 +1211,10 @@ static struct spec_list static_specs[] =
>   INIT_STATIC_SPEC ("multilib_options",                &multilib_options),
>   INIT_STATIC_SPEC ("linker",                  &linker_name_spec),
>   INIT_STATIC_SPEC ("linker_plugin_file",      &linker_plugin_file_spec),
> +  INIT_STATIC_SPEC ("func_reorder_linker_plugin_file",
> +                   &func_reorder_linker_plugin_file_spec),
> +  INIT_STATIC_SPEC ("func_reorder_linker_plugin_opt",
> +                   &func_reorder_linker_plugin_opt),
>   INIT_STATIC_SPEC ("lto_wrapper",             &lto_wrapper_spec),
>   INIT_STATIC_SPEC ("lto_gcc",                 &lto_gcc_spec),
>   INIT_STATIC_SPEC ("link_libgcc",             &link_libgcc_spec),
> @@ -6108,6 +6117,51 @@ compare_files (char *cmpfile[])
>   return ret;
>  }
>
> +/* Set func_reorder_linker_plugin_file_spec and
func_reorder_linker_plugin_opt
> +   here.  This is the linker plugin to do global function reordering and is
> +   enabled with -freorder-functions=*. */
> +
> +static void
> +set_func_reorder_linker_plugin_spec (void)
> +{
> +  int i;
> +  const char *plugin_opt_none = "group=none";
> +  const char *plugin_opt_cgedge = "group=cgedge";
> +
> +  /* Find the linker plugin that does function ordering.  */
> +  func_reorder_linker_plugin_file_spec = find_a_file (&exec_prefixes,
> +                                           FRPLUGINSONAME, R_OK, false);
> +
> +  if (!func_reorder_linker_plugin_file_spec)
> +      fatal_error ("-freorder-functions=*, but "
> +                  FRPLUGINSONAME " file not found");
> +
> +  func_reorder_linker_plugin_opt = plugin_opt_none;
> +
> +  /* Set linker plugin options here.  Option ordering is also checked here.
> +     -fno-reorder-functions or -freorder-functions should disable any
> +     previous -freorder-functions=*. */
> +  for (i = 0; (int) i < n_switches; i++)
> +    {
> +      /* Check for match with "-freorder-functions=cgedge".  */
> +      if (func_reorder_linker_plugin_opt != plugin_opt_cgedge
> +         && !strcmp (switches[i].part1, "freorder-functions=cgedge"))
> +       {
> +         func_reorder_linker_plugin_opt = plugin_opt_cgedge;
> +         continue;
> +       }
> +      /* Set option to none if it matches -fno-reorder-functions
> +        or -freorder-functions  */
> +      if (func_reorder_linker_plugin_opt != plugin_opt_none
> +         && (!strcmp (switches[i].part1, "fno-reorder-functions")
> +             || !strcmp (switches[i].part1, "freorder-functions")))
> +       {
> +         func_reorder_linker_plugin_opt = plugin_opt_none;
> +         continue;
> +       }
> +    }
> +}
> +
>  extern int main (int, char **);
>
>  int
> @@ -6843,6 +6897,8 @@ warranty; not even for MERCHANTABILITY or FITNESS
>       const char *fuse_linker_plugin = "fuse-linker-plugin";
>  #endif
>
> +      const char *freorder_functions_ = "freorder-functions=";
> +
>       /* We'll use ld if we can't find collect2.  */
>       if (! strcmp (linker_name_spec, "collect2"))
>        {
> @@ -6866,6 +6922,12 @@ warranty; not even for MERCHANTABILITY or FITNESS
>            fatal_error ("-fuse-linker-plugin, but " LTOPLUGINSONAME " not
found");
>        }
>       lto_gcc_spec = argv[0];
> +
> +      /* The function reordering linker plugin will be loaded if the option
> +        -freorder-functions= is present in the command-line.  */
> +      if (switch_matches (freorder_functions_,
> +                         freorder_functions_ + strlen (freorder_functions_),
1))
> +       set_func_reorder_linker_plugin_spec ();
>
>       /* Rebuild the COMPILER_PATH and LIBRARY_PATH environment variables
>         for collect.  */
> Index: gcc/config.in
> ===================================================================
> --- gcc/config.in       (revision 185372)
> +++ gcc/config.in       (working copy)
> @@ -1583,13 +1583,18 @@
>  #endif
>
>
> +/* Define to the name of the function reordering plugin DSO that must be
> +   passed to the linker's -plugin=LIB option. */
> +#ifndef USED_FOR_TARGET
> +#undef FRPLUGINSONAME
> +#endif
> +
>  /* Define to the name of the LTO plugin DSO that must be passed to the
>    linker's -plugin=LIB option. */
>  #ifndef USED_FOR_TARGET
>  #undef LTOPLUGINSONAME
>  #endif
>
> -
>  /* Define to the sub-directory in which libtool stores uninstalled libraries.
>    */
>  #ifndef USED_FOR_TARGET
> Index: gcc/configure.ac
> ===================================================================
> --- gcc/configure.ac    (revision 185372)
> +++ gcc/configure.ac    (working copy)
> @@ -1858,6 +1858,10 @@ case $use_collect2 in
>     ;;
>  esac
>
>
+AC_DEFINE_UNQUOTED(FRPLUGINSONAME,"${host_function_reordering_plugin_soname}",
> +[Define to the name of the function reordering plugin DSO that must be
> +  passed to the linker's -plugin=LIB option.])
> +
>  AC_DEFINE_UNQUOTED(LTOPLUGINSONAME,"${host_lto_plugin_soname}",
>  [Define to the name of the LTO plugin DSO that must be
>   passed to the linker's -plugin=LIB option.])
> Index: gcc/ChangeLog.google-4_6
> ===================================================================
> --- gcc/ChangeLog.google-4_6    (revision 185372)
> +++ gcc/ChangeLog.google-4_6    (working copy)
> @@ -1,3 +1,21 @@
> +2012-03-14  Sriraman Tallam  <tmsriram@google.com>
> +
> +       * cgraphbuild.c (remove_cgraph_callee_edges): Replace
> +       flag_callgraph_profiles_sections with flag_reorder_functions.
> +       * final.c (rest_of_handle_final): Ditto.
> +       * configure: Regenerate.
> +       * config.in: undef FRPLUGINSONAME.
> +       * congifure.ac: Define FRPLUGINSONAME.
> +       * config.host: Set host_function_reordering_plugin_soname.
> +       * gcc.c: Invoke function_reordering linker plugin with
> +       -freorder-functions=* option.
> +       (set_func_reorder_linker_plugin_spec): New function.
> +       (main): Call set_func_reorder_linker_plugin_spec when
> +       -freorder-functions=* option is seen.
> +       * common.opt: Add new option -freorder-functions=.  Remove
> +       -fcallgraph-profiles-sections.
> +
> +
>  2012-03-12  Sterling Augustine  <saugustine@google.com>
>            Cary Coutant  <ccoutant@google.com>
>
> Index: gcc/common.opt
> ===================================================================
> --- gcc/common.opt      (revision 185372)
> +++ gcc/common.opt      (working copy)
> @@ -920,10 +920,6 @@ fcaller-saves
>  Common Report Var(flag_caller_saves) Optimization
>  Save registers around function calls
>
> -fcallgraph-profiles-sections
> -Common Report Var(flag_callgraph_profiles_sections) Init(0)
> -Generate .note.callgraph.text sections listing callees and edge counts.
> -
>  fcheck-data-deps
>  Common Report Var(flag_check_data_deps)
>  Compare the results of several data dependence analyzers.
> @@ -1758,9 +1754,19 @@ Common Report Var(flag_reorder_blocks_and_partitio
>  Reorder basic blocks and partition into hot and cold sections
>
>  freorder-functions
> -Common Report Var(flag_reorder_functions) Optimization
> +Common Report Var(flag_reorder_functions,1) Init(0) Optimization
>  Reorder functions to improve code placement
>
> +freorder-functions=
> +Common Joined RejectNegative Enum(function_reorder)
Var(flag_reorder_functions) Init(0) Optimization
> +-freorder-functions=[cgedge]   Select the scheme for function reordering.
This invokes a linker plugin.  Generate .note.callgraph.text sections listing
callees and edge counts.
> +
> +Enum
> +Name(function_reorder) Type(int) UnknownError(unrecognized function reorder
value %qs)
> +
> +EnumValue
> +Enum(function_reorder) String(cgedge) Value(2)
> +
>  frerun-cse-after-loop
>  Common Report Var(flag_rerun_cse_after_loop) Optimization
>  Add a common subexpression elimination pass after loop optimizations
> Index: gcc/config.host
> ===================================================================
> --- gcc/config.host     (revision 185372)
> +++ gcc/config.host     (working copy)
> @@ -76,6 +76,7 @@ out_host_hook_obj=host-default.o
>  host_can_use_collect2=yes
>  use_long_long_for_widest_fast_int=no
>  host_lto_plugin_soname=liblto_plugin.so
> +host_function_reordering_plugin_soname=libfunction_reordering_plugin.so
>
>  # Unsupported hosts list.  Generally, only include hosts known to fail here,
>  # since we allow hosts not listed to be supported generically.
>
> --
> This patch is available for review at http://codereview.appspot.com/5825054

[Sriraman, 2012-03-15 01:56:10]

Made the following changes:

* -ffunction-sections turned on with -freorder-functions=
* Change name from cgedge to callgraph.
* The plugin should not dump final layout to file by default.


	* cgraphbuild.c (remove_cgraph_callee_edges): Replace
	flag_callgraph_profiles_sections with flag_reorder_functions.
	* final.c (rest_of_handle_final): Ditto.
	* configure: Regenerate.
	* config.in: undef FRPLUGINSONAME.
	* congifure.ac: Define FRPLUGINSONAME.
	* config.host: Set host_function_reordering_plugin_soname.
	* gcc.c: Invoke function_reordering linker plugin with
	-freorder-functions=* option.
	(set_func_reorder_linker_plugin_spec): New function.
	(main): Call set_func_reorder_linker_plugin_spec when
	-freorder-functions=* option is seen.
	* common.opt (fcallgraph-profiles-sections):  Remove.
	(freorder-functions=): New option.
	* testsuite/g++.dg/tree-prof/callgraph-profiles.C: Replace
	-fcallgraph-profiles-sections with -freorder-functions=callgraph.
	* opts.c (finish_options): Turn on -ffunction-sections when
	-freorder-functions= is used.

	* function_reordering_plugin.c (register_claim_file_hook): New function
	pointer.
	(register_all_symbols_read_hook): Ditto.
	(no_op): New global.
	(out_file): Make static.
	(is_api_exist): Make static.
	(process_option): New function.
	(onload): Return if no_op is set. Do not register handlers until
	necessary.
	(get_load): Check if out_file is NULL before opening & closing the
	file.
	* callgraph.c (find_pettis_hansen_function_layout):
	(get_layout): Dump only when fp is not NULL.


Index: gcc/cgraphbuild.c
===================================================================
--- gcc/cgraphbuild.c	(revision 185372)
+++ gcc/cgraphbuild.c	(working copy)
@@ -703,10 +703,10 @@ extern bool cgraph_callee_edges_final_cleanup;
 static unsigned int
 remove_cgraph_callee_edges (void)
 {
-  /* The -fcallgraph-profiles-sections flag needs the call-graph preserved
+  /* The -freorder-functions=* flag needs the call-graph preserved
      till pass_final.  */
   if (cgraph_callee_edges_final_cleanup
-      && flag_callgraph_profiles_sections)
+      && flag_reorder_functions > 1)
       return 0;
 
   cgraph_node_remove_callees (cgraph_node (current_function_decl));
Index: gcc/configure
===================================================================
--- gcc/configure	(revision 185372)
+++ gcc/configure	(working copy)
@@ -4861,7 +4861,7 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $acx_cv_cc_gcc_supports_ada"
>&5
 $as_echo "$acx_cv_cc_gcc_supports_ada" >&6; }
 
-if test x$GNATBIND != xno && test x$GNATMAKE != xno && test
x$acx_cv_cc_gcc_supports_ada != xno; then
+if test "x$GNATBIND" != xno && test "x$GNATMAKE" != xno && test
x$acx_cv_cc_gcc_supports_ada != xno; then
   have_gnat=yes
 else
   have_gnat=no
@@ -11480,6 +11480,12 @@ esac
 
 
 cat >>confdefs.h <<_ACEOF
+#define FRPLUGINSONAME "${host_function_reordering_plugin_soname}"
+_ACEOF
+
+
+
+cat >>confdefs.h <<_ACEOF
 #define LTOPLUGINSONAME "${host_lto_plugin_soname}"
 _ACEOF
 
@@ -17572,7 +17578,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 17575 "configure"
+#line 17581 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -17678,7 +17684,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 17681 "configure"
+#line 17687 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
Index: gcc/final.c
===================================================================
--- gcc/final.c	(revision 185372)
+++ gcc/final.c	(working copy)
@@ -4421,9 +4421,9 @@ rest_of_handle_final (void)
 				decl_fini_priority_lookup
 				  (current_function_decl));
 
-  /* With -fcallgraph-profiles-sections, add ".gnu.callgraph.text" section
+  /* With -freorder-functions=, add ".gnu.callgraph.text" section
      for storing profiling information. */
-  if (flag_callgraph_profiles_sections
+  if (flag_reorder_functions > 1
       && flag_profile_use
       && cgraph_node (current_function_decl) != NULL
       && (cgraph_node (current_function_decl))->callees != NULL)
Index: gcc/gcc.c
===================================================================
--- gcc/gcc.c	(revision 185372)
+++ gcc/gcc.c	(working copy)
@@ -663,6 +663,9 @@ proper position among the other output files.  */
     -plugin-opt=-fresolution=%u.res \
     %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}}
\
     }"PLUGIN_COND_CLOSE" \
+    %{freorder-functions=*: \
+    -plugin %(func_reorder_linker_plugin_file) \
+    -plugin-opt=%(func_reorder_linker_plugin_opt)} \
     %{flto|flto=*:%<fcompare-debug*} \
     %{flto} %{flto=*} %l " LINK_PIE_SPEC \
    "%{fuse-ld=gold:%{fuse-ld=bfd:%e-fuse-ld=gold and -fuse-ld=bfd may not be
used together}} \
@@ -717,6 +720,8 @@ static const char *endfile_spec = ENDFILE_SPEC;
 static const char *startfile_spec = STARTFILE_SPEC;
 static const char *linker_name_spec = LINKER_NAME;
 static const char *linker_plugin_file_spec = "";
+static const char *func_reorder_linker_plugin_file_spec = "";
+static const char *func_reorder_linker_plugin_opt = "";
 static const char *lto_wrapper_spec = "";
 static const char *lto_gcc_spec = "";
 static const char *link_command_spec = LINK_COMMAND_SPEC;
@@ -1206,6 +1211,10 @@ static struct spec_list static_specs[] =
   INIT_STATIC_SPEC ("multilib_options",		&multilib_options),
   INIT_STATIC_SPEC ("linker",			&linker_name_spec),
   INIT_STATIC_SPEC ("linker_plugin_file",	&linker_plugin_file_spec),
+  INIT_STATIC_SPEC ("func_reorder_linker_plugin_file",
+		    &func_reorder_linker_plugin_file_spec),
+  INIT_STATIC_SPEC ("func_reorder_linker_plugin_opt",
+	  	    &func_reorder_linker_plugin_opt),
   INIT_STATIC_SPEC ("lto_wrapper",		&lto_wrapper_spec),
   INIT_STATIC_SPEC ("lto_gcc",			&lto_gcc_spec),
   INIT_STATIC_SPEC ("link_libgcc",		&link_libgcc_spec),
@@ -6108,6 +6117,51 @@ compare_files (char *cmpfile[])
   return ret;
 }
 
+/* Set func_reorder_linker_plugin_file_spec and func_reorder_linker_plugin_opt
+   here.  This is the linker plugin to do global function reordering and is
+   enabled with -freorder-functions=*. */
+
+static void
+set_func_reorder_linker_plugin_spec (void)
+{
+  int i;
+  const char *plugin_opt_none = "group=none";
+  const char *plugin_opt_callgraph = "group=callgraph";
+  
+  /* Find the linker plugin that does function ordering.  */
+  func_reorder_linker_plugin_file_spec = find_a_file (&exec_prefixes,
+					    FRPLUGINSONAME, R_OK, false);
+
+  if (!func_reorder_linker_plugin_file_spec)
+      fatal_error ("-freorder-functions=*, but "
+		   FRPLUGINSONAME " file not found");
+
+  func_reorder_linker_plugin_opt = plugin_opt_none;
+
+  /* Set linker plugin options here.  Option ordering is also checked here.
+     -fno-reorder-functions or -freorder-functions should disable any
+     previous -freorder-functions=*. */
+  for (i = 0; (int) i < n_switches; i++)
+    {
+      /* Check for match with "-freorder-functions=callgraph".  */
+      if (func_reorder_linker_plugin_opt != plugin_opt_callgraph
+	  && !strcmp (switches[i].part1, "freorder-functions=callgraph"))
+	{
+	  func_reorder_linker_plugin_opt = plugin_opt_callgraph;
+	  continue;
+	}
+      /* Set option to none if it matches -fno-reorder-functions
+	 or -freorder-functions  */
+      if (func_reorder_linker_plugin_opt != plugin_opt_none
+	  && (!strcmp (switches[i].part1, "fno-reorder-functions")
+	      || !strcmp (switches[i].part1, "freorder-functions")))
+	{
+	  func_reorder_linker_plugin_opt = plugin_opt_none;
+	  continue;
+	}
+    }
+}
+
 extern int main (int, char **);
 
 int
@@ -6843,6 +6897,8 @@ warranty; not even for MERCHANTABILITY or FITNESS
       const char *fuse_linker_plugin = "fuse-linker-plugin";
 #endif
 
+      const char *freorder_functions_ = "freorder-functions=";
+
       /* We'll use ld if we can't find collect2.  */
       if (! strcmp (linker_name_spec, "collect2"))
 	{
@@ -6866,6 +6922,12 @@ warranty; not even for MERCHANTABILITY or FITNESS
 	    fatal_error ("-fuse-linker-plugin, but " LTOPLUGINSONAME " not found");
 	}
       lto_gcc_spec = argv[0];
+  
+      /* The function reordering linker plugin will be loaded if the option
+	 -freorder-functions= is present in the command-line.  */ 
+      if (switch_matches (freorder_functions_,
+			  freorder_functions_ + strlen (freorder_functions_), 1))
+	set_func_reorder_linker_plugin_spec ();
 
       /* Rebuild the COMPILER_PATH and LIBRARY_PATH environment variables
 	 for collect.  */
Index: gcc/testsuite/g++.dg/tree-prof/callgraph-profiles.C
===================================================================
--- gcc/testsuite/g++.dg/tree-prof/callgraph-profiles.C	(revision 185372)
+++ gcc/testsuite/g++.dg/tree-prof/callgraph-profiles.C	(working copy)
@@ -1,6 +1,6 @@
 /* Verify if call-graph profile sections are created
-   with -fcallgraph-profiles-sections. */
-/* { dg-options "-O2 -fcallgraph-profiles-sections -ffunction-sections
--save-temps" } */
+   with -freorder-functions=. */
+/* { dg-options "-O2 -freorder-functions=callgraph -ffunction-sections
--save-temps" } */
 /* { dg-require-section-exclude "" } */
 
 int __attribute__ ((noinline))
Index: gcc/config.in
===================================================================
--- gcc/config.in	(revision 185372)
+++ gcc/config.in	(working copy)
@@ -1583,13 +1583,18 @@
 #endif
 
 
+/* Define to the name of the function reordering plugin DSO that must be
+   passed to the linker's -plugin=LIB option. */
+#ifndef USED_FOR_TARGET
+#undef FRPLUGINSONAME
+#endif
+
 /* Define to the name of the LTO plugin DSO that must be passed to the
    linker's -plugin=LIB option. */
 #ifndef USED_FOR_TARGET
 #undef LTOPLUGINSONAME
 #endif
 
-
 /* Define to the sub-directory in which libtool stores uninstalled libraries.
    */
 #ifndef USED_FOR_TARGET
Index: gcc/opts.c
===================================================================
--- gcc/opts.c	(revision 185372)
+++ gcc/opts.c	(working copy)
@@ -846,6 +846,10 @@ finish_options (struct gcc_options *opts, struct g
 	(PARAM_MAX_INLINE_INSNS_AUTO, 1000,
 	 opts->x_param_values, opts_set->x_param_values);
     }
+
+  /* Turn on -ffunction-sections when -freorder-functions=* is used.  */
+  if (opts->x_flag_reorder_functions > 1)
+    opts->x_flag_function_sections = 1;
 }
 
 #define LEFT_COLUMN	27
Index: gcc/configure.ac
===================================================================
--- gcc/configure.ac	(revision 185372)
+++ gcc/configure.ac	(working copy)
@@ -1858,6 +1858,10 @@ case $use_collect2 in
     ;;
 esac
 
+AC_DEFINE_UNQUOTED(FRPLUGINSONAME,"${host_function_reordering_plugin_soname}",
+[Define to the name of the function reordering plugin DSO that must be
+  passed to the linker's -plugin=LIB option.])
+
 AC_DEFINE_UNQUOTED(LTOPLUGINSONAME,"${host_lto_plugin_soname}",
 [Define to the name of the LTO plugin DSO that must be
   passed to the linker's -plugin=LIB option.])
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 185372)
+++ gcc/common.opt	(working copy)
@@ -920,10 +920,6 @@ fcaller-saves
 Common Report Var(flag_caller_saves) Optimization
 Save registers around function calls
 
-fcallgraph-profiles-sections
-Common Report Var(flag_callgraph_profiles_sections) Init(0)
-Generate .note.callgraph.text sections listing callees and edge counts.
-
 fcheck-data-deps
 Common Report Var(flag_check_data_deps)
 Compare the results of several data dependence analyzers.
@@ -1758,9 +1754,19 @@ Common Report Var(flag_reorder_blocks_and_partitio
 Reorder basic blocks and partition into hot and cold sections
 
 freorder-functions
-Common Report Var(flag_reorder_functions) Optimization
+Common Report Var(flag_reorder_functions,1) Init(0) Optimization
 Reorder functions to improve code placement
 
+freorder-functions=
+Common Joined RejectNegative Enum(function_reorder) Var(flag_reorder_functions)
Init(0) Optimization
+-freorder-functions=[callgraph]	Select the scheme for function reordering. This
invokes a linker plugin.  Generate .note.callgraph.text sections listing callees
and edge counts.
+
+Enum
+Name(function_reorder) Type(int) UnknownError(unrecognized function reorder
value %qs)
+ 
+EnumValue
+Enum(function_reorder) String(callgraph) Value(2)
+
 frerun-cse-after-loop
 Common Report Var(flag_rerun_cse_after_loop) Optimization
 Add a common subexpression elimination pass after loop optimizations
Index: gcc/config.host
===================================================================
--- gcc/config.host	(revision 185372)
+++ gcc/config.host	(working copy)
@@ -76,6 +76,7 @@ out_host_hook_obj=host-default.o
 host_can_use_collect2=yes
 use_long_long_for_widest_fast_int=no
 host_lto_plugin_soname=liblto_plugin.so
+host_function_reordering_plugin_soname=libfunction_reordering_plugin.so
 
 # Unsupported hosts list.  Generally, only include hosts known to fail here,
 # since we allow hosts not listed to be supported generically.
Index: function_reordering_plugin/function_reordering_plugin.c
===================================================================
--- function_reordering_plugin/function_reordering_plugin.c	(revision 185372)
+++ function_reordering_plugin/function_reordering_plugin.c	(working copy)
@@ -64,6 +64,9 @@ enum ld_plugin_status claim_file_hook (const struc
                                        int *claimed);
 enum ld_plugin_status all_symbols_read_hook ();
 
+static ld_plugin_register_claim_file register_claim_file_hook = NULL;
+static ld_plugin_register_all_symbols_read
+  register_all_symbols_read_hook = NULL;
 static ld_plugin_get_input_section_count get_input_section_count = NULL;
 static ld_plugin_get_input_section_type get_input_section_type = NULL;
 static ld_plugin_get_input_section_name get_input_section_name = NULL;
@@ -72,23 +75,54 @@ static ld_plugin_update_section_order update_secti
 static ld_plugin_allow_section_ordering allow_section_ordering = NULL;
 
 /* The file where the final function order will be stored.
-   It is "./final_layout.txt".  It can be changed by passing
-   new name to --plugin-opt  */
+   It can be set by using the  plugin option  as --plugin-opt
+   "file=<name>".  To dump to stderr, say --plugin-opt "file=stderr".  */
 
-char *out_file = "./final_layout.txt";
+static char *out_file = NULL;
 
-int is_api_exist = 0;
+static int is_api_exist = 0;
 
+/* The plugin does nothing when no-op is 1.  */
+static int no_op = 0;
+
 /* Copies new output file name out_file  */
 void get_filename (const char *name)
 {
-  if (strcmp (name, "stderr") == 0)
+  out_file = (char *) malloc (strlen (name) + 1);
+  strcpy (out_file, name);
+}
+
+/* Process options to plugin.  Options with prefix "group=" are special.
+   They specify the type of grouping. The option "group=none" makes the
+   plugin do nothing.   Options with prefix "file=" set the output file
+   where the final function order must be stored.  */
+void
+process_option (const char *name)
+{
+  const char *option_group = "group=";
+  const char *option_file = "file=";
+
+  /* Check if option is "group="  */
+  if (strncmp (name, option_group, strlen (option_group)) == 0)
     {
-      out_file = NULL;
+      if (strcmp (name + strlen (option_group), "none") == 0)
+	no_op = 1;
+      else
+	no_op = 0;
       return;
     }
-  out_file = (char *) malloc (strlen (name) + 1);
-  strcpy (out_file, name);
+
+  /* Check if option is "file=" */
+  if (strncmp (name, option_file, strlen (option_file)) == 0)
+    {
+      get_filename (name + strlen (option_file));
+      return;
+    }
+
+  /* Unknown option, set no_op to 1.  */
+  no_op = 1;
+  fprintf (stderr, "Unknown option to function reordering plugin :%s\n",
+	   name);
 }
 
 /* Plugin entry point.  */
@@ -105,14 +139,16 @@ onload (struct ld_plugin_tv *tv)
         case LDPT_GOLD_VERSION:
           break;
         case LDPT_OPTION:
-	  get_filename (entry->tv_u.tv_string);
+	  process_option (entry->tv_u.tv_string);
+	  /* If no_op is set, do not do anything else.  */
+	  if (no_op) return LDPS_OK;
 	  break;
         case LDPT_REGISTER_CLAIM_FILE_HOOK:
-          assert ((*entry->tv_u.tv_register_claim_file) (claim_file_hook) ==
LDPS_OK);
+	  register_claim_file_hook = *entry->tv_u.tv_register_claim_file;
           break;
 	case LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK:
-          assert ((*entry->tv_u.tv_register_all_symbols_read)
(all_symbols_read_hook)
-		   == LDPS_OK);
+	  register_all_symbols_read_hook
+	    = *entry->tv_u.tv_register_all_symbols_read;
           break;
         case LDPT_GET_INPUT_SECTION_COUNT:
           get_input_section_count = *entry->tv_u.tv_get_input_section_count;
@@ -137,14 +173,25 @@ onload (struct ld_plugin_tv *tv)
         }
     }
 
-  if (get_input_section_count != NULL
+  assert (!no_op);
+
+  if (register_all_symbols_read_hook != NULL
+      && register_claim_file_hook != NULL
+      && get_input_section_count != NULL
       && get_input_section_type != NULL
       && get_input_section_name != NULL
       && get_input_section_contents != NULL
       && update_section_order != NULL
       && allow_section_ordering != NULL)
     is_api_exist = 1;
+  else
+    return LDPS_OK;
 
+  /* Register handlers.  */
+  assert ((*register_all_symbols_read_hook) (all_symbols_read_hook)
+	   == LDPS_OK);
+  assert ((*register_claim_file_hook) (claim_file_hook)
+	  == LDPS_OK);
   return LDPS_OK;
 }
 
@@ -161,9 +208,8 @@ claim_file_hook (const struct ld_plugin_input_file
 
   (void) claimed;
 
-  /* Silently return if the plugin APIs are not supported.  */
-  if (!is_api_exist)
-    return LDPS_OK;
+  /* Plugin APIs are supported if this is called.  */
+  assert (is_api_exist);
 
   if (is_ordering_specified == 0)
     {
@@ -219,24 +265,26 @@ all_symbols_read_hook (void)
   struct ld_plugin_section *section_list;
   void **handles;
   unsigned int *shndx;
-  FILE *fp;
+  FILE *fp = NULL;
 
-  /* Silently return if the plugin APIs are not supported.  */
-  if (!is_api_exist)
-    return LDPS_OK;
+  /* Plugin APIs are supported if this is called.  */
+  assert (is_api_exist);
 
   if (is_callgraph_empty ())
     return LDPS_OK;
 
   /* Open the file to write the final layout  */
-  if (out_file == NULL)
-    fp = stderr;
-  else
-    fp = fopen (out_file, "w");
+  if (out_file != NULL)
+    {
+      if (strcmp (out_file, "stderr") == 0)
+	fp = stderr;
+      else
+	fp = fopen (out_file, "w");
 
-  fprintf (fp, "# Remove lines starting with \'#\' to"
-	       " pass to --section-ordering-file\n");
-  fprintf (fp, "# Lines starting with \'#\' are the edge profiles\n");
+      fprintf (fp, "# Remove lines starting with \'#\' to"
+		   " pass to --section-ordering-file\n");
+      fprintf (fp, "# Lines starting with \'#\' are the edge profiles\n");
+    }
 
   find_pettis_hansen_function_layout (fp);
   num_entries = get_layout (fp, &handles, &shndx);
@@ -248,7 +296,8 @@ all_symbols_read_hook (void)
       section_list[i].shndx = shndx[i];
     }
 
-  if (out_file != NULL)
+  if (out_file != NULL
+      && strcmp (out_file, "stderr") != 0)
     fclose (fp);
   /* Pass the new order of functions to the linker.  */
   update_section_order (section_list, num_entries);
Index: function_reordering_plugin/callgraph.c
===================================================================
--- function_reordering_plugin/callgraph.c	(revision 185372)
+++ function_reordering_plugin/callgraph.c	(working copy)
@@ -435,10 +435,9 @@ find_pettis_hansen_function_layout (FILE *fp)
 
   assert (node_chain != NULL);
   assert (active_edges != NULL);
-  assert (fp != NULL);
+  if (fp != NULL)
+    dump_edges (fp);
 
-  dump_edges (fp);
-
   /* Go over all the nodes and set it as real node only if a corresponding
      function section exists.  */
   for (n_it = node_chain; n_it != NULL; n_it = n_it->next)
@@ -509,7 +508,8 @@ write_out_node (FILE *fp, char *name,
       Section_id *s = (Section_id *)slot;
       handles[position] = s->handle;
       shndx[position] = s->shndx;
-      fprintf (fp, "%s\n", s->full_name);
+      if (fp != NULL)
+	fprintf (fp, "%s\n", s->full_name);
       /* No more use of full_name  */
       free (s->full_name);
     }
@@ -524,8 +524,6 @@ get_layout (FILE *fp, void*** handles,
   Node *it;
   int position = 0;
 
-  assert (fp != NULL);
-
   *handles = XNEWVEC (void *, num_real_nodes);
   *shndx = XNEWVEC (unsigned int, num_real_nodes);
 

--
This patch is available for review at http://codereview.appspot.com/5825054

[Sriraman, 2012-03-15 01:58:12]

On Wed, Mar 14, 2012 at 6:56 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> Made the following changes:
>
> * -ffunction-sections turned on with -freorder-functions=
> * Change name from cgedge to callgraph.
> * The plugin should not dump final layout to file by default.
>
>
>        * cgraphbuild.c (remove_cgraph_callee_edges): Replace
>        flag_callgraph_profiles_sections with flag_reorder_functions.
>        * final.c (rest_of_handle_final): Ditto.
>        * configure: Regenerate.
>        * config.in: undef FRPLUGINSONAME.
>        * congifure.ac: Define FRPLUGINSONAME.
>        * config.host: Set host_function_reordering_plugin_soname.
>        * gcc.c: Invoke function_reordering linker plugin with
>        -freorder-functions=* option.
>        (set_func_reorder_linker_plugin_spec): New function.
>        (main): Call set_func_reorder_linker_plugin_spec when
>        -freorder-functions=* option is seen.
>        * common.opt (fcallgraph-profiles-sections):  Remove.
>        (freorder-functions=): New option.
>        * testsuite/g++.dg/tree-prof/callgraph-profiles.C: Replace
>        -fcallgraph-profiles-sections with -freorder-functions=callgraph.
>        * opts.c (finish_options): Turn on -ffunction-sections when
>        -freorder-functions= is used.
>
>        * function_reordering_plugin.c (register_claim_file_hook): New function
>        pointer.
>        (register_all_symbols_read_hook): Ditto.
>        (no_op): New global.
>        (out_file): Make static.
>        (is_api_exist): Make static.
>        (process_option): New function.
>        (onload): Return if no_op is set. Do not register handlers until
>        necessary.
>        (get_load): Check if out_file is NULL before opening & closing the
>        file.
>        * callgraph.c (find_pettis_hansen_function_layout):
>        (get_layout): Dump only when fp is not NULL.
>
>
> Index: gcc/cgraphbuild.c
> ===================================================================
> --- gcc/cgraphbuild.c   (revision 185372)
> +++ gcc/cgraphbuild.c   (working copy)
> @@ -703,10 +703,10 @@ extern bool cgraph_callee_edges_final_cleanup;
>  static unsigned int
>  remove_cgraph_callee_edges (void)
>  {
> -  /* The -fcallgraph-profiles-sections flag needs the call-graph preserved
> +  /* The -freorder-functions=* flag needs the call-graph preserved
>      till pass_final.  */
>   if (cgraph_callee_edges_final_cleanup
> -      && flag_callgraph_profiles_sections)
> +      && flag_reorder_functions > 1)
>       return 0;
>
>   cgraph_node_remove_callees (cgraph_node (current_function_decl));
> Index: gcc/configure
> ===================================================================
> --- gcc/configure       (revision 185372)
> +++ gcc/configure       (working copy)
> @@ -4861,7 +4861,7 @@ fi
>  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $acx_cv_cc_gcc_supports_ada"
>&5
>  $as_echo "$acx_cv_cc_gcc_supports_ada" >&6; }
>
> -if test x$GNATBIND != xno && test x$GNATMAKE != xno && test
x$acx_cv_cc_gcc_supports_ada != xno; then
> +if test "x$GNATBIND" != xno && test "x$GNATMAKE" != xno && test
x$acx_cv_cc_gcc_supports_ada != xno; then
>   have_gnat=yes
>  else
>   have_gnat=no
> @@ -11480,6 +11480,12 @@ esac
>
>
>  cat >>confdefs.h <<_ACEOF
> +#define FRPLUGINSONAME "${host_function_reordering_plugin_soname}"
> +_ACEOF
> +
> +
> +
> +cat >>confdefs.h <<_ACEOF
>  #define LTOPLUGINSONAME "${host_lto_plugin_soname}"
>  _ACEOF
>
> @@ -17572,7 +17578,7 @@ else
>   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>   lt_status=$lt_dlunknown
>   cat > conftest.$ac_ext <<_LT_EOF
> -#line 17575 "configure"
> +#line 17581 "configure"
>  #include "confdefs.h"
>
>  #if HAVE_DLFCN_H
> @@ -17678,7 +17684,7 @@ else
>   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>   lt_status=$lt_dlunknown
>   cat > conftest.$ac_ext <<_LT_EOF
> -#line 17681 "configure"
> +#line 17687 "configure"
>  #include "confdefs.h"
>
>  #if HAVE_DLFCN_H
> Index: gcc/final.c
> ===================================================================
> --- gcc/final.c (revision 185372)
> +++ gcc/final.c (working copy)
> @@ -4421,9 +4421,9 @@ rest_of_handle_final (void)
>                                decl_fini_priority_lookup
>                                  (current_function_decl));
>
> -  /* With -fcallgraph-profiles-sections, add ".gnu.callgraph.text" section
> +  /* With -freorder-functions=, add ".gnu.callgraph.text" section
>      for storing profiling information. */
> -  if (flag_callgraph_profiles_sections
> +  if (flag_reorder_functions > 1
>       && flag_profile_use
>       && cgraph_node (current_function_decl) != NULL
>       && (cgraph_node (current_function_decl))->callees != NULL)
> Index: gcc/gcc.c
> ===================================================================
> --- gcc/gcc.c   (revision 185372)
> +++ gcc/gcc.c   (working copy)
> @@ -663,6 +663,9 @@ proper position among the other output files.  */
>     -plugin-opt=-fresolution=%u.res \
>     %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}}
\
>     }"PLUGIN_COND_CLOSE" \
> +    %{freorder-functions=*: \
> +    -plugin %(func_reorder_linker_plugin_file) \
> +    -plugin-opt=%(func_reorder_linker_plugin_opt)} \
>     %{flto|flto=*:%<fcompare-debug*} \
>     %{flto} %{flto=*} %l " LINK_PIE_SPEC \
>    "%{fuse-ld=gold:%{fuse-ld=bfd:%e-fuse-ld=gold and -fuse-ld=bfd may not be
used together}} \
> @@ -717,6 +720,8 @@ static const char *endfile_spec = ENDFILE_SPEC;
>  static const char *startfile_spec = STARTFILE_SPEC;
>  static const char *linker_name_spec = LINKER_NAME;
>  static const char *linker_plugin_file_spec = "";
> +static const char *func_reorder_linker_plugin_file_spec = "";
> +static const char *func_reorder_linker_plugin_opt = "";
>  static const char *lto_wrapper_spec = "";
>  static const char *lto_gcc_spec = "";
>  static const char *link_command_spec = LINK_COMMAND_SPEC;
> @@ -1206,6 +1211,10 @@ static struct spec_list static_specs[] =
>   INIT_STATIC_SPEC ("multilib_options",                &multilib_options),
>   INIT_STATIC_SPEC ("linker",                  &linker_name_spec),
>   INIT_STATIC_SPEC ("linker_plugin_file",      &linker_plugin_file_spec),
> +  INIT_STATIC_SPEC ("func_reorder_linker_plugin_file",
> +                   &func_reorder_linker_plugin_file_spec),
> +  INIT_STATIC_SPEC ("func_reorder_linker_plugin_opt",
> +                   &func_reorder_linker_plugin_opt),
>   INIT_STATIC_SPEC ("lto_wrapper",             &lto_wrapper_spec),
>   INIT_STATIC_SPEC ("lto_gcc",                 &lto_gcc_spec),
>   INIT_STATIC_SPEC ("link_libgcc",             &link_libgcc_spec),
> @@ -6108,6 +6117,51 @@ compare_files (char *cmpfile[])
>   return ret;
>  }
>
> +/* Set func_reorder_linker_plugin_file_spec and
func_reorder_linker_plugin_opt
> +   here.  This is the linker plugin to do global function reordering and is
> +   enabled with -freorder-functions=*. */
> +
> +static void
> +set_func_reorder_linker_plugin_spec (void)
> +{
> +  int i;
> +  const char *plugin_opt_none = "group=none";
> +  const char *plugin_opt_callgraph = "group=callgraph";
> +
> +  /* Find the linker plugin that does function ordering.  */
> +  func_reorder_linker_plugin_file_spec = find_a_file (&exec_prefixes,
> +                                           FRPLUGINSONAME, R_OK, false);
> +
> +  if (!func_reorder_linker_plugin_file_spec)
> +      fatal_error ("-freorder-functions=*, but "
> +                  FRPLUGINSONAME " file not found");
> +
> +  func_reorder_linker_plugin_opt = plugin_opt_none;
> +
> +  /* Set linker plugin options here.  Option ordering is also checked here.
> +     -fno-reorder-functions or -freorder-functions should disable any
> +     previous -freorder-functions=*. */
> +  for (i = 0; (int) i < n_switches; i++)
> +    {
> +      /* Check for match with "-freorder-functions=callgraph".  */
> +      if (func_reorder_linker_plugin_opt != plugin_opt_callgraph
> +         && !strcmp (switches[i].part1, "freorder-functions=callgraph"))
> +       {
> +         func_reorder_linker_plugin_opt = plugin_opt_callgraph;
> +         continue;
> +       }
> +      /* Set option to none if it matches -fno-reorder-functions
> +        or -freorder-functions  */
> +      if (func_reorder_linker_plugin_opt != plugin_opt_none
> +         && (!strcmp (switches[i].part1, "fno-reorder-functions")
> +             || !strcmp (switches[i].part1, "freorder-functions")))
> +       {
> +         func_reorder_linker_plugin_opt = plugin_opt_none;
> +         continue;
> +       }
> +    }
> +}
> +
>  extern int main (int, char **);
>
>  int
> @@ -6843,6 +6897,8 @@ warranty; not even for MERCHANTABILITY or FITNESS
>       const char *fuse_linker_plugin = "fuse-linker-plugin";
>  #endif
>
> +      const char *freorder_functions_ = "freorder-functions=";
> +
>       /* We'll use ld if we can't find collect2.  */
>       if (! strcmp (linker_name_spec, "collect2"))
>        {
> @@ -6866,6 +6922,12 @@ warranty; not even for MERCHANTABILITY or FITNESS
>            fatal_error ("-fuse-linker-plugin, but " LTOPLUGINSONAME " not
found");
>        }
>       lto_gcc_spec = argv[0];
> +
> +      /* The function reordering linker plugin will be loaded if the option
> +        -freorder-functions= is present in the command-line.  */
> +      if (switch_matches (freorder_functions_,
> +                         freorder_functions_ + strlen (freorder_functions_),
1))
> +       set_func_reorder_linker_plugin_spec ();
>
>       /* Rebuild the COMPILER_PATH and LIBRARY_PATH environment variables
>         for collect.  */
> Index: gcc/testsuite/g++.dg/tree-prof/callgraph-profiles.C
> ===================================================================
> --- gcc/testsuite/g++.dg/tree-prof/callgraph-profiles.C (revision 185372)
> +++ gcc/testsuite/g++.dg/tree-prof/callgraph-profiles.C (working copy)
> @@ -1,6 +1,6 @@
>  /* Verify if call-graph profile sections are created
> -   with -fcallgraph-profiles-sections. */
> -/* { dg-options "-O2 -fcallgraph-profiles-sections -ffunction-sections
--save-temps" } */
> +   with -freorder-functions=. */
> +/* { dg-options "-O2 -freorder-functions=callgraph -ffunction-sections
--save-temps" } */
>  /* { dg-require-section-exclude "" } */
>
>  int __attribute__ ((noinline))
> Index: gcc/config.in
> ===================================================================
> --- gcc/config.in       (revision 185372)
> +++ gcc/config.in       (working copy)
> @@ -1583,13 +1583,18 @@
>  #endif
>
>
> +/* Define to the name of the function reordering plugin DSO that must be
> +   passed to the linker's -plugin=LIB option. */
> +#ifndef USED_FOR_TARGET
> +#undef FRPLUGINSONAME
> +#endif
> +
>  /* Define to the name of the LTO plugin DSO that must be passed to the
>    linker's -plugin=LIB option. */
>  #ifndef USED_FOR_TARGET
>  #undef LTOPLUGINSONAME
>  #endif
>
> -
>  /* Define to the sub-directory in which libtool stores uninstalled libraries.
>    */
>  #ifndef USED_FOR_TARGET
> Index: gcc/opts.c
> ===================================================================
> --- gcc/opts.c  (revision 185372)
> +++ gcc/opts.c  (working copy)
> @@ -846,6 +846,10 @@ finish_options (struct gcc_options *opts, struct g
>        (PARAM_MAX_INLINE_INSNS_AUTO, 1000,
>         opts->x_param_values, opts_set->x_param_values);
>     }
> +
> +  /* Turn on -ffunction-sections when -freorder-functions=* is used.  */
> +  if (opts->x_flag_reorder_functions > 1)
> +    opts->x_flag_function_sections = 1;
>  }
>
>  #define LEFT_COLUMN    27
> Index: gcc/configure.ac
> ===================================================================
> --- gcc/configure.ac    (revision 185372)
> +++ gcc/configure.ac    (working copy)
> @@ -1858,6 +1858,10 @@ case $use_collect2 in
>     ;;
>  esac
>
>
+AC_DEFINE_UNQUOTED(FRPLUGINSONAME,"${host_function_reordering_plugin_soname}",
> +[Define to the name of the function reordering plugin DSO that must be
> +  passed to the linker's -plugin=LIB option.])
> +
>  AC_DEFINE_UNQUOTED(LTOPLUGINSONAME,"${host_lto_plugin_soname}",
>  [Define to the name of the LTO plugin DSO that must be
>   passed to the linker's -plugin=LIB option.])
> Index: gcc/common.opt
> ===================================================================
> --- gcc/common.opt      (revision 185372)
> +++ gcc/common.opt      (working copy)
> @@ -920,10 +920,6 @@ fcaller-saves
>  Common Report Var(flag_caller_saves) Optimization
>  Save registers around function calls
>
> -fcallgraph-profiles-sections
> -Common Report Var(flag_callgraph_profiles_sections) Init(0)
> -Generate .note.callgraph.text sections listing callees and edge counts.
> -
>  fcheck-data-deps
>  Common Report Var(flag_check_data_deps)
>  Compare the results of several data dependence analyzers.
> @@ -1758,9 +1754,19 @@ Common Report Var(flag_reorder_blocks_and_partitio
>  Reorder basic blocks and partition into hot and cold sections
>
>  freorder-functions
> -Common Report Var(flag_reorder_functions) Optimization
> +Common Report Var(flag_reorder_functions,1) Init(0) Optimization
>  Reorder functions to improve code placement
>
> +freorder-functions=
> +Common Joined RejectNegative Enum(function_reorder)
Var(flag_reorder_functions) Init(0) Optimization
> +-freorder-functions=[callgraph]        Select the scheme for function
reordering. This invokes a linker plugin.  Generate .note.callgraph.text
sections listing callees and edge counts.
> +
> +Enum
> +Name(function_reorder) Type(int) UnknownError(unrecognized function reorder
value %qs)
> +
> +EnumValue
> +Enum(function_reorder) String(callgraph) Value(2)
> +
>  frerun-cse-after-loop
>  Common Report Var(flag_rerun_cse_after_loop) Optimization
>  Add a common subexpression elimination pass after loop optimizations
> Index: gcc/config.host
> ===================================================================
> --- gcc/config.host     (revision 185372)
> +++ gcc/config.host     (working copy)
> @@ -76,6 +76,7 @@ out_host_hook_obj=host-default.o
>  host_can_use_collect2=yes
>  use_long_long_for_widest_fast_int=no
>  host_lto_plugin_soname=liblto_plugin.so
> +host_function_reordering_plugin_soname=libfunction_reordering_plugin.so
>
>  # Unsupported hosts list.  Generally, only include hosts known to fail here,
>  # since we allow hosts not listed to be supported generically.
> Index: function_reordering_plugin/function_reordering_plugin.c
> ===================================================================
> --- function_reordering_plugin/function_reordering_plugin.c     (revision
185372)
> +++ function_reordering_plugin/function_reordering_plugin.c     (working copy)
> @@ -64,6 +64,9 @@ enum ld_plugin_status claim_file_hook (const struc
>                                        int *claimed);
>  enum ld_plugin_status all_symbols_read_hook ();
>
> +static ld_plugin_register_claim_file register_claim_file_hook = NULL;
> +static ld_plugin_register_all_symbols_read
> +  register_all_symbols_read_hook = NULL;
>  static ld_plugin_get_input_section_count get_input_section_count = NULL;
>  static ld_plugin_get_input_section_type get_input_section_type = NULL;
>  static ld_plugin_get_input_section_name get_input_section_name = NULL;
> @@ -72,23 +75,54 @@ static ld_plugin_update_section_order update_secti
>  static ld_plugin_allow_section_ordering allow_section_ordering = NULL;
>
>  /* The file where the final function order will be stored.
> -   It is "./final_layout.txt".  It can be changed by passing
> -   new name to --plugin-opt  */
> +   It can be set by using the  plugin option  as --plugin-opt
> +   "file=<name>".  To dump to stderr, say --plugin-opt "file=stderr".  */
>
> -char *out_file = "./final_layout.txt";
> +static char *out_file = NULL;
>
> -int is_api_exist = 0;
> +static int is_api_exist = 0;
>
> +/* The plugin does nothing when no-op is 1.  */
> +static int no_op = 0;
> +
>  /* Copies new output file name out_file  */
>  void get_filename (const char *name)
>  {
> -  if (strcmp (name, "stderr") == 0)
> +  out_file = (char *) malloc (strlen (name) + 1);
> +  strcpy (out_file, name);
> +}
> +
> +/* Process options to plugin.  Options with prefix "group=" are special.
> +   They specify the type of grouping. The option "group=none" makes the
> +   plugin do nothing.   Options with prefix "file=" set the output file
> +   where the final function order must be stored.  */
> +void
> +process_option (const char *name)
> +{
> +  const char *option_group = "group=";
> +  const char *option_file = "file=";
> +
> +  /* Check if option is "group="  */
> +  if (strncmp (name, option_group, strlen (option_group)) == 0)
>     {
> -      out_file = NULL;
> +      if (strcmp (name + strlen (option_group), "none") == 0)
> +       no_op = 1;
> +      else
> +       no_op = 0;
>       return;
>     }
> -  out_file = (char *) malloc (strlen (name) + 1);
> -  strcpy (out_file, name);
> +
> +  /* Check if option is "file=" */
> +  if (strncmp (name, option_file, strlen (option_file)) == 0)
> +    {
> +      get_filename (name + strlen (option_file));
> +      return;
> +    }
> +
> +  /* Unknown option, set no_op to 1.  */
> +  no_op = 1;
> +  fprintf (stderr, "Unknown option to function reordering plugin :%s\n",
> +          name);
>  }
>
>  /* Plugin entry point.  */
> @@ -105,14 +139,16 @@ onload (struct ld_plugin_tv *tv)
>         case LDPT_GOLD_VERSION:
>           break;
>         case LDPT_OPTION:
> -         get_filename (entry->tv_u.tv_string);
> +         process_option (entry->tv_u.tv_string);
> +         /* If no_op is set, do not do anything else.  */
> +         if (no_op) return LDPS_OK;
>          break;
>         case LDPT_REGISTER_CLAIM_FILE_HOOK:
> -          assert ((*entry->tv_u.tv_register_claim_file) (claim_file_hook) ==
LDPS_OK);
> +         register_claim_file_hook = *entry->tv_u.tv_register_claim_file;
>           break;
>        case LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK:
> -          assert ((*entry->tv_u.tv_register_all_symbols_read)
(all_symbols_read_hook)
> -                  == LDPS_OK);
> +         register_all_symbols_read_hook
> +           = *entry->tv_u.tv_register_all_symbols_read;
>           break;
>         case LDPT_GET_INPUT_SECTION_COUNT:
>           get_input_section_count = *entry->tv_u.tv_get_input_section_count;
> @@ -137,14 +173,25 @@ onload (struct ld_plugin_tv *tv)
>         }
>     }
>
> -  if (get_input_section_count != NULL
> +  assert (!no_op);
> +
> +  if (register_all_symbols_read_hook != NULL
> +      && register_claim_file_hook != NULL
> +      && get_input_section_count != NULL
>       && get_input_section_type != NULL
>       && get_input_section_name != NULL
>       && get_input_section_contents != NULL
>       && update_section_order != NULL
>       && allow_section_ordering != NULL)
>     is_api_exist = 1;
> +  else
> +    return LDPS_OK;
>
> +  /* Register handlers.  */
> +  assert ((*register_all_symbols_read_hook) (all_symbols_read_hook)
> +          == LDPS_OK);
> +  assert ((*register_claim_file_hook) (claim_file_hook)
> +         == LDPS_OK);
>   return LDPS_OK;
>  }
>
> @@ -161,9 +208,8 @@ claim_file_hook (const struct ld_plugin_input_file
>
>   (void) claimed;
>
> -  /* Silently return if the plugin APIs are not supported.  */
> -  if (!is_api_exist)
> -    return LDPS_OK;
> +  /* Plugin APIs are supported if this is called.  */
> +  assert (is_api_exist);
>
>   if (is_ordering_specified == 0)
>     {
> @@ -219,24 +265,26 @@ all_symbols_read_hook (void)
>   struct ld_plugin_section *section_list;
>   void **handles;
>   unsigned int *shndx;
> -  FILE *fp;
> +  FILE *fp = NULL;
>
> -  /* Silently return if the plugin APIs are not supported.  */
> -  if (!is_api_exist)
> -    return LDPS_OK;
> +  /* Plugin APIs are supported if this is called.  */
> +  assert (is_api_exist);
>
>   if (is_callgraph_empty ())
>     return LDPS_OK;
>
>   /* Open the file to write the final layout  */
> -  if (out_file == NULL)
> -    fp = stderr;
> -  else
> -    fp = fopen (out_file, "w");
> +  if (out_file != NULL)
> +    {
> +      if (strcmp (out_file, "stderr") == 0)
> +       fp = stderr;
> +      else
> +       fp = fopen (out_file, "w");
>
> -  fprintf (fp, "# Remove lines starting with \'#\' to"
> -              " pass to --section-ordering-file\n");
> -  fprintf (fp, "# Lines starting with \'#\' are the edge profiles\n");
> +      fprintf (fp, "# Remove lines starting with \'#\' to"
> +                  " pass to --section-ordering-file\n");
> +      fprintf (fp, "# Lines starting with \'#\' are the edge profiles\n");
> +    }
>
>   find_pettis_hansen_function_layout (fp);
>   num_entries = get_layout (fp, &handles, &shndx);
> @@ -248,7 +296,8 @@ all_symbols_read_hook (void)
>       section_list[i].shndx = shndx[i];
>     }
>
> -  if (out_file != NULL)
> +  if (out_file != NULL
> +      && strcmp (out_file, "stderr") != 0)
>     fclose (fp);
>   /* Pass the new order of functions to the linker.  */
>   update_section_order (section_list, num_entries);
> Index: function_reordering_plugin/callgraph.c
> ===================================================================
> --- function_reordering_plugin/callgraph.c      (revision 185372)
> +++ function_reordering_plugin/callgraph.c      (working copy)
> @@ -435,10 +435,9 @@ find_pettis_hansen_function_layout (FILE *fp)
>
>   assert (node_chain != NULL);
>   assert (active_edges != NULL);
> -  assert (fp != NULL);
> +  if (fp != NULL)
> +    dump_edges (fp);
>
> -  dump_edges (fp);
> -
>   /* Go over all the nodes and set it as real node only if a corresponding
>      function section exists.  */
>   for (n_it = node_chain; n_it != NULL; n_it = n_it->next)
> @@ -509,7 +508,8 @@ write_out_node (FILE *fp, char *name,
>       Section_id *s = (Section_id *)slot;
>       handles[position] = s->handle;
>       shndx[position] = s->shndx;
> -      fprintf (fp, "%s\n", s->full_name);
> +      if (fp != NULL)
> +       fprintf (fp, "%s\n", s->full_name);
>       /* No more use of full_name  */
>       free (s->full_name);
>     }
> @@ -524,8 +524,6 @@ get_layout (FILE *fp, void*** handles,
>   Node *it;
>   int position = 0;
>
> -  assert (fp != NULL);
> -
>   *handles = XNEWVEC (void *, num_real_nodes);
>   *shndx = XNEWVEC (unsigned int, num_real_nodes);
>
>
> --
> This patch is available for review at http://codereview.appspot.com/5825054

[davidxl, 2012-03-18 06:58:14]

Ok for google branches after updating the doc/invoke.texi file.

David

[Sriraman, 2012-03-19 03:09:20]

Updated doc/invoke.texi and submitted.

Thanks,
-Sri.

On Sat, Mar 17, 2012 at 11:58 PM,  <davidxl@google.com> wrote:
>
> Ok for google branches after updating the doc/invoke.texi file.
>
> David
>
>
> http://codereview.appspot.com/5825054/