From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11685 invoked by alias); 25 May 2014 23:43:08 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 11668 invoked by uid 89); 25 May 2014 23:43:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,UNSUBSCRIBE_BODY autolearn=no version=3.3.2 X-HELO: nikam.ms.mff.cuni.cz Received: from nikam.ms.mff.cuni.cz (HELO nikam.ms.mff.cuni.cz) (195.113.20.16) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Sun, 25 May 2014 23:43:04 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 2FE7A54172F; Mon, 26 May 2014 01:43:00 +0200 (CEST) Date: Sun, 25 May 2014 23:43:00 -0000 From: Jan Hubicka To: Rong Xu Cc: Jan Hubicka , GCC Patches , Xinliang David Li , Teresa Johnson , Dehao Chen Subject: Re: [PATCH] offline gcda profile processing tool Message-ID: <20140525234259.GA11448@kam.mff.cuni.cz> References: <20140415213850.GA23141@atrey.karlin.mff.cuni.cz> <20140417033448.GC3157@kam.mff.cuni.cz> <20140515203700.GA29194@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2014-05/txt/msg02127.txt.bz2 > >> /* 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 > > * 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] Merge coverage file contents\n"); > + fnotice (file, " -v, --verbose Verbose mode\n"); > + fnotice (file, " -o, --output Output directory\n"); > + fnotice (file, " -w, --weight 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 > +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