Hi Xiong,

> > It seems the parameters no longer do anything. i.e. -flto --param ipa-cp-
> eval-threshold=1 --param ipa-cp-unit-growth=80 doesn't have any effect
> anymore.
> 
> Strange that you don't need -fno-inline to achieve the 30% performance
> boost, I found that some specialized nodes are recursively inlined cause
> register spill and no performance improve.  What's your platform and could
> you please try it with -fno-inline and see any difference please?
> 

You do indeed need to restrict inlining, but that wasn't needed for this 
regression report so I
omitted the extra options. 

Regards,
Tamar

> Thanks
> Xiong Hu
> >
> > Thanks,
> > Tamar
> >
> >> -----Original Message-----
> >> From: gcc-patches-ow...@gcc.gnu.org <gcc-patches-ow...@gcc.gnu.org>
> >> On Behalf Of Martin Jambor
> >> Sent: Wednesday, January 8, 2020 4:14 PM
> >> To: Martin Liška <mli...@suse.cz>; gcc-patches@gcc.gnu.org
> >> Cc: hubi...@ucw.cz; Richard Biener <richard.guent...@gmail.com>
> >> Subject: Re: [PATCH] Add Optimization for various IPA parameters.
> >>
> >> Hi,
> >>
> >> On Fri, Jan 03 2020, Martin Liška wrote:
> >>> Hi.
> >>>
> >>> This is similar transformation for IPA passes. This time, one needs
> >>> to use opt_for_fn in order to get the right parameter values.
> >>>
> >>> @Martin, Honza:
> >>> There are last few remaining parameters which should use
> >>> opt_for_fn:
> >>>
> >>> param_ipa_cp_unit_growth
> >>
> >> So as we discussed, picking this one from one particular node is not
> >> what one would expect to happen, but inlining does it too and so anyway:
> >>
> >>
> >> IPA-CP: Always access param_ipcp_unit_growth through opt_for_fn
> >>
> >> 2020-01-07  Martin Jambor  <mjam...@suse.cz>
> >>
> >>    * params.opt (param_ipcp_unit_growth): Mark as Optimization.
> >>    * ipa-cp.c (max_new_size): Removed.
> >>    (orig_overall_size): New variable.
> >>    (get_max_overall_size): New function.
> >>    (estimate_local_effects): Use it.  Adjust dump.
> >>    (decide_about_value): Likewise.
> >>    (ipcp_propagate_stage): Do not calculate max_new_size, just store
> >>    orig_overall_size.  Adjust dump.
> >>    (ipa_cp_c_finalize): Clear orig_overall_size instead of max_new_size.
> >> ---
> >>   gcc/ipa-cp.c   | 39 ++++++++++++++++++++++++++-------------
> >>   gcc/params.opt |  2 +-
> >>   2 files changed, 27 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c index
> >> 9e20e278eff..c2572e3e0e8
> >> 100644
> >> --- a/gcc/ipa-cp.c
> >> +++ b/gcc/ipa-cp.c
> >> @@ -375,7 +375,7 @@ static profile_count max_count;
> >>
> >>   /* Original overall size of the program.  */
> >>
> >> -static long overall_size, max_new_size;
> >> +static long overall_size, orig_overall_size;
> >>
> >>   /* Node name to unique clone suffix number map.  */  static
> >> hash_map<const char *, unsigned> *clone_num_suffixes; @@ -3395,6
> >> +3395,23 @@ perform_estimation_of_a_value (cgraph_node *node,
> >> vec<tree> known_csts,
> >>     val->local_size_cost = size;
> >>   }
> >>
> >> +/* Get the overall limit oof growth based on parameters extracted
> >> +from
> >> growth.
> >> +   it does not really make sense to mix functions with different
> >> + overall
> >> growth
> >> +   limits but it is possible and if it happens, we do not want to select 
> >> one
> >> +   limit at random.  */
> >> +
> >> +static long
> >> +get_max_overall_size (cgraph_node *node) {
> >> +  long max_new_size = orig_overall_size;
> >> +  long large_unit = opt_for_fn (node->decl, param_large_unit_insns);
> >> +  if (max_new_size < large_unit)
> >> +    max_new_size = large_unit;
> >> +  int unit_growth = opt_for_fn (node->decl, param_ipcp_unit_growth);
> >> +  max_new_size += max_new_size * unit_growth / 100 + 1;
> >> +  return max_new_size;
> >> +}
> >> +
> >>   /* Iterate over known values of parameters of NODE and estimate the
> local
> >>      effects in terms of time and size they have.  */
> >>
> >> @@ -3457,7 +3474,7 @@ estimate_local_effects (struct cgraph_node
> *node)
> >>                                       stats.freq_sum, stats.count_sum,
> >>                                       size))
> >>    {
> >> -    if (size + overall_size <= max_new_size)
> >> +    if (size + overall_size <= get_max_overall_size (node))
> >>        {
> >>          info->do_clone_for_all_contexts = true;
> >>          overall_size += size;
> >> @@ -3467,8 +3484,8 @@ estimate_local_effects (struct cgraph_node
> *node)
> >>                     "known contexts, growth deemed beneficial.\n");
> >>        }
> >>      else if (dump_file && (dump_flags & TDF_DETAILS))
> >> -      fprintf (dump_file, "   Not cloning for all contexts because "
> >> -               "max_new_size would be reached with %li.\n",
> >> +      fprintf (dump_file, "  Not cloning for all contexts because "
> >> +               "maximum unit size would be reached with %li.\n",
> >>                 size + overall_size);
> >>    }
> >>         else if (dump_file && (dump_flags & TDF_DETAILS)) @@ -3860,14
> >> +3877,10 @@ ipcp_propagate_stage (class ipa_topo_info *topo)
> >>       max_count = max_count.max (node->count.ipa ());
> >>     }
> >>
> >> -  max_new_size = overall_size;
> >> -  if (max_new_size < param_large_unit_insns)
> >> -    max_new_size = param_large_unit_insns;
> >> -  max_new_size += max_new_size * param_ipcp_unit_growth / 100 + 1;
> >> +  orig_overall_size = overall_size;
> >>
> >>     if (dump_file)
> >> -    fprintf (dump_file, "\noverall_size: %li, max_new_size: %li\n",
> >> -       overall_size, max_new_size);
> >> +    fprintf (dump_file, "\noverall_size: %li\n", overall_size);
> >>
> >>     propagate_constants_topo (topo);
> >>     if (flag_checking)
> >> @@ -5380,11 +5393,11 @@ decide_about_value (struct cgraph_node
> *node,
> >> int index, HOST_WIDE_INT offset,
> >>         perhaps_add_new_callers (node, val);
> >>         return false;
> >>       }
> >> -  else if (val->local_size_cost + overall_size > max_new_size)
> >> +  else if (val->local_size_cost + overall_size >
> >> + get_max_overall_size
> >> + (node))
> >>       {
> >>         if (dump_file && (dump_flags & TDF_DETAILS))
> >>    fprintf (dump_file, "   Ignoring candidate value because "
> >> -           "max_new_size would be reached with %li.\n",
> >> +           "maximum unit size would be reached with %li.\n",
> >>             val->local_size_cost + overall_size);
> >>         return false;
> >>       }
> >> @@ -5928,6 +5941,6 @@ ipa_cp_c_finalize (void)  {
> >>     max_count = profile_count::uninitialized ();
> >>     overall_size = 0;
> >> -  max_new_size = 0;
> >> +  orig_overall_size = 0;
> >>     ipcp_free_transformation_sum ();
> >>   }
> >> diff --git a/gcc/params.opt b/gcc/params.opt index
> >> d5f0d9a69ce..3cd6d7d481d 100644
> >> --- a/gcc/params.opt
> >> +++ b/gcc/params.opt
> >> @@ -243,7 +243,7 @@ Common Joined UInteger
> >> Var(param_ipa_sra_ptr_growth_factor) Init(2) Param  Maximum allowed
> >> growth of number and total size of new parameters that ipa-sra
> >> replaces a pointer to an aggregate with.
> >>
> >>   -param=ipcp-unit-growth=
> >> -Common Joined UInteger Var(param_ipcp_unit_growth) Init(10) Param
> >> +Common Joined UInteger Var(param_ipcp_unit_growth) Optimization
> >> +Init(10) Param
> >>   How much can given compilation unit grow because of the
> >> interprocedural constant propagation (in percent).
> >>
> >>   -param=ira-loop-reserved-regs=
> >> --
> >> 2.24.1
> >

Reply via email to