Re: Ipa function summary pass

[Christophe Lyon <christophe dot lyon at linaro dot org>]

Hi,

On 23 May 2017 at 18:23, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> this patch finishes the breakup of ipa-inline and function analysis.
> The analysis is now done by separate pass and I will work on cleaning
> up the interfaces now.
>
> Honza
>
>         * cgraphunit.c (symbol_table::process_new_functions): Update.
>         * ipa-fnsummary.c (pass_data_inline_parameters): Remove.
>         (inline_generate_summary): Rename to ...
>         (ipa_fn_summary_generate): ... this one.
>         (inline_read_summary): Rename to ...
>         (ipa_fn_summary_read): ... this one.
>         (inline_write_summary): Rename to ...
>         (ipa_fn_summary_write): ... this one.
>         (inline_free_summary): Rename to ...
>         (ipa_free_fn_summary): ... this one.
>         (pass_data_local_fn_summary, pass_local_fn_summary,
>         make_pass_local_fn_summary, pass_data_ipa_free_fn_summary,
>         pass_ipa_free_fn_summary, make_pass_ipa_free_fn_summary,
>         pass_data_ipa_fn_summary, pass_ipa_fn_summary,
>         make_pass_ipa_fn_summary): New.
>         * ipa-fnsummary.h (inline_generate_summary, inline_read_summary,
>         inline_write_summary, inline_free_summary): Remove.
>         (ipa_free_fn_summary) : New.
>         * ipa-inline.c (ipa_inline): Update.
>         (pass_ipa_inline): Do not generate summaries.
>         * ipa.c (pass_data_ipa_free_fn_summary, pass_ipa_free_fn_summary):
>         Remove.
>         * passes.def: Replace pass_inline_parameters by pass_local_fn_summary
>         and add pass_ipa_fn_summary.
>         * tree-pass.h (make_pass_ipa_fn_summary, make_pass_local_fn_summary):
>         New.
>         (make_pass_inline_parameters): Remove.
>
>         * lto.c (do_whole_program_analysis): Replace inline_free_summary
>         by ipa_free_fn_summary.
>
>         * gcc.dg/ipa/ctor-empty-1.c: Update template.
>         * gcc.dg/ipa/inline-5.c: Likewise.
>         * gfortran.dg/pr48636.f90: Likewise.
> Index: cgraphunit.c
> ===================================================================
> --- cgraphunit.c        (revision 248365)
> +++ cgraphunit.c        (working copy)
> @@ -339,7 +339,7 @@ symbol_table::process_new_functions (voi
>                  and splitting.  This is redundant for functions added late.
>                  Just throw away whatever it did.  */
>               if (!summaried_computed)
> -               inline_free_summary ();
> +               ipa_free_fn_summary ();
>             }
>           else if (ipa_fn_summaries != NULL)
>             compute_fn_summary (node, true);
> Index: ipa-fnsummary.c
> ===================================================================
> --- ipa-fnsummary.c     (revision 248366)
> +++ ipa-fnsummary.c     (working copy)
> @@ -2504,46 +2504,6 @@ compute_fn_summary_for_current (void)
>    return 0;
>  }
>
> -namespace {
> -
> -const pass_data pass_data_inline_parameters =
> -{
> -  GIMPLE_PASS, /* type */
> -  "inline_param", /* name */
> -  OPTGROUP_INLINE, /* optinfo_flags */
> -  TV_INLINE_PARAMETERS, /* tv_id */
> -  0, /* properties_required */
> -  0, /* properties_provided */
> -  0, /* properties_destroyed */
> -  0, /* todo_flags_start */
> -  0, /* todo_flags_finish */
> -};
> -
> -class pass_inline_parameters : public gimple_opt_pass
> -{
> -public:
> -  pass_inline_parameters (gcc::context *ctxt)
> -    : gimple_opt_pass (pass_data_inline_parameters, ctxt)
> -  {}
> -
> -  /* opt_pass methods: */
> -  opt_pass * clone () { return new pass_inline_parameters (m_ctxt); }
> -  virtual unsigned int execute (function *)
> -    {
> -      return compute_fn_summary_for_current ();
> -    }
> -
> -}; // class pass_inline_parameters
> -
> -} // anon namespace
> -
> -gimple_opt_pass *
> -make_pass_inline_parameters (gcc::context *ctxt)
> -{
> -  return new pass_inline_parameters (ctxt);
> -}
> -
> -
>  /* Estimate benefit devirtualizing indirect edge IE, provided KNOWN_VALS,
>     KNOWN_CONTEXTS and KNOWN_AGGS.  */
>
> @@ -3207,8 +3167,8 @@ ipa_fn_summary_t::insert (struct cgraph_
>
>  /* Note function body size.  */
>
> -void
> -inline_generate_summary (void)
> +static void
> +ipa_fn_summary_generate (void)
>  {
>    struct cgraph_node *node;
>
> @@ -3226,7 +3186,7 @@ inline_generate_summary (void)
>    ipa_fn_summaries->enable_insertion_hook ();
>
>    ipa_register_cgraph_hooks ();
> -  inline_free_summary ();
> +  ipa_free_fn_summary ();
>
>    FOR_EACH_DEFINED_FUNCTION (node)
>      if (!node->alias)
> @@ -3358,8 +3318,8 @@ inline_read_section (struct lto_file_dec
>     and inliner, so when ipa-cp is active, we don't need to write them
>     twice.  */
>
> -void
> -inline_read_summary (void)
> +static void
> +ipa_fn_summary_read (void)
>  {
>    struct lto_file_decl_data **file_data_vec = lto_get_file_decl_data ();
>    struct lto_file_decl_data *file_data;
> @@ -3419,8 +3379,8 @@ write_ipa_call_summary (struct output_bl
>     Jump functions are shared among ipa-cp and inliner, so when ipa-cp is
>     active, we don't need to write them twice.  */
>
> -void
> -inline_write_summary (void)
> +static void
> +ipa_fn_summary_write (void)
>  {
>    struct output_block *ob = create_output_block (LTO_section_ipa_fn_summary);
>    lto_symtab_encoder_t encoder = ob->decl_state->symtab_node_encoder;
> @@ -3510,7 +3470,7 @@ inline_write_summary (void)
>  /* Release inline summary.  */
>
>  void
> -inline_free_summary (void)
> +ipa_free_fn_summary (void)
>  {
>    struct cgraph_node *node;
>    if (!ipa_call_summaries)
> @@ -3525,3 +3485,131 @@ inline_free_summary (void)
>    ipa_call_summaries = NULL;
>    edge_predicate_pool.release ();
>  }
> +
> +namespace {
> +
> +const pass_data pass_data_local_fn_summary =
> +{
> +  GIMPLE_PASS, /* type */
> +  "local-fnsummary", /* name */
> +  OPTGROUP_INLINE, /* optinfo_flags */
> +  TV_INLINE_PARAMETERS, /* tv_id */
> +  0, /* properties_required */
> +  0, /* properties_provided */
> +  0, /* properties_destroyed */
> +  0, /* todo_flags_start */
> +  0, /* todo_flags_finish */
> +};
> +
> +class pass_local_fn_summary : public gimple_opt_pass
> +{
> +public:
> +  pass_local_fn_summary (gcc::context *ctxt)
> +    : gimple_opt_pass (pass_data_local_fn_summary, ctxt)
> +  {}
> +
> +  /* opt_pass methods: */
> +  opt_pass * clone () { return new pass_local_fn_summary (m_ctxt); }
> +  virtual unsigned int execute (function *)
> +    {
> +      return compute_fn_summary_for_current ();
> +    }
> +
> +}; // class pass_local_fn_summary
> +
> +} // anon namespace
> +
> +gimple_opt_pass *
> +make_pass_local_fn_summary (gcc::context *ctxt)
> +{
> +  return new pass_local_fn_summary (ctxt);
> +}
> +
> +
> +/* Free inline summary.  */
> +
> +namespace {
> +
> +const pass_data pass_data_ipa_free_fn_summary =
> +{
> +  SIMPLE_IPA_PASS, /* type */
> +  "free-fnsummary", /* name */
> +  OPTGROUP_NONE, /* optinfo_flags */
> +  TV_IPA_FREE_INLINE_SUMMARY, /* tv_id */
> +  0, /* properties_required */
> +  0, /* properties_provided */
> +  0, /* properties_destroyed */
> +  0, /* todo_flags_start */
> +  /* Early optimizations may make function unreachable.  We can not
> +     remove unreachable functions as part of the ealry opts pass because
> +     TODOs are run before subpasses.  Do it here.  */
> +  ( TODO_remove_functions | TODO_dump_symtab ), /* todo_flags_finish */
> +};
> +
> +class pass_ipa_free_fn_summary : public simple_ipa_opt_pass
> +{
> +public:
> +  pass_ipa_free_fn_summary (gcc::context *ctxt)
> +    : simple_ipa_opt_pass (pass_data_ipa_free_fn_summary, ctxt)
> +  {}
> +
> +  /* opt_pass methods: */
> +  virtual unsigned int execute (function *)
> +    {
> +      ipa_free_fn_summary ();
> +      return 0;
> +    }
> +
> +}; // class pass_ipa_free_fn_summary
> +
> +} // anon namespace
> +
> +simple_ipa_opt_pass *
> +make_pass_ipa_free_fn_summary (gcc::context *ctxt)
> +{
> +  return new pass_ipa_free_fn_summary (ctxt);
> +}
> +
> +namespace {
> +
> +const pass_data pass_data_ipa_fn_summary =
> +{
> +  IPA_PASS, /* type */
> +  "fnsummary", /* name */
> +  OPTGROUP_INLINE, /* optinfo_flags */
> +  TV_IPA_INLINING, /* tv_id */
> +  0, /* properties_required */
> +  0, /* properties_provided */
> +  0, /* properties_destroyed */
> +  0, /* todo_flags_start */
> +  ( TODO_dump_symtab ), /* todo_flags_finish */
> +};
> +
> +class pass_ipa_fn_summary : public ipa_opt_pass_d
> +{
> +public:
> +  pass_ipa_fn_summary (gcc::context *ctxt)
> +    : ipa_opt_pass_d (pass_data_ipa_fn_summary, ctxt,
> +                     ipa_fn_summary_generate, /* generate_summary */
> +                     ipa_fn_summary_write, /* write_summary */
> +                     ipa_fn_summary_read, /* read_summary */
> +                     NULL, /* write_optimization_summary */
> +                     NULL, /* read_optimization_summary */
> +                     NULL, /* stmt_fixup */
> +                     0, /* function_transform_todo_flags_start */
> +                     NULL, /* function_transform */
> +                     NULL) /* variable_transform */
> +  {}
> +
> +  /* opt_pass methods: */
> +  virtual unsigned int execute (function *) { return 0; }
> +
> +}; // class pass_ipa_fn_summary
> +
> +} // anon namespace
> +
> +ipa_opt_pass_d *
> +make_pass_ipa_fn_summary (gcc::context *ctxt)
> +{
> +  return new pass_ipa_fn_summary (ctxt);
> +}
> Index: ipa-fnsummary.h
> ===================================================================
> --- ipa-fnsummary.h     (revision 248365)
> +++ ipa-fnsummary.h     (working copy)
> @@ -231,10 +231,7 @@ void ipa_debug_fn_summary (struct cgraph
>  void ipa_dump_fn_summaries (FILE *f);
>  void ipa_dump_fn_summary (FILE *f, struct cgraph_node *node);
>  void ipa_dump_hints (FILE *f, ipa_hints);
> -void inline_generate_summary (void);
> -void inline_read_summary (void);
> -void inline_write_summary (void);
> -void inline_free_summary (void);
> +void ipa_free_fn_summary (void);
>  void inline_analyze_function (struct cgraph_node *node);
>  void estimate_ipcp_clone_size_and_time (struct cgraph_node *,
>                                         vec<tree>,
> Index: ipa-inline.c
> ===================================================================
> --- ipa-inline.c        (revision 248365)
> +++ ipa-inline.c        (working copy)
> @@ -2513,7 +2513,7 @@ ipa_inline (void)
>      ipa_dump_fn_summaries (dump_file);
>    /* In WPA we use inline summaries for partitioning process.  */
>    if (!flag_wpa)
> -    inline_free_summary ();
> +    ipa_free_fn_summary ();
>    return remove_functions ? TODO_remove_functions : 0;
>  }
>
> @@ -2819,9 +2819,9 @@ class pass_ipa_inline : public ipa_opt_p
>  public:
>    pass_ipa_inline (gcc::context *ctxt)
>      : ipa_opt_pass_d (pass_data_ipa_inline, ctxt,
> -                     inline_generate_summary, /* generate_summary */
> -                     inline_write_summary, /* write_summary */
> -                     inline_read_summary, /* read_summary */
> +                     NULL, /* generate_summary */
> +                     NULL, /* write_summary */
> +                     NULL, /* read_summary */
>                       NULL, /* write_optimization_summary */
>                       NULL, /* read_optimization_summary */
>                       NULL, /* stmt_fixup */
> Index: ipa.c
> ===================================================================
> --- ipa.c       (revision 248365)
> +++ ipa.c       (working copy)
> @@ -845,50 +845,6 @@ ipa_discover_readonly_nonaddressable_var
>    return remove_p;
>  }
>
> -/* Free inline summary.  */
> -
> -namespace {
> -
> -const pass_data pass_data_ipa_free_fn_summary =
> -{
> -  SIMPLE_IPA_PASS, /* type */
> -  "free-inline-summary", /* name */
> -  OPTGROUP_NONE, /* optinfo_flags */
> -  TV_IPA_FREE_INLINE_SUMMARY, /* tv_id */
> -  0, /* properties_required */
> -  0, /* properties_provided */
> -  0, /* properties_destroyed */
> -  0, /* todo_flags_start */
> -  /* Early optimizations may make function unreachable.  We can not
> -     remove unreachable functions as part of the ealry opts pass because
> -     TODOs are run before subpasses.  Do it here.  */
> -  ( TODO_remove_functions | TODO_dump_symtab ), /* todo_flags_finish */
> -};
> -
> -class pass_ipa_free_fn_summary : public simple_ipa_opt_pass
> -{
> -public:
> -  pass_ipa_free_fn_summary (gcc::context *ctxt)
> -    : simple_ipa_opt_pass (pass_data_ipa_free_fn_summary, ctxt)
> -  {}
> -
> -  /* opt_pass methods: */
> -  virtual unsigned int execute (function *)
> -    {
> -      inline_free_summary ();
> -      return 0;
> -    }
> -
> -}; // class pass_ipa_free_fn_summary
> -
> -} // anon namespace
> -
> -simple_ipa_opt_pass *
> -make_pass_ipa_free_fn_summary (gcc::context *ctxt)
> -{
> -  return new pass_ipa_free_fn_summary (ctxt);
> -}
> -
>  /* Generate and emit a static constructor or destructor.  WHICH must
>     be one of 'I' (for a constructor), 'D' (for a destructor), 'P'
>     (for chp static vars constructor) or 'B' (for chkp static bounds
> Index: lto/lto.c
> ===================================================================
> --- lto/lto.c   (revision 248336)
> +++ lto/lto.c   (working copy)
> @@ -3129,7 +3129,7 @@ do_whole_program_analysis (void)
>
>    /* Inline summaries are needed for balanced partitioning.  Free them now so
>       the memory can be used for streamer caches.  */
> -  inline_free_summary ();
> +  ipa_free_fn_summary ();
>
>    /* AUX pointers are used by partitioning code to bookkeep number of
>       partitions symbol is in.  This is no longer needed.  */
> Index: passes.def
> ===================================================================
> --- passes.def  (revision 248365)
> +++ passes.def  (working copy)
> @@ -75,7 +75,7 @@ along with GCC; see the file COPYING3.
>    PUSH_INSERT_PASSES_WITHIN (pass_local_optimization_passes)
>        NEXT_PASS (pass_fixup_cfg);
>        NEXT_PASS (pass_rebuild_cgraph_edges);
> -      NEXT_PASS (pass_inline_parameters);
> +      NEXT_PASS (pass_local_fn_summary);
>        NEXT_PASS (pass_early_inline);
>        NEXT_PASS (pass_all_early_optimizations);
>        PUSH_INSERT_PASSES_WITHIN (pass_all_early_optimizations)
> @@ -110,7 +110,7 @@ along with GCC; see the file COPYING3.
>        POP_INSERT_PASSES ()
>        NEXT_PASS (pass_release_ssa_names);
>        NEXT_PASS (pass_rebuild_cgraph_edges);
> -      NEXT_PASS (pass_inline_parameters);
> +      NEXT_PASS (pass_local_fn_summary);
>    POP_INSERT_PASSES ()
>
>    NEXT_PASS (pass_ipa_oacc);
> @@ -157,6 +157,7 @@ along with GCC; see the file COPYING3.
>    NEXT_PASS (pass_ipa_cp);
>    NEXT_PASS (pass_ipa_cdtor_merge);
>    NEXT_PASS (pass_ipa_hsa);
> +  NEXT_PASS (pass_ipa_fn_summary);
>    NEXT_PASS (pass_ipa_inline);
>    NEXT_PASS (pass_ipa_pure_const);
>    NEXT_PASS (pass_ipa_reference);
> Index: tree-pass.h
> ===================================================================
> --- tree-pass.h (revision 248365)
> +++ tree-pass.h (working copy)
> @@ -494,6 +494,7 @@ extern ipa_opt_pass_d *make_pass_ipa_who
>                                                                *ctxt);
>  extern simple_ipa_opt_pass *make_pass_ipa_increase_alignment (gcc::context
>                                                               *ctxt);
> +extern ipa_opt_pass_d *make_pass_ipa_fn_summary (gcc::context *ctxt);
>  extern ipa_opt_pass_d *make_pass_ipa_inline (gcc::context *ctxt);
>  extern simple_ipa_opt_pass *make_pass_ipa_free_lang_data (gcc::context *ctxt);
>  extern simple_ipa_opt_pass *make_pass_ipa_free_fn_summary (gcc::context *ctxt);
> @@ -617,7 +618,7 @@ extern rtl_opt_pass *make_pass_final (gc
>  extern rtl_opt_pass *make_pass_rtl_seqabstr (gcc::context *ctxt);
>  extern gimple_opt_pass *make_pass_release_ssa_names (gcc::context *ctxt);
>  extern gimple_opt_pass *make_pass_early_inline (gcc::context *ctxt);
> -extern gimple_opt_pass *make_pass_inline_parameters (gcc::context *ctxt);
> +extern gimple_opt_pass *make_pass_local_fn_summary (gcc::context *ctxt);
>  extern gimple_opt_pass *make_pass_update_address_taken (gcc::context *ctxt);
>  extern gimple_opt_pass *make_pass_convert_switch (gcc::context *ctxt);
>  extern gimple_opt_pass *make_pass_lower_vaarg (gcc::context *ctxt);
> Index: gcc.dg/ipa/ctor-empty-1.c
> ===================================================================
> --- gcc.dg/ipa/ctor-empty-1.c   (revision 248374)
> +++ gcc.dg/ipa/ctor-empty-1.c   (working copy)
> @@ -1,7 +1,7 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O3 -c -fdump-ipa-free-inline-summary"  } */
> +/* { dg-options "-O3 -c -fdump-ipa-free-fnsummary"  } */
>  static __attribute__((constructor))
>  void empty_constructor()
>  {
>  }
> -/* { dg-final { scan-ipa-dump "Reclaiming functions: empty_constructor"  "free-inline-summary"  } } */
> +/* { dg-final { scan-ipa-dump "Reclaiming functions: empty_constructor"  "free-fnsummary"  } } */
> Index: gcc.dg/ipa/inline-5.c
> ===================================================================
> --- gcc.dg/ipa/inline-5.c       (revision 248374)
> +++ gcc.dg/ipa/inline-5.c       (working copy)
> @@ -1,6 +1,6 @@
>  /* Check statements that are eliminated by inlining.  */
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fdump-ipa-inline-details -fno-early-inlining -fno-partial-inlining -fno-ipa-cp"  } */
> +/* { dg-options "-O2 -fdump-ipa-fnsummary-details -fno-early-inlining -fno-partial-inlining -fno-ipa-cp"  } */
>
>  struct a {int a,b,c,d,e;};
>  void t(int);
> @@ -34,5 +34,5 @@ accessreference (struct a *a)
>      /* Will be eliminated by inlining */
>  }
>
> -/* { dg-final { scan-ipa-dump-times "Will be eliminated" 4 "inline" { xfail { { hppa*-*-* } && { ! lp64 } } } } } */
> -/* { dg-final { scan-ipa-dump-times "50. will be eliminated" 1 "inline"  } } */
> +/* { dg-final { scan-ipa-dump-times "Will be eliminated" 4 "fnsummary" { xfail { { hppa*-*-* } && { ! lp64 } } } } } */
> +/* { dg-final { scan-ipa-dump-times "50. will be eliminated" 1 "fnsummary"  } } */
> Index: gfortran.dg/pr48636.f90
> ===================================================================
> --- gfortran.dg/pr48636.f90     (revision 248374)
> +++ gfortran.dg/pr48636.f90     (working copy)
> @@ -1,5 +1,5 @@
>  ! { dg-do compile }
> -! { dg-options "-O3 -fdump-ipa-inline-details -fno-ipa-cp" }
> +! { dg-options "-O3 -fdump-ipa-inline-details -fdump-ipa-fnsummary-details -fno-ipa-cp" }
>
>  module foo
>    implicit none
> @@ -34,5 +34,5 @@ program main
>  end program main
>
>  ! { dg-final { scan-ipa-dump "bar\[^\\n\]*inline copy in MAIN" "inline" } }
> -! { dg-final { scan-ipa-dump-times "phi predicate:" 3 "inline" } }
> -! { dg-final { scan-ipa-dump "inline hints: loop_iterations" "inline" } }
> +! { dg-final { scan-ipa-dump-times "phi predicate:" 3 "fnsummary" } }
> +! { dg-final { scan-ipa-dump "inline hints: loop_iterations" "fnsummary" } }


With this patch (commit r248375), I now see:
FAIL: gfortran.dg/pr48636.f90   -O   scan-ipa-dump fnsummary "inline
hints: loop_iterations"
on aarch64 and arm targets.

Christophe