> > Here are the results: > > > > Param Size (GCC5) Time (GCC5) Time (GCC7) > > 0 44686265 (-8.26%) 58.772s 66.332s > > 1 45692793 (-6.19%) 40.684s 39.220s > > 2 45556185 (-6.47%) 35.292s 34.328s > > 3 46251049 (-5.05%) 28.820s 27.136s > > 4 47028873 (-3.45%) 24.616s 22.200s > > 5 47495641 (-2.49%) 20.160s 17.800s > > 6 47520153 (-2.44%) 16.444s 15.656s > > 14 48708873 5.620s 5.556s > > Thanks for data! I meant to run the benchmark myself, but had little time to > do > it over past week becuase of traveling and was also wondering what to do given > that spec is rather poor benchmark in this area. Tramp3d is biassed but we are > in stage1 and can fine tune latter. I am debugging the libxul crashes in FDO > binary now, so we can re-run talos.
It seems to be memory corruption. The content of a pointer becomes 0xe5e5...e5. I have tried Firefox 48.0.2, 49b10 and mozilla-central with GCC 6. Same error. > > Param: value of PARAM_EARLY_INLINING_INSNS > > Size: code size (.text) of optimized libxul.so > > Time: execution time of instrumented tramp3d (-n 25) > > > > To balance between size reduction of optimized binary and speed penalty > > of instrumented binary, I set param=6 as baseline and compare: > > > > Param Size score Time score Total > > 0 3.39 -3.57 -0.18 > > 1 2.54 -2.47 0.07 > > 2 2.65 -2.15 0.50 > > 3 2.07 -1.75 0.32 > > 4 1.41 -1.50 -0.09 > > 5 1.02 -1.23 -0.21 > > 6 1.00 -1.00 0.00 > > 14 0.00 -0.34 -0.34 > > > > Therefore, I think param=2 is the best choice. > > > > Is the attached patch OK? > > Setting param to 2 looks fine > > > gcc/ChangeLog > > * opts.c (finish_options): Adjust PARAM_EARLY_INLINING_INSNS > > when FDO is enabled. > > > > > > diff --git a/gcc/opts.c b/gcc/opts.c > > index 39c190d..b59c700 100644 > > --- a/gcc/opts.c > > +++ b/gcc/opts.c > > @@ -826,8 +826,14 @@ finish_options (struct gcc_options *opts, struct > > gcc_options *opts_set, > > maybe_set_param_value (PARAM_STACK_FRAME_GROWTH, 40, > > opts->x_param_values, > > opts_set->x_param_values); > > } > > > > + /* Adjust PARAM_EARLY_INLINING_INSNS when FDO is enabled. */ > > + if ((opts->x_profile_arc_flag && !opts->x_flag_test_coverage) > > + || (opts->x_flag_branch_probabilities && !opts->x_flag_auto_profile)) > > + maybe_set_param_value (PARAM_EARLY_INLINING_INSNS, 2, > > + opts->x_param_values, opts_set->x_param_values); > > + > > I would actually preffer to have PARAM_EARLY_ININING_INSNS_FEEDBACK. We > already have TRACER_DYNAMIC_COVERAGE_FEEDBACK and other params. The reason is > that profile is not a global property of program. It may or may not be > available for given function, while params are global. Whether profile is available for specific functions is not important here because profile is loaded after pass_early_inline. Therefore I think setting the param globally is fine. > Even at compile time profile may be selectively missing for example for > COMDATs that did not win in the linking process. > > There is also need to update the documentation. > Thanks for the work! > Honza Here is the new patch. Regards, Yuan, Pengfei gcc/ChangeLog * doc/invoke.texi (early-inlining-insns): Value is adjusted when FDO is enabled. * opts.c (finish_options): Adjust PARAM_EARLY_INLINING_INSNS when FDO is enabled. diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 1ca4dcc..880a28c 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -10272,9 +10272,11 @@ The default value is 10. @item early-inlining-insns Specify growth that the early inliner can make. In effect it increases the amount of inlining for code having a large abstraction penalty. -The default value is 14. +The default value is 14. When feedback-directed optimizations is enabled (by +@option{-fprofile-generate} or @option{-fprofile-use}), the value is adjusted +to 2. @item max-early-inliner-iterations Limit of iterations of the early inliner. This basically bounds the number of nested indirect calls the early inliner can resolve. diff --git a/gcc/opts.c b/gcc/opts.c index 39c190d..b59c700 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -826,8 +826,14 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, maybe_set_param_value (PARAM_STACK_FRAME_GROWTH, 40, opts->x_param_values, opts_set->x_param_values); } + /* Adjust PARAM_EARLY_INLINING_INSNS when FDO is enabled. */ + if ((opts->x_profile_arc_flag && !opts->x_flag_test_coverage) + || (opts->x_flag_branch_probabilities && !opts->x_flag_auto_profile)) + maybe_set_param_value (PARAM_EARLY_INLINING_INSNS, 2, + opts->x_param_values, opts_set->x_param_values); + if (opts->x_flag_lto) { #ifdef ENABLE_LTO opts->x_flag_generate_lto = 1;