[RFC, PATCH] Disable -fprofile-use related optimizations if corresponding .gcda file not found.

[RFC, PATCH] Disable -fprofile-use related optimizations if corresponding .gcda file not found.

[Maxim Ostapenko]

[-- Attachment #1: Type: text/plain, Size: 806 bytes --]

Hi,

when testing OpenSSL performance, I found out that sometimes PGO-built 
binaries can actually introduce performance regressions. We could 
identify affected object files and disable PGO for them by simply 
removing corresponding .gcda file. However, even if profile data is not 
presented, GCC doesn't switch back -fprofile-use dependent optimizations 
(e.g. -fbranch-probabilities, -fvpt, etc). This may also lead to 
performance degradations.

The issue had already raised quite time ago 
(https://gcc.gnu.org/ml/gcc-patches/2009-09/msg02119.html), but for some 
reasons wasn't discussed.

Here a draft patch that disables -fprofile-use related optimizations if 
profile data wasn't found (perhaps it makes sense to introduce a special 
switch for this?). Does this look reasonable?

Thanks,
-Maxim

[-- Attachment #2: disable_profile_optimizations-4.diff --]
[-- Type: text/x-patch, Size: 1416 bytes --]

gcc/ChangeLog:

2015-10-07  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>

	* coverage.c (disable_profile_use_flags): New function.
	(read_counts_file): Call it if corresponding .gcda file wasn't found.

diff --git a/gcc/coverage.c b/gcc/coverage.c
index 4c06fa4..37e16b7 100644
--- a/gcc/coverage.c
+++ b/gcc/coverage.c
@@ -134,6 +134,7 @@ static bool coverage_obj_init (void);
 static vec<constructor_elt, va_gc> *coverage_obj_fn
 (vec<constructor_elt, va_gc> *, tree, struct coverage_data const *);
 static void coverage_obj_finish (vec<constructor_elt, va_gc> *);
+static void disable_profile_use_flags (void);
 \f
 /* Return the type node for gcov_type.  */
 
@@ -190,7 +191,10 @@ read_counts_file (void)
   unsigned cfg_checksum = 0;
 
   if (!gcov_open (da_file_name, 1))
-    return;
+    {
+      disable_profile_use_flags ();
+      return;
+    }
 
   if (!gcov_magic (gcov_read_unsigned (), GCOV_DATA_MAGIC))
     {
@@ -1219,4 +1223,14 @@ coverage_finish (void)
   da_file_name = NULL;
 }
 
+/* Reset flags if a .gcda file is not found.  */ 
+static void
+disable_profile_use_flags (void)
+{
+  flag_branch_probabilities = flag_profile_values = flag_unroll_loops = false;
+  flag_value_profile_transformations
+   = flag_tree_loop_distribute_patterns = false;
+  flag_rename_registers = flag_peel_loops = false;
+  flag_profile_reorder_functions = flag_tracer = false;
+}
+
 #include "gt-coverage.h"

Re: [RFC, PATCH] Disable -fprofile-use related optimizations if corresponding .gcda file not found.

[Andrew Pinski]

On Wed, Oct 7, 2015 at 9:11 AM, Maxim Ostapenko
<m.ostapenko@partner.samsung.com> wrote:
> Hi,
>
> when testing OpenSSL performance, I found out that sometimes PGO-built
> binaries can actually introduce performance regressions. We could identify
> affected object files and disable PGO for them by simply removing
> corresponding .gcda file. However, even if profile data is not presented,
> GCC doesn't switch back -fprofile-use dependent optimizations (e.g.
> -fbranch-probabilities, -fvpt, etc). This may also lead to performance
> degradations.
>
> The issue had already raised quite time ago
> (https://gcc.gnu.org/ml/gcc-patches/2009-09/msg02119.html), but for some
> reasons wasn't discussed.
>
> Here a draft patch that disables -fprofile-use related optimizations if
> profile data wasn't found (perhaps it makes sense to introduce a special
> switch for this?). Does this look reasonable?

I thought if profile is not present, then branch probabilities goes
back to the original heuristics?
Which option is really causing the performance degradation here?

Also I think your patch is very incomplete as someone could use
-frename-registers with -fprofile-use and then you just turned it off.

Thanks,
Andrew Pinski

>
> Thanks,
> -Maxim

Re: [RFC, PATCH] Disable -fprofile-use related optimizations if corresponding .gcda file not found.

[Maxim Ostapenko]

On 07/10/15 19:18, Andrew Pinski wrote:
> On Wed, Oct 7, 2015 at 9:11 AM, Maxim Ostapenko
> <m.ostapenko@partner.samsung.com> wrote:
>> Hi,
>>
>> when testing OpenSSL performance, I found out that sometimes PGO-built
>> binaries can actually introduce performance regressions. We could identify
>> affected object files and disable PGO for them by simply removing
>> corresponding .gcda file. However, even if profile data is not presented,
>> GCC doesn't switch back -fprofile-use dependent optimizations (e.g.
>> -fbranch-probabilities, -fvpt, etc). This may also lead to performance
>> degradations.
>>
>> The issue had already raised quite time ago
>> (https://gcc.gnu.org/ml/gcc-patches/2009-09/msg02119.html), but for some
>> reasons wasn't discussed.
>>
>> Here a draft patch that disables -fprofile-use related optimizations if
>> profile data wasn't found (perhaps it makes sense to introduce a special
>> switch for this?). Does this look reasonable?
> I thought if profile is not present, then branch probabilities goes
> back to the original heuristics?
> Which option is really causing the performance degradation here?

