Issue 4428074: Disable tracer by default for profile use
|
Created:
13 years ago by Sharad Singhai Modified:
13 years ago CC:
gcc-patches_gcc.gnu.org Base URL:
svn://gcc.gnu.org/svn/gcc/branches/google/main/gcc/ Visibility:
Public. |
Patch Set 1 #MessagesTotal messages: 11
This patch disables -ftracer for profile use. Okay for google/main? Thanks, Sharad 2011-04-28 Sharad Singhai <singhai@google.com> Google Ref 40087 * opts.c (common_handle_option): Disable -ftracer for profile use. * doc/invoke.texi: Update doc that -ftracer is no longer enabled for FDO. Index: doc/invoke.texi =================================================================== --- doc/invoke.texi (revision 173048) +++ doc/invoke.texi (working copy) @@ -7930,7 +7930,7 @@ generally profitable only with profile feedback available. The following options are enabled: @code{-fbranch-probabilities}, @code{-fvpt}, -@code{-funroll-loops}, @code{-fpeel-loops}, @code{-ftracer} +@code{-funroll-loops}, @code{-fpeel-loops}. By default, GCC emits an error message if the feedback profiles do not match the source code. This error can be turned into a warning by using Index: opts.c =================================================================== --- opts.c (revision 173048) +++ opts.c (working copy) @@ -1531,8 +1531,6 @@ opts->x_flag_unroll_loops = value; if (!opts_set->x_flag_peel_loops) opts->x_flag_peel_loops = value; - if (!opts_set->x_flag_tracer) - opts->x_flag_tracer = value; if (!opts_set->x_flag_value_profile_transformations) opts->x_flag_value_profile_transformations = value; if (!opts_set->x_flag_inline_functions) -- This patch is available for review at http://codereview.appspot.com/4428074
Sign in to reply to this message.
On Thu, Apr 28, 2011 at 14:51, Sharad Singhai <singhai@google.com> wrote: > This patch disables -ftracer for profile use. Okay for google/main? Could you elaborate on why doing this is beneficial? Are you proposing this for trunk as well? > 2011-04-28 Sharad Singhai <singhai@google.com> > > Google Ref 40087 > * opts.c (common_handle_option): Disable -ftracer for profile use. > * doc/invoke.texi: Update doc that -ftracer is no longer enabled for FDO. OK. Diego.
Sign in to reply to this message.
On Thu, Apr 28, 2011 at 11:53 AM, Diego Novillo <dnovillo@google.com> wrote: > On Thu, Apr 28, 2011 at 14:51, Sharad Singhai <singhai@google.com> wrote: > > This patch disables -ftracer for profile use. Okay for google/main? > > Could you elaborate on why doing this is beneficial? Are you > proposing this for trunk as well? > > The -ftracer pass causes much more code growth and hurts performance by 1-2%. It would need some tuning to get right. For now, disabling seemed the easiest option. No, I am not proposing this for the trunk as I have not yet quantified its effects on SPEC benchmarks. After more detailed performance analysis, it may not be needed for trunk and instead tuning the pass might be a better idea. Thanks, Sharad > > 2011-04-28 Sharad Singhai <singhai@google.com> > > > > Google Ref 40087 > > * opts.c (common_handle_option): Disable -ftracer for profile use. > > * doc/invoke.texi: Update doc that -ftracer is no longer enabled > for FDO. > > OK. > > > Diego. >
Sign in to reply to this message.
On Thu, Apr 28, 2011 at 8:53 PM, Diego Novillo <dnovillo@google.com> wrote: > On Thu, Apr 28, 2011 at 14:51, Sharad Singhai <singhai@google.com> wrote: >> This patch disables -ftracer for profile use. Okay for google/main? > > Could you elaborate on why doing this is beneficial? Are you > proposing this for trunk as well? It indeed doesn't make sense to me at all. Richard. >> 2011-04-28 Sharad Singhai <singhai@google.com> >> >> Google Ref 40087 >> * opts.c (common_handle_option): Disable -ftracer for profile use. >> * doc/invoke.texi: Update doc that -ftracer is no longer enabled for FDO. > > OK. > > > Diego. >
Sign in to reply to this message.
Sharad can provide some some performance data -- we have seen up to 2% degradation to with tracer turned on for one of google's most important program. Perhaps Sharad can collect some SPEC numbers. I agree a better approach should be to fix the problem in tracer instead of turning it off in trunk. David On Thu, Apr 28, 2011 at 4:23 PM, Richard Guenther <richard.guenther@gmail.com> wrote: > On Thu, Apr 28, 2011 at 8:53 PM, Diego Novillo <dnovillo@google.com> wrote: >> On Thu, Apr 28, 2011 at 14:51, Sharad Singhai <singhai@google.com> wrote: >>> This patch disables -ftracer for profile use. Okay for google/main? >> >> Could you elaborate on why doing this is beneficial? Are you >> proposing this for trunk as well? > > It indeed doesn't make sense to me at all. > > Richard. > >>> 2011-04-28 Sharad Singhai <singhai@google.com> >>> >>> Google Ref 40087 >>> * opts.c (common_handle_option): Disable -ftracer for profile use. >>> * doc/invoke.texi: Update doc that -ftracer is no longer enabled for FDO. >> >> OK. >> >> >> Diego. >> >
Sign in to reply to this message.
On Fri, Apr 29, 2011 at 1:34 AM, Xinliang David Li <davidxl@google.com> wrote: > Sharad can provide some some performance data -- we have seen up to 2% > degradation to with tracer turned on for one of google's most > important program. Perhaps Sharad can collect some SPEC numbers. > > I agree a better approach should be to fix the problem in tracer > instead of turning it off in trunk. Esp. not turning it off for profile-use only where it should have the most precise input. Richard. > David > > > On Thu, Apr 28, 2011 at 4:23 PM, Richard Guenther > <richard.guenther@gmail.com> wrote: >> On Thu, Apr 28, 2011 at 8:53 PM, Diego Novillo <dnovillo@google.com> wrote: >>> On Thu, Apr 28, 2011 at 14:51, Sharad Singhai <singhai@google.com> wrote: >>>> This patch disables -ftracer for profile use. Okay for google/main? >>> >>> Could you elaborate on why doing this is beneficial? Are you >>> proposing this for trunk as well? >> >> It indeed doesn't make sense to me at all. >> >> Richard. >> >>>> 2011-04-28 Sharad Singhai <singhai@google.com> >>>> >>>> Google Ref 40087 >>>> * opts.c (common_handle_option): Disable -ftracer for profile use. >>>> * doc/invoke.texi: Update doc that -ftracer is no longer enabled for FDO. >>> >>> OK. >>> >>> >>> Diego. >>> >> >
Sign in to reply to this message.
Tracer is not turned on by default for non-FDO case -- Sharad's change simply turns it off by default for all cases. David On Thu, Apr 28, 2011 at 4:38 PM, Richard Guenther <richard.guenther@gmail.com> wrote: > On Fri, Apr 29, 2011 at 1:34 AM, Xinliang David Li <davidxl@google.com> wrote: >> Sharad can provide some some performance data -- we have seen up to 2% >> degradation to with tracer turned on for one of google's most >> important program. Perhaps Sharad can collect some SPEC numbers. >> >> I agree a better approach should be to fix the problem in tracer >> instead of turning it off in trunk. > > Esp. not turning it off for profile-use only where it should have the most > precise input. > > Richard. > >> David >> >> >> On Thu, Apr 28, 2011 at 4:23 PM, Richard Guenther >> <richard.guenther@gmail.com> wrote: >>> On Thu, Apr 28, 2011 at 8:53 PM, Diego Novillo <dnovillo@google.com> wrote: >>>> On Thu, Apr 28, 2011 at 14:51, Sharad Singhai <singhai@google.com> wrote: >>>>> This patch disables -ftracer for profile use. Okay for google/main? >>>> >>>> Could you elaborate on why doing this is beneficial? Are you >>>> proposing this for trunk as well? >>> >>> It indeed doesn't make sense to me at all. >>> >>> Richard. >>> >>>>> 2011-04-28 Sharad Singhai <singhai@google.com> >>>>> >>>>> Google Ref 40087 >>>>> * opts.c (common_handle_option): Disable -ftracer for profile use. >>>>> * doc/invoke.texi: Update doc that -ftracer is no longer enabled for FDO. >>>> >>>> OK. >>>> >>>> >>>> Diego. >>>> >>> >> >
Sign in to reply to this message.
On Thu, Apr 28, 2011 at 4:38 PM, Richard Guenther < richard.guenther@gmail.com> wrote: > On Fri, Apr 29, 2011 at 1:34 AM, Xinliang David Li <davidxl@google.com> > wrote: > > Sharad can provide some some performance data -- we have seen up to 2% > > degradation to with tracer turned on for one of google's most > > important program. Perhaps Sharad can collect some SPEC numbers. > > > > I agree a better approach should be to fix the problem in tracer > > instead of turning it off in trunk. > > Esp. not turning it off for profile-use only where it should have the most > precise input. > > Yes. As I explained, I am not proposing to turn -ftracer off in the trunk. After some tuning the pass should be improved. As of now, during the performance analysis of the internal benchmarks, I have seen up to 2% performance loss with -ftracer and hence turned it off during FDO. I would do SPEC performance numbers and send those out. Thanks, Sharad > Richard. > > > David > > > > > > On Thu, Apr 28, 2011 at 4:23 PM, Richard Guenther > > <richard.guenther@gmail.com> wrote: > >> On Thu, Apr 28, 2011 at 8:53 PM, Diego Novillo <dnovillo@google.com> > wrote: > >>> On Thu, Apr 28, 2011 at 14:51, Sharad Singhai <singhai@google.com> > wrote: > >>>> This patch disables -ftracer for profile use. Okay for google/main? > >>> > >>> Could you elaborate on why doing this is beneficial? Are you > >>> proposing this for trunk as well? > >> > >> It indeed doesn't make sense to me at all. > >> > >> Richard. > >> > >>>> 2011-04-28 Sharad Singhai <singhai@google.com> > >>>> > >>>> Google Ref 40087 > >>>> * opts.c (common_handle_option): Disable -ftracer for profile > use. > >>>> * doc/invoke.texi: Update doc that -ftracer is no longer > enabled for FDO. > >>> > >>> OK. > >>> > >>> > >>> Diego. > >>> > >> > > >
Sign in to reply to this message.
2011/4/29 Sharad Singhai (शरद सिंघई) <singhai@google.com>: > On Thu, Apr 28, 2011 at 4:38 PM, Richard Guenther > <richard.guenther@gmail.com> wrote: >> >> On Fri, Apr 29, 2011 at 1:34 AM, Xinliang David Li <davidxl@google.com> >> wrote: >> > Sharad can provide some some performance data -- we have seen up to 2% >> > degradation to with tracer turned on for one of google's most >> > important program. Perhaps Sharad can collect some SPEC numbers. >> > >> > I agree a better approach should be to fix the problem in tracer >> > instead of turning it off in trunk. >> >> Esp. not turning it off for profile-use only where it should have the most >> precise input. >> > > Yes. As I explained, I am not proposing to turn -ftracer off in the trunk. Ah, sorry for the confusion on my side. If you can back up the change with SPEC performance numbers it's ok for trunk as well. But indeed coverage of -ftracer will be zero then, and I wonder what it is useful for if it doesn't even do good with FDO ... Richard. > After some tuning the pass should be improved. > As of now, during the performance analysis of the internal benchmarks, I > have seen up to 2% performance loss with -ftracer and hence turned it off > during FDO. > I would do SPEC performance numbers and send those out. > Thanks, > Sharad > >> >> Richard. >> >> > David >> > >> > >> > On Thu, Apr 28, 2011 at 4:23 PM, Richard Guenther >> > <richard.guenther@gmail.com> wrote: >> >> On Thu, Apr 28, 2011 at 8:53 PM, Diego Novillo <dnovillo@google.com> >> >> wrote: >> >>> On Thu, Apr 28, 2011 at 14:51, Sharad Singhai <singhai@google.com> >> >>> wrote: >> >>>> This patch disables -ftracer for profile use. Okay for google/main? >> >>> >> >>> Could you elaborate on why doing this is beneficial? Are you >> >>> proposing this for trunk as well? >> >> >> >> It indeed doesn't make sense to me at all. >> >> >> >> Richard. >> >> >> >>>> 2011-04-28 Sharad Singhai <singhai@google.com> >> >>>> >> >>>> Google Ref 40087 >> >>>> * opts.c (common_handle_option): Disable -ftracer for profile >> >>>> use. >> >>>> * doc/invoke.texi: Update doc that -ftracer is no longer >> >>>> enabled for FDO. >> >>> >> >>> OK. >> >>> >> >>> >> >>> Diego. >> >>> >> >> >> > > >
Sign in to reply to this message.
On 2011/04/28 18:53:24, Diego Novillo wrote: > OK. Committed rev 173186. Diego.
Sign in to reply to this message.
> 2011/4/29 Sharad Singhai (????????? ???????????????) <singhai@google.com>: > > On Thu, Apr 28, 2011 at 4:38 PM, Richard Guenther > > <richard.guenther@gmail.com> wrote: > >> > >> On Fri, Apr 29, 2011 at 1:34 AM, Xinliang David Li <davidxl@google.com> > >> wrote: > >> > Sharad can provide some some performance data -- we have seen up to 2% > >> > degradation to with tracer turned on for one of google's most > >> > important program. Perhaps Sharad can collect some SPEC numbers. > >> > > >> > I agree a better approach should be to fix the problem in tracer > >> > instead of turning it off in trunk. > >> > >> Esp. not turning it off for profile-use only where it should have the most > >> precise input. > >> > > > > Yes. As I explained, I am not proposing to turn -ftracer off in the trunk. > > Ah, sorry for the confusion on my side. If you can back up the change > with SPEC performance numbers it's ok for trunk as well. But indeed > coverage of -ftracer will be zero then, and I wonder what it is useful > for if it doesn't even do good with FDO ... Tracer definitely did measurable SPEC speedups at a time it was implemented. So we are most likely running into some bug, like misupdated profile or tracer confusing something more important. If you provide me some testcase, I can try to debug it. Honza
Sign in to reply to this message.
|
