Disable tracer by default for profile use

[Sharad Singhai, 2011-04-28 18:51:28]

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

[Diego Novillo, 2011-04-28 18:53:24]

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.

[Sharad Singhai, 2011-04-28 19:02:03]

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.
>

[richard.guenther_gmail.com, 2011-04-28 23:23:19]

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.
>

[davidxl, 2011-04-28 23:34:26]

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.
>>
>

[richard.guenther_gmail.com, 2011-04-28 23:38:04]

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.
>>>
>>
>

[davidxl, 2011-04-28 23:43:19]

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.
>>>>
>>>
>>
>

[Sharad Singhai, 2011-04-28 23:43:27]

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.
> >>>
> >>
> >
>

[richard.guenther_gmail.com, 2011-04-29 08:26:02]

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.
>> >>>
>> >>
>> >
>
>

[Diego Novillo, 2011-04-29 20:57:28]

On 2011/04/28 18:53:24, Diego Novillo wrote:

> OK.

Committed rev 173186.


Diego.

[hubicka_ucw.cz, 2011-04-29 23:25:08]

> 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