[PATCH] Remove params for a specific optimization option.

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Remove params for a specific optimization option.

Martin Liška-2
Hello.

The patch is about removal of special *-O2 parameters that Honza added some time
ago. Right now, we have a better mechanism how to have a different default value
for a parameter. I'm planning to send a patch that will decorate some params
with Optimization attribute and so that one can sensitively use them per-function.

The patch is not a NOP, mainly because of param_max_inline_insns_auto params.
As one can see:

gcc/ipa-cp.c:      if (size <= param_max_inline_insns_auto / 4)
gcc/ipa-cp.c:      else if (size <= param_max_inline_insns_auto / 2)
gcc/ipa-cp.c:      else if (size <= param_max_inline_insns_auto
gcc/ipa-inline.c:inline_insns_auto (cgraph_node *n, bool hint)
gcc/ipa-inline.c:       return param_max_inline_insns_auto
gcc/ipa-inline.c:      return param_max_inline_insns_auto;
gcc/ipa-inline.c:       return param_max_inline_insns_auto_o2
gcc/ipa-inline.c:      return param_max_inline_insns_auto_o2;
gcc/ipa-inline.c:                                   inline_insns_auto (caller, false))))
gcc/ipa-inline.c:             > inline_insns_auto (e->caller, true))
gcc/ipa-inline.c:      /* Apply param_max_inline_insns_auto limit for functions not declared
gcc/ipa-inline.c:              && growth >= inline_insns_auto (e->caller, apply_hints)
gcc/ipa-inline.c:                  || growth >= inline_insns_auto (e->caller, true)
gcc/ipa-inline.c:                   : inline_insns_auto (caller, false)))
gcc/ipa-split.c:                        : param_max_inline_insns_auto) + 10)
gcc/ipa-split.c:      && current->split_size >= (unsigned int) param_max_inline_insns_auto + 10)
gcc/opts.c:      SET_OPTION_IF_UNSET (opts, opts_set, param_max_inline_insns_auto,
gcc/params.opt:Common Joined UInteger Var(param_max_inline_insns_auto) Init(30) Param
gcc/params.opt:Common Joined UInteger Var(param_max_inline_insns_auto_o2) Init(15) Param

The param_max_inline_insns_auto_o2 version is used with an optimization level smaller then -O3
in ipa-inline.c (but not in ipa-cp.c and ipa-split.c). That's very inconsistent hack.
That's why I needed to adjust an asan test, where the expected backtrace has changed.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2019-11-12  Martin Liska  <[hidden email]>

        * cif-code.def (MAX_INLINE_INSNS_SINGLE_O2_LIMIT): Remove.
        (MAX_INLINE_INSNS_AUTO_O2_LIMIT): Remove.
        * ipa-inline.c (inline_insns_single): Remove n argument
        and do not use *_o2 parameters.
        (inline_insns_auto): Likewise.
        (can_inline_edge_by_limits_p): Do not use *_o2 parameters.
        (want_early_inline_function_p): Likewise.
        (big_speedup_p): Likewise.
        (want_inline_small_function_p): Likewise.
        (edge_badness): Likewise.
        * opts.c (maybe_default_option): Do not drop default value
        for parameters.
        (default_options_table): Add -O3+ changes for parameters.
        * params.opt: Remove all *-O2 params.

gcc/testsuite/ChangeLog:

2019-11-12  Martin Liska  <[hidden email]>

        * g++.dg/tree-ssa/pr53844.C: Remove all *-O2 params.
        * g++.dg/tree-ssa/pr61034.C: Likewise.
        * g++.dg/tree-ssa/pr8781.C: Likewise.
        * g++.dg/warn/Wstringop-truncation-1.C: Likewise.
        * gcc.dg/ipa/pr63416.c: Likewise.
        * gcc.dg/tree-ssa/ssa-thread-12.c: Likewise.
        * gcc.dg/vect/pr66142.c: Likewise.
        * gcc.dg/winline-3.c: Likewise.
        * gcc.target/powerpc/pr72804.c: Likewise.

gcc/testsuite/ChangeLog:

2019-11-12  Martin Liska  <[hidden email]>

        * c-c++-common/asan/memcmp-1.c: Update expected backtrace.
---
  gcc/cif-code.def                              |   4 -
  gcc/ipa-inline.c                              | 104 ++++++------------
  gcc/opts.c                                    |  11 +-
  gcc/params.opt                                |  32 +-----
  gcc/testsuite/c-c++-common/asan/memcmp-1.c    |   4 +-
  gcc/testsuite/g++.dg/tree-ssa/pr53844.C       |   2 +-
  gcc/testsuite/g++.dg/tree-ssa/pr61034.C       |   2 +-
  gcc/testsuite/g++.dg/tree-ssa/pr8781.C        |   2 +-
  .../g++.dg/warn/Wstringop-truncation-1.C      |   2 +-
  gcc/testsuite/gcc.dg/ipa/pr63416.c            |   2 +-
  gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-12.c |   2 +-
  gcc/testsuite/gcc.dg/vect/pr66142.c           |   2 +-
  gcc/testsuite/gcc.dg/winline-3.c              |   2 +-
  gcc/testsuite/gcc.target/powerpc/pr72804.c    |   2 +-
  14 files changed, 58 insertions(+), 115 deletions(-)



0001-Remove-params-for-a-specific-optimization-option.patch (19K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Remove params for a specific optimization option.

Jan Hubicka-2
> Hello.
>
> The patch is about removal of special *-O2 parameters that Honza added some time
> ago. Right now, we have a better mechanism how to have a different default value
> for a parameter. I'm planning to send a patch that will decorate some params
> with Optimization attribute and so that one can sensitively use them per-function.
>
> The patch is not a NOP, mainly because of param_max_inline_insns_auto params.
> As one can see:
>
> gcc/ipa-cp.c:      if (size <= param_max_inline_insns_auto / 4)
> gcc/ipa-cp.c:      else if (size <= param_max_inline_insns_auto / 2)
> gcc/ipa-cp.c:      else if (size <= param_max_inline_insns_auto
> gcc/ipa-inline.c:inline_insns_auto (cgraph_node *n, bool hint)
> gcc/ipa-inline.c:       return param_max_inline_insns_auto
> gcc/ipa-inline.c:      return param_max_inline_insns_auto;
> gcc/ipa-inline.c:       return param_max_inline_insns_auto_o2
> gcc/ipa-inline.c:      return param_max_inline_insns_auto_o2;
> gcc/ipa-inline.c:                                   inline_insns_auto (caller, false))))
> gcc/ipa-inline.c:             > inline_insns_auto (e->caller, true))
> gcc/ipa-inline.c:      /* Apply param_max_inline_insns_auto limit for functions not declared
> gcc/ipa-inline.c:              && growth >= inline_insns_auto (e->caller, apply_hints)
> gcc/ipa-inline.c:                  || growth >= inline_insns_auto (e->caller, true)
> gcc/ipa-inline.c:                   : inline_insns_auto (caller, false)))
> gcc/ipa-split.c:                        : param_max_inline_insns_auto) + 10)
> gcc/ipa-split.c:      && current->split_size >= (unsigned int) param_max_inline_insns_auto + 10)
> gcc/opts.c:      SET_OPTION_IF_UNSET (opts, opts_set, param_max_inline_insns_auto,
> gcc/params.opt:Common Joined UInteger Var(param_max_inline_insns_auto) Init(30) Param
> gcc/params.opt:Common Joined UInteger Var(param_max_inline_insns_auto_o2) Init(15) Param
>
> diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
> index df6f991ad79..9b96ca66137 100644
> --- a/gcc/ipa-inline.c
> +++ b/gcc/ipa-inline.c
> @@ -393,44 +393,24 @@ can_inline_edge_p (struct cgraph_edge *e, bool report,
>     scale up the bound.  */
>  
>  static int
> -inline_insns_single (cgraph_node *n, bool hint)
> +inline_insns_single (bool hint)
>  {
> -  if (opt_for_fn (n->decl, optimize) >= 3)
> -    {
> -      if (hint)
> - return param_max_inline_insns_single
> -       * param_inline_heuristics_hint_percent / 100;
> -      return param_max_inline_insns_single;
> -    }
> -  else
> -    {
> -      if (hint)
> - return param_max_inline_insns_single_o2
> -       * param_inline_heuristics_hint_percent_o2 / 100;
> -      return param_max_inline_insns_single_o2;
> -    }
> +  if (hint)
> +    return param_max_inline_insns_single
> +      * param_inline_heuristics_hint_percent / 100;
> +  return param_max_inline_insns_single;

I do not see how this can be nop for LTO where you combine -O2 and -O3
sources? With removing the N parameter there is no way to say what
value should be used here.

I think we need to decorate them with Optimization first and
then do something like param_for_fn and use it here?

Honza
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Remove params for a specific optimization option.

Martin Liška-2
On 11/13/19 9:20 AM, Jan Hubicka wrote:

>> Hello.
>>
>> The patch is about removal of special *-O2 parameters that Honza added some time
>> ago. Right now, we have a better mechanism how to have a different default value
>> for a parameter. I'm planning to send a patch that will decorate some params
>> with Optimization attribute and so that one can sensitively use them per-function.
>>
>> The patch is not a NOP, mainly because of param_max_inline_insns_auto params.
>> As one can see:
>>
>> gcc/ipa-cp.c:      if (size <= param_max_inline_insns_auto / 4)
>> gcc/ipa-cp.c:      else if (size <= param_max_inline_insns_auto / 2)
>> gcc/ipa-cp.c:      else if (size <= param_max_inline_insns_auto
>> gcc/ipa-inline.c:inline_insns_auto (cgraph_node *n, bool hint)
>> gcc/ipa-inline.c:       return param_max_inline_insns_auto
>> gcc/ipa-inline.c:      return param_max_inline_insns_auto;
>> gcc/ipa-inline.c:       return param_max_inline_insns_auto_o2
>> gcc/ipa-inline.c:      return param_max_inline_insns_auto_o2;
>> gcc/ipa-inline.c:                                   inline_insns_auto (caller, false))))
>> gcc/ipa-inline.c:             > inline_insns_auto (e->caller, true))
>> gcc/ipa-inline.c:      /* Apply param_max_inline_insns_auto limit for functions not declared
>> gcc/ipa-inline.c:              && growth >= inline_insns_auto (e->caller, apply_hints)
>> gcc/ipa-inline.c:                  || growth >= inline_insns_auto (e->caller, true)
>> gcc/ipa-inline.c:                   : inline_insns_auto (caller, false)))
>> gcc/ipa-split.c:                        : param_max_inline_insns_auto) + 10)
>> gcc/ipa-split.c:      && current->split_size >= (unsigned int) param_max_inline_insns_auto + 10)
>> gcc/opts.c:      SET_OPTION_IF_UNSET (opts, opts_set, param_max_inline_insns_auto,
>> gcc/params.opt:Common Joined UInteger Var(param_max_inline_insns_auto) Init(30) Param
>> gcc/params.opt:Common Joined UInteger Var(param_max_inline_insns_auto_o2) Init(15) Param
>>
>> diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
>> index df6f991ad79..9b96ca66137 100644
>> --- a/gcc/ipa-inline.c
>> +++ b/gcc/ipa-inline.c
>> @@ -393,44 +393,24 @@ can_inline_edge_p (struct cgraph_edge *e, bool report,
>>      scale up the bound.  */
>>  
>>   static int
>> -inline_insns_single (cgraph_node *n, bool hint)
>> +inline_insns_single (bool hint)
>>   {
>> -  if (opt_for_fn (n->decl, optimize) >= 3)
>> -    {
>> -      if (hint)
>> - return param_max_inline_insns_single
>> -       * param_inline_heuristics_hint_percent / 100;
>> -      return param_max_inline_insns_single;
>> -    }
>> -  else
>> -    {
>> -      if (hint)
>> - return param_max_inline_insns_single_o2
>> -       * param_inline_heuristics_hint_percent_o2 / 100;
>> -      return param_max_inline_insns_single_o2;
>> -    }
>> +  if (hint)
>> +    return param_max_inline_insns_single
>> +      * param_inline_heuristics_hint_percent / 100;
>> +  return param_max_inline_insns_single;
>
> I do not see how this can be nop for LTO where you combine -O2 and -O3
> sources? With removing the N parameter there is no way to say what
> value should be used here.
Ah, I see, you are right. So let's do it the other way around and let's
start with the Optimization keyword first.

>
> I think we need to decorate them with Optimization first and
> then do something like param_for_fn and use it here?

No, we'll use opt_for_fn and it will all work as expected.

I'm sending draft of the patch where I add Optimization keyword.
I put the keyword on params that affect behavior of a function
optimizations (TREE/RTL). I skipped GGC params, ASAN params and basically
all params that influence IPA optimization. I know that most of the inliner
parameters should become Optimization and I'm leaving that to Martin and
Honza.

Martin

>
> Honza
>


params-optimization.patch (52K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Remove params for a specific optimization option.

Richard Biener-2
On Wed, Nov 13, 2019 at 9:30 AM Martin Liška <[hidden email]> wrote:

>
> On 11/13/19 9:20 AM, Jan Hubicka wrote:
> >> Hello.
> >>
> >> The patch is about removal of special *-O2 parameters that Honza added some time
> >> ago. Right now, we have a better mechanism how to have a different default value
> >> for a parameter. I'm planning to send a patch that will decorate some params
> >> with Optimization attribute and so that one can sensitively use them per-function.
> >>
> >> The patch is not a NOP, mainly because of param_max_inline_insns_auto params.
> >> As one can see:
> >>
> >> gcc/ipa-cp.c:      if (size <= param_max_inline_insns_auto / 4)
> >> gcc/ipa-cp.c:      else if (size <= param_max_inline_insns_auto / 2)
> >> gcc/ipa-cp.c:      else if (size <= param_max_inline_insns_auto
> >> gcc/ipa-inline.c:inline_insns_auto (cgraph_node *n, bool hint)
> >> gcc/ipa-inline.c:       return param_max_inline_insns_auto
> >> gcc/ipa-inline.c:      return param_max_inline_insns_auto;
> >> gcc/ipa-inline.c:       return param_max_inline_insns_auto_o2
> >> gcc/ipa-inline.c:      return param_max_inline_insns_auto_o2;
> >> gcc/ipa-inline.c:                                   inline_insns_auto (caller, false))))
> >> gcc/ipa-inline.c:             > inline_insns_auto (e->caller, true))
> >> gcc/ipa-inline.c:      /* Apply param_max_inline_insns_auto limit for functions not declared
> >> gcc/ipa-inline.c:              && growth >= inline_insns_auto (e->caller, apply_hints)
> >> gcc/ipa-inline.c:                  || growth >= inline_insns_auto (e->caller, true)
> >> gcc/ipa-inline.c:                   : inline_insns_auto (caller, false)))
> >> gcc/ipa-split.c:                        : param_max_inline_insns_auto) + 10)
> >> gcc/ipa-split.c:      && current->split_size >= (unsigned int) param_max_inline_insns_auto + 10)
> >> gcc/opts.c:      SET_OPTION_IF_UNSET (opts, opts_set, param_max_inline_insns_auto,
> >> gcc/params.opt:Common Joined UInteger Var(param_max_inline_insns_auto) Init(30) Param
> >> gcc/params.opt:Common Joined UInteger Var(param_max_inline_insns_auto_o2) Init(15) Param
> >>
> >> diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
> >> index df6f991ad79..9b96ca66137 100644
> >> --- a/gcc/ipa-inline.c
> >> +++ b/gcc/ipa-inline.c
> >> @@ -393,44 +393,24 @@ can_inline_edge_p (struct cgraph_edge *e, bool report,
> >>      scale up the bound.  */
> >>
> >>   static int
> >> -inline_insns_single (cgraph_node *n, bool hint)
> >> +inline_insns_single (bool hint)
> >>   {
> >> -  if (opt_for_fn (n->decl, optimize) >= 3)
> >> -    {
> >> -      if (hint)
> >> -    return param_max_inline_insns_single
> >> -           * param_inline_heuristics_hint_percent / 100;
> >> -      return param_max_inline_insns_single;
> >> -    }
> >> -  else
> >> -    {
> >> -      if (hint)
> >> -    return param_max_inline_insns_single_o2
> >> -           * param_inline_heuristics_hint_percent_o2 / 100;
> >> -      return param_max_inline_insns_single_o2;
> >> -    }
> >> +  if (hint)
> >> +    return param_max_inline_insns_single
> >> +      * param_inline_heuristics_hint_percent / 100;
> >> +  return param_max_inline_insns_single;
> >
> > I do not see how this can be nop for LTO where you combine -O2 and -O3
> > sources? With removing the N parameter there is no way to say what
> > value should be used here.
>
> Ah, I see, you are right. So let's do it the other way around and let's
> start with the Optimization keyword first.
>
> >
> > I think we need to decorate them with Optimization first and
> > then do something like param_for_fn and use it here?
>
> No, we'll use opt_for_fn and it will all work as expected.
>
> I'm sending draft of the patch where I add Optimization keyword.

Hmm, can you please - as exercise - add Optimization only for
the "formerly" _o2 params you remove in the other patch to see
if with this you indeed get at a NOP effect?

Thanks,
Richard.

> I put the keyword on params that affect behavior of a function
> optimizations (TREE/RTL). I skipped GGC params, ASAN params and basically
> all params that influence IPA optimization. I know that most of the inliner
> parameters should become Optimization and I'm leaving that to Martin and
> Honza.
>
> Martin
>
> >
> > Honza
> >
>
Reply | Threaded
Open this post in threaded view
|

[PATCH] Add Optimization keyword for param_max_inline_insns_auto param.

Martin Liška-2
On 11/13/19 2:36 PM, Richard Biener wrote:
> Hmm, can you please - as exercise - add Optimization only for
> the "formerly" _o2 params you remove in the other patch to see
> if with this you indeed get at a NOP effect?

Sure, there's a patch that removed max-inline-insns-auto-O2.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
I see the same backtrace change in libsanitizer as in the original
patch series.

Ready to be installed?
Thanks,
Martin

0001-Add-Optimization-keyword-for-param_max_inline_insns_.patch (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add Optimization keyword for param_max_inline_insns_auto param.

Richard Biener-2
On Thu, Nov 14, 2019 at 1:00 PM Martin Liška <[hidden email]> wrote:

>
> On 11/13/19 2:36 PM, Richard Biener wrote:
> > Hmm, can you please - as exercise - add Optimization only for
> > the "formerly" _o2 params you remove in the other patch to see
> > if with this you indeed get at a NOP effect?
>
> Sure, there's a patch that removed max-inline-insns-auto-O2.
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> I see the same backtrace change in libsanitizer as in the original
> patch series.
>
> Ready to be installed?

OK.

Richard.

> Thanks,
> Martin