This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] offline gcda profile processing tool
- From: Jan Hubicka <hubicka at ucw dot cz>
- To: Rong Xu <xur at google dot com>
- Cc: Jan Hubicka <hubicka at ucw dot cz>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Xinliang David Li <xinliangli at gmail dot com>, Teresa Johnson <tejohnson at google dot com>, Dehao Chen <dehao at google dot com>
- Date: Mon, 26 May 2014 01:43:00 +0200
- Subject: Re: [PATCH] offline gcda profile processing tool
- Authentication-results: sourceware.org; auth=none
- References: <CAF1bQ=T7xuC7O4=n90eN-pKh0me0d1e+50iXFtEcOD4o8s0ArQ at mail dot gmail dot com> <CAF1bQ=TypsiC1Aa3Bz14cxX5sP9Zt02ecVPuU_xnS=2+zmDAQg at mail dot gmail dot com> <CAF1bQ=SPdSH8ffMSEBkGQqH-w7b5ye3mC4n2CwqenH8EbGR+PA at mail dot gmail dot com> <20140415213850 dot GA23141 at atrey dot karlin dot mff dot cuni dot cz> <CAF1bQ=Tsi9JmmZSQo4NJwfes-1EAzzVC_645YEpO5k6PRaeKqA at mail dot gmail dot com> <20140417033448 dot GC3157 at kam dot mff dot cuni dot cz> <CAF1bQ=RD2d1E8PGi9FW5WffPupNcG84TBv53GH62k5vVwu2fGQ at mail dot gmail dot com> <20140515203700 dot GA29194 at kam dot mff dot cuni dot cz> <CAF1bQ=SxfQXK8qxhoaV+chJa+f7v3fEq53Qr-39t1utPvD2ahg at mail dot gmail dot com>
> >> /* Size of the longest file name. */
> >> -static size_t gcov_max_filename = 0;
> >> +/* We need to expose this static variable when compiling for gcov-tool. */
> >> +#ifndef IN_GCOV_TOOL
> >> +static
> >> +#endif
> >> +size_t gcov_max_filename = 0;
> >
> >
> > Why max_filename needs to be exported?
>
> For code efficiency, we allocate the gi_filename buffer only once in
> the dumping, using the maximum filename length, which
> is set in gcov_init(). For gcov-tool, we don't have gcov_init, and we
> set the value when reading the gcda files.
> Now libgcov-driver.o is a separated compilation unit. I need to make
> then static gi_filename global to allow the access.
Note sure if code efficiency wins here over convolutin APIs, but I guess
it is OK.
Please update comment to explain how it is used, so we keep track of it
more easily.
> 2014-05-20 Rong Xu <xur@google.com>
>
> * gcc/gcov-io.c (gcov_position): Make avaialble to gcov-tool.
> (gcov_is_error): Ditto.
> (gcov_read_string): Ditto.
> (gcov_read_sync): Ditto.
> * gcc/gcov-io.h: Move counter defines to gcov-counter.def.
> * gcc/gcov-dump.c (tag_counters): Use gcov-counter.def.
> * gcc/coverage.c: Ditto.
> * gcc/gcov-tool.c: Offline gcda profile processing tool.
> (unlink_gcda_file): Remove one gcda file.
> (unlink_profile_dir): Remove gcda files from the profile path.
> (profile_merge): Merge two profiles in directory.
> (print_merge_usage_message): Print merge usage.
> (merge_usage): Print merge usage and exit.
> (do_merge): Driver for profile merge sub-command.
> (profile_rewrite): Rewrite profile.
> (print_rewrite_usage_message): Print rewrite usage.
> (rewrite_usage): Print rewrite usage and exit.
> (do_rewrite): Driver for profile rewrite sub-command.
> (print_usage): Print gcov-info usage and exit.
> (print_version): Print gcov-info version.
> (process_args): Process arguments.
> (main): Main routine for gcov-tool.
> * gcc/Makefile.in: Build and install gcov-tool.
> * gcc/gcov-counter.def: New file split from gcov-io.h.
> * libgcc/libgcov-driver.c (gcov_max_filename): Make available
> to gcov-tool.
> * libgcc/libgcov-merge.c (__gcov_merge_add): Replace
> gcov_read_counter() with a Macro.
> (__gcov_merge_ior): Ditto.
> (__gcov_merge_time_profile): Ditto.
> (__gcov_merge_single): Ditto.
> (__gcov_merge_delta): Ditto.
> * libgcc/libgcov-util.c (void gcov_set_verbose): Set the verbose flag
> in the utility functions.
> (set_fn_ctrs): Utility function for reading gcda files to in-memory
> gcov_list object link lists.
> (tag_function): Ditto.
> (tag_blocks): Ditto.
> (tag_arcs): Ditto.
> (tag_lines): Ditto.
> (tag_counters): Ditto.
> (tag_summary): Ditto.
> (read_gcda_finalize): Ditto.
> (read_gcda_file): Ditto.
> (ftw_read_file): Ditto.
> (read_profile_dir_init): Ditto.
> (gcov_read_profile_dir): Ditto.
> (gcov_read_counter_mem): Ditto.
> (gcov_get_merge_weight): Ditto.
> (merge_wrapper): A wrapper function that calls merging handler.
> (gcov_merge): Merge two gcov_info objects with weights.
> (find_match_gcov_info): Find the matched gcov_info in the list.
> (gcov_profile_merge): Merge two gcov_info object lists.
> (__gcov_add_counter_op): Process edge profile counter values.
> (__gcov_ior_counter_op): Process IOR profile counter values.
> (__gcov_delta_counter_op): Process delta profile counter values.
> (__gcov_single_counter_op): Process single profile counter values.
> (fp_scale): Callback function for float-point scaling.
> (int_scale): Callback function for integer fraction scaling.
> (gcov_profile_scale): Scaling profile counters.
> (gcov_profile_normalize): Normalize profile counters.
> * libgcc/libgcov.h: Add headers and functions for gcov-tool use.
> (gcov_get_counter): New.
> (gcov_get_counter_target): Ditto.
> (struct gcov_info): Make the functions field mutable in gcov-tool
> compilation.
> * gcc/doc/gcc.texi: Include gcov-tool.texi.
> * gcc/doc/gcov-tool.texi: Document for gcov-tool.
OK, with changes bellow.
I apologize for the delay - it is a long patch and I am bit swamped in tasks
these days.
> Index: gcc/gcov-tool.c
> ===================================================================
> --- gcc/gcov-tool.c (revision 0)
> +++ gcc/gcov-tool.c (revision 0)
> @@ -0,0 +1,466 @@
> +/* Gcc offline profile processing tool support. */
> +/* Compile this one with gcc. */
What "compile this one with gcc" means? :)
> +
> +static int verbose;
Perhaps in C++ times, it could be bool and have comment in front of it
(per coding standards)
> +
> +/* Remove file NAME if it has a gcda suffix. */
> +
> +static int
> +unlink_gcda_file (const char *name,
> + const struct stat *status ATTRIBUTE_UNUSED,
> + int type ATTRIBUTE_UNUSED,
> + struct FTW *ftwbuf ATTRIBUTE_UNUSED)
> +{
> + int ret = 0;
> + int len = strlen (name);
> + int len1 = strlen (GCOV_DATA_SUFFIX);
> +
> + if (len > len1 && !strncmp (len -len1 + name, GCOV_DATA_SUFFIX, len1))
> + remove (name);
> +
> + if (ret)
ret is not set here.
> + {
> + fnotice (stderr, "error in removing %s\n", name);
> + exit (FATAL_EXIT_CODE);
> + }
I think you want to use fatal here, that exists for you.
> +
> +/* Merging profile D1 and D2 with weight as W1 and W2, respectively.
> + The result profile is written to directory OUT.
> + Return 0 on success. */
> +
> +static int
> +profile_merge (const char *d1, const char *d2, const char *out, int w1, int w2)
> +{
> + char *pwd;
> + int ret;
> + struct gcov_info * d1_profile;
> + struct gcov_info * d2_profile;
> +
> +
> + d1_profile = gcov_read_profile_dir (d1, 0);
> + if (!d1_profile)
> + return 1;
> +
> + if (d2)
> + {
> + d2_profile = gcov_read_profile_dir (d2, 0);
> + if (!d2_profile)
> + return 1;
> +
> + /* The actual merge: we overwrite to d1_profile. */
> + ret = gcov_profile_merge (d1_profile, d2_profile, w1, w2);
> +
> + if (ret)
> + return ret;
> + }
> +
> + /* Output new profile. */
> + unlink_profile_dir (out);
> + mkdir (out, 0755);
> + pwd = getcwd (NULL, 0);
> + gcc_assert (pwd);
> + ret = chdir (out);
> + gcc_assert (ret == 0);
> +
> + set_gcov_list (d1_profile);
> + gcov_exit ();
> +
> + ret = chdir (pwd);
ret is unused here, I suppose we should fatal if it fails.
I wonder how safe this is considered in command tools (i.e. change directory and go back)
> + free (pwd);
> + return 0;
> +}
> +
> +/* Usage message for profile merge. */
> +
> +static void
> +print_merge_usage_message (int error_p)
> +{
> + FILE *file = error_p ? stderr : stdout;
> +
> + fnotice (file, " merge [options] <dir1> <dir2> Merge coverage file contents\n");
> + fnotice (file, " -v, --verbose Verbose mode\n");
> + fnotice (file, " -o, --output <dir> Output directory\n");
> + fnotice (file, " -w, --weight <w1,w2> Set weights (float point values)\n");
> +}
> +
> +static const struct option merge_options[] =
> +{
> + { "verbose", no_argument, NULL, 'v' },
> + { "output", required_argument, NULL, 'o' },
> + { "weight", required_argument, NULL, 'w' },
> + { 0, 0, 0, 0 }
> +};
> +
> +/* Print merge usage and exit. */
> +
> +static void
> +merge_usage (void)
> +{
> + fnotice (stderr, "Merge subcomand usage:");
> + print_merge_usage_message (true);
> + exit (FATAL_EXIT_CODE);
> +}
> +
> +/* If N_VAL is no-zero, normalize the profile by setting the largest counter
> + counter value to N_VAL and scale others counters proportionally.
> + Otherwise, multiply the all counters by SCALE. */
> +
> +static int
> +profile_rewrite (const char *d1, const char *out, long long n_val,
> + float scale, int n, int d)
> +{
> + char *pwd;
> + int ret;
> + struct gcov_info * d1_profile;
> +
> +
> + d1_profile = gcov_read_profile_dir (d1, 0);
> + if (!d1_profile)
> + return 1;
> +
> + /* Output new profile. */
> + unlink_profile_dir (out);
> + mkdir (out, 0755);
> + pwd = getcwd (NULL, 0);
> + gcc_assert (pwd);
> + ret = chdir (out);
> + gcc_assert (ret == 0);
Perhaps we want user readable error messages in both cases?
I think both can legally fail if you remove the CWD or when you specify wrong
output directory.
> +
> + ret = chdir (pwd);
> + free (pwd);
Again, I think you want to check (probably have chdir wrapper that will do the fatal for you?)
> + case 's':
> + ret = 0;
> + do_scaling = 1;
> + if (strstr (optarg, "/"))
> + {
> + ret = sscanf (optarg, "%d/%d", &numerator, &denominator);
> + if (ret == 2)
> + {
> + gcc_assert (numerator >= 0);
> + gcc_assert (denominator > 0);
> + }
> + }
> + if (ret != 2)
> + {
> + ret = sscanf (optarg, "%f", &scale);
> + gcc_assert (ret == 1);
> + denominator = 0;
> + }
Probably user readable sanity checking here, too (rather than assert)
> +
> + if (scale < 0.0)
> + {
> + fnotice (stderr, "scale needs to be non-negative\n");
> + exit (FATAL_EXIT_CODE);
fatal (here and at other cases)
> +/* This part of the code is to merge profile counters. */
> +
> +static gcov_type *gcov_value_buf;
> +static gcov_unsigned_t gcov_value_buf_size;
> +static gcov_unsigned_t gcov_value_buf_pos;
> +static unsigned gcov_merge_weight;
Add comments for individual vars.
> +@c Copyright (C) 1996-2014 Free Software Foundation, Inc.
> +@c This is part of the GCC manual.
> +@c For copying conditions, see the file gcc.texi.
> +
> +@ignore
> +@c man begin COPYRIGHT
> +Copyright @copyright{} 1996-2014 Free Software Foundation, Inc.
Perhaps just 2014?
> +
> +Permission is granted to copy, distribute and/or modify this document
> +under the terms of the GNU Free Documentation License, Version 1.3 or
> +any later version published by the Free Software Foundation; with the
> +Invariant Sections being ``GNU General Public License'' and ``Funding
> +Free Software'', the Front-Cover texts being (a) (see below), and with
> +the Back-Cover Texts being (b) (see below). A copy of the license is
> +included in the gfdl(7) man page.
> +
> +(a) The FSF's Front-Cover Text is:
> +
> + A GNU Manual
> +
> +(b) The FSF's Back-Cover Text is:
> +
> + You have freedom to copy and modify this GNU Manual, like GNU
> + software. Copies published by the Free Software Foundation raise
> + funds for GNU development.
> +@c man end
> +@c Set file name and title for the man page.
> +@setfilename gcov-tool
> +@settitle offline gcda profile processing tool
> +@end ignore
> +
> +@node Gcov-tool
> +@chapter @command{gcov-tool}---an offline gcda profile processing tool
> +
> +@command{gcov-tool} is a tool you can use in conjunction with GCC to
> +manipulate or process gcda profile files offline.
> +
> +@menu
> +* Gcov-tool Intro:: Introduction to gcov-tool.
> +* Invoking Gcov-tool:: How to use gcov-tool.
> +@end menu
> +
> +@node Gcov-tool Intro
> +@section Introduction to @command{gcov-tool}
> +@c man begin DESCRIPTION
> +
> +@command{gcov-tool} is an offline tool to process gcc's gcda profile files.
> +
> +Current gcov-tool supports the following functionalities:
> +
> +@itemize @bullet
> +@item
> +merge two sets of profiles with weights.
> +
> +@item
> +read one set of profile and rewrite profile contents. One can scale or
> +normalize the count values.
> +@end itemize
> +
> +Note that for the merging operation, this profile generated offline may
> +contain a slight different values from the online merged profile. Here are
> +a list of typical differences:
> +
> +@itemize @bullet
> +@item
> +histogram difference: This offline tool recomputes the histogram after merging
> +the counters. The resulting histogram, therefore, is precise. The online
> +merging does not have this capability -- the histogram is merged from two
> +histograms and the result is an approximation.
> +
> +@item
> +summary checksum difference: Summary checksum uses a CRC32 operation. The value
> +depends on the link list order of gcov-info objects. This order is different in
> +gcov-tool from that in the online merge. It's expected to have different
> +summary checksums. It does not really matter as the compiler does not use this
> +checksum anywhere.
> +
> +@item
> +value profile counter values difference: Some counter values for value profile
> +are runtime dependent, like heap addresses. It's normal to see some difference
> +in these kind of counters.
> +@end itemize
> +
> +@c man end
> +
> +@node Invoking Gcov-tool
> +@section Invoking @command{gcov-tool}
> +
> +@smallexample
> +gcov-tool @r{[}@var{global-options}@r{]} SUB_COMMAND
> +@r{[}@var{sub_command-options}@r{]} @var{profile_dir}
> +@end smallexample
> +
> +@command{gcov-tool} accepts the following options:
> +
> +@ignore
> +@c man begin SYNOPSIS
> +gcov-tool [@option{-v}|@option{--version}] [@option{-h}|@option{--help}]
> +
> +gcov-tool merge [merge-options] @var{directory1} @var{directory2}
> + [@option{-v}|@option{--verbose}]
> + [@option{-o}|@option{ --output} @var{directory}]
> + [@option{-w}|@option{--weight} @var{w1,w2}]
> +
> +gcov-tool rewrite [rewrite-options] @var{directory}
> + [@option{-v}|@option{--verbose}]
> + [@option{-o}|@option{--output} @var{directory}]
> + [@option{-s}|@option{--scale} @var{float_or_simple-frac_value}]
> + [@option{-n}|@option{--normalize} @var{long_long_value}]
> +@c man end
> +@c man begin SEEALSO
> +gpl(7), gfdl(7), fsf-funding(7), gcc(1), gcov(1) and the Info entry for
> +@file{gcc}.
> +@c man end
> +@end ignore
> +
> +@c man begin OPTIONS
> +@table @gcctabopt
> +@item -h
> +@itemx --help
> +Display help about using @command{gcov-tool} (on the standard output), and
> +exit without doing any further processing.
> +
> +@item -v
> +@itemx --version
> +Display the @command{gcov-tool} version number (on the standard output),
> +and exit without doing any further processing.
> +
> +@item merge
> +Merge two profile directories.
> +
> +@table @gcctabopt
> +@item -v
> +@itemx --verbose
> +Set the verbose mode.
> +
> +@item -o @var{directory}
> +@itemx --output @var{directory}
> +Set the output profile directory. Default output directory name is
> +@var{merged_profile}.
> +
> +@item -w @var{w1,w2}
> +@itemx --weight @var{w1,w2}
> +Set the merge weights of the @var{directory1} and @var{directory2},
> +respectively. The default weights are 1 for both.
> +@end table
> +
> +@item rewrite
> +Read the specified profile directory and rewrite to a new directory.
> +
> +@table @gcctabopt
> +@item -v
> +@itemx --verbose
> +Set the verbose mode.
> +
> +@item -o @var{directory}
> +@itemx --output @var{directory}
> +Set the output profile directory. Default output name is @var{rewrite_profile}.
> +
> +@item -s @var{float_or_simple-frac_value}
> +@itemx --scale @var{float_or_simple-frac_value}
> +Scale the profile counters. The specified value can be in floating point value,
> +or simple fraction value form, such 1, 2, 2/3, and 5/3.
> +
> +@item -n @var{long_long_value}
> +@itemx --normalize <long_long_value>
> +Normalize the profile. The specified value is the max counter value
> +in the new profile.
> +
> +@end table
> +@end table
> +
> +@c man end
Thanks for writting the docs. Perhaps you could add bit extra information about when one would
like to do the supported operations.
Thanks!
Honza