Re: [GOOGLE] Patch to fix AutoFDO LIPO performance regression

[Xinliang David Li <davidxl at google dot com>]

On Wed, Sep 18, 2013 at 4:51 PM, Dehao Chen <dehao@google.com> wrote:
> This patch fixup the call graph edge targets during AutoFDO pass, so
> that when rebuilding call graph edges, it can find the correct callee.
>
> Bootstrapped and passed regression test. Benchmark tests on-going.
>
> Ok for google-4_8 branch?
>
> Thanks,
> Dehao
>
> Index: gcc/Makefile.in
> ===================================================================
> --- gcc/Makefile.in (revision 202725)
> +++ gcc/Makefile.in (working copy)
> @@ -2960,7 +2960,7 @@ coverage.o : coverage.c $(GCOV_IO_H) $(CONFIG_H) $
>  auto-profile.o : auto-profile.c $(CONFIG_H) $(SYSTEM_H) $(FLAGS_H) \
>     $(BASIC_BLOCK_H) $(DIAGNOSTIC_CORE_H) $(GCOV_IO_H) $(INPUT_H) profile.h \
>     $(LANGHOOKS_H) $(OPTS_H) $(TREE_PASS_H) $(CGRAPH_H) $(GIMPLE_H)
> value-prof.h \
> -   $(COVERAGE_H) coretypes.h $(TREE_H) $(PARAMS_H) $(AUTO_PROFILE_H)
> +   $(COVERAGE_H) coretypes.h $(TREE_H) $(PARAMS_H) l-ipo.h $(AUTO_PROFILE_H)
>  cselib.o : cselib.c $(CONFIG_H) $(SYSTEM_H) coretypes.h dumpfile.h
> $(TM_H) $(RTL_H) \
>     $(REGS_H) hard-reg-set.h $(FLAGS_H) insn-config.h $(RECOG_H) \
>     $(EMIT_RTL_H) $(DIAGNOSTIC_CORE_H) $(FUNCTION_H) \
> Index: gcc/auto-profile.c
> ===================================================================
> --- gcc/auto-profile.c (revision 202725)
> +++ gcc/auto-profile.c (working copy)
> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "value-prof.h"
>  #include "coverage.h"
>  #include "params.h"
> +#include "l-ipo.h"
>  #include "auto-profile.h"
>
>  /* The following routines implements AutoFDO optimization.
> @@ -1290,6 +1291,13 @@ auto_profile (void)
>    init_node_map ();
>    profile_info = autofdo::afdo_profile_info;
>
> +  cgraph_pre_profiling_inlining_done = true;
> +  cgraph_process_module_scope_statics ();
> +  /* Now perform link to allow cross module inlining.  */
> +  cgraph_do_link ();
> +  varpool_do_link ();
> +  cgraph_unify_type_alias_sets ();
> +
>    FOR_EACH_FUNCTION (node)
>      {
>        if (!gimple_has_body_p (node->symbol.decl))
> @@ -1301,6 +1309,21 @@ auto_profile (void)
>
>        push_cfun (DECL_STRUCT_FUNCTION (node->symbol.decl));
>
> +      if (L_IPO_COMP_MODE)
> +        {
> +  basic_block bb;
> +  FOR_EACH_BB (bb)
> +    {
> +      gimple_stmt_iterator gsi;
> +      for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
> + {
> +  gimple stmt = gsi_stmt (gsi);
> +  if (is_gimple_call (stmt))
> +    lipo_fixup_cgraph_edge_call_target (stmt);
> + }
> +    }
> + }
> +

Need this:


      if (execute_fixup_cfg () & TODO_cleanup_cfg)
          cleanup_tree_cfg ();


as in tree-profiling. Changing call stmt targets can lead to CFG changes.



David

>        autofdo::afdo_annotate_cfg ();
>        compute_function_frequency ();
>        update_ssa (TODO_update_ssa);
> @@ -1309,13 +1332,6 @@ auto_profile (void)
>        pop_cfun ();
>      }
>
> -  cgraph_pre_profiling_inlining_done = true;
> -  cgraph_process_module_scope_statics ();
> -  /* Now perform link to allow cross module inlining.  */
> -  cgraph_do_link ();
> -  varpool_do_link ();
> -  cgraph_unify_type_alias_sets ();
> -
>    return TODO_rebuild_cgraph_edges;
>  }