Patchwork [google,gcc-4_8] Restore max peeled instructions to old default

login
register
mail settings
Submitter Teresa Johnson
Date June 6, 2013, 8:22 p.m.
Message ID <CAAe5K+WiCh0vBSSb14UJMD=tjLpJCLdH2p_oup2Z7Q3C71HMnA@mail.gmail.com>
Download mbox | patch
Permalink /patch/249541/
State New
Headers show

Comments

Teresa Johnson - June 6, 2013, 8:22 p.m.
The default for the max instructions in peeled loops was reduced on
trunk in r193570. This is causing a performance regression on an internal
benchmark. This change will revert to the old higher limits.

Google ref b/8839137.

Bootstrapped and tested. Ok for google/4_8?

Thanks,
Teresa

2013-06-06  Teresa Johnson  <tejohnson@google.com>

* params.def (PARAM_MAX_PEELED_INSNS): Revert to 400.
        (PARAM_MAX_COMPLETELY_PEELED_INSNS): Ditto.

  "max-completely-peel-times",


--
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
Diego Novillo - June 6, 2013, 8:26 p.m.
On 2013-06-06 16:22 , Teresa Johnson wrote:
> The default for the max instructions in peeled loops was reduced on
> trunk in r193570. This is causing a performance regression on an internal
> benchmark. This change will revert to the old higher limits.
>
> Google ref b/8839137.
>
> Bootstrapped and tested. Ok for google/4_8?

I wonder if this isn't something we want to set in our internal build 
configuration files.  I don't have a strong opinion one way or the 
other, however.  I'm only saying this because it would mean 1 less patch 
for us to maintain.


Diego.
Xinliang David Li - June 6, 2013, 8:30 p.m.
We should make the default setting right for our environment. The
patch is trivial to maintain.

thanks,

David

On Thu, Jun 6, 2013 at 1:26 PM, Diego Novillo <dnovillo@google.com> wrote:
> On 2013-06-06 16:22 , Teresa Johnson wrote:
>>
>> The default for the max instructions in peeled loops was reduced on
>> trunk in r193570. This is causing a performance regression on an internal
>> benchmark. This change will revert to the old higher limits.
>>
>> Google ref b/8839137.
>>
>> Bootstrapped and tested. Ok for google/4_8?
>
>
> I wonder if this isn't something we want to set in our internal build
> configuration files.  I don't have a strong opinion one way or the other,
> however.  I'm only saying this because it would mean 1 less patch for us to
> maintain.
>
>
> Diego.
Xinliang David Li - June 6, 2013, 8:33 p.m.
ok.   Wht is the rational for dropping the limit in trunk?  Ideally,
the limit should be lifted up and to enable other heuristics to kick
in.

David

