[PATCH] PR86957

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] PR86957

Indu Bhagat-2
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"  <"[hidden email]">

         * 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 (9K) Download Attachment
pr86957-missing-profile-diagnostic (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Martin Liška-2
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"  <"[hidden email]">
>
>         * 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
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Martin Liška-2
In reply to this post by Indu Bhagat-2
On 09/05/2018 09:28 PM, Indu Bhagat wrote:

> 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"  <"[hidden email]">
>
>         * 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.
>

... and please add a test-case for the missing file. That's easy to achieve by running
a test-case with -fprofile-use.

Martin
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Indu Bhagat-2
In reply to this post by Martin Liška-2
Thanks for the reviews. I have incorporated all but one (See below; its the change in the warning's
brief summary in common.opt) in the patch.

In this patch,

1. -Wmissing-profile is a warning by default and is ON by default with
    -fprofile-use
2. Attached pr86957-missing-profile-diagnostic-2 shows the warning messages
3. Added a testcase for warning in the case of missing profile feedback data
    file for a compilation unit

Thanks

gcc/ChangeLog:

2018-09-14  "Indu Bhagat"<[hidden email]>

         * common.opt: New warning option -Wmissing-profile.
         * coverage.c (get_coverage_counts): Add warning for missing .gcda file.
         * doc/invoke.texi: Document -Wmissing-profile.

gcc/testsuite/ChangeLog:

2018-09-14  "Indu Bhagat"<[hidden email]>

         * gcc.dg/Wmissing-profile.c: New test.


On 09/11/2018 02:21 AM, Martin Liška wrote:

>> 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.' ?
>
Since, Wmissing-profile also warns when feedback data file is missing for a compilation unit, the
suggested text above will be more restrictive. So I did not change.


pr86957-missing-profile-diagnostic-2 (2K) Download Attachment
pr86957-ver2.patch (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Martin Liška-2
In reply to this post by Martin Liška-2
On 9/16/18 12:58 AM, Indu Bhagat wrote:

> Thanks for the reviews. I have incorporated them in this patch except the one
> (changes in common.opt) below.
>
> In this patch,
>
> 1. -Wmissing-profile is a warning by default and is ON by default with
>    -fprofile-use
> 2. Attached pr86957-missing-profile-diagnostic-2 shows the warning messages
> 3. Added a testcase for warning in the case of missing profile feedback data
>    file for a compilation unit
>
> Thanks

Hi.

The patch looks fine for me now. Honza can you approve it?

Martin

>
> gcc/ChangeLog:
>
> 2018-09-14  "Indu Bhagat"  <"[hidden email]">
>
>         * common.opt: New warning option -Wmissing-profile.
>         * coverage.c (get_coverage_counts): Add warning for missing .gcda file.
>         * doc/invoke.texi: Document -Wmissing-profile.
>
> gcc/testsuite/ChangeLog:
>
> 2018-09-14  "Indu Bhagat"  <"[hidden email]">
>
>         * gcc.dg/Wmissing-profile.c: New test.
>
>
> On 09/11/2018 02:21 AM, Martin Liška wrote:
>>> --- 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.' ?
>>
> Since, it also warns when feedback file is missing for a compilation unit, the
> suggested text above will be more restrictive. So I did not change.
>
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Jan Hubicka-2
> On 9/16/18 12:58 AM, Indu Bhagat wrote:
> > Thanks for the reviews. I have incorporated them in this patch except the one
> > (changes in common.opt) below.
> >
> > In this patch,
> >
> > 1. -Wmissing-profile is a warning by default and is ON by default with
> >    -fprofile-use
> > 2. Attached pr86957-missing-profile-diagnostic-2 shows the warning messages
> > 3. Added a testcase for warning in the case of missing profile feedback data
> >    file for a compilation unit
> >
> > Thanks
>
> Hi.
>
> The patch looks fine for me now. Honza can you approve it?
Patch looks OK.  We used to have this warning but it was removed by google floks
because if you build a static library it may happen that object file was never linked
into your test application, so its use is a bit limited. Having an option for this makes
sense to me though.

Honza

>
> Martin
>
> >
> > gcc/ChangeLog:
> >
> > 2018-09-14  "Indu Bhagat"  <"[hidden email]">
> >
> >         * common.opt: New warning option -Wmissing-profile.
> >         * coverage.c (get_coverage_counts): Add warning for missing .gcda file.
> >         * doc/invoke.texi: Document -Wmissing-profile.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2018-09-14  "Indu Bhagat"  <"[hidden email]">
> >
> >         * gcc.dg/Wmissing-profile.c: New test.
> >
> >
> > On 09/11/2018 02:21 AM, Martin Liška wrote:
> >>> --- 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.' ?
> >>
> > Since, it also warns when feedback file is missing for a compilation unit, the
> > suggested text above will be more restrictive. So I did not change.

Perhaps we want also to have reference from -fprofile-use documentation so users notice
this option.

Honza
> >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Indu Bhagat-2

On 09/17/2018 03:52 AM, Jan Hubicka wrote:

>>> On 09/11/2018 02:21 AM, Martin Liška wrote:
>>>>> --- 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.' ?
>>>>
>>> Since, it also warns when feedback file is missing for a compilation unit, the
>>> suggested text above will be more restrictive. So I did not change.
> Perhaps we want also to have reference from -fprofile-use documentation so users notice
> this option.
>
> Honza
Thanks

Yes, I had added that in the patch. Following additional text will
appear at the end of
-fprofile-use :

+ Additionally, by default, GCC also emits a warning message if
+the feedback profiles do not exist (See @option{-Wmissing-profile}).
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Indu Bhagat-2
In reply to this post by Jan Hubicka-2
Attached is the refreshed patch for trunk.

After commit 264462 (Remove arc profile histogram in non-LTO mode.), the API
of get_coverage_counts was changed a bit. So the main difference between the
current version of my patch from the previous one is that :

Now I use
+      if (counter == GCOV_COUNTER_ARCS)
+       warning_at (DECL_SOURCE_LOCATION (current_function_decl),
+                   OPT_Wmissing_profile,
+                   "profile for function %qD not found in profile data",
+                   current_function_decl);

instead of
+      if (summary)
+       warning_at (DECL_SOURCE_LOCATION (current_function_decl),
+                   OPT_Wmissing_profile,
+                   "profile for function %qD not found in profile data",
+                   current_function_decl);

to warn about missing profile of a function only once (get_coverage_counts is
called from two diff flows : getting exec counts for arc counter and computing
histogram for other other counters)

I am not sure of any better way to avoid superfluous warnings per function.

Is the patch OK ?


pr86957-ver3.patch (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Martin Sebor-2
On 09/21/2018 05:27 PM, Indu Bhagat wrote:

> Attached is the refreshed patch for trunk.
>
> After commit 264462 (Remove arc profile histogram in non-LTO mode.), the
> API
> of get_coverage_counts was changed a bit. So the main difference between
> the
> current version of my patch from the previous one is that :
>
> Now I use
> +      if (counter == GCOV_COUNTER_ARCS)
> +       warning_at (DECL_SOURCE_LOCATION (current_function_decl),
> +                   OPT_Wmissing_profile,
> +                   "profile for function %qD not found in profile data",
> +                   current_function_decl);
>
> instead of
> +      if (summary)
> +       warning_at (DECL_SOURCE_LOCATION (current_function_decl),
> +                   OPT_Wmissing_profile,
> +                   "profile for function %qD not found in profile data",
> +                   current_function_decl);
>
> to warn about missing profile of a function only once
> (get_coverage_counts is
> called from two diff flows : getting exec counts for arc counter and
> computing
> histogram for other other counters)
>
> I am not sure of any better way to avoid superfluous warnings per function.
>
> Is the patch OK ?
>

In the text below:

@@ -4823,6 +4823,23 @@
  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.
+This option diagnoses those cases where a new function or a new file is
added
+to the user code between compiling with @option{-fprofile-generate} and
with
+@option{-fprofile-use}, without regenerating the profiles.  In these
cases, the
+profile feedback data files do not contain any profile feedback
information for
+the newly added function or file respectively.  Also, in the case when
profile
+count data (.gcda) files are wiped out, GCC cannot use any profile
feedback
+information.

I would suggest to use the term "remove" or "delete" instead of
the informal "wipe out" when referring to removing files or their
contents.

Martin
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Indu Bhagat-2
Done. Attached is updated patch.

Patch is tested on x86_64

Thanks


On 09/24/2018 09:37 AM, Martin Sebor wrote:
> I would suggest to use the term "remove" or "delete" instead of
> the informal "wipe out" when referring to removing files or their
> contents.
>
> Martin


pr86957-ver4.patch (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Martin Liška-2
On 9/24/18 9:21 PM, Indu Bhagat wrote:
> Done. Attached is updated patch.

Thanks for it, I tested that right now.
You have ACK, so please install the patch. Please do not forget
to install ChangeLog entry and I would include PR entry:
https://www.gnu.org/software/gcc/contribute.html

example can be seen here:
https://github.com/gcc-mirror/gcc/blob/089d1a5f4939282881c108e0d04d026bdd82f6c6/ChangeLog#L178

Martin

>
> Patch is tested on x86_64
>
> Thanks
>
>
> On 09/24/2018 09:37 AM, Martin Sebor wrote:
>> I would suggest to use the term "remove" or "delete" instead of
>> the informal "wipe out" when referring to removing files or their
>> contents.
>>
>> Martin
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Richard Biener-2
In reply to this post by Indu Bhagat-2
On Mon, Sep 24, 2018 at 9:14 PM Indu Bhagat <[hidden email]> wrote:
>
> Done. Attached is updated patch.
>
> Patch is tested on x86_64

You obviously did _not_ properly test the patch since it causes a
bunch of new testsuite
failures:

FAIL: g++.dg/pr60518.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/pr60518.C  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/pr60518.C  -std=gnu++98 (test for excess errors)
FAIL: g++.dg/tree-ssa/dom-invalid.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/tree-ssa/dom-invalid.C  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/tree-ssa/dom-invalid.C  -std=gnu++98 (test for excess errors)
FAIL: g++.dg/guality/pr55665.C   -O2 -flto -fno-use-linker-plugin
-flto-partition=none  line 23 p == 40
FAIL: g++.dg/torture/pr59265.C   -O0  (test for excess errors)
FAIL: g++.dg/torture/pr59265.C   -O1  (test for excess errors)
FAIL: g++.dg/torture/pr59265.C   -O2  (test for excess errors)
FAIL: g++.dg/torture/pr59265.C   -O2 -flto -fno-use-linker-plugin
-flto-partition=none  (test for excess errors)
FAIL: g++.dg/torture/pr59265.C   -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects  (test for excess errors)
FAIL: g++.dg/torture/pr59265.C   -O3 -g  (test for excess errors)
FAIL: g++.dg/torture/pr59265.C   -Os  (test for excess errors)
FAIL: g++.dg/tree-prof/morefunc.C compilation,  -fprofile-use -D_PROFILE_USE
UNRESOLVED: g++.dg/tree-prof/morefunc.C execution,    -fprofile-use
-D_PROFILE_USE

and more.  Please get up to speed with GCC development and testing requirements!

Richard.

> Thanks
>
>
> On 09/24/2018 09:37 AM, Martin Sebor wrote:
> > I would suggest to use the term "remove" or "delete" instead of
> > the informal "wipe out" when referring to removing files or their
> > contents.
> >
> > Martin
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Martin Liška-2
On 9/27/18 11:47 AM, Richard Biener wrote:

> On Mon, Sep 24, 2018 at 9:14 PM Indu Bhagat <[hidden email]> wrote:
>>
>> Done. Attached is updated patch.
>>
>> Patch is tested on x86_64
>
> You obviously did _not_ properly test the patch since it causes a
> bunch of new testsuite
> failures:
>
> FAIL: g++.dg/pr60518.C  -std=gnu++11 (test for excess errors)
> FAIL: g++.dg/pr60518.C  -std=gnu++14 (test for excess errors)
> FAIL: g++.dg/pr60518.C  -std=gnu++98 (test for excess errors)
> FAIL: g++.dg/tree-ssa/dom-invalid.C  -std=gnu++11 (test for excess errors)
> FAIL: g++.dg/tree-ssa/dom-invalid.C  -std=gnu++14 (test for excess errors)
> FAIL: g++.dg/tree-ssa/dom-invalid.C  -std=gnu++98 (test for excess errors)
> FAIL: g++.dg/guality/pr55665.C   -O2 -flto -fno-use-linker-plugin
> -flto-partition=none  line 23 p == 40
> FAIL: g++.dg/torture/pr59265.C   -O0  (test for excess errors)
> FAIL: g++.dg/torture/pr59265.C   -O1  (test for excess errors)
> FAIL: g++.dg/torture/pr59265.C   -O2  (test for excess errors)
> FAIL: g++.dg/torture/pr59265.C   -O2 -flto -fno-use-linker-plugin
> -flto-partition=none  (test for excess errors)
> FAIL: g++.dg/torture/pr59265.C   -O2 -flto -fuse-linker-plugin
> -fno-fat-lto-objects  (test for excess errors)
> FAIL: g++.dg/torture/pr59265.C   -O3 -g  (test for excess errors)
> FAIL: g++.dg/torture/pr59265.C   -Os  (test for excess errors)
> FAIL: g++.dg/tree-prof/morefunc.C compilation,  -fprofile-use -D_PROFILE_USE
> UNRESOLVED: g++.dg/tree-prof/morefunc.C execution,    -fprofile-use
> -D_PROFILE_USE
>
> and more.  Please get up to speed with GCC development and testing requirements!
>
> Richard.

I'll cook patch for it in order to remove the failures as soon as possible.

Martin

>
>> Thanks
>>
>>
>> On 09/24/2018 09:37 AM, Martin Sebor wrote:
>>> I would suggest to use the term "remove" or "delete" instead of
>>> the informal "wipe out" when referring to removing files or their
>>> contents.
>>>
>>> Martin
>>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Martin Liška-2
On 9/27/18 12:14 PM, Martin Liška wrote:

> On 9/27/18 11:47 AM, Richard Biener wrote:
>> On Mon, Sep 24, 2018 at 9:14 PM Indu Bhagat <[hidden email]> wrote:
>>>
>>> Done. Attached is updated patch.
>>>
>>> Patch is tested on x86_64
>>
>> You obviously did _not_ properly test the patch since it causes a
>> bunch of new testsuite
>> failures:
>>
>> FAIL: g++.dg/pr60518.C  -std=gnu++11 (test for excess errors)
>> FAIL: g++.dg/pr60518.C  -std=gnu++14 (test for excess errors)
>> FAIL: g++.dg/pr60518.C  -std=gnu++98 (test for excess errors)
>> FAIL: g++.dg/tree-ssa/dom-invalid.C  -std=gnu++11 (test for excess errors)
>> FAIL: g++.dg/tree-ssa/dom-invalid.C  -std=gnu++14 (test for excess errors)
>> FAIL: g++.dg/tree-ssa/dom-invalid.C  -std=gnu++98 (test for excess errors)
>> FAIL: g++.dg/guality/pr55665.C   -O2 -flto -fno-use-linker-plugin
>> -flto-partition=none  line 23 p == 40
>> FAIL: g++.dg/torture/pr59265.C   -O0  (test for excess errors)
>> FAIL: g++.dg/torture/pr59265.C   -O1  (test for excess errors)
>> FAIL: g++.dg/torture/pr59265.C   -O2  (test for excess errors)
>> FAIL: g++.dg/torture/pr59265.C   -O2 -flto -fno-use-linker-plugin
>> -flto-partition=none  (test for excess errors)
>> FAIL: g++.dg/torture/pr59265.C   -O2 -flto -fuse-linker-plugin
>> -fno-fat-lto-objects  (test for excess errors)
>> FAIL: g++.dg/torture/pr59265.C   -O3 -g  (test for excess errors)
>> FAIL: g++.dg/torture/pr59265.C   -Os  (test for excess errors)
>> FAIL: g++.dg/tree-prof/morefunc.C compilation,  -fprofile-use -D_PROFILE_USE
>> UNRESOLVED: g++.dg/tree-prof/morefunc.C execution,    -fprofile-use
>> -D_PROFILE_USE
>>
>> and more.  Please get up to speed with GCC development and testing requirements!
>>
>> Richard.
>
> I'll cook patch for it in order to remove the failures as soon as possible.
>
> Martin
>
>>
>>> Thanks
>>>
>>>
>>> On 09/24/2018 09:37 AM, Martin Sebor wrote:
>>>> I would suggest to use the term "remove" or "delete" instead of
>>>> the informal "wipe out" when referring to removing files or their
>>>> contents.
>>>>
>>>> Martin
>>>
>
There's tested patch that I made. It's tested on x86_64-linux-gnu. Feel free
to install it after approval. I'll be back on Monday.

Martin

0001-Fix-fallout-of-Wmissing-profile-warning.patch (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Richard Biener-2
On Thu, Sep 27, 2018 at 3:10 PM Martin Liška <[hidden email]> wrote:

>
> On 9/27/18 12:14 PM, Martin Liška wrote:
> > On 9/27/18 11:47 AM, Richard Biener wrote:
> >> On Mon, Sep 24, 2018 at 9:14 PM Indu Bhagat <[hidden email]> wrote:
> >>>
> >>> Done. Attached is updated patch.
> >>>
> >>> Patch is tested on x86_64
> >>
> >> You obviously did _not_ properly test the patch since it causes a
> >> bunch of new testsuite
> >> failures:
> >>
> >> FAIL: g++.dg/pr60518.C  -std=gnu++11 (test for excess errors)
> >> FAIL: g++.dg/pr60518.C  -std=gnu++14 (test for excess errors)
> >> FAIL: g++.dg/pr60518.C  -std=gnu++98 (test for excess errors)
> >> FAIL: g++.dg/tree-ssa/dom-invalid.C  -std=gnu++11 (test for excess errors)
> >> FAIL: g++.dg/tree-ssa/dom-invalid.C  -std=gnu++14 (test for excess errors)
> >> FAIL: g++.dg/tree-ssa/dom-invalid.C  -std=gnu++98 (test for excess errors)
> >> FAIL: g++.dg/guality/pr55665.C   -O2 -flto -fno-use-linker-plugin
> >> -flto-partition=none  line 23 p == 40
> >> FAIL: g++.dg/torture/pr59265.C   -O0  (test for excess errors)
> >> FAIL: g++.dg/torture/pr59265.C   -O1  (test for excess errors)
> >> FAIL: g++.dg/torture/pr59265.C   -O2  (test for excess errors)
> >> FAIL: g++.dg/torture/pr59265.C   -O2 -flto -fno-use-linker-plugin
> >> -flto-partition=none  (test for excess errors)
> >> FAIL: g++.dg/torture/pr59265.C   -O2 -flto -fuse-linker-plugin
> >> -fno-fat-lto-objects  (test for excess errors)
> >> FAIL: g++.dg/torture/pr59265.C   -O3 -g  (test for excess errors)
> >> FAIL: g++.dg/torture/pr59265.C   -Os  (test for excess errors)
> >> FAIL: g++.dg/tree-prof/morefunc.C compilation,  -fprofile-use -D_PROFILE_USE
> >> UNRESOLVED: g++.dg/tree-prof/morefunc.C execution,    -fprofile-use
> >> -D_PROFILE_USE
> >>
> >> and more.  Please get up to speed with GCC development and testing requirements!
> >>
> >> Richard.
> >
> > I'll cook patch for it in order to remove the failures as soon as possible.
> >
> > Martin
> >
> >>
> >>> Thanks
> >>>
> >>>
> >>> On 09/24/2018 09:37 AM, Martin Sebor wrote:
> >>>> I would suggest to use the term "remove" or "delete" instead of
> >>>> the informal "wipe out" when referring to removing files or their
> >>>> contents.
> >>>>
> >>>> Martin
> >>>
> >
>
> There's tested patch that I made. It's tested on x86_64-linux-gnu. Feel free
> to install it after approval. I'll be back on Monday.

Patch is OK (and I installed it).

Richard.

> Martin
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Thomas Schwinge-8
In reply to this post by Richard Biener-2
Hi!

Sorry for my late follow-up; had a lot of catch up to do back then.

On Thu, 27 Sep 2018 11:47:31 +0200, Richard Biener <[hidden email]> wrote:

> On Mon, Sep 24, 2018 at 9:14 PM Indu Bhagat <[hidden email]> wrote:
> >
> > Done. Attached is updated patch.
> >
> > Patch is tested on x86_64
>
> You obviously did _not_ properly test the patch since it causes a
> bunch of new testsuite
> failures: [...]
>
> and more.  Please get up to speed with GCC development and testing requirements!

Also, with this commit, several test cases regressed from PASS to
UNSUPPORTED because of "{ dg-require-effective-target freorder }" running
into:

    fprofile_use_freorder16732.c: In function 'void foo()':
    fprofile_use_freorder16732.c:2:20: warning: '[...]/gcc/testsuite/g++/fprofile_use_freorder16732.gcda' profile count data file not found [-Wmissing-profile]

Iain had just mentioned that in <https://gcc.gnu.org/PR88310>, and fixed
in r266785, <https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00209.html>.
(Back then, I had produced the same fix, but not yet posted.)  :-|

With "{ dg-require-effective-target freorder }" thusly restored, I then
also saw two other regressions/FAILs, back then:

    [-UNSUPPORTED: g++.dg/tree-prof/pr63581.C-]
    UNSUPPORTED: g++.dg/tree-prof/pr63581.C -fauto-profile
    {+PASS: g++.dg/tree-prof/pr63581.C compilation,  -fprofile-generate -D_PROFILE_GENERATE+}
    {+FAIL: g++.dg/tree-prof/pr63581.C compilation,  -fprofile-use -D_PROFILE_USE+}
    {+PASS: g++.dg/tree-prof/pr63581.C execution,    -fprofile-generate -D_PROFILE_GENERATE+}
    {+UNRESOLVED: g++.dg/tree-prof/pr63581.C execution,    -fprofile-use -D_PROFILE_USE+}
   
    [...]/gcc/testsuite/g++.dg/tree-prof/pr63581.C: In function 'int uptodate(page*)':
    [...]/gcc/testsuite/g++.dg/tree-prof/pr63581.C:28:19: warning: profile for function 'int uptodate(page*)' not found in profile data [-Wmissing-profile]

..., and:

    [-UNSUPPORTED: gcc.dg/tree-prof/20041218-1.c-]
    UNSUPPORTED: gcc.dg/tree-prof/20041218-1.c -fauto-profile
    {+PASS: gcc.dg/tree-prof/20041218-1.c compilation,  -fprofile-generate -D_PROFILE_GENERATE+}
    {+FAIL: gcc.dg/tree-prof/20041218-1.c compilation,  -fprofile-use -D_PROFILE_USE+}
    {+PASS: gcc.dg/tree-prof/20041218-1.c execution,    -fprofile-generate -D_PROFILE_GENERATE+}
    {+UNRESOLVED: gcc.dg/tree-prof/20041218-1.c execution,    -fprofile-use -D_PROFILE_USE+}
   
    [...]/gcc/testsuite/gcc.dg/tree-prof/20041218-1.c: In function 'bar':
    [...]/gcc/testsuite/gcc.dg/tree-prof/20041218-1.c:58:1: warning: profile for function 'bar' not found in profile data [-Wmissing-profile]

..., for which I came up with the following patch.

But, this doesn't now seem to be necessary anymore, or am I confused?
Maybe this got fixed differently -- or is anybody still seeing these
FAILs?  (If not, then I'm not proposing to commit this, of course.)

commit 04ad00f79075a0c5eff1f806959918081e54684c
Author: Thomas Schwinge <[hidden email]>
Date:   Wed Oct 31 23:29:32 2018 +0100

    Avoid two -Wmissing-profile diagnostics
---
 gcc/testsuite/g++.dg/tree-prof/pr63581.C    |    5 +++++
 gcc/testsuite/gcc.dg/tree-prof/20041218-1.c |    5 +++++
 2 files changed, 10 insertions(+)

diff --git gcc/testsuite/g++.dg/tree-prof/pr63581.C gcc/testsuite/g++.dg/tree-prof/pr63581.C
index c8caf07..9800851 100644
--- gcc/testsuite/g++.dg/tree-prof/pr63581.C
+++ gcc/testsuite/g++.dg/tree-prof/pr63581.C
@@ -25,10 +25,15 @@ __attribute__((noinline)) static int zero (void)
   return ii;
 }
 
+#pragma GCC diagnostic push
+/* WARNING: profopt.exp does not support dg-warning */
+/* "profile for function 'int uptodate(page*)' not found in profile data" */
+#pragma GCC diagnostic ignored "-Wmissing-profile"
 static inline int uptodate (struct page* p)
 {
   return (p->i < 709);
 }
+#pragma GCC diagnostic pop
 
 static struct page* bar(int i)
 {
diff --git gcc/testsuite/gcc.dg/tree-prof/20041218-1.c gcc/testsuite/gcc.dg/tree-prof/20041218-1.c
index cbd1c7c..41c60a7 100644
--- gcc/testsuite/gcc.dg/tree-prof/20041218-1.c
+++ gcc/testsuite/gcc.dg/tree-prof/20041218-1.c
@@ -54,6 +54,10 @@ check (void *x, struct S *y)
   return 1;
 }
 
+#pragma GCC diagnostic push
+/* WARNING: profopt.exp does not support dg-warning */
+/* "profile for function 'bar' not found in profile data" */
+#pragma GCC diagnostic ignored "-Wmissing-profile"
 static struct V *
 bar (unsigned int x, void *y)
 {
@@ -75,6 +79,7 @@ bar (unsigned int x, void *y)
     return (void *) 0;
   return u;
 }
+#pragma GCC diagnostic pop
 
 int
 foo (unsigned int *x, unsigned int y, void **z)


Grüße
 Thomas
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Indu Bhagat-2
On 12/05/2018 03:33 AM, Thomas Schwinge wrote:

> Hi!
>
> Sorry for my late follow-up; had a lot of catch up to do back then.
>
> On Thu, 27 Sep 2018 11:47:31 +0200, Richard Biener<[hidden email]>  wrote:
>> On Mon, Sep 24, 2018 at 9:14 PM Indu Bhagat<[hidden email]>  wrote:
>>> Done. Attached is updated patch.
>>>
>>> Patch is tested on x86_64
>> You obviously did_not_  properly test the patch since it causes a
>> bunch of new testsuite
>> failures: [...]
>>
>> and more.  Please get up to speed with GCC development and testing requirements!
> Also, with this commit, several test cases regressed from PASS to
> UNSUPPORTED because of "{ dg-require-effective-target freorder }" running
> into:
>
>      fprofile_use_freorder16732.c: In function 'void foo()':
>      fprofile_use_freorder16732.c:2:20: warning: '[...]/gcc/testsuite/g++/fprofile_use_freorder16732.gcda' profile count data file not found [-Wmissing-profile]
>
> Iain had just mentioned that in<https://gcc.gnu.org/PR88310>, and fixed
> in r266785,<https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00209.html>.
> (Back then, I had produced the same fix, but not yet posted.)  :-|
>
> With "{ dg-require-effective-target freorder }" thusly restored, I then
> also saw two other regressions/FAILs, back then:
>
>      [-UNSUPPORTED: g++.dg/tree-prof/pr63581.C-]
>      UNSUPPORTED: g++.dg/tree-prof/pr63581.C -fauto-profile
>      {+PASS: g++.dg/tree-prof/pr63581.C compilation,  -fprofile-generate -D_PROFILE_GENERATE+}
>      {+FAIL: g++.dg/tree-prof/pr63581.C compilation,  -fprofile-use -D_PROFILE_USE+}
>      {+PASS: g++.dg/tree-prof/pr63581.C execution,    -fprofile-generate -D_PROFILE_GENERATE+}
>      {+UNRESOLVED: g++.dg/tree-prof/pr63581.C execution,    -fprofile-use -D_PROFILE_USE+}
>      
>      [...]/gcc/testsuite/g++.dg/tree-prof/pr63581.C: In function 'int uptodate(page*)':
>      [...]/gcc/testsuite/g++.dg/tree-prof/pr63581.C:28:19: warning: profile for function 'int uptodate(page*)' not found in profile data [-Wmissing-profile]
>
> ..., and:
>
>      [-UNSUPPORTED: gcc.dg/tree-prof/20041218-1.c-]
>      UNSUPPORTED: gcc.dg/tree-prof/20041218-1.c -fauto-profile
>      {+PASS: gcc.dg/tree-prof/20041218-1.c compilation,  -fprofile-generate -D_PROFILE_GENERATE+}
>      {+FAIL: gcc.dg/tree-prof/20041218-1.c compilation,  -fprofile-use -D_PROFILE_USE+}
>      {+PASS: gcc.dg/tree-prof/20041218-1.c execution,    -fprofile-generate -D_PROFILE_GENERATE+}
>      {+UNRESOLVED: gcc.dg/tree-prof/20041218-1.c execution,    -fprofile-use -D_PROFILE_USE+}
>      
>      [...]/gcc/testsuite/gcc.dg/tree-prof/20041218-1.c: In function 'bar':
>      [...]/gcc/testsuite/gcc.dg/tree-prof/20041218-1.c:58:1: warning: profile for function 'bar' not found in profile data [-Wmissing-profile]
>
> ..., for which I came up with the following patch.
>
> But, this doesn't now seem to be necessary anymore, or am I confused?
> Maybe this got fixed differently -- or is anybody still seeing these
> FAILs?  (If not, then I'm not proposing to commit this, of course.)
I tested the trunk as of commit 85df98d7fbb6ef5c5b6e97190fff4028f4b70747
(Dec 5 PR c/87028) with and without the -Wmissing-profile on by default
in the compiler. I.e., with the following change :

Common Var(warn_missing_profile) Init(1) Warning
+Common Var(warn_missing_profile) Init(0) Warning

Two observations :
1. The two issues (tree-prof/pr63581.C and tree-prof/20041218-1.c) that you run
    into are not repeatable on my end. I am not sure why you run into them.

2. I do however see other tests (a total of 23) which are have regressed from
    PASS --> UNRESOLVED. A diff is attached.

    Each one of them is due to "Error/Warning threshold exceeded:  1 0 (max. 1 3)"

E.g.,
ERROR: gcc.dg/tree-prof/20050826-2.c: error executing dg-final-use: couldn't open "20050826-2.c.116t.dom2 20050826-2.c.115t.dom2": no such file or directory
   Error/Warning threshold exceeded:  1 0 (max. 1 3)

(I have not been able to see the -Wmissing-profile warning explicitly in my logs
  yet. I was thinking a RUNTESTFLAGS="-v -v -v" should have helped, but it didnt.)

Any suggestions to resolve these tests are appreciated. I am not sure yet of the
most maintainable way to "allow the warning in all tests that it appears". I
need to look around a bit (Is Thomas' patch doing just what needs to be done here ?).

Thanks
Indu


UNRESOLVED-tests-Wmissing-profile (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Indu Bhagat-2

On 12/06/2018 05:54 PM, Indu Bhagat wrote:
> 2. I do however see other tests (a total of 23) which are have
> regressed from
>    PASS --> UNRESOLVED. A diff is attached.
>
>    Each one of them is due to "Error/Warning threshold exceeded: 1 0
> (max. 1 3)"
False alarm.

Looks like there is some flakiness I ran into.

New testsuite runs (make check-gcc) on a fresh builds of the GCC (with
and without missing-profile) enabled look OK.
One additional failure as expected (Wmissing-profile.c). No additional
unresolved tests.

Indu
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] PR86957

Thomas Schwinge-8
Hi Indu!

I recently saw that you just started contributing to GCC, so: welcome,
and enjoy to journey!

On Mon, 10 Dec 2018 12:54:09 -0800, Indu Bhagat <[hidden email]> wrote:
> On 12/06/2018 05:54 PM, Indu Bhagat wrote:
> > [...]

Thanks for looking into this issue again.  As I said in private email,
such things (like looking at the wrong build's test logs) happen to all
of us.  As long as we learn -- or, at least try to learn ;-) -- something
from that, that's fine, as far as I'm concerned.

Evidently, the peer review/collaboration that we have in these Free
Software/Open Source software projects worked, and we eventually caught
this issue.  :-)

> > 2. I do however see other tests (a total of 23) which are have
> > regressed from
> >    PASS --> UNRESOLVED. A diff is attached.
> >
> >    Each one of them is due to "Error/Warning threshold exceeded: 1 0
> > (max. 1 3)"
> False alarm.
>
> Looks like there is some flakiness I ran into.

Anything we could help you with, or that you have questions about?

> New testsuite runs (make check-gcc) on a fresh builds of the GCC (with
> and without missing-profile) enabled look OK.
> One additional failure as expected (Wmissing-profile.c). No additional
> unresolved tests.

Good, thanks for verifying!


Grüße
 Thomas