-fprofile-use enabled -fpeel-loops that in turn enabled 
-frename-registers. This caused the scheduler to transform the code in 
sched2 pass.

> Also I think your patch is very incomplete as someone could use
> -frename-registers with -fprofile-use and then you just turned it off.
>
> Thanks,
> Andrew Pinski

Doesn't -fprofile-use enable -frename-registers transitively through 
-fpeel-loops?

>> Thanks,
>> -Maxim

Re: [RFC, PATCH] Disable -fprofile-use related optimizations if corresponding .gcda file not found.

[Markus Trippelsdorf]

On 2015.10.07 at 19:11 +0300, Maxim Ostapenko wrote:
> when testing OpenSSL performance, I found out that sometimes PGO-built 
> binaries can actually introduce performance regressions. We could 
> identify affected object files and disable PGO for them by simply 
> removing corresponding .gcda file. However, even if profile data is not 
> presented, GCC doesn't switch back -fprofile-use dependent optimizations 
> (e.g. -fbranch-probabilities, -fvpt, etc). This may also lead to 
> performance degradations.
> 
> The issue had already raised quite time ago 
> (https://gcc.gnu.org/ml/gcc-patches/2009-09/msg02119.html), but for some 
> reasons wasn't discussed.
> 
> Here a draft patch that disables -fprofile-use related optimizations if 
> profile data wasn't found (perhaps it makes sense to introduce a special 
> switch for this?). Does this look reasonable?

I find -fprofile-use a useful shortcut to: -fbranch-probabilities,
-fvpt, -funroll-loops, -fpeel-loops, -ftracer, -ftree-vectorize, and
ftree-loop-distribute-patterns.

There are many applications were the bulk of the speedup surprisingly
comes from these flags alone, even without any gcda file. E.g. with
tramp3d-v4 (-Ofast) I get ~5.3 percent speedup (without gcda) and an
additional 2.2 percent when using a gcda file.

> +/* Reset flags if a .gcda file is not found.  */ 
> +static void
> +disable_profile_use_flags (void)
> +{
> +  flag_branch_probabilities = flag_profile_values = flag_unroll_loops = false;
> +  flag_value_profile_transformations
> +   = flag_tree_loop_distribute_patterns = false;
> +  flag_rename_registers = flag_peel_loops = false;
> +  flag_profile_reorder_functions = flag_tracer = false;
> +}

Here you are disabling unrelated flags, e.g. flag_rename_registers.

-- 
Markus

Re: [RFC, PATCH] Disable -fprofile-use related optimizations if corresponding .gcda file not found.

[pinskia]

> On Oct 7, 2015, at 9:28 AM, Maxim Ostapenko <m.ostapenko@partner.samsung.com> wrote:
> 
> 
> 
>> On 07/10/15 19:18, Andrew Pinski wrote:
>> On Wed, Oct 7, 2015 at 9:11 AM, Maxim Ostapenko
>> <m.ostapenko@partner.samsung.com> wrote:
>>> Hi,
>>> 
>>> when testing OpenSSL performance, I found out that sometimes PGO-built
>>> binaries can actually introduce performance regressions. We could identify
>>> affected object files and disable PGO for them by simply removing
>>> corresponding .gcda file. However, even if profile data is not presented,
>>> GCC doesn't switch back -fprofile-use dependent optimizations (e.g.
>>> -fbranch-probabilities, -fvpt, etc). This may also lead to performance
>>> degradations.
>>> 
>>> The issue had already raised quite time ago
>>> (https://gcc.gnu.org/ml/gcc-patches/2009-09/msg02119.html), but for some
>>> reasons wasn't discussed.
>>> 
>>> Here a draft patch that disables -fprofile-use related optimizations if
>>> profile data wasn't found (perhaps it makes sense to introduce a special
>>> switch for this?). Does this look reasonable?
>> I thought if profile is not present, then branch probabilities goes
>> back to the original heuristics?
>> Which option is really causing the performance degradation here?
> 
> -fprofile-use enabled -fpeel-loops that in turn enabled -frename-registers. This caused the scheduler to transform the code in sched2 pass.

Why not figure out how much to improve that instead. Rename register is just doing renaming of register usage and not undoing any scheduling at all (well it might cause some moves to be removed).  I think you really should dig into deeper why it is causing a performance issue. 

> 
>> Also I think your patch is very incomplete as someone could use
>> -frename-registers with -fprofile-use and then you just turned it off.
>> 
>> Thanks,
>> Andrew Pinski
> 
> Doesn't -fprofile-use enable -frename-registers transitively through -fpeel-loops?

Yes. But I am saying some one could do -fprofile-use -frename-registers and expect rename registers to stay on even if there is no profile. 

Thanks,
Andrew


> 
>>> Thanks,
>>> -Maxim
> 

Re: [RFC, PATCH] Disable -fprofile-use related optimizations if corresponding .gcda file not found.

[Nikolai Bozhenov]

On 10/07/2015 08:07 PM, pinskia@gmail.com wrote:
> Yes. But I am saying some one could do -fprofile-use -frename-registers and expect rename registers to stay on even if there is no profile.

That's true, we shouldn't disable any options that were explicitly 
requested by
the user.

> Why not figure out how much to improve that instead. Rename register is just doing renaming of register usage and not undoing any scheduling at all (well it might cause some moves to be removed).  I think you really should dig into deeper why it is causing a performance issue.

Yep, the better way is to fix all the problems in all the passes. We're
working on it but it may take a while. Meanwhile, we want to use PGO and we
need a way to avoid degradations caused by it. We want to fallback to the
usual compilation process for a source file if we don't like how it is
compiled with PGO. And what we want to do is to simply remove the
corresponding gcda file and have the compiler ignore the -fprofile-use 
option
for the translation unit.

Anyway, I find current GCC's behavior to be confusing. Currently it silently
ignores missed profile data but triggers some opaque internal options which
affect further compilation. I think in case of a missed gcda file GCC should
either ignore -fprofile-use completely or issue a warning/error.

Thanks,
Nikolai

Re: [RFC, PATCH] Disable -fprofile-use related optimizations if corresponding .gcda file not found.

[Richard Biener]

On Thu, Oct 8, 2015 at 9:54 AM, Nikolai Bozhenov <n.bozhenov@samsung.com> wrote:
>
> On 10/07/2015 08:07 PM, pinskia@gmail.com wrote:
>>
>> Yes. But I am saying some one could do -fprofile-use -frename-registers
>> and expect rename registers to stay on even if there is no profile.
>
>
> That's true, we shouldn't disable any options that were explicitly requested
> by
> the user.
>
>> Why not figure out how much to improve that instead. Rename register is
>> just doing renaming of register usage and not undoing any scheduling at all
>> (well it might cause some moves to be removed).  I think you really should
>> dig into deeper why it is causing a performance issue.
>
>
> Yep, the better way is to fix all the problems in all the passes. We're
> working on it but it may take a while. Meanwhile, we want to use PGO and we
> need a way to avoid degradations caused by it. We want to fallback to the
> usual compilation process for a source file if we don't like how it is
> compiled with PGO. And what we want to do is to simply remove the
> corresponding gcda file and have the compiler ignore the -fprofile-use
> option
> for the translation unit.
>
> Anyway, I find current GCC's behavior to be confusing. Currently it silently
> ignores missed profile data but triggers some opaque internal options which
> affect further compilation. I think in case of a missed gcda file GCC should
> either ignore -fprofile-use completely or issue a warning/error.

I agree that it is confusing.  You have to watch out for explicit
-fprofile-use -fXXX
and not clear flags set explicitely though.  Didn't look at the patch in detail.

Richard.

> Thanks,
> Nikolai

[RFC, PATCH v2] Disable -fprofile-use related optimizations if corresponding .gcda file not found.

[Maxim Ostapenko]

[-- Attachment #1: Type: text/plain, Size: 1160 bytes --]

Hi!

As was pointed out in previous thread 
(https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00723.html), sometimes 
PGO-built binaries can actually introduce performance regressions. We 
could identify affected object files and disable PGO for them by simply 
removing corresponding .gcda file.
My previous patch was incomplete and had two major drawbacks:

* It disabled unrelated options (e.g. -frename-registers) and 
PGO-related options, set up by user explicitly, if corresponding .gcda 
file is not found.
* As Markus pointed out, in many cases we actually don't want to disable 
PGO-related options even if .gcda file is missing.

This patch tries to solve these issues in the following way:

* Introduce new -fprofile-use-partial switch.
* If -fprofile-use-partial is ON, try to find corresponding .gcda file 
in the very early stage during compiler invoking (actually, when common 
command line options are parsed). If .gcda file exists, enable 
PGO-related optimizations and emit warning otherwise. I believe this 
should not break existing code.

Regtested and bootstrapped on x86_64-unknown-linux-gnu.

Does the patch look sensible?

Thanks,
-Maxim

[-- Attachment #2: disable_profile_optimizations-11.diff --]
[-- Type: text/x-patch, Size: 10759 bytes --]

gcc/ChangeLog:

2015-10-27  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>

	* common.opt (profile_file_name): New variable.
	(fprofile-use-partial): Likewise.
	(fprofile-use-partial=): Likewise.
	* opts.c: Include gcov-io.h.
	(common_handle_option): Defer enabling PGO-related optimizations until
	we know if corresponding .gcda file exists.
	(maybe_setup_aux_base_name): New function.
	(setup_coverage_filename): Likewise.
	(enable_fdo_optimizations): Move up in source file.
	(finish_options): Call maybe_setup_aux_base_name and setup coverage
	filename. Enable PGO-related optimizations if corresponding .gcda file
	exists if -fprofile-use-partial is used.  If -fprofile-use is used,
	enable PGO-related optimizations without any other conditions.
	* coverage.c (coverage_init): Adjust to use profile_file_name.

diff --git a/gcc/common.opt b/gcc/common.opt
index 12ca0d6..fb04201 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -198,6 +198,9 @@ const char *main_input_basename
 Variable
 int main_input_baselength
 
+Variable
+char *profile_file_name
+
 ; Which options have been printed by --help.
 Variable
 char *help_printed
@@ -1861,6 +1864,14 @@ fprofile-use=
 Common Joined RejectNegative
 Enable common options for performing profile feedback directed optimizations, and set -fprofile-dir=.
 
+fprofile-use-partial
+Common Var(flag_profile_use_partial)
+Enable common options for performing profile feedback directed optimizations.
+
+fprofile-use-partial=
+Common Joined RejectNegative
+Enable common options for performing profile feedback directed optimizations, and set -fprofile-dir=.
+
 fprofile-values
 Common Report Var(flag_profile_values)
 Insert code to profile values of expressions.
diff --git a/gcc/coverage.c b/gcc/coverage.c
index 4e08e5f..461dd48 100644
--- a/gcc/coverage.c
+++ b/gcc/coverage.c
@@ -1184,7 +1184,7 @@ void
 coverage_init (const char *filename)
 {
   int len = strlen (filename);
-  int prefix_len = 0;
+  da_file_name = profile_file_name;
 
   /* Since coverage_init is invoked very early, before the pass
      manager, we need to set up the dumping explicitly. This is
@@ -1193,24 +1193,6 @@ coverage_init (const char *filename)
     g->get_passes ()->get_pass_profile ()->static_pass_number;
   g->get_dumps ()->dump_start (profile_pass_num, NULL);
 
-  if (!profile_data_prefix && !IS_ABSOLUTE_PATH (filename))
-    profile_data_prefix = getpwd ();
-
-  if (profile_data_prefix)
-    prefix_len = strlen (profile_data_prefix);
-
-  /* Name of da file.  */
-  da_file_name = XNEWVEC (char, len + strlen (GCOV_DATA_SUFFIX)
-			  + prefix_len + 2);
-
-  if (profile_data_prefix)
-    {
-      memcpy (da_file_name, profile_data_prefix, prefix_len);
-      da_file_name[prefix_len++] = '/';
-    }
-  memcpy (da_file_name + prefix_len, filename, len);
-  strcpy (da_file_name + prefix_len + len, GCOV_DATA_SUFFIX);
-
   bbg_file_stamp = local_tick;
   
   if (flag_auto_profile)
diff --git a/gcc/opts.c b/gcc/opts.c
index 9a3fbb3..2e0cfa4 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -30,6 +30,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "opts-diagnostic.h"
 #include "insn-attr-common.h"
 #include "common/common-target.h"
+#include "gcov-io.h"
 
 static void set_Wstrict_aliasing (struct gcc_options *opts, int onoff);
 
@@ -660,6 +661,97 @@ default_options_optimization (struct gcc_options *opts,
 			 lang_mask, handlers, loc, dc);
 }
 
+/* Enable FDO-related flags.  */
+
+static void
+enable_fdo_optimizations (struct gcc_options *opts,
+			  struct gcc_options *opts_set,
+			  int value)
+{
+  if (!opts_set->x_flag_branch_probabilities)
+    opts->x_flag_branch_probabilities = value;
+  if (!opts_set->x_flag_profile_values)
+    opts->x_flag_profile_values = value;
+  if (!opts_set->x_flag_unroll_loops)
+    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)
+    opts->x_flag_inline_functions = value;
+  if (!opts_set->x_flag_ipa_cp)
+    opts->x_flag_ipa_cp = value;
+  if (!opts_set->x_flag_ipa_cp_clone
+      && value && opts->x_flag_ipa_cp)
+    opts->x_flag_ipa_cp_clone = value;
+  if (!opts_set->x_flag_ipa_cp_alignment
+      && value && opts->x_flag_ipa_cp)
+    opts->x_flag_ipa_cp_alignment = value;
+  if (!opts_set->x_flag_predictive_commoning)
+    opts->x_flag_predictive_commoning = value;
+  if (!opts_set->x_flag_unswitch_loops)
+    opts->x_flag_unswitch_loops = value;
+  if (!opts_set->x_flag_gcse_after_reload)
+    opts->x_flag_gcse_after_reload = value;
+  if (!opts_set->x_flag_tree_loop_vectorize
+      && !opts_set->x_flag_tree_vectorize)
+    opts->x_flag_tree_loop_vectorize = value;
+  if (!opts_set->x_flag_tree_slp_vectorize
+      && !opts_set->x_flag_tree_vectorize)
+    opts->x_flag_tree_slp_vectorize = value;
+  if (!opts_set->x_flag_vect_cost_model)
+    opts->x_flag_vect_cost_model = VECT_COST_MODEL_DYNAMIC;
+  if (!opts_set->x_flag_tree_loop_distribute_patterns)
+    opts->x_flag_tree_loop_distribute_patterns = value;
+}
+
+static void
+maybe_setup_aux_base_name ()
+{
+  /* Set aux_base_name if not already set.  */
+  if (aux_base_name)
+    return;
+  else if (main_input_filename)
+    {
+      char *name = xstrdup (lbasename (main_input_filename));
+      strip_off_ending (name, strlen (name));
+      aux_base_name = name;
+    }
+  else
+    aux_base_name = "gccaux";
+}
+
+static char *
+setup_coverage_filename (const char *basename, struct gcc_options *opts)
+{
+  int len = strlen (basename);
+  int prefix_len = 0;
+  const char *data_prefix = opts->x_profile_data_prefix;
+
+  if (!opts->x_profile_data_prefix && !IS_ABSOLUTE_PATH (basename))
+    data_prefix = getpwd ();
+
+  if (data_prefix)
+    prefix_len = strlen (data_prefix);
+
+  /* Name of da file.  */
+  char *da_file_name = XNEWVEC (char, len + strlen (GCOV_DATA_SUFFIX)
+				+ prefix_len + 2);
+
+  if (data_prefix)
+    {
+      memcpy (da_file_name, data_prefix, prefix_len);
+      da_file_name[prefix_len++] = '/';
+    }
+  memcpy (da_file_name + prefix_len, basename, len);
+  strcpy (da_file_name + prefix_len + len, GCOV_DATA_SUFFIX);
+  return da_file_name;
+}
+
+
 /* After all options at LOC have been read into OPTS and OPTS_SET,
    finalize settings of those options and diagnose incompatible
    combinations.  */
@@ -949,6 +1041,28 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
       opts->x_flag_aggressive_loop_optimizations = 0;
       opts->x_flag_strict_overflow = 0;
     }
+
+  maybe_setup_aux_base_name ();
+  opts->x_profile_file_name = setup_coverage_filename (aux_base_name, opts);
+  if (opts->x_flag_profile_use || opts->x_flag_profile_use_partial)
+    {
+      /* Warn only -fprofile-use-partial switch is enabled and corresponding
+         .gcda doesn't exist.  */
+      if (opts->x_flag_profile_use_partial
+	  && access (opts->x_profile_file_name, F_OK))
+	fprintf (stderr, "%s not found!\n", opts->x_profile_file_name);
+      else
+       {
+	 enable_fdo_optimizations (opts, opts_set, true);
+	 if (!opts_set->x_flag_profile_reorder_functions)
+	   opts->x_flag_profile_reorder_functions = true;
+	   /* Indirect call profiling should do all useful transformations
+	      speculative devirtualization does.  */
+	 if (!opts_set->x_flag_devirtualize_speculatively
+	     && opts->x_flag_value_profile_transformations)
+	   opts->x_flag_devirtualize_speculatively = false;
+      }
+    }
 }
 
 #define LEFT_COLUMN	27
@@ -1368,53 +1482,6 @@ print_specific_help (unsigned int include_flags,
 		       opts->x_help_columns, opts, lang_mask);
 }
 
-/* Enable FDO-related flags.  */
-
-static void
-enable_fdo_optimizations (struct gcc_options *opts,
-			  struct gcc_options *opts_set,
-			  int value)
-{
-  if (!opts_set->x_flag_branch_probabilities)
-    opts->x_flag_branch_probabilities = value;
-  if (!opts_set->x_flag_profile_values)
-    opts->x_flag_profile_values = value;
-  if (!opts_set->x_flag_unroll_loops)
-    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)
-    opts->x_flag_inline_functions = value;
-  if (!opts_set->x_flag_ipa_cp)
-    opts->x_flag_ipa_cp = value;
-  if (!opts_set->x_flag_ipa_cp_clone
-      && value && opts->x_flag_ipa_cp)
-    opts->x_flag_ipa_cp_clone = value;
-  if (!opts_set->x_flag_ipa_cp_alignment
-      && value && opts->x_flag_ipa_cp)
-    opts->x_flag_ipa_cp_alignment = value;
-  if (!opts_set->x_flag_predictive_commoning)
-    opts->x_flag_predictive_commoning = value;
-  if (!opts_set->x_flag_unswitch_loops)
-    opts->x_flag_unswitch_loops = value;
-  if (!opts_set->x_flag_gcse_after_reload)
-    opts->x_flag_gcse_after_reload = value;
-  if (!opts_set->x_flag_tree_loop_vectorize
-      && !opts_set->x_flag_tree_vectorize)
-    opts->x_flag_tree_loop_vectorize = value;
-  if (!opts_set->x_flag_tree_slp_vectorize
-      && !opts_set->x_flag_tree_vectorize)
-    opts->x_flag_tree_slp_vectorize = value;
-  if (!opts_set->x_flag_vect_cost_model)
-    opts->x_flag_vect_cost_model = VECT_COST_MODEL_DYNAMIC;
-  if (!opts_set->x_flag_tree_loop_distribute_patterns)
-    opts->x_flag_tree_loop_distribute_patterns = value;
-}
-
 /* Handle target- and language-independent options.  Return zero to
    generate an "unknown option" message.  Only options that need
    extra handling need to be listed here; if you simply want
@@ -1923,15 +1990,12 @@ common_handle_option (struct gcc_options *opts,
       opts->x_flag_profile_use = true;
       value = true;
       /* No break here - do -fprofile-use processing. */
+    case OPT_fprofile_use_partial_:
+      opts->x_flag_profile_use_partial = true;
     case OPT_fprofile_use:
-      enable_fdo_optimizations (opts, opts_set, value);
-      if (!opts_set->x_flag_profile_reorder_functions)
-	  opts->x_flag_profile_reorder_functions = value;
-	/* Indirect call profiling should do all useful transformations
-	   speculative devirtualization does.  */
-      if (!opts_set->x_flag_devirtualize_speculatively
-	  && opts->x_flag_value_profile_transformations)
-	opts->x_flag_devirtualize_speculatively = false;
+    case OPT_fprofile_use_partial:
+      /* Deferred until we know if we really need to enable PGO - related
+         optimizations.  */
       break;
 
     case OPT_fauto_profile_:

Re: [RFC, PATCH v2] Disable -fprofile-use related optimizations if corresponding .gcda file not found.

[Richard Biener]

On Tue, Oct 27, 2015 at 10:14 AM, Maxim Ostapenko
<m.ostapenko@partner.samsung.com> wrote:
> Hi!
>
> As was pointed out in previous thread
> (https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00723.html), sometimes
> PGO-built binaries can actually introduce performance regressions. We could
> identify affected object files and disable PGO for them by simply removing
> corresponding .gcda file.

Did you analyze where the performance regressions came from?

> My previous patch was incomplete and had two major drawbacks:
>
> * It disabled unrelated options (e.g. -frename-registers) and PGO-related
> options, set up by user explicitly, if corresponding .gcda file is not
> found.
> * As Markus pointed out, in many cases we actually don't want to disable
> PGO-related options even if .gcda file is missing.
>
> This patch tries to solve these issues in the following way:
>
> * Introduce new -fprofile-use-partial switch.
> * If -fprofile-use-partial is ON, try to find corresponding .gcda file in
> the very early stage during compiler invoking (actually, when common command
> line options are parsed). If .gcda file exists, enable PGO-related
> optimizations and emit warning otherwise. I believe this should not break
> existing code.
>
> Regtested and bootstrapped on x86_64-unknown-linux-gnu.
>
> Does the patch look sensible?

It still somehow feels like a hack.

I think it should operate on a function granularity and the user
should be able to specify
a fallback to use (do we actually guess the profile if it fails to
read?  what if the profile
is missing just for some functions, like if the gcda file is older
than sources?).

Which means that we should be able to somehow "late" disable some options
from inside the IPA tree-profile pass.  In principle we have
DECL_FUNCTION_SPECIFIC_OPTIMIZATION
nodes on each function so disabling of flags should be possible (but
we've lost whether
they were the default or user-controlled at that point).

Honza, any further ideas here?

Thanks,
Richard.

> Thanks,
> -Maxim