On Thu, Jun 6, 2013 at 1:22 PM, Teresa Johnson <tejohnson@google.com> wrote:
> The default for the max instructions in peeled loops was reduced on
> trunk in r193570. This is causing a performance regression on an internal
> benchmark. This change will revert to the old higher limits.
>
> Google ref b/8839137.
>
> Bootstrapped and tested. Ok for google/4_8?
>
> Thanks,
> Teresa
>
> 2013-06-06  Teresa Johnson  <tejohnson@google.com>
>
> * params.def (PARAM_MAX_PEELED_INSNS): Revert to 400.
>         (PARAM_MAX_COMPLETELY_PEELED_INSNS): Ditto.
>
> Index: params.def
> ===================================================================
> --- params.def (revision 199753)
> +++ params.def (working copy)
> @@ -306,7 +306,7 @@ DEFPARAM(PARAM_MAX_UNROLL_TIMES,
>  DEFPARAM(PARAM_MAX_PEELED_INSNS,
>   "max-peeled-insns",
>   "The maximum number of insns of a peeled loop",
> - 100, 0, 0)
> + 400, 0, 0)
>  /* The maximum number of peelings of a single loop.  */
>  DEFPARAM(PARAM_MAX_PEEL_TIMES,
>   "max-peel-times",
> @@ -321,7 +321,7 @@ DEFPARAM(PARAM_MAX_PEEL_BRANCHES,
>  DEFPARAM(PARAM_MAX_COMPLETELY_PEELED_INSNS,
>   "max-completely-peeled-insns",
>   "The maximum number of insns of a completely peeled loop",
> - 100, 0, 0)
> + 400, 0, 0)
>  /* The maximum number of peelings of a single loop that is peeled
> completely.  */
>  DEFPARAM(PARAM_MAX_COMPLETELY_PEEL_TIMES,
>   "max-completely-peel-times",
>
>
> --
> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
Teresa Johnson - June 6, 2013, 11:15 p.m.
On Thu, Jun 6, 2013 at 1:33 PM, Xinliang David Li <davidxl@google.com> wrote:
> ok.   Wht is the rational for dropping the limit in trunk?  Ideally,
> the limit should be lifted up and to enable other heuristics to kick
> in.

Here is the message about it from Honza:

http://gcc.gnu.org/ml/gcc-patches/2012-11/msg01193.html

Basically, it was to reduce code bloat, and it didn't show spec regressions.

Teresa

>
> David
>
> On Thu, Jun 6, 2013 at 1:22 PM, Teresa Johnson <tejohnson@google.com> wrote:
>> The default for the max instructions in peeled loops was reduced on
>> trunk in r193570. This is causing a performance regression on an internal
>> benchmark. This change will revert to the old higher limits.
>>
>> Google ref b/8839137.
>>
>> Bootstrapped and tested. Ok for google/4_8?
>>
>> Thanks,
>> Teresa
>>
>> 2013-06-06  Teresa Johnson  <tejohnson@google.com>
>>
>> * params.def (PARAM_MAX_PEELED_INSNS): Revert to 400.
>>         (PARAM_MAX_COMPLETELY_PEELED_INSNS): Ditto.
>>
>> Index: params.def
>> ===================================================================
>> --- params.def (revision 199753)
>> +++ params.def (working copy)
>> @@ -306,7 +306,7 @@ DEFPARAM(PARAM_MAX_UNROLL_TIMES,
>>  DEFPARAM(PARAM_MAX_PEELED_INSNS,
>>   "max-peeled-insns",
>>   "The maximum number of insns of a peeled loop",
>> - 100, 0, 0)
>> + 400, 0, 0)
>>  /* The maximum number of peelings of a single loop.  */
>>  DEFPARAM(PARAM_MAX_PEEL_TIMES,
>>   "max-peel-times",
>> @@ -321,7 +321,7 @@ DEFPARAM(PARAM_MAX_PEEL_BRANCHES,
>>  DEFPARAM(PARAM_MAX_COMPLETELY_PEELED_INSNS,
>>   "max-completely-peeled-insns",
>>   "The maximum number of insns of a completely peeled loop",
>> - 100, 0, 0)
>> + 400, 0, 0)
>>  /* The maximum number of peelings of a single loop that is peeled
>> completely.  */
>>  DEFPARAM(PARAM_MAX_COMPLETELY_PEEL_TIMES,
>>   "max-completely-peel-times",
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413



--
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

Patch

Index: params.def
===================================================================
--- params.def (revision 199753)
+++ params.def (working copy)
@@ -306,7 +306,7 @@  DEFPARAM(PARAM_MAX_UNROLL_TIMES,
 DEFPARAM(PARAM_MAX_PEELED_INSNS,
  "max-peeled-insns",
  "The maximum number of insns of a peeled loop",
- 100, 0, 0)
+ 400, 0, 0)
 /* The maximum number of peelings of a single loop.  */
 DEFPARAM(PARAM_MAX_PEEL_TIMES,
  "max-peel-times",
@@ -321,7 +321,7 @@  DEFPARAM(PARAM_MAX_PEEL_BRANCHES,
 DEFPARAM(PARAM_MAX_COMPLETELY_PEELED_INSNS,
  "max-completely-peeled-insns",
  "The maximum number of insns of a completely peeled loop",
- 100, 0, 0)
+ 400, 0, 0)
 /* The maximum number of peelings of a single loop that is peeled
completely.  */
 DEFPARAM(PARAM_MAX_COMPLETELY_PEEL_TIMES,