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 <tmsri...@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 <tmsri...@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 <tmsri...@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", <o_wrapper_spec),
> INIT_STATIC_SPEC ("lto_gcc", <o_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 <tmsri...@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 <saugust...@google.com>
> Cary Coutant <ccout...@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
