diff mbox

[LTO/PGO] Warn when both -flto and -fprofile-generate are enabled

Message ID 20140402115031.GA23632@x4
State New
Headers show

Commit Message

Markus Trippelsdorf April 2, 2014, 11:50 a.m. UTC
It is a common mistake to enable both -flto and -fprofile-generate when
building projects. This is not a good idea, because memory use will
skyrocket due to instrumentation. So just warn the user.

OK for next stage1?

2014-04-02  Markus Trippelsdorf  <markus@trippelsdorf.de>

	* common.opt (fprofile-generate): Add flag.
	* opts.c (finish_options): Add new warning.
	(common_handle_option): Set flag.

Comments

Marek Polacek April 2, 2014, 11:56 a.m. UTC | #1
On Wed, Apr 02, 2014 at 01:50:31PM +0200, Markus Trippelsdorf wrote:
> +  if (opts->x_flag_generate_lto && opts->x_flag_profile_generate)
> +    warning_at (loc, 0, "Enabling both -fprofile-generate and -flto is a bad idea.");

s/Enabling/enabling/ + no dot at the end.

	Marek
Richard Biener April 2, 2014, 12:07 p.m. UTC | #2
On Wed, Apr 2, 2014 at 1:50 PM, Markus Trippelsdorf
<markus@trippelsdorf.de> wrote:
> It is a common mistake to enable both -flto and -fprofile-generate when
> building projects. This is not a good idea, because memory use will
> skyrocket due to instrumentation. So just warn the user.
>
> OK for next stage1?

I'd rather see if we can fix the underlying issue.  For example as we
are now instrumenting as IPA pass we can allocate a single
counter array (if the number of global vars is the issue).  Basically
split analysis and instrumentation into two phases for that.

Or even better, do profile instrumentation as "real" IPA pass.

Richard.

