| Message ID | 4EFE021C.3040406@acm.org |
|---|---|
| State | New |
| Headers | show |
On Fri, Dec 30, 2011 at 10:25 AM, Nathan Sidwell <nathan@acm.org> wrote: > I've committed this patch to fix and improve coverage reporting: > > 1) the time stamp local_tick will be -1 if the user overrides the random > seed. In such cases the gcov data file should be deleted, just as it would > if the time cannot be determined. The end result is that after a the profile data is used in the compilation (with option -fprofile-use -frandom-seed=), it is then deleted. Is this intended? This basically makes FDO very hard to use when -frandom-seed is used. David > > 2) there was a typo in gcov_dump when dumping function data > > 3) when processing coverage data files, it's informative for gcov to emit > the overall line coverage -- in addition to the individual line coverage. > > built & tested on i686-pc-linux-gnu > > nathan
On 06/03/12 05:51, Xinliang David Li wrote: > On Fri, Dec 30, 2011 at 10:25 AM, Nathan Sidwell<nathan@acm.org> wrote: >> I've committed this patch to fix and improve coverage reporting: >> >> 1) the time stamp local_tick will be -1 if the user overrides the random >> seed. In such cases the gcov data file should be deleted, just as it would >> if the time cannot be determined. > > The end result is that after a the profile data is used in the > compilation (with option -fprofile-use -frandom-seed=), it is then > deleted. Is this intended? This basically makes FDO very hard to use > when -frandom-seed is used. IIUC the file is deleted after being used. What is the problem you are encountering? (if you are having a problem, that suggests others on systems without local_tick are also having problems) nathan
On Sun, Jun 3, 2012 at 6:10 AM, Nathan Sidwell <nathan@acm.org> wrote: > On 06/03/12 05:51, Xinliang David Li wrote: >> >> On Fri, Dec 30, 2011 at 10:25 AM, Nathan Sidwell<nathan@acm.org> wrote: >>> >>> I've committed this patch to fix and improve coverage reporting: >>> >>> 1) the time stamp local_tick will be -1 if the user overrides the random >>> seed. In such cases the gcov data file should be deleted, just as it >>> would >>> if the time cannot be determined. >> >> >> The end result is that after a the profile data is used in the >> compilation (with option -fprofile-use -frandom-seed=), it is then >> deleted. Is this intended? This basically makes FDO very hard to use >> when -frandom-seed is used. > > > IIUC the file is deleted after being used. What is the problem you are > encountering? (if you are having a problem, that suggests others on systems > without local_tick are also having problems) Basically it makes it very difficult to rebuild the file with the profile data --- which makes problem triaging impossible. What is worse is that when profile data is missing, gcc silently takes it which gives you no clue you chasing in a completely different path .. David > > nathan
On 06/03/12 17:16, Xinliang David Li wrote: > Basically it makes it very difficult to rebuild the file with the > profile data --- which makes problem triaging impossible. What is Can you explain this more -- what exactly are trying to do? Are you trying to rebuild multiple times with the same coverage data, or something different?
On Sun, Jun 3, 2012 at 10:24 AM, Nathan Sidwell <nathan@acm.org> wrote: > On 06/03/12 17:16, Xinliang David Li wrote: > >> Basically it makes it very difficult to rebuild the file with the >> profile data --- which makes problem triaging impossible. What is > > > Can you explain this more -- what exactly are trying to do? Are you trying > to rebuild multiple times with the same coverage data, yes -- for instance, in the context of debugging a compiler problem, you will need to compile the same file multiple times with the same coverage data. David > or something > different?
On 06/03/12 21:40, Xinliang David Li wrote: >> Can you explain this more -- what exactly are trying to do? Are you trying >> to rebuild multiple times with the same coverage data, > > yes -- for instance, in the context of debugging a compiler problem, > you will need to compile the same file multiple times with the same > coverage data. ok, debugging the compiler is not the typical user mode of use. Do any of the following work for you? *) don't use -frandom-seed (you don't say why you're using this flag) *) copy and restore the coverage data file *) stub out the unlink code at the end of coverage.c. (I presume that you're rebuilding cc1 with optimization off anyway, and as you're debugging a compiler problem with FDO you're somewhat familiar with coverage.c). nathan
On Mon, Jun 4, 2012 at 12:49 AM, Nathan Sidwell <nathan@acm.org> wrote: > On 06/03/12 21:40, Xinliang David Li wrote: > >>> Can you explain this more -- what exactly are trying to do? Are you >>> trying >>> to rebuild multiple times with the same coverage data, >> >> >> yes -- for instance, in the context of debugging a compiler problem, >> you will need to compile the same file multiple times with the same >> coverage data. > > > ok, debugging the compiler is not the typical user mode of use. Another usage is FDO performance tuning trying build with different options/parameters, so it is common. > > Do any of the following work for you? > *) don't use -frandom-seed (you don't say why you're using this flag) This is in our build system for build reproducibility. > *) copy and restore the coverage data file > *) stub out the unlink code at the end of coverage.c. (I presume that you're > rebuilding cc1 with optimization off anyway, and as you're debugging a > compiler problem with FDO you're somewhat familiar with coverage.c). I know how to workaround/fix the problem --- I am asking the motivation for deleting the coverage file? What does it buy us? This looks like a regression in gcc4.7 to me. thanks, David > > nathan
On 06/04/12 17:19, Xinliang David Li wrote: > Another usage is FDO performance tuning trying build with different > options/parameters, so it is common. > This is in our build system for build reproducibility. If you're trying different options, your builds are not repeatable, so I'm not totally sure why you're using random seed. The problem is that, when generating a new set of object files from the coverage files of the previous compilation, the old coverage files no longer match the new object files. Without some kind of way of distinguishing this, the coverage machinery gets confused. The random seed is the way of making this distinction, and allows the coverage machinery to know that when dumping data from executing the new object file, that the coverage file contains information about an old object file -- and so can truncate the file. If the random seed matches, it can't do that and then (if you're lucky) notices the function checksums don't match. I wonder if there's some way of propagating some kind of 'generation' count in the face of using the same random-seed. I'll have a think. nathan
Index: gcov.c =================================================================== --- gcov.c (revision 182730) +++ gcov.c (working copy) @@ -278,6 +278,9 @@ static unsigned a_names; /* Allocated static unsigned object_runs; static unsigned program_count; +static unsigned total_lines; +static unsigned total_executed; + /* Modification time of graph file. */ static time_t bbg_file_time; @@ -380,6 +383,7 @@ static void solve_flow_graph (function_t static void find_exception_blocks (function_t *); static void add_branch_counts (coverage_t *, const arc_t *); static void add_line_counts (coverage_t *, function_t *); +static void executed_summary (unsigned, unsigned); static void function_summary (const coverage_t *, const char *); static const char *format_gcov (gcov_type, gcov_type, int); static void accumulate_line_counts (source_t *); @@ -702,6 +706,8 @@ generate_results (const char *file_name) accumulate_line_counts (src); function_summary (&src->coverage, "File"); + total_lines += src->coverage.lines; + total_executed += src->coverage.lines_executed; if (flag_gcov_file && src->coverage.lines) { char *gcov_file_name @@ -724,6 +730,9 @@ generate_results (const char *file_name) } fnotice (stdout, "\n"); } + + if (!file_name) + executed_summary (total_lines, total_executed); } /* Release a function structure */ @@ -1666,20 +1675,25 @@ format_gcov (gcov_type top, gcov_type bo return buffer; } - -/* Output summary info for a function. */ +/* Summary of execution */ static void -function_summary (const coverage_t *coverage, const char *title) +executed_summary (unsigned lines, unsigned executed) { - fnotice (stdout, "%s '%s'\n", title, coverage->name); - - if (coverage->lines) + if (lines) fnotice (stdout, "Lines executed:%s of %d\n", - format_gcov (coverage->lines_executed, coverage->lines, 2), - coverage->lines); + format_gcov (executed, lines, 2), lines); else fnotice (stdout, "No executable lines\n"); +} + +/* Output summary info for a function or file. */ + +static void +function_summary (const coverage_t *coverage, const char *title) +{ + fnotice (stdout, "%s '%s'\n", title, coverage->name); + executed_summary (coverage->lines, coverage->lines_executed); if (flag_branches) { Index: coverage.c =================================================================== --- coverage.c (revision 182730) +++ coverage.c (working copy) @@ -1119,7 +1119,7 @@ coverage_finish (void) if (bbg_file_name && gcov_close ()) unlink (bbg_file_name); - if (!local_tick) + if (!local_tick || local_tick == (unsigned)-1) /* Only remove the da file, if we cannot stamp it. If we can stamp it, libgcov will DTRT. */ unlink (da_file_name); Index: gcov-dump.c =================================================================== --- gcov-dump.c (revision 182730) +++ gcov-dump.c (working copy) @@ -286,7 +286,7 @@ tag_function (const char *filename ATTRI { printf (" ident=%u", gcov_read_unsigned ()); printf (", lineno_checksum=0x%08x", gcov_read_unsigned ()); - printf (", cfg_checksum_checksum=0x%08x", gcov_read_unsigned ()); + printf (", cfg_checksum=0x%08x", gcov_read_unsigned ()); if (gcov_position () - pos < length) {