Message ID | CAAe5K+WiCh0vBSSb14UJMD=tjLpJCLdH2p_oup2Z7Q3C71HMnA@mail.gmail.com |
---|---|
State | New |
Headers | show |
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.
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.
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
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
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,