> 2014-04-02  Markus Trippelsdorf  <markus@trippelsdorf.de>
>
>         * common.opt (fprofile-generate): Add flag.
>         * opts.c (finish_options): Add new warning.
>         (common_handle_option): Set flag.
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 62c72f0d2fbf..61e9adfa0df5 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -1689,7 +1689,7 @@ Common Report Var(flag_profile_correction)
>  Enable correction of flow inconsistent profile data input
>
>  fprofile-generate
> -Common
> +Common Var(flag_profile_generate)
>  Enable common options for generating profile info for profile feedback directed optimizations
>
>  fprofile-generate=
> diff --git a/gcc/opts.c b/gcc/opts.c
> index fdc903f9271a..b62a0d626d94 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -833,6 +833,9 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
>         error_at (loc, "only one -flto-partition value can be specified");
>      }
>
> +  if (opts->x_flag_generate_lto && opts->x_flag_profile_generate)
> +    warning_at (loc, 0, "Enabling both -fprofile-generate and -flto is a bad idea.");
> +
>    /* We initialize opts->x_flag_split_stack to -1 so that targets can set a
>       default value if they choose based on other options.  */
>    if (opts->x_flag_split_stack == -1)
> @@ -1728,6 +1731,7 @@ common_handle_option (struct gcc_options *opts,
>
>      case OPT_fprofile_generate_:
>        opts->x_profile_data_prefix = xstrdup (arg);
> +      opts->x_flag_profile_generate = true;
>        value = true;
>        /* No break here - do -fprofile-generate processing. */
>      case OPT_fprofile_generate:
> --
> Markus
Richard Biener April 2, 2014, 12:10 p.m. UTC | #3
On Wed, Apr 2, 2014 at 2:07 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Wed, Apr 2, 2014 at 1:50 PM, Markus Trippelsdorf
> <markus@trippelsdorf.de> wrote:
>> It is a common mistake to enable both -flto and -fprofile-generate when
>> building projects. This is not a good idea, because memory use will
>> skyrocket due to instrumentation. So just warn the user.
>>
>> OK for next stage1?
>
> I'd rather see if we can fix the underlying issue.  For example as we
> are now instrumenting as IPA pass we can allocate a single
> counter array (if the number of global vars is the issue).  Basically
> split analysis and instrumentation into two phases for that.
>
> Or even better, do profile instrumentation as "real" IPA pass.

Thus, isn't -coverage also facing the same issue?  Thus, is it
really -fprofile-arcs already or only one of the value profiling pieces?

Richard.

> Richard.
>
>> 2014-04-02  Markus Trippelsdorf  <markus@trippelsdorf.de>
>>
>>         * common.opt (fprofile-generate): Add flag.
>>         * opts.c (finish_options): Add new warning.
>>         (common_handle_option): Set flag.
>>
>> diff --git a/gcc/common.opt b/gcc/common.opt
>> index 62c72f0d2fbf..61e9adfa0df5 100644
>> --- a/gcc/common.opt
>> +++ b/gcc/common.opt
>> @@ -1689,7 +1689,7 @@ Common Report Var(flag_profile_correction)
>>  Enable correction of flow inconsistent profile data input
>>
>>  fprofile-generate
>> -Common
>> +Common Var(flag_profile_generate)
>>  Enable common options for generating profile info for profile feedback directed optimizations
>>
>>  fprofile-generate=
>> diff --git a/gcc/opts.c b/gcc/opts.c
>> index fdc903f9271a..b62a0d626d94 100644
>> --- a/gcc/opts.c
>> +++ b/gcc/opts.c
>> @@ -833,6 +833,9 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
>>         error_at (loc, "only one -flto-partition value can be specified");
>>      }
>>
>> +  if (opts->x_flag_generate_lto && opts->x_flag_profile_generate)
>> +    warning_at (loc, 0, "Enabling both -fprofile-generate and -flto is a bad idea.");
>> +
>>    /* We initialize opts->x_flag_split_stack to -1 so that targets can set a
>>       default value if they choose based on other options.  */
>>    if (opts->x_flag_split_stack == -1)
>> @@ -1728,6 +1731,7 @@ common_handle_option (struct gcc_options *opts,
>>
>>      case OPT_fprofile_generate_:
>>        opts->x_profile_data_prefix = xstrdup (arg);
>> +      opts->x_flag_profile_generate = true;
>>        value = true;
>>        /* No break here - do -fprofile-generate processing. */
>>      case OPT_fprofile_generate:
>> --
>> Markus
Jan Hubicka April 2, 2014, 11:52 p.m. UTC | #4
> On Wed, Apr 2, 2014 at 2:07 PM, Richard Biener
> <richard.guenther@gmail.com> wrote:
> > On Wed, Apr 2, 2014 at 1:50 PM, Markus Trippelsdorf
> > <markus@trippelsdorf.de> wrote:
> >> It is a common mistake to enable both -flto and -fprofile-generate when
> >> building projects. This is not a good idea, because memory use will
> >> skyrocket due to instrumentation. So just warn the user.
> >>
> >> OK for next stage1?
> >
> > I'd rather see if we can fix the underlying issue.  For example as we
> > are now instrumenting as IPA pass we can allocate a single
> > counter array (if the number of global vars is the issue).  Basically
> > split analysis and instrumentation into two phases for that.
> >
> > Or even better, do profile instrumentation as "real" IPA pass.
> 
> Thus, isn't -coverage also facing the same issue?  Thus, is it
> really -fprofile-arcs already or only one of the value profiling pieces?

Yep, -fprofile-arcs will cause similar issues.
Implementing instrumentation as real IPA is on my TODO list, but pretty low,
since it is quite some work; we need to stream CFG into summaries and make
the instrumentation code independent of function bodies, that needs quite some
reorg (at moment we have no way to load cfg alone).

Note that -fprofile-generate -flto gives you a bit more precise profiles than
-fprofile-generate alone, this is because of COMDAT functions from static libraries
that may be lost in the first case.

Honza
> 
> Richard.
> 
> > Richard.
> >
> >> 2014-04-02  Markus Trippelsdorf  <markus@trippelsdorf.de>
> >>
> >>         * common.opt (fprofile-generate): Add flag.
> >>         * opts.c (finish_options): Add new warning.
> >>         (common_handle_option): Set flag.
> >>
> >> diff --git a/gcc/common.opt b/gcc/common.opt
> >> index 62c72f0d2fbf..61e9adfa0df5 100644
> >> --- a/gcc/common.opt
> >> +++ b/gcc/common.opt
> >> @@ -1689,7 +1689,7 @@ Common Report Var(flag_profile_correction)
> >>  Enable correction of flow inconsistent profile data input
> >>
> >>  fprofile-generate
> >> -Common
> >> +Common Var(flag_profile_generate)
> >>  Enable common options for generating profile info for profile feedback directed optimizations
> >>
> >>  fprofile-generate=
> >> diff --git a/gcc/opts.c b/gcc/opts.c
> >> index fdc903f9271a..b62a0d626d94 100644
> >> --- a/gcc/opts.c
> >> +++ b/gcc/opts.c
> >> @@ -833,6 +833,9 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
> >>         error_at (loc, "only one -flto-partition value can be specified");
> >>      }
> >>
> >> +  if (opts->x_flag_generate_lto && opts->x_flag_profile_generate)
> >> +    warning_at (loc, 0, "Enabling both -fprofile-generate and -flto is a bad idea.");
> >> +
> >>    /* We initialize opts->x_flag_split_stack to -1 so that targets can set a
> >>       default value if they choose based on other options.  */
> >>    if (opts->x_flag_split_stack == -1)
> >> @@ -1728,6 +1731,7 @@ common_handle_option (struct gcc_options *opts,
> >>
> >>      case OPT_fprofile_generate_:
> >>        opts->x_profile_data_prefix = xstrdup (arg);
> >> +      opts->x_flag_profile_generate = true;
> >>        value = true;
> >>        /* No break here - do -fprofile-generate processing. */
> >>      case OPT_fprofile_generate:
> >> --
> >> Markus
diff mbox

Patch

diff --git a/gcc/common.opt b/gcc/common.opt
index 62c72f0d2fbf..61e9adfa0df5 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1689,7 +1689,7 @@  Common Report Var(flag_profile_correction)
 Enable correction of flow inconsistent profile data input
 
 fprofile-generate
-Common
+Common Var(flag_profile_generate)
 Enable common options for generating profile info for profile feedback directed optimizations
 
 fprofile-generate=
diff --git a/gcc/opts.c b/gcc/opts.c
index fdc903f9271a..b62a0d626d94 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -833,6 +833,9 @@  finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
 	error_at (loc, "only one -flto-partition value can be specified");
     }
 
+  if (opts->x_flag_generate_lto && opts->x_flag_profile_generate)
+    warning_at (loc, 0, "Enabling both -fprofile-generate and -flto is a bad idea.");
+
   /* We initialize opts->x_flag_split_stack to -1 so that targets can set a
      default value if they choose based on other options.  */
   if (opts->x_flag_split_stack == -1)
@@ -1728,6 +1731,7 @@  common_handle_option (struct gcc_options *opts,
 
     case OPT_fprofile_generate_:
       opts->x_profile_data_prefix = xstrdup (arg);
+      opts->x_flag_profile_generate = true;
       value = true;
       /* No break here - do -fprofile-generate processing. */
     case OPT_fprofile_generate: