| Message ID | CACmZjJL=PPKMuCspMPfS82zud_rgshLM7VLzbMMkTQNs2DbupA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Sorry, tabs seems converted to spaces automatically. Please use the attachment instead. 2014-07-21 13:13 GMT+08:00 Pengfei Yuan <0xcoolypf@gmail.com>: > Hi, > > This patch tunes optimization options based on profile data: > * Disable PGO options if profile is not available or empty. > * Optimize for size if profile is available but empty. > > Here is an experiment on Firefox PGO build: > > CPU Intel Core i7-4770 > RAM 32 GB > OS Debian sid amd64 > Firefox source mozilla-central, changeset 4bafe35cfb65 > Compiler GCC 4.9.2 20140721 (prerelease) > > Result: > > Size of libxul.so | Octane benchmark score > PGO w/o this patch 67206232 32440.4 +/- 0.35% > PGO w/ this patch 66604312 32765.8 +/- 0.56% > > With this patch, the size of PGO-built libxul.so decreases by 0.9% and the > performance improves slightly. > > Regards, > > Yuan Pengfei > Peking University
On Mon, Jul 21, 2014 at 7:13 AM, Pengfei Yuan <0xcoolypf@gmail.com> wrote: > Hi, > > This patch tunes optimization options based on profile data: > * Disable PGO options if profile is not available or empty. > * Optimize for size if profile is available but empty. Err ... these don't seem interesting cases to "optimize" for? Richard. > Here is an experiment on Firefox PGO build: > > CPU Intel Core i7-4770 > RAM 32 GB > OS Debian sid amd64 > Firefox source mozilla-central, changeset 4bafe35cfb65 > Compiler GCC 4.9.2 20140721 (prerelease) > > Result: > > Size of libxul.so | Octane benchmark score > PGO w/o this patch 67206232 32440.4 +/- 0.35% > PGO w/ this patch 66604312 32765.8 +/- 0.56% > > With this patch, the size of PGO-built libxul.so decreases by 0.9% and the > performance improves slightly. > > Regards, > > Yuan Pengfei > Peking University > > > gcc/ChangeLog: > > * coverage.c (coverage_check): New function. > * coverage.h (coverage_check): New function. > * toplev.c (profile_based_option_override): New function. > (process_options): Add profile based option tuning. > > > diff --git a/gcc/coverage.c b/gcc/coverage.c > index 4c06fa4..205bee5 100644 > --- a/gcc/coverage.c > +++ b/gcc/coverage.c > @@ -1128,6 +1128,75 @@ coverage_obj_finish (vec<constructor_elt, va_gc> *ctor) > varpool_finalize_decl (gcov_info_var); > } > > +/* Check the profile data file. > + Return -1 if the file is not available or corrupted, > + 0 if the file is available and all counters are zero, > + 1 otherwise. */ > + > +int > +coverage_check (const char *filename) > +{ > + int ret = 0; > + int len = strlen (filename); > + int prefix_len = 0; > + gcov_unsigned_t tag; > + char *data_filename; > + > + if (!profile_data_prefix && !IS_ABSOLUTE_PATH (filename)) > + profile_data_prefix = getpwd (); > + > + if (profile_data_prefix) > + prefix_len = strlen (profile_data_prefix); > + > + data_filename = XNEWVEC (char, len + strlen (GCOV_DATA_SUFFIX) > + + prefix_len + 2); > + > + if (profile_data_prefix) > + { > + memcpy (data_filename, profile_data_prefix, prefix_len); > + data_filename[prefix_len++] = '/'; > + } > + memcpy (data_filename + prefix_len, filename, len); > + strcpy (data_filename + prefix_len + len, GCOV_DATA_SUFFIX); > + > + if (!gcov_open (data_filename, 1)) > + return -1; > + if (!gcov_magic (gcov_read_unsigned (), GCOV_DATA_MAGIC) > + || gcov_read_unsigned () != GCOV_VERSION) > + { > + gcov_close (); > + return -1; > + } > + gcov_read_unsigned (); > + > + while ((tag = gcov_read_unsigned ())) > + { > + gcov_unsigned_t length = gcov_read_unsigned (); > + gcov_position_t offset = gcov_position (); > + > + if (GCOV_TAG_IS_COUNTER (tag)) > + { > + unsigned n_counts = GCOV_TAG_COUNTER_NUM (length); > + unsigned ix; > + > + for (ix = 0; ix != n_counts; ix++) > + if (gcov_read_counter ()) > + ret = 1; > + } > + gcov_sync (offset, length); > + > + if (gcov_is_error ()) > + { > + ret = -1; > + break; > + } > + } > + > + gcov_close (); > + > + return ret; > +} > + > /* Perform file-level initialization. Read in data file, generate name > of notes file. */ > > diff --git a/gcc/coverage.h b/gcc/coverage.h > index 81f87a6..51d1119 100644 > --- a/gcc/coverage.h > +++ b/gcc/coverage.h > @@ -22,6 +22,7 @@ along with GCC; see the file COPYING3. If not see > > #include "gcov-io.h" > > +extern int coverage_check (const char *); > extern void coverage_init (const char *); > extern void coverage_finish (void); > > diff --git a/gcc/toplev.c b/gcc/toplev.c > index d646faf..b0c3906 100644 > --- a/gcc/toplev.c > +++ b/gcc/toplev.c > @@ -1222,6 +1222,77 @@ init_alignments (void) > align_functions_log = floor_log2 (align_functions * 2 - 1); > } > > +/* Override options based on profile. */ > + > +static void > +profile_based_option_override (void) > +{ > + int status; > + const char *name = aux_base_name; > + > + if (!flag_branch_probabilities) > + return; > + > + if (!name) > + { > + char *newname; > + if (!main_input_filename) > + return; > + newname = xstrdup (lbasename (main_input_filename)); > + strip_off_ending (newname, strlen (newname)); > + name = newname; > + } > + > + status = coverage_check (name); > + if (status > 0) > + return; > + > + /* Profile data file is valid and all profile counters are zero. > + Prefer optimizing code size. */ > + if (status == 0) > + { > + optimize = 2; > + optimize_size = 1; > + maybe_set_param_value (PARAM_MIN_CROSSJUMP_INSNS, 1, > + param_values, global_options_set.x_param_values); > + > + /* Ignore coverage mismatch since all counters are zero. */ > + diagnostic_classify_diagnostic (global_dc, OPT_Wcoverage_mismatch, > + DK_IGNORED, UNKNOWN_LOCATION); > + } > + > + if (!flag_profile_use) > + return; > + > + /* Disable optimization options for PGO. */ > + if (!global_options_set.x_flag_profile_values) > + flag_profile_values = false; > + if (!global_options_set.x_flag_unroll_loops) > + flag_unroll_loops = false; > + if (!global_options_set.x_flag_peel_loops) > + flag_peel_loops = false; > + if (!global_options_set.x_flag_tracer) > + flag_tracer = false; > + if (!global_options_set.x_flag_value_profile_transformations) > + flag_value_profile_transformations = false; > + if (!global_options_set.x_flag_ipa_cp_clone) > + flag_ipa_cp_clone = false; > + if (!global_options_set.x_flag_predictive_commoning) > + flag_predictive_commoning = false; > + if (!global_options_set.x_flag_unswitch_loops) > + flag_unswitch_loops = false; > + if (!global_options_set.x_flag_gcse_after_reload) > + flag_gcse_after_reload = false; > + if (!global_options_set.x_flag_tree_loop_vectorize > + && !global_options_set.x_flag_tree_vectorize) > + flag_tree_loop_vectorize = false; > + if (!global_options_set.x_flag_tree_slp_vectorize > + && !global_options_set.x_flag_tree_vectorize) > + flag_tree_slp_vectorize = false; > + if (!global_options_set.x_flag_vect_cost_model) > + flag_vect_cost_model = VECT_COST_MODEL_CHEAP; > +} > + > /* Process the options that have been parsed. */ > static void > process_options (void) > @@ -1245,6 +1316,8 @@ process_options (void) > so we can correctly initialize debug output. */ > no_backend = lang_hooks.post_options (&main_input_filename); > > + profile_based_option_override (); > + > /* Some machines may reject certain combinations of options. */ > targetm.target_option.override ();
In the experiment, about 60% (1019/1699) profile data files are empty (all counters are zero). 2014-07-22 21:39 GMT+08:00 Richard Biener <richard.guenther@gmail.com>: > On Mon, Jul 21, 2014 at 7:13 AM, Pengfei Yuan <0xcoolypf@gmail.com> wrote: >> Hi, >> >> This patch tunes optimization options based on profile data: >> * Disable PGO options if profile is not available or empty. >> * Optimize for size if profile is available but empty. > > Err ... these don't seem interesting cases to "optimize" for? > > Richard.
On Wed, Jul 23, 2014 at 2:39 AM, Pengfei Yuan <0xcoolypf@gmail.com> wrote: > In the experiment, about 60% (1019/1699) profile data files are empty > (all counters are zero). Well, but you are globally overriding options even for the parts with profile. The whole point of profile-feedback is to get at the interesting parts (those with non-zero counters). What you say is that not enough parts of the compiler care for the actual profiles and thus portions with all-zero counters are treated as if they were hot? Then better fix that. Richard. > 2014-07-22 21:39 GMT+08:00 Richard Biener <richard.guenther@gmail.com>: >> On Mon, Jul 21, 2014 at 7:13 AM, Pengfei Yuan <0xcoolypf@gmail.com> wrote: >>> Hi, >>> >>> This patch tunes optimization options based on profile data: >>> * Disable PGO options if profile is not available or empty. >>> * Optimize for size if profile is available but empty. >> >> Err ... these don't seem interesting cases to "optimize" for? >> >> Richard.
I guess some optimizations are controlled only by "optimize_size", not by the profile. Other optimizations are controlled by the profile. So this patch does not have very much effectiveness (only 0.9% size reduction). 2014-07-23 17:26 GMT+08:00 Richard Biener <richard.guenther@gmail.com>: > On Wed, Jul 23, 2014 at 2:39 AM, Pengfei Yuan <0xcoolypf@gmail.com> wrote: >> In the experiment, about 60% (1019/1699) profile data files are empty >> (all counters are zero). > > Well, but you are globally overriding options even for the parts with > profile. The whole point of profile-feedback is to get at the interesting > parts (those with non-zero counters). > > What you say is that not enough parts of the compiler care for > the actual profiles and thus portions with all-zero counters are > treated as if they were hot? Then better fix that. > > Richard. > >> 2014-07-22 21:39 GMT+08:00 Richard Biener <richard.guenther@gmail.com>: >>> On Mon, Jul 21, 2014 at 7:13 AM, Pengfei Yuan <0xcoolypf@gmail.com> wrote: >>>> Hi, >>>> >>>> This patch tunes optimization options based on profile data: >>>> * Disable PGO options if profile is not available or empty. >>>> * Optimize for size if profile is available but empty. >>> >>> Err ... these don't seem interesting cases to "optimize" for? >>> >>> Richard.
On Wed, Jul 23, 2014 at 1:04 PM, Pengfei Yuan <0xcoolypf@gmail.com> wrote: > I guess some optimizations are controlled only by "optimize_size", not > by the profile. I only see tree-inline.c:estimate_move_cost which we should indeed fix, it could make a significant difference. One other use in tree-ssa-phiopt.c, but probably doesn't really matter. Richard. > Other optimizations are controlled by the profile. > So this patch does not have very much effectiveness (only 0.9% size reduction). > > 2014-07-23 17:26 GMT+08:00 Richard Biener <richard.guenther@gmail.com>: >> On Wed, Jul 23, 2014 at 2:39 AM, Pengfei Yuan <0xcoolypf@gmail.com> wrote: >>> In the experiment, about 60% (1019/1699) profile data files are empty >>> (all counters are zero). >> >> Well, but you are globally overriding options even for the parts with >> profile. The whole point of profile-feedback is to get at the interesting >> parts (those with non-zero counters). >> >> What you say is that not enough parts of the compiler care for >> the actual profiles and thus portions with all-zero counters are >> treated as if they were hot? Then better fix that. >> >> Richard. >> >>> 2014-07-22 21:39 GMT+08:00 Richard Biener <richard.guenther@gmail.com>: >>>> On Mon, Jul 21, 2014 at 7:13 AM, Pengfei Yuan <0xcoolypf@gmail.com> wrote: >>>>> Hi, >>>>> >>>>> This patch tunes optimization options based on profile data: >>>>> * Disable PGO options if profile is not available or empty. >>>>> * Optimize for size if profile is available but empty. >>>> >>>> Err ... these don't seem interesting cases to "optimize" for? >>>> >>>> Richard.
> On Wed, Jul 23, 2014 at 2:39 AM, Pengfei Yuan <0xcoolypf@gmail.com> wrote: > > In the experiment, about 60% (1019/1699) profile data files are empty > > (all counters are zero). > > Well, but you are globally overriding options even for the parts with > profile. The whole point of profile-feedback is to get at the interesting > parts (those with non-zero counters). > > What you say is that not enough parts of the compiler care for > the actual profiles and thus portions with all-zero counters are > treated as if they were hot? Then better fix that. Most of the compiler should use the optimize_for_size/time predicates that should get the case of empty profile right. For missing profile I am not terribly decided what to do - it may make sense to ignore all changes of -fprofile-use then, but it seems users should just arrange profile when they want to use it (and we already warn in that case) What benefits you see from the patch? Honza > > Richard. > > > 2014-07-22 21:39 GMT+08:00 Richard Biener <richard.guenther@gmail.com>: > >> On Mon, Jul 21, 2014 at 7:13 AM, Pengfei Yuan <0xcoolypf@gmail.com> wrote: > >>> Hi, > >>> > >>> This patch tunes optimization options based on profile data: > >>> * Disable PGO options if profile is not available or empty. > >>> * Optimize for size if profile is available but empty. > >> > >> Err ... these don't seem interesting cases to "optimize" for? > >> > >> Richard.
> I guess some optimizations are controlled only by "optimize_size", not > by the profile. > Other optimizations are controlled by the profile. > So this patch does not have very much effectiveness (only 0.9% size reduction). 0.9% size reduction counts as very much in compiler developers perspective :). Indeed not all optimizations have detailed size/speed gates - I basically reviewed the compiler for places that seemed important to me. I think the proper fix would be to figure out from where the reduction comes from and fix the offending pass. Do you have any idea where you get the savings? Honza > > 2014-07-23 17:26 GMT+08:00 Richard Biener <richard.guenther@gmail.com>: > > On Wed, Jul 23, 2014 at 2:39 AM, Pengfei Yuan <0xcoolypf@gmail.com> wrote: > >> In the experiment, about 60% (1019/1699) profile data files are empty > >> (all counters are zero). > > > > Well, but you are globally overriding options even for the parts with > > profile. The whole point of profile-feedback is to get at the interesting > > parts (those with non-zero counters). > > > > What you say is that not enough parts of the compiler care for > > the actual profiles and thus portions with all-zero counters are > > treated as if they were hot? Then better fix that. > > > > Richard. > > > >> 2014-07-22 21:39 GMT+08:00 Richard Biener <richard.guenther@gmail.com>: > >>> On Mon, Jul 21, 2014 at 7:13 AM, Pengfei Yuan <0xcoolypf@gmail.com> wrote: > >>>> Hi, > >>>> > >>>> This patch tunes optimization options based on profile data: > >>>> * Disable PGO options if profile is not available or empty. > >>>> * Optimize for size if profile is available but empty. > >>> > >>> Err ... these don't seem interesting cases to "optimize" for? > >>> > >>> Richard.
There are more.
In toplev.c:
/* One region RA really helps to decrease the code size. */
if (flag_ira_region == IRA_REGION_AUTODETECT)
flag_ira_region
= optimize_size || !optimize ? IRA_REGION_ONE : IRA_REGION_MIXED;
In config/i386/i386.c:
* Assignment of ix86_cost
* Decision of alignment
2014-07-23 19:32 GMT+08:00 Richard Biener <richard.guenther@gmail.com>:
> On Wed, Jul 23, 2014 at 1:04 PM, Pengfei Yuan <0xcoolypf@gmail.com> wrote:
>> I guess some optimizations are controlled only by "optimize_size", not
>> by the profile.
>
> I only see tree-inline.c:estimate_move_cost which we should indeed fix,
> it could make a significant difference.
>
> One other use in tree-ssa-phiopt.c, but probably doesn't really matter.
>
> Richard.
On Thu, Jul 24, 2014 at 3:52 AM, Pengfei Yuan <0xcoolypf@gmail.com> wrote: > There are more. > > In toplev.c: > /* One region RA really helps to decrease the code size. */ > if (flag_ira_region == IRA_REGION_AUTODETECT) > flag_ira_region > = optimize_size || !optimize ? IRA_REGION_ONE : IRA_REGION_MIXED; This could be fixed by moving this to ira.c > In config/i386/i386.c: > * Assignment of ix86_cost > * Decision of alignment True, I didn't grep backends. Did you investigate where the savings come from? I meanwhile fixed the estimate_move_cost bit. Thanks, Richard. > 2014-07-23 19:32 GMT+08:00 Richard Biener <richard.guenther@gmail.com>: >> On Wed, Jul 23, 2014 at 1:04 PM, Pengfei Yuan <0xcoolypf@gmail.com> wrote: >>> I guess some optimizations are controlled only by "optimize_size", not >>> by the profile. >> >> I only see tree-inline.c:estimate_move_cost which we should indeed fix, >> it could make a significant difference. >> >> One other use in tree-ssa-phiopt.c, but probably doesn't really matter. >> >> Richard.
No, I didn't. 2014-07-24 16:50 GMT+08:00 Richard Biener <richard.guenther@gmail.com>: > On Thu, Jul 24, 2014 at 3:52 AM, Pengfei Yuan <0xcoolypf@gmail.com> wrote: >> There are more. >> >> In toplev.c: >> /* One region RA really helps to decrease the code size. */ >> if (flag_ira_region == IRA_REGION_AUTODETECT) >> flag_ira_region >> = optimize_size || !optimize ? IRA_REGION_ONE : IRA_REGION_MIXED; > > This could be fixed by moving this to ira.c > >> In config/i386/i386.c: >> * Assignment of ix86_cost >> * Decision of alignment > > True, I didn't grep backends. > > Did you investigate where the savings come from? I meanwhile fixed > the estimate_move_cost bit. > > Thanks, > Richard.
diff --git a/gcc/coverage.c b/gcc/coverage.c index 4c06fa4..205bee5 100644 --- a/gcc/coverage.c +++ b/gcc/coverage.c @@ -1128,6 +1128,75 @@ coverage_obj_finish (vec<constructor_elt, va_gc> *ctor) varpool_finalize_decl (gcov_info_var); } +/* Check the profile data file. + Return -1 if the file is not available or corrupted, + 0 if the file is available and all counters are zero, + 1 otherwise. */ + +int +coverage_check (const char *filename) +{ + int ret = 0; + int len = strlen (filename); + int prefix_len = 0; + gcov_unsigned_t tag; + char *data_filename; + + if (!profile_data_prefix && !IS_ABSOLUTE_PATH (filename)) + profile_data_prefix = getpwd (); + + if (profile_data_prefix) + prefix_len = strlen (profile_data_prefix); + + data_filename = XNEWVEC (char, len + strlen (GCOV_DATA_SUFFIX) + + prefix_len + 2); + + if (profile_data_prefix) + { + memcpy (data_filename, profile_data_prefix, prefix_len); + data_filename[prefix_len++] = '/'; + } + memcpy (data_filename + prefix_len, filename, len); + strcpy (data_filename + prefix_len + len, GCOV_DATA_SUFFIX); + + if (!gcov_open (data_filename, 1)) + return -1; + if (!gcov_magic (gcov_read_unsigned (), GCOV_DATA_MAGIC) + || gcov_read_unsigned () != GCOV_VERSION) + { + gcov_close (); + return -1; + } + gcov_read_unsigned (); + + while ((tag = gcov_read_unsigned ())) + { + gcov_unsigned_t length = gcov_read_unsigned (); + gcov_position_t offset = gcov_position (); + + if (GCOV_TAG_IS_COUNTER (tag)) + { + unsigned n_counts = GCOV_TAG_COUNTER_NUM (length); + unsigned ix; + + for (ix = 0; ix != n_counts; ix++) + if (gcov_read_counter ()) + ret = 1; + } + gcov_sync (offset, length); + + if (gcov_is_error ()) + { + ret = -1; + break; + } + } + + gcov_close (); + + return ret; +} + /* Perform file-level initialization. Read in data file, generate name of notes file. */ diff --git a/gcc/coverage.h b/gcc/coverage.h index 81f87a6..51d1119 100644 --- a/gcc/coverage.h +++ b/gcc/coverage.h @@ -22,6 +22,7 @@ along with GCC; see the file COPYING3. If not see #include "gcov-io.h" +extern int coverage_check (const char *); extern void coverage_init (const char *); extern void coverage_finish (void); diff --git a/gcc/toplev.c b/gcc/toplev.c index d646faf..b0c3906 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -1222,6 +1222,77 @@ init_alignments (void) align_functions_log = floor_log2 (align_functions * 2 - 1); } +/* Override options based on profile. */ + +static void +profile_based_option_override (void) +{ + int status; + const char *name = aux_base_name; + + if (!flag_branch_probabilities) + return; + + if (!name) + { + char *newname; + if (!main_input_filename) + return; + newname = xstrdup (lbasename (main_input_filename)); + strip_off_ending (newname, strlen (newname)); + name = newname; + } + + status = coverage_check (name); + if (status > 0) + return; + + /* Profile data file is valid and all profile counters are zero. + Prefer optimizing code size. */ + if (status == 0) + { + optimize = 2; + optimize_size = 1; + maybe_set_param_value (PARAM_MIN_CROSSJUMP_INSNS, 1, + param_values, global_options_set.x_param_values); + + /* Ignore coverage mismatch since all counters are zero. */ + diagnostic_classify_diagnostic (global_dc, OPT_Wcoverage_mismatch, + DK_IGNORED, UNKNOWN_LOCATION); + } + + if (!flag_profile_use) + return; + + /* Disable optimization options for PGO. */ + if (!global_options_set.x_flag_profile_values) + flag_profile_values = false; + if (!global_options_set.x_flag_unroll_loops) + flag_unroll_loops = false; + if (!global_options_set.x_flag_peel_loops) + flag_peel_loops = false; + if (!global_options_set.x_flag_tracer) + flag_tracer = false; + if (!global_options_set.x_flag_value_profile_transformations) + flag_value_profile_transformations = false; + if (!global_options_set.x_flag_ipa_cp_clone) + flag_ipa_cp_clone = false; + if (!global_options_set.x_flag_predictive_commoning) + flag_predictive_commoning = false; + if (!global_options_set.x_flag_unswitch_loops) + flag_unswitch_loops = false; + if (!global_options_set.x_flag_gcse_after_reload) + flag_gcse_after_reload = false; + if (!global_options_set.x_flag_tree_loop_vectorize + && !global_options_set.x_flag_tree_vectorize) + flag_tree_loop_vectorize = false; + if (!global_options_set.x_flag_tree_slp_vectorize + && !global_options_set.x_flag_tree_vectorize) + flag_tree_slp_vectorize = false; + if (!global_options_set.x_flag_vect_cost_model) + flag_vect_cost_model = VECT_COST_MODEL_CHEAP; +} + /* Process the options that have been parsed. */ static void process_options (void) @@ -1245,6 +1316,8 @@ process_options (void) so we can correctly initialize debug output. */ no_backend = lang_hooks.post_options (&main_input_filename); + profile_based_option_override (); + /* Some machines may reject certain combinations of options. */ targetm.target_option.override ();
Hi, This patch tunes optimization options based on profile data: * Disable PGO options if profile is not available or empty. * Optimize for size if profile is available but empty. Here is an experiment on Firefox PGO build: CPU Intel Core i7-4770 RAM 32 GB OS Debian sid amd64 Firefox source mozilla-central, changeset 4bafe35cfb65 Compiler GCC 4.9.2 20140721 (prerelease) Result: Size of libxul.so | Octane benchmark score PGO w/o this patch 67206232 32440.4 +/- 0.35% PGO w/ this patch 66604312 32765.8 +/- 0.56% With this patch, the size of PGO-built libxul.so decreases by 0.9% and the performance improves slightly. Regards, Yuan Pengfei Peking University gcc/ChangeLog: * coverage.c (coverage_check): New function. * coverage.h (coverage_check): New function. * toplev.c (profile_based_option_override): New function. (process_options): Add profile based option tuning.