On 09/05/2018 09:28 PM, Indu Bhagat wrote: Hi.
Thanks for working on that. I believe it's useful enhancement. Note that I'm not profile feedback maintainer, but I'll provide some feedback: > Patch for PR 86957 " gcc should warn about missing profiles for a compilation > unit or a new function with -fprofile-use". > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86957 > > The patch adds -Wmissing-profile warning flag to alert user about cases when > there is no profile data for a compilation unit or a function when using > -fprofile-use. > > The flag is on by default when -fprofile-use is specified and generates errors > by default (like -Wcoverage-mismatch). > > The attachment pr86957-missing-profile-diagnostic shows the behavior of GCC > with the patch. > > Thanks > Indu > > ------------------------------------------------------ > > > gcc/ChangeLog: > > 2018-09-05 "Indu Bhagat" <"indu.bha...@oracle.com"> > > * common.opt: New warning option -Wmissing-profile. > * coverage.c (get_coverage_counts): Add warning for missing .gcda > file. > * doc/invoke.texi: Document -Wmissing-profile. > * profile.c (get_exec_counts): Add warning for missing feedback > profile for a function. > * toplev.c (process_options): -Wmissing-profile warning as error. > > > pr86957-patch > > > diff --git a/gcc/common.opt b/gcc/common.opt > index ebc3ef4..d93ddca 100644 > --- a/gcc/common.opt > +++ b/gcc/common.opt > @@ -811,6 +811,10 @@ Wcoverage-mismatch > Common Var(warn_coverage_mismatch) Init(1) Warning > Warn in case profiles in -fprofile-use do not match. > > +Wmissing-profile > +Common Var(warn_missing_profile) Init(1) Warning > +Warn in case profiles in -fprofile-use do not exist. Maybe 'Want about missing profile for a function in -fprofile-use build.' ? > + > Wvector-operation-performance > Common Var(warn_vector_operation_performance) Warning > Warn when a vector operation is compiled outside the SIMD. > diff --git a/gcc/coverage.c b/gcc/coverage.c > index bae6f5c..df031df 100644 > --- a/gcc/coverage.c > +++ b/gcc/coverage.c > @@ -341,16 +341,24 @@ get_coverage_counts (unsigned counter, unsigned > expected, > { > static int warned = 0; > > - if (!warned++ && dump_enabled_p ()) > + if (!warned++) > { > - dump_user_location_t loc > - = dump_user_location_t::from_location_t (input_location); > - dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc, > + warning (OPT_Wmissing_profile, > + "%qs profile count data file not found," > + " regenerating profiles may help", I would not append 'regenerating profiles may help'. We don't do it for other profile corruption warnings/errors. > + da_file_name); > + if (dump_enabled_p ()) > + { > + dump_user_location_t loc > + = dump_user_location_t::from_location_t (input_location); > + dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc, > + "file %s not found\n", > + da_file_name); > + dump_printf (MSG_OPTIMIZED_LOCATIONS, > (flag_guess_branch_prob > - ? "file %s not found, execution counts estimated\n" > - : "file %s not found, execution counts assumed to " > - "be zero\n"), > - da_file_name); > + ? "execution counts estimated\n" > + : "execution counts assumed to be zero\n")); > + } > } > return NULL; > } > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index ca92028..e62bcae 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -314,7 +314,7 @@ Objective-C and Objective-C++ Dialects}. > -Wlogical-op -Wlogical-not-parentheses -Wlong-long @gol > -Wmain -Wmaybe-uninitialized -Wmemset-elt-size -Wmemset-transposed-args > @gol > -Wmisleading-indentation -Wmissing-attributes -Wmissing-braces @gol > --Wmissing-field-initializers -Wmissing-include-dirs @gol > +-Wmissing-field-initializers -Wmissing-include-dirs -Wmissing-profile @gol > -Wno-multichar -Wmultistatement-macros -Wnonnull -Wnonnull-compare @gol > -Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol > -Wnull-dereference -Wodr -Wno-overflow -Wopenmp-simd @gol > @@ -4816,6 +4816,25 @@ This warning is enabled by @option{-Wall}. > @opindex Wno-missing-include-dirs > Warn if a user-supplied include directory does not exist. > > +@item -Wmissing-profile > +@opindex Wmissing-profile > +@opindex Wno-missing-profile > +Warn if feedback profiles are missing when using the > +@option{-fprofile-use} option. > +If a new function or a new file is added to the user code between compiling > +with @option{-fprofile-gen} and with @option{-fprofile-use} without s/profile-gen/profile-generate > +regenerating the profiles, the profile feedback data files do not contain any > +profile feedback information for the newly > +added function or file respectively. I would suggest to split the sentence. Also, in the case when profile count data > +(.gcda) files are wiped out, GCC cannot use any profile feedback > +information. In all these cases, warnings are issued to inform the user > that a > +profile generation step is due. By default, this warning is enabled and is > +treated as an error. @option{-Wno-missing-profile} can be used to disable > the > +warning or @option{-Wno-error=missing-profile} can be used to disable the > +error. Disabling the error for this warning can result in poorly optimized > +code and is useful only in the cases when non-existent profile data is > +justified. Completely disabling the warning is not recommended. > + > @item -Wmultistatement-macros > @opindex Wmultistatement-macros > @opindex Wno-multistatement-macros > @@ -9899,8 +9918,10 @@ Before you can use this option, you must first > generate profiling information. > > 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 > -@option{-Wcoverage-mismatch}. Note this may result in poorly optimized > -code. > +@option{-Wno-error=coverage-mismatch}. Note this may result in poorly > +optimized code. Additionally, by default, GCC also emits an error message if > +the feedback profiles do not exist. The latter error can be turned into a > +warning by using @option{-Wno-error=missing-profile}. > > If @var{path} is specified, GCC looks at the @var{path} to find > the profile feedback data files. See @option{-fprofile-dir}. > diff --git a/gcc/profile.c b/gcc/profile.c > index cb51e0d..40f9763 100644 > --- a/gcc/profile.c > +++ b/gcc/profile.c > @@ -286,7 +286,13 @@ get_exec_counts (unsigned cfg_checksum, unsigned > lineno_checksum) > counts = get_coverage_counts (GCOV_COUNTER_ARCS, num_edges, cfg_checksum, > lineno_checksum, &profile_info); > if (!counts) > - return NULL; > + { > + warning (OPT_Wmissing_profile, > + "profile for function %qE not found in profile data," > + " regenerating profiles may help", > + DECL_ASSEMBLER_NAME (current_function_decl)); > + return NULL; > + } I hope we can do better here: warning_at (DECL_SOURCE_LOCATION (current_function_decl), OPT_Wmissing_profile, "profile for function %qD not found in profile data", current_function_decl); Then you'll see better location: main.c:19:5: warning: profile for function ‘main’ not found in profile data [-Wmissing-profile] 19 | int main(int argc, char **argv) | ^~~~ I'm wondering whether we want to report missing profiles for functions that live in a file for which we reported missing .gcda file? main.c: In function ‘main’: main.c:22:1: warning: ‘/tmp/main.gcda’ profile count data file not found, regenerating profiles may help [-Wmissing-profile] 22 | } | ^ main.c:19:5: warning: profile for function ‘main’ not found in profile data [-Wmissing-profile] 19 | int main(int argc, char **argv) | ^~~~ main.c: In function ‘bar’: main.c:14:1: warning: profile for function ‘bar’ not found in profile data [-Wmissing-profile] 14 | bar() | ^~~ main.c: In function ‘foo’: main.c:7:1: warning: profile for function ‘foo’ not found in profile data [-Wmissing-profile] 7 | foo() | ^~~ I would limit that just to the first warning. Otherwise one can end up with a wall of text. > > get_working_sets (); > > diff --git a/gcc/toplev.c b/gcc/toplev.c > index 9fb83d4..5891554 100644 > --- a/gcc/toplev.c > +++ b/gcc/toplev.c > @@ -1782,14 +1782,24 @@ process_options (void) > || !targetm.have_prologue () || !targetm.have_epilogue ()) > flag_ipa_ra = 0; > > - /* Enable -Werror=coverage-mismatch when -Werror and -Wno-error > - have not been set. */ > - if (!global_options_set.x_warnings_are_errors > - && warn_coverage_mismatch > - && (global_dc->classify_diagnostic[OPT_Wcoverage_mismatch] == > - DK_UNSPECIFIED)) > - diagnostic_classify_diagnostic (global_dc, OPT_Wcoverage_mismatch, > - DK_ERROR, UNKNOWN_LOCATION); > + if (!global_options_set.x_warnings_are_errors) > + { > + /* Enable -Werror=coverage-mismatch when -Werror and -Wno-error > + have not been set. */ > + if (warn_coverage_mismatch > + && (global_dc->classify_diagnostic[OPT_Wcoverage_mismatch] == > + DK_UNSPECIFIED)) > + diagnostic_classify_diagnostic (global_dc, OPT_Wcoverage_mismatch, > + DK_ERROR, UNKNOWN_LOCATION); > + > + /* Enable -Werror=missing-profile when -Werror and -Wno-error > + have not been set. */ > + if (warn_missing_profile > + && (global_dc->classify_diagnostic[OPT_Wmissing_profile] == > + DK_UNSPECIFIED)) > + diagnostic_classify_diagnostic (global_dc, OPT_Wmissing_profile, > + DK_ERROR, UNKNOWN_LOCATION); > + } I would prefer not to enable error of the option. Mainly due to configure scripts that have missing profiles (example from postgres): configure: loading site script /usr/share/site/x86_64-unknown-linux-gnu checking build system type... x86_64-pc-linux-gnu checking host system type... x86_64-pc-linux-gnu checking which template to use... linux checking whether NLS is wanted... no checking for default port number... 5432 checking for block size... 8kB checking for segment size... 1GB checking for WAL block size... 8kB checking for gcc... gcc checking whether the C compiler works... no configure: error: in `/home/marxin/Programming/postgres': configure: error: C compiler cannot create executables See `config.log' for more details configure:3942: gcc -O2 -fprofile-use -g -O2 -fprofile-use -g conftest.c >&5 conftest.c: In function 'main': conftest.c:23:1: error: '/home/marxin/Programming/postgres/conftest.gcda' profile count data file not found, regenerating profiles may help [-Werror=missing-profile] 23 | } | ^ conftest.c:23:1: error: profile for function 'main' not found in profile data, regenerating profiles may help [-Werror=missing-profile] Anyway, thanks for working on that. Martin > > /* Save the current optimization options. */ > optimization_default_node = build_optimization_node (&global_options); > > > pr86957-missing-profile-diagnostic > > > [Testcase 1] Missing profile data file with -fprofile-use. The sort.c file > contains two functions main() and stop() > > gcc -c sort.c -fprofile-use -O1 > -fprofile-dir=/data2/user/gcc-temp/fdo/profdata/ > sort.c: In function ‘main’: > sort.c:29:1: error: > ‘/data2/user/gcc-temp/fdo/profdata//#data2#user#gcc-temp#fdo#sort.gcda’ > profile count data file not found, regenerating profiles may help > [-Werror=missing-profile] > 29 | } > | ^ > sort.c:29:1: error: profile for function ‘main’ not found in profile data, > regenerating profiles may help [-Werror=missing-profile] > sort.c: In function ‘stop’: > sort.c:29:1: error: profile for function ‘stop’ not found in profile data, > regenerating profiles may help [-Werror=missing-profile] > cc1: some warnings being treated as errors > make: *** [sort.o] Error 1 > > [Testcase 2] bubble_sort.c has a non-empty function bubble_sort() for which > profile data exists. > Now at profile-use phase, a user adds empty function before1() before an > existing lone function bubble_sort() and empty function after1() after the > lone existing function bubble_sort() > > gcc -c bubble_sort.c -fprofile-use -O1 > -fprofile-dir=/data2/user/gcc-temp/fdo/profdata/ > bubble_sort.c: In function ‘after1’: > bubble_sort.c:19:1: error: profile for function ‘after1’ not found in profile > data, regenerating profiles may help [-Werror=missing-profile] > 19 | void after1() { } > | ^~~~ > bubble_sort.c: In function ‘bubble_sort’: > bubble_sort.c:19:1: error: source locations for function ‘bubble_sort’ have > changed, the profile data may be out of date [-Werror=coverage-mismatch] > bubble_sort.c: In function ‘before1’: > bubble_sort.c:19:1: error: profile for function ‘before1’ not found in > profile data, regenerating profiles may help [-Werror=missing-profile] > cc1: some warnings being treated as errors > make: *** [bubble_sort.o] Error 1 > > [Testcase 3] Use -Wno-error=missing-profile to treat them as warnings, not > errors > > gcc -c bubble_sort.c -fprofile-use -O1 -Wno-error=missing-profile > -fprofile-dir=/data2/user/gcc-temp/fdo/profdata/ > bubble_sort.c: In function ‘after1’: > bubble_sort.c:19:1: warning: profile for function ‘after1’ not found in > profile data, regenerating profiles may help [-Wmissing-profile] > 19 | void after1() { } > | ^~~~ > bubble_sort.c: In function ‘bubble_sort’: > bubble_sort.c:19:1: error: source locations for function ‘bubble_sort’ have > changed, the profile data may be out of date [-Werror=coverage-mismatch] > bubble_sort.c: In function ‘before1’: > bubble_sort.c:19:1: warning: profile for function ‘before1’ not found in > profile data, regenerating profiles may help [-Wmissing-profile] > cc1: some warnings being treated as errors > make: *** [bubble_sort.o] Error 1 >
