Message ID | CAAe5K+UouGttAzYMwqHtyMSu2sD=KSVhGZ=5oySq=a-7oy2rwg@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Wed, Dec 11, 2013 at 10:05 PM, Teresa Johnson <tejohnson@google.com> wrote: > On Fri, Dec 6, 2013 at 6:23 AM, Jan Hubicka <hubicka@ucw.cz> wrote: >>> Hi, all >>> >>> This is the new patch for gcov-tool (previously profile-tool). >>> >>> Honza: can you comment on the new merge interface? David posted some >>> comments in an earlier email and we want to know what's your opinion. >>> >>> Test patch has been tested with boostrap, regresssion, >>> profiledbootstrap and SPEC2006. >>> >>> Noticeable changes from the earlier version: >>> >>> 1. create a new file libgcov.h and move libgcov-*.h headers to libgcov.h >>> So we can included multiple libgcov-*.c without adding new macros. >>> >>> 2. split libgcov.h specific code in gcvo-io.h to libcc/libgcov.h >>> Avoid multiple-page of code under IN_LIBGCOV macro -- this >>> improves the readability. >>> >>> 3. make gcov_var static, and move the definition from gcov-io.h to >>> gcov-io.c. Also >>> move some static functions accessing gcov_var to gcvo-io.c >>> Current code rely on GCOV_LINKAGE tricks to avoid multi-definition. I don't see >>> a reason that gcov_var needs to exposed as a global. >>> >>> 4. expose gcov_write_strings() and gcov_sync() to gcov_tool usage >>> >>> 5. rename profile-tool to gcov-tool per Honza's suggestion. >>> >>> Thanks, >> >> Hi, >> I did not read in deatil the gcov-tool source itself, but lets first make the interface changes >> needed. >> >>> 2013-11-18 Rong Xu <xur@google.com> >>> >>> * gcc/gcov-io.c (gcov_var): Moved from gcov-io.h and make it static. >>> (gcov_position): Move from gcov-io.h >>> (gcov_is_error): Ditto. >>> (gcov_rewrite): Ditto. >>> * gcc/gcov-io.h: Re-factoring. Move gcov_var to gcov-io.h and >>> move the libgcov only part of libgcc/libgcov.h. >>> * libgcc/libgcov.h: New common header files for libgcov-*.h >>> * libgcc/Makefile.in: Add dependence to libgcov.h >>> * libgcc/libgcov-profiler.c: Use libgcov.h >>> * libgcc/libgcov-driver.c: Ditto. >>> * libgcc/libgcov-interface.c: Ditto. >>> * libgcc/libgcov-driver-system.c (allocate_filename_struct): use >>> xmalloc instread of malloc. >>> * libgcc/libgcov-merge.c (void __gcov_merge_delta): Add more >>> parameters to merge function. >>> (__gcov_merge_add): Ditto. >>> (__gcov_merge_ior): Ditto. >>> (__gcov_merge_time_profile): Ditto. >>> (__gcov_merge_single): Ditto. >>> (__gcov_merge_delta): Ditto. >>> * libgcc/libgcov-tool.c (void gcov_tool_set_verbose): New for >>> gcov-tool support. >>> (set_fn_ctrs): Ditto. >>> (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_merge): Ditto. >>> (find_match_gcov_inf Ditto.o): >>> (gcov_profile_merge): Ditto. >>> (__gcov_scale_add): Ditto. >>> (__gcov_scale_ior): Ditto. >>> (__gcov_scale_delta): Ditto. >>> (__gcov_scale_single): Ditto. >>> (gcov_profile_scale): Ditto. >>> (gcov_profile_normalize): Ditto. >>> (__gcov_scale2_add): Ditto. >>> (__gcov_scale2_ior): Ditto. >>> (__gcov_scale2_delta): Ditto. >>> (__gcov_scale2_single): Ditto. >>> (gcov_profile_scale2): Ditto. >>> * gcc/gcov-tool.c (unlink_file): Gcov-tool driver support. >>> (unlink_dir): Ditto. >>> (profile_merge): Ditto. >>> (print_merge_usage_message): Ditto. >>> (merge_usage): Ditto. >>> (do_merge): Ditto. >>> (profile_rewrite2): Ditto. >>> (profile_rewrite): Ditto. >>> (print_rewrite_usage_message): Ditto. >>> (rewrite_usage): Ditto. >>> (do_rewrite): Ditto. >>> (print_usage): Ditto. >>> (print_version): Ditto. >>> (process_args): Ditto. >>> (main): Ditto. >>> * gcc/Makefile.in: Build and install gcov-tool. >> >>> Index: gcc/gcov-io.c >>> =================================================================== >>> --- gcc/gcov-io.c (revision 204895) >>> +++ gcc/gcov-io.c (working copy) >>> @@ -36,6 +36,37 @@ static const gcov_unsigned_t *gcov_read_words (uns >>> static void gcov_allocate (unsigned); >>> #endif >>> >>> +/* Moved for gcov-io.h and make it static. */ >>> +static struct gcov_var gcov_var; >> >> This is more an changelog message than a comment in source file. >> Just describe what gcov_var is. > > I changed this so gcov_var is no longer static, but global as before. > >> >> Do you know how the size of libgcov changed with your patch? >> Quick check of current mainline on compiling empty main gives: >> >> jh@gcc10:~/trunk/build/gcc$ cat t.c >> main() >> { >> } >> jh@gcc10:~/trunk/build/gcc$ ./xgcc -B ./ -O2 -fprofile-generate -o a.out-new --static t.c >> jh@gcc10:~/trunk/build/gcc$ gcc -O2 -fprofile-generate -o a.out-old --static t.c >> jh@gcc10:~/trunk/build/gcc$ size a.out-old >> text data bss dec hex filename >> 608141 3560 16728 628429 996cd a.out-old >> jh@gcc10:~/trunk/build/gcc$ size a.out-new >> text data bss dec hex filename >> 612621 3688 22880 639189 9c0d5 a.out-new >> >> Without profiling I get: >> jh@gcc10:~/trunk/build/gcc$ size a.out-new-no >> jh@gcc10:~/trunk/build/gcc$ size a.out-old-no >> text data bss dec hex filename >> 599719 3448 12568 615735 96537 a.out-old-no >> text data bss dec hex filename >> 600247 3448 12568 616263 96747 a.out-new-no >> >> Quite big for empty program, but mostly glibc fault, I suppose >> (that won't be an issue for embedded platforms). But anyway >> we increased text size overhead from 8k to 12k, BSS size >> overhead from 4k to 10k and data by another 1k. >> >> text data bss dec hex filename >> 126 0 0 126 7e _gcov_merge_add.o (ex libgcov.a) >> 251 0 0 251 fb _gcov_merge_single.o (ex libgcov.a) >> 242 0 0 242 f2 _gcov_merge_delta.o (ex libgcov.a) >> 126 0 0 126 7e _gcov_merge_ior.o (ex libgcov.a) >> 156 0 0 156 9c _gcov_merge_time_profile.o (ex libgcov.a) >> 89 0 0 89 59 _gcov_interval_profiler.o (ex libgcov.a) >> 69 0 0 69 45 _gcov_pow2_profiler.o (ex libgcov.a) >> 115 0 0 115 73 _gcov_one_value_profiler.o (ex libgcov.a) >> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex libgcov.a) >> 57 0 0 57 39 _gcov_average_profiler.o (ex libgcov.a) >> 52 0 0 52 34 _gcov_ior_profiler.o (ex libgcov.a) >> 178 0 16 194 c2 _gcov_indirect_call_profiler_v2.o (ex libgcov.a) >> 77 0 8 85 55 _gcov_time_profiler.o (ex libgcov.a) >> 126 0 40 166 a6 _gcov_flush.o (ex libgcov.a) >> 101 0 0 101 65 _gcov_fork.o (ex libgcov.a) >> 471 0 0 471 1d7 _gcov_execl.o (ex libgcov.a) >> 471 0 0 471 1d7 _gcov_execlp.o (ex libgcov.a) >> 524 0 0 524 20c _gcov_execle.o (ex libgcov.a) >> 98 0 0 98 62 _gcov_execv.o (ex libgcov.a) >> 98 0 0 98 62 _gcov_execvp.o (ex libgcov.a) >> 108 0 0 108 6c _gcov_execve.o (ex libgcov.a) >> 66 0 0 66 42 _gcov_reset.o (ex libgcov.a) >> 66 0 0 66 42 _gcov_dump.o (ex libgcov.a) >> 9505 0 6144 15649 3d21 _gcov.o (ex libgcov.a) >> >> I think we definitely need to move those 6k of bss space out. I think those are new >> static vars you introduced that I think are unsafe anyway because multiple streaming >> may run at once in threaded program where locking mechanizm fails. >> (it will probably do other bad things, but definitely we do not want to >> conflict on things like filename to write into). >> >> Compared to my system gcov: >> text data bss dec hex filename >> 9765 0 64 9829 2665 _gcov.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 125 0 0 125 7d _gcov_merge_add.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 251 0 0 251 fb _gcov_merge_single.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 242 0 0 242 f2 _gcov_merge_delta.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 101 0 0 101 65 _gcov_fork.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 471 0 0 471 1d7 _gcov_execl.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 471 0 0 471 1d7 _gcov_execlp.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 524 0 0 524 20c _gcov_execle.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 98 0 0 98 62 _gcov_execv.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 98 0 0 98 62 _gcov_execvp.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 108 0 0 108 6c _gcov_execve.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 72 0 0 72 48 _gcov_reset.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 72 0 0 72 48 _gcov_dump.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 89 0 0 89 59 _gcov_interval_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 69 0 0 69 45 _gcov_pow2_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 115 0 0 115 73 _gcov_one_value_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 57 0 0 57 39 _gcov_average_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 52 0 0 52 34 _gcov_ior_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> 125 0 0 125 7d _gcov_merge_ior.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>> Index: gcc/gcov-io.h >>> =================================================================== >>> --- gcc/gcov-io.h (revision 204895) >>> +++ gcc/gcov-io.h (working copy) >>> @@ -164,51 +164,7 @@ see the files COPYING3 and COPYING.RUNTIME respect >>> #ifndef GCC_GCOV_IO_H >>> #define GCC_GCOV_IO_H >>> >>> -#if IN_LIBGCOV >>> -/* About the target */ >>> - >>> -#if BITS_PER_UNIT == 8 >>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (SI))); >>> -typedef unsigned gcov_position_t __attribute__ ((mode (SI))); >>> -#if LONG_LONG_TYPE_SIZE > 32 >>> -typedef signed gcov_type __attribute__ ((mode (DI))); >>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (DI))); >>> -#else >>> -typedef signed gcov_type __attribute__ ((mode (SI))); >>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); >>> -#endif >>> -#else >>> -#if BITS_PER_UNIT == 16 >>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (HI))); >>> -typedef unsigned gcov_position_t __attribute__ ((mode (HI))); >>> -#if LONG_LONG_TYPE_SIZE > 32 >>> -typedef signed gcov_type __attribute__ ((mode (SI))); >>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); >>> -#else >>> -typedef signed gcov_type __attribute__ ((mode (HI))); >>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); >>> -#endif >>> -#else >>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (QI))); >>> -typedef unsigned gcov_position_t __attribute__ ((mode (QI))); >>> -#if LONG_LONG_TYPE_SIZE > 32 >>> -typedef signed gcov_type __attribute__ ((mode (HI))); >>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); >>> -#else >>> -typedef signed gcov_type __attribute__ ((mode (QI))); >>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (QI))); >>> -#endif >>> -#endif >>> -#endif >> >> So this part basically moves libgcov specific bits into libgcov.h? That OK fine by >> itself. >>> Index: libgcc/libgcov-profiler.c >>> =================================================================== >>> --- libgcc/libgcov-profiler.c (revision 204895) >>> +++ libgcc/libgcov-profiler.c (working copy) >>> @@ -23,15 +23,8 @@ a copy of the GCC Runtime Library Exception along >>> see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >>> <http://www.gnu.org/licenses/>. */ >>> >>> -#include "tconfig.h" >>> -#include "tsystem.h" >>> -#include "coretypes.h" >>> -#include "tm.h" >>> -#include "libgcc_tm.h" >>> - >>> +#include "libgcov.h" >>> #if !defined(inhibit_libc) >>> -#define IN_LIBGCOV 1 >>> -#include "gcov-io.h" >> >> I did not pay that much attention into the current include file changes, but wasn't >> idea to avoid #include file to include random other #includes? >> So perhaps the first block of includes should stay, followed by libgcov.h and gcov-io.h >> last? > > I'm not sure I understand the issue here? The patch basically moves > the same includes into libgcov.h, and includes that instead. I see > many other header files in gcc that include other headers. > >>> @@ -325,6 +311,9 @@ static struct gcov_summary all_prg; >>> #endif >>> /* crc32 for this program. */ >>> static gcov_unsigned_t crc32; >>> +/* Use this summary checksum rather the computed one if the value is >>> + * non-zero. */ >>> +static gcov_unsigned_t saved_summary_checksum; >> >> Why do you need to save the checksum? Won't it reset summary back with multiple streaming? > > This has been removed. > >> >> I would really like to avoid introducing those static vars that are used exclusively >> by gcov_exit. What about putting them into an gcov_context structure that >> is passed around the functions that was broken out? >>> Index: libgcc/libgcov-merge.c >>> =================================================================== >>> --- libgcc/libgcov-merge.c (revision 204895) >>> +++ libgcc/libgcov-merge.c (working copy) >>> @@ -23,107 +23,150 @@ a copy of the GCC Runtime Library Exception along >>> see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >>> <http://www.gnu.org/licenses/>. */ >>> >>> -#include "tconfig.h" >>> -#include "tsystem.h" >>> -#include "coretypes.h" >>> -#include "tm.h" >>> -#include "libgcc_tm.h" >>> +#include "libgcov.h" >>> >>> -#if defined(inhibit_libc) >>> -#define IN_LIBGCOV (-1) >>> -#else >>> -#define IN_LIBGCOV 1 >>> +#include "gcov-io-libgcov.h" >>> #endif >>> >>> -#include "gcov-io.h" >>> - >>> #if defined(inhibit_libc) >>> /* If libc and its header files are not available, provide dummy functions. */ >>> >>> #ifdef L_gcov_merge_add >>> void __gcov_merge_add (gcov_type *counters __attribute__ ((unused)), >>> - unsigned n_counters __attribute__ ((unused))) {} >>> + unsigned n_counters __attribute__ ((unused)), >>> + unsigned gcov_type *src __attribute__ ((unused)), >>> + unsigned w __attribute__ ((unused))) {} >>> #endif >>> >>> #ifdef L_gcov_merge_single >>> void __gcov_merge_single (gcov_type *counters __attribute__ ((unused)), >>> - unsigned n_counters __attribute__ ((unused))) {} >>> + unsigned n_counters __attribute__ ((unused)), >>> + unsigned gcov_type *src __attribute__ ((unused)), >>> + unsigned w __attribute__ ((unused))) {} >>> #endif >>> >>> #ifdef L_gcov_merge_delta >>> void __gcov_merge_delta (gcov_type *counters __attribute__ ((unused)), >>> - unsigned n_counters __attribute__ ((unused))) {} >>> + unsigned n_counters __attribute__ ((unused)), >>> + unsigned gcov_type *src __attribute__ ((unused)), >>> + unsigned w __attribute__ ((unused))) {} >>> #endif >>> >>> #else >>> >>> #ifdef L_gcov_merge_add >>> /* The profile merging function that just adds the counters. It is given >>> - an array COUNTERS of N_COUNTERS old counters and it reads the same number >>> - of counters from the gcov file. */ >>> + an array COUNTERS of N_COUNTERS old counters. >>> + When SRC==NULL, it reads the same number of counters from the gcov file. >>> + Otherwise, it reads from SRC array. These read values will be multiplied >>> + by weight W before adding to the old counters. */ >>> void >>> -__gcov_merge_add (gcov_type *counters, unsigned n_counters) >>> +__gcov_merge_add (gcov_type *counters, unsigned n_counters, >>> + gcov_type *src, unsigned w) >>> { >>> + int in_mem = (src != 0); >>> + >>> for (; n_counters; counters++, n_counters--) >>> - *counters += gcov_read_counter (); >>> + { >>> + gcov_type value; >>> + >>> + if (in_mem) >>> + value = *(src++); >>> + else >>> + value = gcov_read_counter (); >>> + >>> + *counters += w * value; >>> + } >>> } >>> #endif /* L_gcov_merge_add */ >>> >>> #ifdef L_gcov_merge_ior >>> /* The profile merging function that just adds the counters. It is given >>> - an array COUNTERS of N_COUNTERS old counters and it reads the same number >>> - of counters from the gcov file. */ >>> + an array COUNTERS of N_COUNTERS old counters. >>> + When SRC==NULL, it reads the same number of counters from the gcov file. >>> + Otherwise, it reads from SRC array. */ >>> void >>> -__gcov_merge_ior (gcov_type *counters, unsigned n_counters) >>> +__gcov_merge_ior (gcov_type *counters, unsigned n_counters, >>> + gcov_type *src, unsigned w __attribute__ ((unused))) >> >> So the new in-memory variants are introduced for merging tool, while libgcc use gcov_read_counter >> interface? >> Perhaps we can actually just duplicate the functions to avoid runtime to do all the scalling >> and in_mem tests it won't need? >> >> I would suggest going with libgcov.h changes and clenaups first, with interface changes next >> and the gcov-tool is probably quite obvious at the end? >> Do you think you can split the patch this way? >> >> Thanks and sorry for taking long to review. I should have more time again now. >> Honza > > The libgcov.h related changes are in the attached patch. I think it > addresses your concerns. Bootstrapped and tested on > x86-64-unknown-linux-gnu. A profiledbootstrap is in progress. > > Ok for trunk if profiledbootstrap passes? Both a profiledbootstrap and LTO profiledbootstrap pass. Teresa > > Thanks, > Teresa > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
gcov_rewrite function is only needed (and defined) with IN_LIBGCOV. Should it be moved from common file gcov-io.c to libgcov.c? David On Thu, Dec 12, 2013 at 12:11 PM, Teresa Johnson <tejohnson@google.com> wrote: > On Wed, Dec 11, 2013 at 10:05 PM, Teresa Johnson <tejohnson@google.com> wrote: >> On Fri, Dec 6, 2013 at 6:23 AM, Jan Hubicka <hubicka@ucw.cz> wrote: >>>> Hi, all >>>> >>>> This is the new patch for gcov-tool (previously profile-tool). >>>> >>>> Honza: can you comment on the new merge interface? David posted some >>>> comments in an earlier email and we want to know what's your opinion. >>>> >>>> Test patch has been tested with boostrap, regresssion, >>>> profiledbootstrap and SPEC2006. >>>> >>>> Noticeable changes from the earlier version: >>>> >>>> 1. create a new file libgcov.h and move libgcov-*.h headers to libgcov.h >>>> So we can included multiple libgcov-*.c without adding new macros. >>>> >>>> 2. split libgcov.h specific code in gcvo-io.h to libcc/libgcov.h >>>> Avoid multiple-page of code under IN_LIBGCOV macro -- this >>>> improves the readability. >>>> >>>> 3. make gcov_var static, and move the definition from gcov-io.h to >>>> gcov-io.c. Also >>>> move some static functions accessing gcov_var to gcvo-io.c >>>> Current code rely on GCOV_LINKAGE tricks to avoid multi-definition. I don't see >>>> a reason that gcov_var needs to exposed as a global. >>>> >>>> 4. expose gcov_write_strings() and gcov_sync() to gcov_tool usage >>>> >>>> 5. rename profile-tool to gcov-tool per Honza's suggestion. >>>> >>>> Thanks, >>> >>> Hi, >>> I did not read in deatil the gcov-tool source itself, but lets first make the interface changes >>> needed. >>> >>>> 2013-11-18 Rong Xu <xur@google.com> >>>> >>>> * gcc/gcov-io.c (gcov_var): Moved from gcov-io.h and make it static. >>>> (gcov_position): Move from gcov-io.h >>>> (gcov_is_error): Ditto. >>>> (gcov_rewrite): Ditto. >>>> * gcc/gcov-io.h: Re-factoring. Move gcov_var to gcov-io.h and >>>> move the libgcov only part of libgcc/libgcov.h. >>>> * libgcc/libgcov.h: New common header files for libgcov-*.h >>>> * libgcc/Makefile.in: Add dependence to libgcov.h >>>> * libgcc/libgcov-profiler.c: Use libgcov.h >>>> * libgcc/libgcov-driver.c: Ditto. >>>> * libgcc/libgcov-interface.c: Ditto. >>>> * libgcc/libgcov-driver-system.c (allocate_filename_struct): use >>>> xmalloc instread of malloc. >>>> * libgcc/libgcov-merge.c (void __gcov_merge_delta): Add more >>>> parameters to merge function. >>>> (__gcov_merge_add): Ditto. >>>> (__gcov_merge_ior): Ditto. >>>> (__gcov_merge_time_profile): Ditto. >>>> (__gcov_merge_single): Ditto. >>>> (__gcov_merge_delta): Ditto. >>>> * libgcc/libgcov-tool.c (void gcov_tool_set_verbose): New for >>>> gcov-tool support. >>>> (set_fn_ctrs): Ditto. >>>> (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_merge): Ditto. >>>> (find_match_gcov_inf Ditto.o): >>>> (gcov_profile_merge): Ditto. >>>> (__gcov_scale_add): Ditto. >>>> (__gcov_scale_ior): Ditto. >>>> (__gcov_scale_delta): Ditto. >>>> (__gcov_scale_single): Ditto. >>>> (gcov_profile_scale): Ditto. >>>> (gcov_profile_normalize): Ditto. >>>> (__gcov_scale2_add): Ditto. >>>> (__gcov_scale2_ior): Ditto. >>>> (__gcov_scale2_delta): Ditto. >>>> (__gcov_scale2_single): Ditto. >>>> (gcov_profile_scale2): Ditto. >>>> * gcc/gcov-tool.c (unlink_file): Gcov-tool driver support. >>>> (unlink_dir): Ditto. >>>> (profile_merge): Ditto. >>>> (print_merge_usage_message): Ditto. >>>> (merge_usage): Ditto. >>>> (do_merge): Ditto. >>>> (profile_rewrite2): Ditto. >>>> (profile_rewrite): Ditto. >>>> (print_rewrite_usage_message): Ditto. >>>> (rewrite_usage): Ditto. >>>> (do_rewrite): Ditto. >>>> (print_usage): Ditto. >>>> (print_version): Ditto. >>>> (process_args): Ditto. >>>> (main): Ditto. >>>> * gcc/Makefile.in: Build and install gcov-tool. >>> >>>> Index: gcc/gcov-io.c >>>> =================================================================== >>>> --- gcc/gcov-io.c (revision 204895) >>>> +++ gcc/gcov-io.c (working copy) >>>> @@ -36,6 +36,37 @@ static const gcov_unsigned_t *gcov_read_words (uns >>>> static void gcov_allocate (unsigned); >>>> #endif >>>> >>>> +/* Moved for gcov-io.h and make it static. */ >>>> +static struct gcov_var gcov_var; >>> >>> This is more an changelog message than a comment in source file. >>> Just describe what gcov_var is. >> >> I changed this so gcov_var is no longer static, but global as before. >> >>> >>> Do you know how the size of libgcov changed with your patch? >>> Quick check of current mainline on compiling empty main gives: >>> >>> jh@gcc10:~/trunk/build/gcc$ cat t.c >>> main() >>> { >>> } >>> jh@gcc10:~/trunk/build/gcc$ ./xgcc -B ./ -O2 -fprofile-generate -o a.out-new --static t.c >>> jh@gcc10:~/trunk/build/gcc$ gcc -O2 -fprofile-generate -o a.out-old --static t.c >>> jh@gcc10:~/trunk/build/gcc$ size a.out-old >>> text data bss dec hex filename >>> 608141 3560 16728 628429 996cd a.out-old >>> jh@gcc10:~/trunk/build/gcc$ size a.out-new >>> text data bss dec hex filename >>> 612621 3688 22880 639189 9c0d5 a.out-new >>> >>> Without profiling I get: >>> jh@gcc10:~/trunk/build/gcc$ size a.out-new-no >>> jh@gcc10:~/trunk/build/gcc$ size a.out-old-no >>> text data bss dec hex filename >>> 599719 3448 12568 615735 96537 a.out-old-no >>> text data bss dec hex filename >>> 600247 3448 12568 616263 96747 a.out-new-no >>> >>> Quite big for empty program, but mostly glibc fault, I suppose >>> (that won't be an issue for embedded platforms). But anyway >>> we increased text size overhead from 8k to 12k, BSS size >>> overhead from 4k to 10k and data by another 1k. >>> >>> text data bss dec hex filename >>> 126 0 0 126 7e _gcov_merge_add.o (ex libgcov.a) >>> 251 0 0 251 fb _gcov_merge_single.o (ex libgcov.a) >>> 242 0 0 242 f2 _gcov_merge_delta.o (ex libgcov.a) >>> 126 0 0 126 7e _gcov_merge_ior.o (ex libgcov.a) >>> 156 0 0 156 9c _gcov_merge_time_profile.o (ex libgcov.a) >>> 89 0 0 89 59 _gcov_interval_profiler.o (ex libgcov.a) >>> 69 0 0 69 45 _gcov_pow2_profiler.o (ex libgcov.a) >>> 115 0 0 115 73 _gcov_one_value_profiler.o (ex libgcov.a) >>> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex libgcov.a) >>> 57 0 0 57 39 _gcov_average_profiler.o (ex libgcov.a) >>> 52 0 0 52 34 _gcov_ior_profiler.o (ex libgcov.a) >>> 178 0 16 194 c2 _gcov_indirect_call_profiler_v2.o (ex libgcov.a) >>> 77 0 8 85 55 _gcov_time_profiler.o (ex libgcov.a) >>> 126 0 40 166 a6 _gcov_flush.o (ex libgcov.a) >>> 101 0 0 101 65 _gcov_fork.o (ex libgcov.a) >>> 471 0 0 471 1d7 _gcov_execl.o (ex libgcov.a) >>> 471 0 0 471 1d7 _gcov_execlp.o (ex libgcov.a) >>> 524 0 0 524 20c _gcov_execle.o (ex libgcov.a) >>> 98 0 0 98 62 _gcov_execv.o (ex libgcov.a) >>> 98 0 0 98 62 _gcov_execvp.o (ex libgcov.a) >>> 108 0 0 108 6c _gcov_execve.o (ex libgcov.a) >>> 66 0 0 66 42 _gcov_reset.o (ex libgcov.a) >>> 66 0 0 66 42 _gcov_dump.o (ex libgcov.a) >>> 9505 0 6144 15649 3d21 _gcov.o (ex libgcov.a) >>> >>> I think we definitely need to move those 6k of bss space out. I think those are new >>> static vars you introduced that I think are unsafe anyway because multiple streaming >>> may run at once in threaded program where locking mechanizm fails. >>> (it will probably do other bad things, but definitely we do not want to >>> conflict on things like filename to write into). >>> >>> Compared to my system gcov: >>> text data bss dec hex filename >>> 9765 0 64 9829 2665 _gcov.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 125 0 0 125 7d _gcov_merge_add.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 251 0 0 251 fb _gcov_merge_single.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 242 0 0 242 f2 _gcov_merge_delta.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 101 0 0 101 65 _gcov_fork.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 471 0 0 471 1d7 _gcov_execl.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 471 0 0 471 1d7 _gcov_execlp.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 524 0 0 524 20c _gcov_execle.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 98 0 0 98 62 _gcov_execv.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 98 0 0 98 62 _gcov_execvp.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 108 0 0 108 6c _gcov_execve.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 72 0 0 72 48 _gcov_reset.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 72 0 0 72 48 _gcov_dump.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 89 0 0 89 59 _gcov_interval_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 69 0 0 69 45 _gcov_pow2_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 115 0 0 115 73 _gcov_one_value_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 57 0 0 57 39 _gcov_average_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 52 0 0 52 34 _gcov_ior_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> 125 0 0 125 7d _gcov_merge_ior.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>> Index: gcc/gcov-io.h >>>> =================================================================== >>>> --- gcc/gcov-io.h (revision 204895) >>>> +++ gcc/gcov-io.h (working copy) >>>> @@ -164,51 +164,7 @@ see the files COPYING3 and COPYING.RUNTIME respect >>>> #ifndef GCC_GCOV_IO_H >>>> #define GCC_GCOV_IO_H >>>> >>>> -#if IN_LIBGCOV >>>> -/* About the target */ >>>> - >>>> -#if BITS_PER_UNIT == 8 >>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (SI))); >>>> -typedef unsigned gcov_position_t __attribute__ ((mode (SI))); >>>> -#if LONG_LONG_TYPE_SIZE > 32 >>>> -typedef signed gcov_type __attribute__ ((mode (DI))); >>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (DI))); >>>> -#else >>>> -typedef signed gcov_type __attribute__ ((mode (SI))); >>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); >>>> -#endif >>>> -#else >>>> -#if BITS_PER_UNIT == 16 >>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (HI))); >>>> -typedef unsigned gcov_position_t __attribute__ ((mode (HI))); >>>> -#if LONG_LONG_TYPE_SIZE > 32 >>>> -typedef signed gcov_type __attribute__ ((mode (SI))); >>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); >>>> -#else >>>> -typedef signed gcov_type __attribute__ ((mode (HI))); >>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); >>>> -#endif >>>> -#else >>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (QI))); >>>> -typedef unsigned gcov_position_t __attribute__ ((mode (QI))); >>>> -#if LONG_LONG_TYPE_SIZE > 32 >>>> -typedef signed gcov_type __attribute__ ((mode (HI))); >>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); >>>> -#else >>>> -typedef signed gcov_type __attribute__ ((mode (QI))); >>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (QI))); >>>> -#endif >>>> -#endif >>>> -#endif >>> >>> So this part basically moves libgcov specific bits into libgcov.h? That OK fine by >>> itself. >>>> Index: libgcc/libgcov-profiler.c >>>> =================================================================== >>>> --- libgcc/libgcov-profiler.c (revision 204895) >>>> +++ libgcc/libgcov-profiler.c (working copy) >>>> @@ -23,15 +23,8 @@ a copy of the GCC Runtime Library Exception along >>>> see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >>>> <http://www.gnu.org/licenses/>. */ >>>> >>>> -#include "tconfig.h" >>>> -#include "tsystem.h" >>>> -#include "coretypes.h" >>>> -#include "tm.h" >>>> -#include "libgcc_tm.h" >>>> - >>>> +#include "libgcov.h" >>>> #if !defined(inhibit_libc) >>>> -#define IN_LIBGCOV 1 >>>> -#include "gcov-io.h" >>> >>> I did not pay that much attention into the current include file changes, but wasn't >>> idea to avoid #include file to include random other #includes? >>> So perhaps the first block of includes should stay, followed by libgcov.h and gcov-io.h >>> last? >> >> I'm not sure I understand the issue here? The patch basically moves >> the same includes into libgcov.h, and includes that instead. I see >> many other header files in gcc that include other headers. >> >>>> @@ -325,6 +311,9 @@ static struct gcov_summary all_prg; >>>> #endif >>>> /* crc32 for this program. */ >>>> static gcov_unsigned_t crc32; >>>> +/* Use this summary checksum rather the computed one if the value is >>>> + * non-zero. */ >>>> +static gcov_unsigned_t saved_summary_checksum; >>> >>> Why do you need to save the checksum? Won't it reset summary back with multiple streaming? >> >> This has been removed. >> >>> >>> I would really like to avoid introducing those static vars that are used exclusively >>> by gcov_exit. What about putting them into an gcov_context structure that >>> is passed around the functions that was broken out? >>>> Index: libgcc/libgcov-merge.c >>>> =================================================================== >>>> --- libgcc/libgcov-merge.c (revision 204895) >>>> +++ libgcc/libgcov-merge.c (working copy) >>>> @@ -23,107 +23,150 @@ a copy of the GCC Runtime Library Exception along >>>> see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >>>> <http://www.gnu.org/licenses/>. */ >>>> >>>> -#include "tconfig.h" >>>> -#include "tsystem.h" >>>> -#include "coretypes.h" >>>> -#include "tm.h" >>>> -#include "libgcc_tm.h" >>>> +#include "libgcov.h" >>>> >>>> -#if defined(inhibit_libc) >>>> -#define IN_LIBGCOV (-1) >>>> -#else >>>> -#define IN_LIBGCOV 1 >>>> +#include "gcov-io-libgcov.h" >>>> #endif >>>> >>>> -#include "gcov-io.h" >>>> - >>>> #if defined(inhibit_libc) >>>> /* If libc and its header files are not available, provide dummy functions. */ >>>> >>>> #ifdef L_gcov_merge_add >>>> void __gcov_merge_add (gcov_type *counters __attribute__ ((unused)), >>>> - unsigned n_counters __attribute__ ((unused))) {} >>>> + unsigned n_counters __attribute__ ((unused)), >>>> + unsigned gcov_type *src __attribute__ ((unused)), >>>> + unsigned w __attribute__ ((unused))) {} >>>> #endif >>>> >>>> #ifdef L_gcov_merge_single >>>> void __gcov_merge_single (gcov_type *counters __attribute__ ((unused)), >>>> - unsigned n_counters __attribute__ ((unused))) {} >>>> + unsigned n_counters __attribute__ ((unused)), >>>> + unsigned gcov_type *src __attribute__ ((unused)), >>>> + unsigned w __attribute__ ((unused))) {} >>>> #endif >>>> >>>> #ifdef L_gcov_merge_delta >>>> void __gcov_merge_delta (gcov_type *counters __attribute__ ((unused)), >>>> - unsigned n_counters __attribute__ ((unused))) {} >>>> + unsigned n_counters __attribute__ ((unused)), >>>> + unsigned gcov_type *src __attribute__ ((unused)), >>>> + unsigned w __attribute__ ((unused))) {} >>>> #endif >>>> >>>> #else >>>> >>>> #ifdef L_gcov_merge_add >>>> /* The profile merging function that just adds the counters. It is given >>>> - an array COUNTERS of N_COUNTERS old counters and it reads the same number >>>> - of counters from the gcov file. */ >>>> + an array COUNTERS of N_COUNTERS old counters. >>>> + When SRC==NULL, it reads the same number of counters from the gcov file. >>>> + Otherwise, it reads from SRC array. These read values will be multiplied >>>> + by weight W before adding to the old counters. */ >>>> void >>>> -__gcov_merge_add (gcov_type *counters, unsigned n_counters) >>>> +__gcov_merge_add (gcov_type *counters, unsigned n_counters, >>>> + gcov_type *src, unsigned w) >>>> { >>>> + int in_mem = (src != 0); >>>> + >>>> for (; n_counters; counters++, n_counters--) >>>> - *counters += gcov_read_counter (); >>>> + { >>>> + gcov_type value; >>>> + >>>> + if (in_mem) >>>> + value = *(src++); >>>> + else >>>> + value = gcov_read_counter (); >>>> + >>>> + *counters += w * value; >>>> + } >>>> } >>>> #endif /* L_gcov_merge_add */ >>>> >>>> #ifdef L_gcov_merge_ior >>>> /* The profile merging function that just adds the counters. It is given >>>> - an array COUNTERS of N_COUNTERS old counters and it reads the same number >>>> - of counters from the gcov file. */ >>>> + an array COUNTERS of N_COUNTERS old counters. >>>> + When SRC==NULL, it reads the same number of counters from the gcov file. >>>> + Otherwise, it reads from SRC array. */ >>>> void >>>> -__gcov_merge_ior (gcov_type *counters, unsigned n_counters) >>>> +__gcov_merge_ior (gcov_type *counters, unsigned n_counters, >>>> + gcov_type *src, unsigned w __attribute__ ((unused))) >>> >>> So the new in-memory variants are introduced for merging tool, while libgcc use gcov_read_counter >>> interface? >>> Perhaps we can actually just duplicate the functions to avoid runtime to do all the scalling >>> and in_mem tests it won't need? >>> >>> I would suggest going with libgcov.h changes and clenaups first, with interface changes next >>> and the gcov-tool is probably quite obvious at the end? >>> Do you think you can split the patch this way? >>> >>> Thanks and sorry for taking long to review. I should have more time again now. >>> Honza >> >> The libgcov.h related changes are in the attached patch. I think it >> addresses your concerns. Bootstrapped and tested on >> x86-64-unknown-linux-gnu. A profiledbootstrap is in progress. >> >> Ok for trunk if profiledbootstrap passes? > > Both a profiledbootstrap and LTO profiledbootstrap pass. > > Teresa > >> >> Thanks, >> Teresa >> >> >> -- >> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 > > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
On Mon, Dec 16, 2013 at 12:55 PM, Xinliang David Li <davidxl@google.com> wrote: > gcov_rewrite function is only needed (and defined) with IN_LIBGCOV. > Should it be moved from common file gcov-io.c to libgcov.c? Possibly. I just looked through gcov-io.c and there are several additional functions that are only defined under "#ifdef IN_LIBGCOV" and only used in libgcov*c (or each other): gcov_write_counter gcov_write_tag_length gcov_write_summary gcov_seek Should they all, plus gcov_rewrite, be moved to libgcov-driver.c? Teresa > > > David > > On Thu, Dec 12, 2013 at 12:11 PM, Teresa Johnson <tejohnson@google.com> wrote: >> On Wed, Dec 11, 2013 at 10:05 PM, Teresa Johnson <tejohnson@google.com> wrote: >>> On Fri, Dec 6, 2013 at 6:23 AM, Jan Hubicka <hubicka@ucw.cz> wrote: >>>>> Hi, all >>>>> >>>>> This is the new patch for gcov-tool (previously profile-tool). >>>>> >>>>> Honza: can you comment on the new merge interface? David posted some >>>>> comments in an earlier email and we want to know what's your opinion. >>>>> >>>>> Test patch has been tested with boostrap, regresssion, >>>>> profiledbootstrap and SPEC2006. >>>>> >>>>> Noticeable changes from the earlier version: >>>>> >>>>> 1. create a new file libgcov.h and move libgcov-*.h headers to libgcov.h >>>>> So we can included multiple libgcov-*.c without adding new macros. >>>>> >>>>> 2. split libgcov.h specific code in gcvo-io.h to libcc/libgcov.h >>>>> Avoid multiple-page of code under IN_LIBGCOV macro -- this >>>>> improves the readability. >>>>> >>>>> 3. make gcov_var static, and move the definition from gcov-io.h to >>>>> gcov-io.c. Also >>>>> move some static functions accessing gcov_var to gcvo-io.c >>>>> Current code rely on GCOV_LINKAGE tricks to avoid multi-definition. I don't see >>>>> a reason that gcov_var needs to exposed as a global. >>>>> >>>>> 4. expose gcov_write_strings() and gcov_sync() to gcov_tool usage >>>>> >>>>> 5. rename profile-tool to gcov-tool per Honza's suggestion. >>>>> >>>>> Thanks, >>>> >>>> Hi, >>>> I did not read in deatil the gcov-tool source itself, but lets first make the interface changes >>>> needed. >>>> >>>>> 2013-11-18 Rong Xu <xur@google.com> >>>>> >>>>> * gcc/gcov-io.c (gcov_var): Moved from gcov-io.h and make it static. >>>>> (gcov_position): Move from gcov-io.h >>>>> (gcov_is_error): Ditto. >>>>> (gcov_rewrite): Ditto. >>>>> * gcc/gcov-io.h: Re-factoring. Move gcov_var to gcov-io.h and >>>>> move the libgcov only part of libgcc/libgcov.h. >>>>> * libgcc/libgcov.h: New common header files for libgcov-*.h >>>>> * libgcc/Makefile.in: Add dependence to libgcov.h >>>>> * libgcc/libgcov-profiler.c: Use libgcov.h >>>>> * libgcc/libgcov-driver.c: Ditto. >>>>> * libgcc/libgcov-interface.c: Ditto. >>>>> * libgcc/libgcov-driver-system.c (allocate_filename_struct): use >>>>> xmalloc instread of malloc. >>>>> * libgcc/libgcov-merge.c (void __gcov_merge_delta): Add more >>>>> parameters to merge function. >>>>> (__gcov_merge_add): Ditto. >>>>> (__gcov_merge_ior): Ditto. >>>>> (__gcov_merge_time_profile): Ditto. >>>>> (__gcov_merge_single): Ditto. >>>>> (__gcov_merge_delta): Ditto. >>>>> * libgcc/libgcov-tool.c (void gcov_tool_set_verbose): New for >>>>> gcov-tool support. >>>>> (set_fn_ctrs): Ditto. >>>>> (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_merge): Ditto. >>>>> (find_match_gcov_inf Ditto.o): >>>>> (gcov_profile_merge): Ditto. >>>>> (__gcov_scale_add): Ditto. >>>>> (__gcov_scale_ior): Ditto. >>>>> (__gcov_scale_delta): Ditto. >>>>> (__gcov_scale_single): Ditto. >>>>> (gcov_profile_scale): Ditto. >>>>> (gcov_profile_normalize): Ditto. >>>>> (__gcov_scale2_add): Ditto. >>>>> (__gcov_scale2_ior): Ditto. >>>>> (__gcov_scale2_delta): Ditto. >>>>> (__gcov_scale2_single): Ditto. >>>>> (gcov_profile_scale2): Ditto. >>>>> * gcc/gcov-tool.c (unlink_file): Gcov-tool driver support. >>>>> (unlink_dir): Ditto. >>>>> (profile_merge): Ditto. >>>>> (print_merge_usage_message): Ditto. >>>>> (merge_usage): Ditto. >>>>> (do_merge): Ditto. >>>>> (profile_rewrite2): Ditto. >>>>> (profile_rewrite): Ditto. >>>>> (print_rewrite_usage_message): Ditto. >>>>> (rewrite_usage): Ditto. >>>>> (do_rewrite): Ditto. >>>>> (print_usage): Ditto. >>>>> (print_version): Ditto. >>>>> (process_args): Ditto. >>>>> (main): Ditto. >>>>> * gcc/Makefile.in: Build and install gcov-tool. >>>> >>>>> Index: gcc/gcov-io.c >>>>> =================================================================== >>>>> --- gcc/gcov-io.c (revision 204895) >>>>> +++ gcc/gcov-io.c (working copy) >>>>> @@ -36,6 +36,37 @@ static const gcov_unsigned_t *gcov_read_words (uns >>>>> static void gcov_allocate (unsigned); >>>>> #endif >>>>> >>>>> +/* Moved for gcov-io.h and make it static. */ >>>>> +static struct gcov_var gcov_var; >>>> >>>> This is more an changelog message than a comment in source file. >>>> Just describe what gcov_var is. >>> >>> I changed this so gcov_var is no longer static, but global as before. >>> >>>> >>>> Do you know how the size of libgcov changed with your patch? >>>> Quick check of current mainline on compiling empty main gives: >>>> >>>> jh@gcc10:~/trunk/build/gcc$ cat t.c >>>> main() >>>> { >>>> } >>>> jh@gcc10:~/trunk/build/gcc$ ./xgcc -B ./ -O2 -fprofile-generate -o a.out-new --static t.c >>>> jh@gcc10:~/trunk/build/gcc$ gcc -O2 -fprofile-generate -o a.out-old --static t.c >>>> jh@gcc10:~/trunk/build/gcc$ size a.out-old >>>> text data bss dec hex filename >>>> 608141 3560 16728 628429 996cd a.out-old >>>> jh@gcc10:~/trunk/build/gcc$ size a.out-new >>>> text data bss dec hex filename >>>> 612621 3688 22880 639189 9c0d5 a.out-new >>>> >>>> Without profiling I get: >>>> jh@gcc10:~/trunk/build/gcc$ size a.out-new-no >>>> jh@gcc10:~/trunk/build/gcc$ size a.out-old-no >>>> text data bss dec hex filename >>>> 599719 3448 12568 615735 96537 a.out-old-no >>>> text data bss dec hex filename >>>> 600247 3448 12568 616263 96747 a.out-new-no >>>> >>>> Quite big for empty program, but mostly glibc fault, I suppose >>>> (that won't be an issue for embedded platforms). But anyway >>>> we increased text size overhead from 8k to 12k, BSS size >>>> overhead from 4k to 10k and data by another 1k. >>>> >>>> text data bss dec hex filename >>>> 126 0 0 126 7e _gcov_merge_add.o (ex libgcov.a) >>>> 251 0 0 251 fb _gcov_merge_single.o (ex libgcov.a) >>>> 242 0 0 242 f2 _gcov_merge_delta.o (ex libgcov.a) >>>> 126 0 0 126 7e _gcov_merge_ior.o (ex libgcov.a) >>>> 156 0 0 156 9c _gcov_merge_time_profile.o (ex libgcov.a) >>>> 89 0 0 89 59 _gcov_interval_profiler.o (ex libgcov.a) >>>> 69 0 0 69 45 _gcov_pow2_profiler.o (ex libgcov.a) >>>> 115 0 0 115 73 _gcov_one_value_profiler.o (ex libgcov.a) >>>> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex libgcov.a) >>>> 57 0 0 57 39 _gcov_average_profiler.o (ex libgcov.a) >>>> 52 0 0 52 34 _gcov_ior_profiler.o (ex libgcov.a) >>>> 178 0 16 194 c2 _gcov_indirect_call_profiler_v2.o (ex libgcov.a) >>>> 77 0 8 85 55 _gcov_time_profiler.o (ex libgcov.a) >>>> 126 0 40 166 a6 _gcov_flush.o (ex libgcov.a) >>>> 101 0 0 101 65 _gcov_fork.o (ex libgcov.a) >>>> 471 0 0 471 1d7 _gcov_execl.o (ex libgcov.a) >>>> 471 0 0 471 1d7 _gcov_execlp.o (ex libgcov.a) >>>> 524 0 0 524 20c _gcov_execle.o (ex libgcov.a) >>>> 98 0 0 98 62 _gcov_execv.o (ex libgcov.a) >>>> 98 0 0 98 62 _gcov_execvp.o (ex libgcov.a) >>>> 108 0 0 108 6c _gcov_execve.o (ex libgcov.a) >>>> 66 0 0 66 42 _gcov_reset.o (ex libgcov.a) >>>> 66 0 0 66 42 _gcov_dump.o (ex libgcov.a) >>>> 9505 0 6144 15649 3d21 _gcov.o (ex libgcov.a) >>>> >>>> I think we definitely need to move those 6k of bss space out. I think those are new >>>> static vars you introduced that I think are unsafe anyway because multiple streaming >>>> may run at once in threaded program where locking mechanizm fails. >>>> (it will probably do other bad things, but definitely we do not want to >>>> conflict on things like filename to write into). >>>> >>>> Compared to my system gcov: >>>> text data bss dec hex filename >>>> 9765 0 64 9829 2665 _gcov.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 125 0 0 125 7d _gcov_merge_add.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 251 0 0 251 fb _gcov_merge_single.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 242 0 0 242 f2 _gcov_merge_delta.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 101 0 0 101 65 _gcov_fork.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 471 0 0 471 1d7 _gcov_execl.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 471 0 0 471 1d7 _gcov_execlp.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 524 0 0 524 20c _gcov_execle.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 98 0 0 98 62 _gcov_execv.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 98 0 0 98 62 _gcov_execvp.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 108 0 0 108 6c _gcov_execve.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 72 0 0 72 48 _gcov_reset.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 72 0 0 72 48 _gcov_dump.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 89 0 0 89 59 _gcov_interval_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 69 0 0 69 45 _gcov_pow2_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 115 0 0 115 73 _gcov_one_value_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 57 0 0 57 39 _gcov_average_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 52 0 0 52 34 _gcov_ior_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> 125 0 0 125 7d _gcov_merge_ior.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> Index: gcc/gcov-io.h >>>>> =================================================================== >>>>> --- gcc/gcov-io.h (revision 204895) >>>>> +++ gcc/gcov-io.h (working copy) >>>>> @@ -164,51 +164,7 @@ see the files COPYING3 and COPYING.RUNTIME respect >>>>> #ifndef GCC_GCOV_IO_H >>>>> #define GCC_GCOV_IO_H >>>>> >>>>> -#if IN_LIBGCOV >>>>> -/* About the target */ >>>>> - >>>>> -#if BITS_PER_UNIT == 8 >>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (SI))); >>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (SI))); >>>>> -#if LONG_LONG_TYPE_SIZE > 32 >>>>> -typedef signed gcov_type __attribute__ ((mode (DI))); >>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (DI))); >>>>> -#else >>>>> -typedef signed gcov_type __attribute__ ((mode (SI))); >>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); >>>>> -#endif >>>>> -#else >>>>> -#if BITS_PER_UNIT == 16 >>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (HI))); >>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (HI))); >>>>> -#if LONG_LONG_TYPE_SIZE > 32 >>>>> -typedef signed gcov_type __attribute__ ((mode (SI))); >>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); >>>>> -#else >>>>> -typedef signed gcov_type __attribute__ ((mode (HI))); >>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); >>>>> -#endif >>>>> -#else >>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (QI))); >>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (QI))); >>>>> -#if LONG_LONG_TYPE_SIZE > 32 >>>>> -typedef signed gcov_type __attribute__ ((mode (HI))); >>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); >>>>> -#else >>>>> -typedef signed gcov_type __attribute__ ((mode (QI))); >>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (QI))); >>>>> -#endif >>>>> -#endif >>>>> -#endif >>>> >>>> So this part basically moves libgcov specific bits into libgcov.h? That OK fine by >>>> itself. >>>>> Index: libgcc/libgcov-profiler.c >>>>> =================================================================== >>>>> --- libgcc/libgcov-profiler.c (revision 204895) >>>>> +++ libgcc/libgcov-profiler.c (working copy) >>>>> @@ -23,15 +23,8 @@ a copy of the GCC Runtime Library Exception along >>>>> see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >>>>> <http://www.gnu.org/licenses/>. */ >>>>> >>>>> -#include "tconfig.h" >>>>> -#include "tsystem.h" >>>>> -#include "coretypes.h" >>>>> -#include "tm.h" >>>>> -#include "libgcc_tm.h" >>>>> - >>>>> +#include "libgcov.h" >>>>> #if !defined(inhibit_libc) >>>>> -#define IN_LIBGCOV 1 >>>>> -#include "gcov-io.h" >>>> >>>> I did not pay that much attention into the current include file changes, but wasn't >>>> idea to avoid #include file to include random other #includes? >>>> So perhaps the first block of includes should stay, followed by libgcov.h and gcov-io.h >>>> last? >>> >>> I'm not sure I understand the issue here? The patch basically moves >>> the same includes into libgcov.h, and includes that instead. I see >>> many other header files in gcc that include other headers. >>> >>>>> @@ -325,6 +311,9 @@ static struct gcov_summary all_prg; >>>>> #endif >>>>> /* crc32 for this program. */ >>>>> static gcov_unsigned_t crc32; >>>>> +/* Use this summary checksum rather the computed one if the value is >>>>> + * non-zero. */ >>>>> +static gcov_unsigned_t saved_summary_checksum; >>>> >>>> Why do you need to save the checksum? Won't it reset summary back with multiple streaming? >>> >>> This has been removed. >>> >>>> >>>> I would really like to avoid introducing those static vars that are used exclusively >>>> by gcov_exit. What about putting them into an gcov_context structure that >>>> is passed around the functions that was broken out? >>>>> Index: libgcc/libgcov-merge.c >>>>> =================================================================== >>>>> --- libgcc/libgcov-merge.c (revision 204895) >>>>> +++ libgcc/libgcov-merge.c (working copy) >>>>> @@ -23,107 +23,150 @@ a copy of the GCC Runtime Library Exception along >>>>> see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >>>>> <http://www.gnu.org/licenses/>. */ >>>>> >>>>> -#include "tconfig.h" >>>>> -#include "tsystem.h" >>>>> -#include "coretypes.h" >>>>> -#include "tm.h" >>>>> -#include "libgcc_tm.h" >>>>> +#include "libgcov.h" >>>>> >>>>> -#if defined(inhibit_libc) >>>>> -#define IN_LIBGCOV (-1) >>>>> -#else >>>>> -#define IN_LIBGCOV 1 >>>>> +#include "gcov-io-libgcov.h" >>>>> #endif >>>>> >>>>> -#include "gcov-io.h" >>>>> - >>>>> #if defined(inhibit_libc) >>>>> /* If libc and its header files are not available, provide dummy functions. */ >>>>> >>>>> #ifdef L_gcov_merge_add >>>>> void __gcov_merge_add (gcov_type *counters __attribute__ ((unused)), >>>>> - unsigned n_counters __attribute__ ((unused))) {} >>>>> + unsigned n_counters __attribute__ ((unused)), >>>>> + unsigned gcov_type *src __attribute__ ((unused)), >>>>> + unsigned w __attribute__ ((unused))) {} >>>>> #endif >>>>> >>>>> #ifdef L_gcov_merge_single >>>>> void __gcov_merge_single (gcov_type *counters __attribute__ ((unused)), >>>>> - unsigned n_counters __attribute__ ((unused))) {} >>>>> + unsigned n_counters __attribute__ ((unused)), >>>>> + unsigned gcov_type *src __attribute__ ((unused)), >>>>> + unsigned w __attribute__ ((unused))) {} >>>>> #endif >>>>> >>>>> #ifdef L_gcov_merge_delta >>>>> void __gcov_merge_delta (gcov_type *counters __attribute__ ((unused)), >>>>> - unsigned n_counters __attribute__ ((unused))) {} >>>>> + unsigned n_counters __attribute__ ((unused)), >>>>> + unsigned gcov_type *src __attribute__ ((unused)), >>>>> + unsigned w __attribute__ ((unused))) {} >>>>> #endif >>>>> >>>>> #else >>>>> >>>>> #ifdef L_gcov_merge_add >>>>> /* The profile merging function that just adds the counters. It is given >>>>> - an array COUNTERS of N_COUNTERS old counters and it reads the same number >>>>> - of counters from the gcov file. */ >>>>> + an array COUNTERS of N_COUNTERS old counters. >>>>> + When SRC==NULL, it reads the same number of counters from the gcov file. >>>>> + Otherwise, it reads from SRC array. These read values will be multiplied >>>>> + by weight W before adding to the old counters. */ >>>>> void >>>>> -__gcov_merge_add (gcov_type *counters, unsigned n_counters) >>>>> +__gcov_merge_add (gcov_type *counters, unsigned n_counters, >>>>> + gcov_type *src, unsigned w) >>>>> { >>>>> + int in_mem = (src != 0); >>>>> + >>>>> for (; n_counters; counters++, n_counters--) >>>>> - *counters += gcov_read_counter (); >>>>> + { >>>>> + gcov_type value; >>>>> + >>>>> + if (in_mem) >>>>> + value = *(src++); >>>>> + else >>>>> + value = gcov_read_counter (); >>>>> + >>>>> + *counters += w * value; >>>>> + } >>>>> } >>>>> #endif /* L_gcov_merge_add */ >>>>> >>>>> #ifdef L_gcov_merge_ior >>>>> /* The profile merging function that just adds the counters. It is given >>>>> - an array COUNTERS of N_COUNTERS old counters and it reads the same number >>>>> - of counters from the gcov file. */ >>>>> + an array COUNTERS of N_COUNTERS old counters. >>>>> + When SRC==NULL, it reads the same number of counters from the gcov file. >>>>> + Otherwise, it reads from SRC array. */ >>>>> void >>>>> -__gcov_merge_ior (gcov_type *counters, unsigned n_counters) >>>>> +__gcov_merge_ior (gcov_type *counters, unsigned n_counters, >>>>> + gcov_type *src, unsigned w __attribute__ ((unused))) >>>> >>>> So the new in-memory variants are introduced for merging tool, while libgcc use gcov_read_counter >>>> interface? >>>> Perhaps we can actually just duplicate the functions to avoid runtime to do all the scalling >>>> and in_mem tests it won't need? >>>> >>>> I would suggest going with libgcov.h changes and clenaups first, with interface changes next >>>> and the gcov-tool is probably quite obvious at the end? >>>> Do you think you can split the patch this way? >>>> >>>> Thanks and sorry for taking long to review. I should have more time again now. >>>> Honza >>> >>> The libgcov.h related changes are in the attached patch. I think it >>> addresses your concerns. Bootstrapped and tested on >>> x86-64-unknown-linux-gnu. A profiledbootstrap is in progress. >>> >>> Ok for trunk if profiledbootstrap passes? >> >> Both a profiledbootstrap and LTO profiledbootstrap pass. >> >> Teresa >> >>> >>> Thanks, >>> Teresa >>> >>> >>> -- >>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 >> >> >> >> -- >> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
I think so -- they are private to libgcov. Honza, what do you think? thanks, David On Mon, Dec 16, 2013 at 1:17 PM, Teresa Johnson <tejohnson@google.com> wrote: > On Mon, Dec 16, 2013 at 12:55 PM, Xinliang David Li <davidxl@google.com> wrote: >> gcov_rewrite function is only needed (and defined) with IN_LIBGCOV. >> Should it be moved from common file gcov-io.c to libgcov.c? > > Possibly. I just looked through gcov-io.c and there are several > additional functions that are only defined under "#ifdef IN_LIBGCOV" > and only used in libgcov*c (or each other): > > gcov_write_counter > gcov_write_tag_length > gcov_write_summary > gcov_seek > > Should they all, plus gcov_rewrite, be moved to libgcov-driver.c? > > Teresa > >> >> >> David >> >> On Thu, Dec 12, 2013 at 12:11 PM, Teresa Johnson <tejohnson@google.com> wrote: >>> On Wed, Dec 11, 2013 at 10:05 PM, Teresa Johnson <tejohnson@google.com> wrote: >>>> On Fri, Dec 6, 2013 at 6:23 AM, Jan Hubicka <hubicka@ucw.cz> wrote: >>>>>> Hi, all >>>>>> >>>>>> This is the new patch for gcov-tool (previously profile-tool). >>>>>> >>>>>> Honza: can you comment on the new merge interface? David posted some >>>>>> comments in an earlier email and we want to know what's your opinion. >>>>>> >>>>>> Test patch has been tested with boostrap, regresssion, >>>>>> profiledbootstrap and SPEC2006. >>>>>> >>>>>> Noticeable changes from the earlier version: >>>>>> >>>>>> 1. create a new file libgcov.h and move libgcov-*.h headers to libgcov.h >>>>>> So we can included multiple libgcov-*.c without adding new macros. >>>>>> >>>>>> 2. split libgcov.h specific code in gcvo-io.h to libcc/libgcov.h >>>>>> Avoid multiple-page of code under IN_LIBGCOV macro -- this >>>>>> improves the readability. >>>>>> >>>>>> 3. make gcov_var static, and move the definition from gcov-io.h to >>>>>> gcov-io.c. Also >>>>>> move some static functions accessing gcov_var to gcvo-io.c >>>>>> Current code rely on GCOV_LINKAGE tricks to avoid multi-definition. I don't see >>>>>> a reason that gcov_var needs to exposed as a global. >>>>>> >>>>>> 4. expose gcov_write_strings() and gcov_sync() to gcov_tool usage >>>>>> >>>>>> 5. rename profile-tool to gcov-tool per Honza's suggestion. >>>>>> >>>>>> Thanks, >>>>> >>>>> Hi, >>>>> I did not read in deatil the gcov-tool source itself, but lets first make the interface changes >>>>> needed. >>>>> >>>>>> 2013-11-18 Rong Xu <xur@google.com> >>>>>> >>>>>> * gcc/gcov-io.c (gcov_var): Moved from gcov-io.h and make it static. >>>>>> (gcov_position): Move from gcov-io.h >>>>>> (gcov_is_error): Ditto. >>>>>> (gcov_rewrite): Ditto. >>>>>> * gcc/gcov-io.h: Re-factoring. Move gcov_var to gcov-io.h and >>>>>> move the libgcov only part of libgcc/libgcov.h. >>>>>> * libgcc/libgcov.h: New common header files for libgcov-*.h >>>>>> * libgcc/Makefile.in: Add dependence to libgcov.h >>>>>> * libgcc/libgcov-profiler.c: Use libgcov.h >>>>>> * libgcc/libgcov-driver.c: Ditto. >>>>>> * libgcc/libgcov-interface.c: Ditto. >>>>>> * libgcc/libgcov-driver-system.c (allocate_filename_struct): use >>>>>> xmalloc instread of malloc. >>>>>> * libgcc/libgcov-merge.c (void __gcov_merge_delta): Add more >>>>>> parameters to merge function. >>>>>> (__gcov_merge_add): Ditto. >>>>>> (__gcov_merge_ior): Ditto. >>>>>> (__gcov_merge_time_profile): Ditto. >>>>>> (__gcov_merge_single): Ditto. >>>>>> (__gcov_merge_delta): Ditto. >>>>>> * libgcc/libgcov-tool.c (void gcov_tool_set_verbose): New for >>>>>> gcov-tool support. >>>>>> (set_fn_ctrs): Ditto. >>>>>> (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_merge): Ditto. >>>>>> (find_match_gcov_inf Ditto.o): >>>>>> (gcov_profile_merge): Ditto. >>>>>> (__gcov_scale_add): Ditto. >>>>>> (__gcov_scale_ior): Ditto. >>>>>> (__gcov_scale_delta): Ditto. >>>>>> (__gcov_scale_single): Ditto. >>>>>> (gcov_profile_scale): Ditto. >>>>>> (gcov_profile_normalize): Ditto. >>>>>> (__gcov_scale2_add): Ditto. >>>>>> (__gcov_scale2_ior): Ditto. >>>>>> (__gcov_scale2_delta): Ditto. >>>>>> (__gcov_scale2_single): Ditto. >>>>>> (gcov_profile_scale2): Ditto. >>>>>> * gcc/gcov-tool.c (unlink_file): Gcov-tool driver support. >>>>>> (unlink_dir): Ditto. >>>>>> (profile_merge): Ditto. >>>>>> (print_merge_usage_message): Ditto. >>>>>> (merge_usage): Ditto. >>>>>> (do_merge): Ditto. >>>>>> (profile_rewrite2): Ditto. >>>>>> (profile_rewrite): Ditto. >>>>>> (print_rewrite_usage_message): Ditto. >>>>>> (rewrite_usage): Ditto. >>>>>> (do_rewrite): Ditto. >>>>>> (print_usage): Ditto. >>>>>> (print_version): Ditto. >>>>>> (process_args): Ditto. >>>>>> (main): Ditto. >>>>>> * gcc/Makefile.in: Build and install gcov-tool. >>>>> >>>>>> Index: gcc/gcov-io.c >>>>>> =================================================================== >>>>>> --- gcc/gcov-io.c (revision 204895) >>>>>> +++ gcc/gcov-io.c (working copy) >>>>>> @@ -36,6 +36,37 @@ static const gcov_unsigned_t *gcov_read_words (uns >>>>>> static void gcov_allocate (unsigned); >>>>>> #endif >>>>>> >>>>>> +/* Moved for gcov-io.h and make it static. */ >>>>>> +static struct gcov_var gcov_var; >>>>> >>>>> This is more an changelog message than a comment in source file. >>>>> Just describe what gcov_var is. >>>> >>>> I changed this so gcov_var is no longer static, but global as before. >>>> >>>>> >>>>> Do you know how the size of libgcov changed with your patch? >>>>> Quick check of current mainline on compiling empty main gives: >>>>> >>>>> jh@gcc10:~/trunk/build/gcc$ cat t.c >>>>> main() >>>>> { >>>>> } >>>>> jh@gcc10:~/trunk/build/gcc$ ./xgcc -B ./ -O2 -fprofile-generate -o a.out-new --static t.c >>>>> jh@gcc10:~/trunk/build/gcc$ gcc -O2 -fprofile-generate -o a.out-old --static t.c >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-old >>>>> text data bss dec hex filename >>>>> 608141 3560 16728 628429 996cd a.out-old >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-new >>>>> text data bss dec hex filename >>>>> 612621 3688 22880 639189 9c0d5 a.out-new >>>>> >>>>> Without profiling I get: >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-new-no >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-old-no >>>>> text data bss dec hex filename >>>>> 599719 3448 12568 615735 96537 a.out-old-no >>>>> text data bss dec hex filename >>>>> 600247 3448 12568 616263 96747 a.out-new-no >>>>> >>>>> Quite big for empty program, but mostly glibc fault, I suppose >>>>> (that won't be an issue for embedded platforms). But anyway >>>>> we increased text size overhead from 8k to 12k, BSS size >>>>> overhead from 4k to 10k and data by another 1k. >>>>> >>>>> text data bss dec hex filename >>>>> 126 0 0 126 7e _gcov_merge_add.o (ex libgcov.a) >>>>> 251 0 0 251 fb _gcov_merge_single.o (ex libgcov.a) >>>>> 242 0 0 242 f2 _gcov_merge_delta.o (ex libgcov.a) >>>>> 126 0 0 126 7e _gcov_merge_ior.o (ex libgcov.a) >>>>> 156 0 0 156 9c _gcov_merge_time_profile.o (ex libgcov.a) >>>>> 89 0 0 89 59 _gcov_interval_profiler.o (ex libgcov.a) >>>>> 69 0 0 69 45 _gcov_pow2_profiler.o (ex libgcov.a) >>>>> 115 0 0 115 73 _gcov_one_value_profiler.o (ex libgcov.a) >>>>> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex libgcov.a) >>>>> 57 0 0 57 39 _gcov_average_profiler.o (ex libgcov.a) >>>>> 52 0 0 52 34 _gcov_ior_profiler.o (ex libgcov.a) >>>>> 178 0 16 194 c2 _gcov_indirect_call_profiler_v2.o (ex libgcov.a) >>>>> 77 0 8 85 55 _gcov_time_profiler.o (ex libgcov.a) >>>>> 126 0 40 166 a6 _gcov_flush.o (ex libgcov.a) >>>>> 101 0 0 101 65 _gcov_fork.o (ex libgcov.a) >>>>> 471 0 0 471 1d7 _gcov_execl.o (ex libgcov.a) >>>>> 471 0 0 471 1d7 _gcov_execlp.o (ex libgcov.a) >>>>> 524 0 0 524 20c _gcov_execle.o (ex libgcov.a) >>>>> 98 0 0 98 62 _gcov_execv.o (ex libgcov.a) >>>>> 98 0 0 98 62 _gcov_execvp.o (ex libgcov.a) >>>>> 108 0 0 108 6c _gcov_execve.o (ex libgcov.a) >>>>> 66 0 0 66 42 _gcov_reset.o (ex libgcov.a) >>>>> 66 0 0 66 42 _gcov_dump.o (ex libgcov.a) >>>>> 9505 0 6144 15649 3d21 _gcov.o (ex libgcov.a) >>>>> >>>>> I think we definitely need to move those 6k of bss space out. I think those are new >>>>> static vars you introduced that I think are unsafe anyway because multiple streaming >>>>> may run at once in threaded program where locking mechanizm fails. >>>>> (it will probably do other bad things, but definitely we do not want to >>>>> conflict on things like filename to write into). >>>>> >>>>> Compared to my system gcov: >>>>> text data bss dec hex filename >>>>> 9765 0 64 9829 2665 _gcov.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 125 0 0 125 7d _gcov_merge_add.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 251 0 0 251 fb _gcov_merge_single.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 242 0 0 242 f2 _gcov_merge_delta.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 101 0 0 101 65 _gcov_fork.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 471 0 0 471 1d7 _gcov_execl.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 471 0 0 471 1d7 _gcov_execlp.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 524 0 0 524 20c _gcov_execle.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 98 0 0 98 62 _gcov_execv.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 98 0 0 98 62 _gcov_execvp.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 108 0 0 108 6c _gcov_execve.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 72 0 0 72 48 _gcov_reset.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 72 0 0 72 48 _gcov_dump.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 89 0 0 89 59 _gcov_interval_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 69 0 0 69 45 _gcov_pow2_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 115 0 0 115 73 _gcov_one_value_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 57 0 0 57 39 _gcov_average_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 52 0 0 52 34 _gcov_ior_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> 125 0 0 125 7d _gcov_merge_ior.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>>> >>>>>> Index: gcc/gcov-io.h >>>>>> =================================================================== >>>>>> --- gcc/gcov-io.h (revision 204895) >>>>>> +++ gcc/gcov-io.h (working copy) >>>>>> @@ -164,51 +164,7 @@ see the files COPYING3 and COPYING.RUNTIME respect >>>>>> #ifndef GCC_GCOV_IO_H >>>>>> #define GCC_GCOV_IO_H >>>>>> >>>>>> -#if IN_LIBGCOV >>>>>> -/* About the target */ >>>>>> - >>>>>> -#if BITS_PER_UNIT == 8 >>>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (SI))); >>>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (SI))); >>>>>> -#if LONG_LONG_TYPE_SIZE > 32 >>>>>> -typedef signed gcov_type __attribute__ ((mode (DI))); >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (DI))); >>>>>> -#else >>>>>> -typedef signed gcov_type __attribute__ ((mode (SI))); >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); >>>>>> -#endif >>>>>> -#else >>>>>> -#if BITS_PER_UNIT == 16 >>>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (HI))); >>>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (HI))); >>>>>> -#if LONG_LONG_TYPE_SIZE > 32 >>>>>> -typedef signed gcov_type __attribute__ ((mode (SI))); >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); >>>>>> -#else >>>>>> -typedef signed gcov_type __attribute__ ((mode (HI))); >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); >>>>>> -#endif >>>>>> -#else >>>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (QI))); >>>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (QI))); >>>>>> -#if LONG_LONG_TYPE_SIZE > 32 >>>>>> -typedef signed gcov_type __attribute__ ((mode (HI))); >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); >>>>>> -#else >>>>>> -typedef signed gcov_type __attribute__ ((mode (QI))); >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (QI))); >>>>>> -#endif >>>>>> -#endif >>>>>> -#endif >>>>> >>>>> So this part basically moves libgcov specific bits into libgcov.h? That OK fine by >>>>> itself. >>>>>> Index: libgcc/libgcov-profiler.c >>>>>> =================================================================== >>>>>> --- libgcc/libgcov-profiler.c (revision 204895) >>>>>> +++ libgcc/libgcov-profiler.c (working copy) >>>>>> @@ -23,15 +23,8 @@ a copy of the GCC Runtime Library Exception along >>>>>> see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >>>>>> <http://www.gnu.org/licenses/>. */ >>>>>> >>>>>> -#include "tconfig.h" >>>>>> -#include "tsystem.h" >>>>>> -#include "coretypes.h" >>>>>> -#include "tm.h" >>>>>> -#include "libgcc_tm.h" >>>>>> - >>>>>> +#include "libgcov.h" >>>>>> #if !defined(inhibit_libc) >>>>>> -#define IN_LIBGCOV 1 >>>>>> -#include "gcov-io.h" >>>>> >>>>> I did not pay that much attention into the current include file changes, but wasn't >>>>> idea to avoid #include file to include random other #includes? >>>>> So perhaps the first block of includes should stay, followed by libgcov.h and gcov-io.h >>>>> last? >>>> >>>> I'm not sure I understand the issue here? The patch basically moves >>>> the same includes into libgcov.h, and includes that instead. I see >>>> many other header files in gcc that include other headers. >>>> >>>>>> @@ -325,6 +311,9 @@ static struct gcov_summary all_prg; >>>>>> #endif >>>>>> /* crc32 for this program. */ >>>>>> static gcov_unsigned_t crc32; >>>>>> +/* Use this summary checksum rather the computed one if the value is >>>>>> + * non-zero. */ >>>>>> +static gcov_unsigned_t saved_summary_checksum; >>>>> >>>>> Why do you need to save the checksum? Won't it reset summary back with multiple streaming? >>>> >>>> This has been removed. >>>> >>>>> >>>>> I would really like to avoid introducing those static vars that are used exclusively >>>>> by gcov_exit. What about putting them into an gcov_context structure that >>>>> is passed around the functions that was broken out? >>>>>> Index: libgcc/libgcov-merge.c >>>>>> =================================================================== >>>>>> --- libgcc/libgcov-merge.c (revision 204895) >>>>>> +++ libgcc/libgcov-merge.c (working copy) >>>>>> @@ -23,107 +23,150 @@ a copy of the GCC Runtime Library Exception along >>>>>> see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >>>>>> <http://www.gnu.org/licenses/>. */ >>>>>> >>>>>> -#include "tconfig.h" >>>>>> -#include "tsystem.h" >>>>>> -#include "coretypes.h" >>>>>> -#include "tm.h" >>>>>> -#include "libgcc_tm.h" >>>>>> +#include "libgcov.h" >>>>>> >>>>>> -#if defined(inhibit_libc) >>>>>> -#define IN_LIBGCOV (-1) >>>>>> -#else >>>>>> -#define IN_LIBGCOV 1 >>>>>> +#include "gcov-io-libgcov.h" >>>>>> #endif >>>>>> >>>>>> -#include "gcov-io.h" >>>>>> - >>>>>> #if defined(inhibit_libc) >>>>>> /* If libc and its header files are not available, provide dummy functions. */ >>>>>> >>>>>> #ifdef L_gcov_merge_add >>>>>> void __gcov_merge_add (gcov_type *counters __attribute__ ((unused)), >>>>>> - unsigned n_counters __attribute__ ((unused))) {} >>>>>> + unsigned n_counters __attribute__ ((unused)), >>>>>> + unsigned gcov_type *src __attribute__ ((unused)), >>>>>> + unsigned w __attribute__ ((unused))) {} >>>>>> #endif >>>>>> >>>>>> #ifdef L_gcov_merge_single >>>>>> void __gcov_merge_single (gcov_type *counters __attribute__ ((unused)), >>>>>> - unsigned n_counters __attribute__ ((unused))) {} >>>>>> + unsigned n_counters __attribute__ ((unused)), >>>>>> + unsigned gcov_type *src __attribute__ ((unused)), >>>>>> + unsigned w __attribute__ ((unused))) {} >>>>>> #endif >>>>>> >>>>>> #ifdef L_gcov_merge_delta >>>>>> void __gcov_merge_delta (gcov_type *counters __attribute__ ((unused)), >>>>>> - unsigned n_counters __attribute__ ((unused))) {} >>>>>> + unsigned n_counters __attribute__ ((unused)), >>>>>> + unsigned gcov_type *src __attribute__ ((unused)), >>>>>> + unsigned w __attribute__ ((unused))) {} >>>>>> #endif >>>>>> >>>>>> #else >>>>>> >>>>>> #ifdef L_gcov_merge_add >>>>>> /* The profile merging function that just adds the counters. It is given >>>>>> - an array COUNTERS of N_COUNTERS old counters and it reads the same number >>>>>> - of counters from the gcov file. */ >>>>>> + an array COUNTERS of N_COUNTERS old counters. >>>>>> + When SRC==NULL, it reads the same number of counters from the gcov file. >>>>>> + Otherwise, it reads from SRC array. These read values will be multiplied >>>>>> + by weight W before adding to the old counters. */ >>>>>> void >>>>>> -__gcov_merge_add (gcov_type *counters, unsigned n_counters) >>>>>> +__gcov_merge_add (gcov_type *counters, unsigned n_counters, >>>>>> + gcov_type *src, unsigned w) >>>>>> { >>>>>> + int in_mem = (src != 0); >>>>>> + >>>>>> for (; n_counters; counters++, n_counters--) >>>>>> - *counters += gcov_read_counter (); >>>>>> + { >>>>>> + gcov_type value; >>>>>> + >>>>>> + if (in_mem) >>>>>> + value = *(src++); >>>>>> + else >>>>>> + value = gcov_read_counter (); >>>>>> + >>>>>> + *counters += w * value; >>>>>> + } >>>>>> } >>>>>> #endif /* L_gcov_merge_add */ >>>>>> >>>>>> #ifdef L_gcov_merge_ior >>>>>> /* The profile merging function that just adds the counters. It is given >>>>>> - an array COUNTERS of N_COUNTERS old counters and it reads the same number >>>>>> - of counters from the gcov file. */ >>>>>> + an array COUNTERS of N_COUNTERS old counters. >>>>>> + When SRC==NULL, it reads the same number of counters from the gcov file. >>>>>> + Otherwise, it reads from SRC array. */ >>>>>> void >>>>>> -__gcov_merge_ior (gcov_type *counters, unsigned n_counters) >>>>>> +__gcov_merge_ior (gcov_type *counters, unsigned n_counters, >>>>>> + gcov_type *src, unsigned w __attribute__ ((unused))) >>>>> >>>>> So the new in-memory variants are introduced for merging tool, while libgcc use gcov_read_counter >>>>> interface? >>>>> Perhaps we can actually just duplicate the functions to avoid runtime to do all the scalling >>>>> and in_mem tests it won't need? >>>>> >>>>> I would suggest going with libgcov.h changes and clenaups first, with interface changes next >>>>> and the gcov-tool is probably quite obvious at the end? >>>>> Do you think you can split the patch this way? >>>>> >>>>> Thanks and sorry for taking long to review. I should have more time again now. >>>>> Honza >>>> >>>> The libgcov.h related changes are in the attached patch. I think it >>>> addresses your concerns. Bootstrapped and tested on >>>> x86-64-unknown-linux-gnu. A profiledbootstrap is in progress. >>>> >>>> Ok for trunk if profiledbootstrap passes? >>> >>> Both a profiledbootstrap and LTO profiledbootstrap pass. >>> >>> Teresa >>> >>>> >>>> Thanks, >>>> Teresa >>>> >>>> >>>> -- >>>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 >>> >>> >>> >>> -- >>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 > > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
> I think so -- they are private to libgcov. Honza, what do you think? Hmm, the purpose of gcov-io was to be low level IO library for the basic gcov file format. I am not sure if gcov_write_tag_length should really resist in other file than gcov_write_tag. I see a desire to isolate actual stdio calls so one can have replacement driver for i.e. Linux kernel. For that reason things like gcov_seek and friends probably should be separated, but what is reason for splitting the file handling itself? Honza > > thanks, > > David > > On Mon, Dec 16, 2013 at 1:17 PM, Teresa Johnson <tejohnson@google.com> wrote: > > On Mon, Dec 16, 2013 at 12:55 PM, Xinliang David Li <davidxl@google.com> wrote: > >> gcov_rewrite function is only needed (and defined) with IN_LIBGCOV. > >> Should it be moved from common file gcov-io.c to libgcov.c? > > > > Possibly. I just looked through gcov-io.c and there are several > > additional functions that are only defined under "#ifdef IN_LIBGCOV" > > and only used in libgcov*c (or each other): > > > > gcov_write_counter > > gcov_write_tag_length > > gcov_write_summary > > gcov_seek > > > > Should they all, plus gcov_rewrite, be moved to libgcov-driver.c? > > > > Teresa > > > >> > >> > >> David > >> > >> On Thu, Dec 12, 2013 at 12:11 PM, Teresa Johnson <tejohnson@google.com> wrote: > >>> On Wed, Dec 11, 2013 at 10:05 PM, Teresa Johnson <tejohnson@google.com> wrote: > >>>> On Fri, Dec 6, 2013 at 6:23 AM, Jan Hubicka <hubicka@ucw.cz> wrote: > >>>>>> Hi, all > >>>>>> > >>>>>> This is the new patch for gcov-tool (previously profile-tool). > >>>>>> > >>>>>> Honza: can you comment on the new merge interface? David posted some > >>>>>> comments in an earlier email and we want to know what's your opinion. > >>>>>> > >>>>>> Test patch has been tested with boostrap, regresssion, > >>>>>> profiledbootstrap and SPEC2006. > >>>>>> > >>>>>> Noticeable changes from the earlier version: > >>>>>> > >>>>>> 1. create a new file libgcov.h and move libgcov-*.h headers to libgcov.h > >>>>>> So we can included multiple libgcov-*.c without adding new macros. > >>>>>> > >>>>>> 2. split libgcov.h specific code in gcvo-io.h to libcc/libgcov.h > >>>>>> Avoid multiple-page of code under IN_LIBGCOV macro -- this > >>>>>> improves the readability. > >>>>>> > >>>>>> 3. make gcov_var static, and move the definition from gcov-io.h to > >>>>>> gcov-io.c. Also > >>>>>> move some static functions accessing gcov_var to gcvo-io.c > >>>>>> Current code rely on GCOV_LINKAGE tricks to avoid multi-definition. I don't see > >>>>>> a reason that gcov_var needs to exposed as a global. > >>>>>> > >>>>>> 4. expose gcov_write_strings() and gcov_sync() to gcov_tool usage > >>>>>> > >>>>>> 5. rename profile-tool to gcov-tool per Honza's suggestion. > >>>>>> > >>>>>> Thanks, > >>>>> > >>>>> Hi, > >>>>> I did not read in deatil the gcov-tool source itself, but lets first make the interface changes > >>>>> needed. > >>>>> > >>>>>> 2013-11-18 Rong Xu <xur@google.com> > >>>>>> > >>>>>> * gcc/gcov-io.c (gcov_var): Moved from gcov-io.h and make it static. > >>>>>> (gcov_position): Move from gcov-io.h > >>>>>> (gcov_is_error): Ditto. > >>>>>> (gcov_rewrite): Ditto. > >>>>>> * gcc/gcov-io.h: Re-factoring. Move gcov_var to gcov-io.h and > >>>>>> move the libgcov only part of libgcc/libgcov.h. > >>>>>> * libgcc/libgcov.h: New common header files for libgcov-*.h > >>>>>> * libgcc/Makefile.in: Add dependence to libgcov.h > >>>>>> * libgcc/libgcov-profiler.c: Use libgcov.h > >>>>>> * libgcc/libgcov-driver.c: Ditto. > >>>>>> * libgcc/libgcov-interface.c: Ditto. > >>>>>> * libgcc/libgcov-driver-system.c (allocate_filename_struct): use > >>>>>> xmalloc instread of malloc. > >>>>>> * libgcc/libgcov-merge.c (void __gcov_merge_delta): Add more > >>>>>> parameters to merge function. > >>>>>> (__gcov_merge_add): Ditto. > >>>>>> (__gcov_merge_ior): Ditto. > >>>>>> (__gcov_merge_time_profile): Ditto. > >>>>>> (__gcov_merge_single): Ditto. > >>>>>> (__gcov_merge_delta): Ditto. > >>>>>> * libgcc/libgcov-tool.c (void gcov_tool_set_verbose): New for > >>>>>> gcov-tool support. > >>>>>> (set_fn_ctrs): Ditto. > >>>>>> (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_merge): Ditto. > >>>>>> (find_match_gcov_inf Ditto.o): > >>>>>> (gcov_profile_merge): Ditto. > >>>>>> (__gcov_scale_add): Ditto. > >>>>>> (__gcov_scale_ior): Ditto. > >>>>>> (__gcov_scale_delta): Ditto. > >>>>>> (__gcov_scale_single): Ditto. > >>>>>> (gcov_profile_scale): Ditto. > >>>>>> (gcov_profile_normalize): Ditto. > >>>>>> (__gcov_scale2_add): Ditto. > >>>>>> (__gcov_scale2_ior): Ditto. > >>>>>> (__gcov_scale2_delta): Ditto. > >>>>>> (__gcov_scale2_single): Ditto. > >>>>>> (gcov_profile_scale2): Ditto. > >>>>>> * gcc/gcov-tool.c (unlink_file): Gcov-tool driver support. > >>>>>> (unlink_dir): Ditto. > >>>>>> (profile_merge): Ditto. > >>>>>> (print_merge_usage_message): Ditto. > >>>>>> (merge_usage): Ditto. > >>>>>> (do_merge): Ditto. > >>>>>> (profile_rewrite2): Ditto. > >>>>>> (profile_rewrite): Ditto. > >>>>>> (print_rewrite_usage_message): Ditto. > >>>>>> (rewrite_usage): Ditto. > >>>>>> (do_rewrite): Ditto. > >>>>>> (print_usage): Ditto. > >>>>>> (print_version): Ditto. > >>>>>> (process_args): Ditto. > >>>>>> (main): Ditto. > >>>>>> * gcc/Makefile.in: Build and install gcov-tool. > >>>>> > >>>>>> Index: gcc/gcov-io.c > >>>>>> =================================================================== > >>>>>> --- gcc/gcov-io.c (revision 204895) > >>>>>> +++ gcc/gcov-io.c (working copy) > >>>>>> @@ -36,6 +36,37 @@ static const gcov_unsigned_t *gcov_read_words (uns > >>>>>> static void gcov_allocate (unsigned); > >>>>>> #endif > >>>>>> > >>>>>> +/* Moved for gcov-io.h and make it static. */ > >>>>>> +static struct gcov_var gcov_var; > >>>>> > >>>>> This is more an changelog message than a comment in source file. > >>>>> Just describe what gcov_var is. > >>>> > >>>> I changed this so gcov_var is no longer static, but global as before. > >>>> > >>>>> > >>>>> Do you know how the size of libgcov changed with your patch? > >>>>> Quick check of current mainline on compiling empty main gives: > >>>>> > >>>>> jh@gcc10:~/trunk/build/gcc$ cat t.c > >>>>> main() > >>>>> { > >>>>> } > >>>>> jh@gcc10:~/trunk/build/gcc$ ./xgcc -B ./ -O2 -fprofile-generate -o a.out-new --static t.c > >>>>> jh@gcc10:~/trunk/build/gcc$ gcc -O2 -fprofile-generate -o a.out-old --static t.c > >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-old > >>>>> text data bss dec hex filename > >>>>> 608141 3560 16728 628429 996cd a.out-old > >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-new > >>>>> text data bss dec hex filename > >>>>> 612621 3688 22880 639189 9c0d5 a.out-new > >>>>> > >>>>> Without profiling I get: > >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-new-no > >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-old-no > >>>>> text data bss dec hex filename > >>>>> 599719 3448 12568 615735 96537 a.out-old-no > >>>>> text data bss dec hex filename > >>>>> 600247 3448 12568 616263 96747 a.out-new-no > >>>>> > >>>>> Quite big for empty program, but mostly glibc fault, I suppose > >>>>> (that won't be an issue for embedded platforms). But anyway > >>>>> we increased text size overhead from 8k to 12k, BSS size > >>>>> overhead from 4k to 10k and data by another 1k. > >>>>> > >>>>> text data bss dec hex filename > >>>>> 126 0 0 126 7e _gcov_merge_add.o (ex libgcov.a) > >>>>> 251 0 0 251 fb _gcov_merge_single.o (ex libgcov.a) > >>>>> 242 0 0 242 f2 _gcov_merge_delta.o (ex libgcov.a) > >>>>> 126 0 0 126 7e _gcov_merge_ior.o (ex libgcov.a) > >>>>> 156 0 0 156 9c _gcov_merge_time_profile.o (ex libgcov.a) > >>>>> 89 0 0 89 59 _gcov_interval_profiler.o (ex libgcov.a) > >>>>> 69 0 0 69 45 _gcov_pow2_profiler.o (ex libgcov.a) > >>>>> 115 0 0 115 73 _gcov_one_value_profiler.o (ex libgcov.a) > >>>>> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex libgcov.a) > >>>>> 57 0 0 57 39 _gcov_average_profiler.o (ex libgcov.a) > >>>>> 52 0 0 52 34 _gcov_ior_profiler.o (ex libgcov.a) > >>>>> 178 0 16 194 c2 _gcov_indirect_call_profiler_v2.o (ex libgcov.a) > >>>>> 77 0 8 85 55 _gcov_time_profiler.o (ex libgcov.a) > >>>>> 126 0 40 166 a6 _gcov_flush.o (ex libgcov.a) > >>>>> 101 0 0 101 65 _gcov_fork.o (ex libgcov.a) > >>>>> 471 0 0 471 1d7 _gcov_execl.o (ex libgcov.a) > >>>>> 471 0 0 471 1d7 _gcov_execlp.o (ex libgcov.a) > >>>>> 524 0 0 524 20c _gcov_execle.o (ex libgcov.a) > >>>>> 98 0 0 98 62 _gcov_execv.o (ex libgcov.a) > >>>>> 98 0 0 98 62 _gcov_execvp.o (ex libgcov.a) > >>>>> 108 0 0 108 6c _gcov_execve.o (ex libgcov.a) > >>>>> 66 0 0 66 42 _gcov_reset.o (ex libgcov.a) > >>>>> 66 0 0 66 42 _gcov_dump.o (ex libgcov.a) > >>>>> 9505 0 6144 15649 3d21 _gcov.o (ex libgcov.a) > >>>>> > >>>>> I think we definitely need to move those 6k of bss space out. I think those are new > >>>>> static vars you introduced that I think are unsafe anyway because multiple streaming > >>>>> may run at once in threaded program where locking mechanizm fails. > >>>>> (it will probably do other bad things, but definitely we do not want to > >>>>> conflict on things like filename to write into). > >>>>> > >>>>> Compared to my system gcov: > >>>>> text data bss dec hex filename > >>>>> 9765 0 64 9829 2665 _gcov.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 125 0 0 125 7d _gcov_merge_add.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 251 0 0 251 fb _gcov_merge_single.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 242 0 0 242 f2 _gcov_merge_delta.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 101 0 0 101 65 _gcov_fork.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 471 0 0 471 1d7 _gcov_execl.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 471 0 0 471 1d7 _gcov_execlp.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 524 0 0 524 20c _gcov_execle.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 98 0 0 98 62 _gcov_execv.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 98 0 0 98 62 _gcov_execvp.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 108 0 0 108 6c _gcov_execve.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 72 0 0 72 48 _gcov_reset.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 72 0 0 72 48 _gcov_dump.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 89 0 0 89 59 _gcov_interval_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 69 0 0 69 45 _gcov_pow2_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 115 0 0 115 73 _gcov_one_value_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 57 0 0 57 39 _gcov_average_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 52 0 0 52 34 _gcov_ior_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> 125 0 0 125 7d _gcov_merge_ior.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) > >>>>> > >>>>>> Index: gcc/gcov-io.h > >>>>>> =================================================================== > >>>>>> --- gcc/gcov-io.h (revision 204895) > >>>>>> +++ gcc/gcov-io.h (working copy) > >>>>>> @@ -164,51 +164,7 @@ see the files COPYING3 and COPYING.RUNTIME respect > >>>>>> #ifndef GCC_GCOV_IO_H > >>>>>> #define GCC_GCOV_IO_H > >>>>>> > >>>>>> -#if IN_LIBGCOV > >>>>>> -/* About the target */ > >>>>>> - > >>>>>> -#if BITS_PER_UNIT == 8 > >>>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (SI))); > >>>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (SI))); > >>>>>> -#if LONG_LONG_TYPE_SIZE > 32 > >>>>>> -typedef signed gcov_type __attribute__ ((mode (DI))); > >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (DI))); > >>>>>> -#else > >>>>>> -typedef signed gcov_type __attribute__ ((mode (SI))); > >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); > >>>>>> -#endif > >>>>>> -#else > >>>>>> -#if BITS_PER_UNIT == 16 > >>>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (HI))); > >>>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (HI))); > >>>>>> -#if LONG_LONG_TYPE_SIZE > 32 > >>>>>> -typedef signed gcov_type __attribute__ ((mode (SI))); > >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); > >>>>>> -#else > >>>>>> -typedef signed gcov_type __attribute__ ((mode (HI))); > >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); > >>>>>> -#endif > >>>>>> -#else > >>>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (QI))); > >>>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (QI))); > >>>>>> -#if LONG_LONG_TYPE_SIZE > 32 > >>>>>> -typedef signed gcov_type __attribute__ ((mode (HI))); > >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); > >>>>>> -#else > >>>>>> -typedef signed gcov_type __attribute__ ((mode (QI))); > >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (QI))); > >>>>>> -#endif > >>>>>> -#endif > >>>>>> -#endif > >>>>> > >>>>> So this part basically moves libgcov specific bits into libgcov.h? That OK fine by > >>>>> itself. > >>>>>> Index: libgcc/libgcov-profiler.c > >>>>>> =================================================================== > >>>>>> --- libgcc/libgcov-profiler.c (revision 204895) > >>>>>> +++ libgcc/libgcov-profiler.c (working copy) > >>>>>> @@ -23,15 +23,8 @@ a copy of the GCC Runtime Library Exception along > >>>>>> see the files COPYING3 and COPYING.RUNTIME respectively. If not, see > >>>>>> <http://www.gnu.org/licenses/>. */ > >>>>>> > >>>>>> -#include "tconfig.h" > >>>>>> -#include "tsystem.h" > >>>>>> -#include "coretypes.h" > >>>>>> -#include "tm.h" > >>>>>> -#include "libgcc_tm.h" > >>>>>> - > >>>>>> +#include "libgcov.h" > >>>>>> #if !defined(inhibit_libc) > >>>>>> -#define IN_LIBGCOV 1 > >>>>>> -#include "gcov-io.h" > >>>>> > >>>>> I did not pay that much attention into the current include file changes, but wasn't > >>>>> idea to avoid #include file to include random other #includes? > >>>>> So perhaps the first block of includes should stay, followed by libgcov.h and gcov-io.h > >>>>> last? > >>>> > >>>> I'm not sure I understand the issue here? The patch basically moves > >>>> the same includes into libgcov.h, and includes that instead. I see > >>>> many other header files in gcc that include other headers. > >>>> > >>>>>> @@ -325,6 +311,9 @@ static struct gcov_summary all_prg; > >>>>>> #endif > >>>>>> /* crc32 for this program. */ > >>>>>> static gcov_unsigned_t crc32; > >>>>>> +/* Use this summary checksum rather the computed one if the value is > >>>>>> + * non-zero. */ > >>>>>> +static gcov_unsigned_t saved_summary_checksum; > >>>>> > >>>>> Why do you need to save the checksum? Won't it reset summary back with multiple streaming? > >>>> > >>>> This has been removed. > >>>> > >>>>> > >>>>> I would really like to avoid introducing those static vars that are used exclusively > >>>>> by gcov_exit. What about putting them into an gcov_context structure that > >>>>> is passed around the functions that was broken out? > >>>>>> Index: libgcc/libgcov-merge.c > >>>>>> =================================================================== > >>>>>> --- libgcc/libgcov-merge.c (revision 204895) > >>>>>> +++ libgcc/libgcov-merge.c (working copy) > >>>>>> @@ -23,107 +23,150 @@ a copy of the GCC Runtime Library Exception along > >>>>>> see the files COPYING3 and COPYING.RUNTIME respectively. If not, see > >>>>>> <http://www.gnu.org/licenses/>. */ > >>>>>> > >>>>>> -#include "tconfig.h" > >>>>>> -#include "tsystem.h" > >>>>>> -#include "coretypes.h" > >>>>>> -#include "tm.h" > >>>>>> -#include "libgcc_tm.h" > >>>>>> +#include "libgcov.h" > >>>>>> > >>>>>> -#if defined(inhibit_libc) > >>>>>> -#define IN_LIBGCOV (-1) > >>>>>> -#else > >>>>>> -#define IN_LIBGCOV 1 > >>>>>> +#include "gcov-io-libgcov.h" > >>>>>> #endif > >>>>>> > >>>>>> -#include "gcov-io.h" > >>>>>> - > >>>>>> #if defined(inhibit_libc) > >>>>>> /* If libc and its header files are not available, provide dummy functions. */ > >>>>>> > >>>>>> #ifdef L_gcov_merge_add > >>>>>> void __gcov_merge_add (gcov_type *counters __attribute__ ((unused)), > >>>>>> - unsigned n_counters __attribute__ ((unused))) {} > >>>>>> + unsigned n_counters __attribute__ ((unused)), > >>>>>> + unsigned gcov_type *src __attribute__ ((unused)), > >>>>>> + unsigned w __attribute__ ((unused))) {} > >>>>>> #endif > >>>>>> > >>>>>> #ifdef L_gcov_merge_single > >>>>>> void __gcov_merge_single (gcov_type *counters __attribute__ ((unused)), > >>>>>> - unsigned n_counters __attribute__ ((unused))) {} > >>>>>> + unsigned n_counters __attribute__ ((unused)), > >>>>>> + unsigned gcov_type *src __attribute__ ((unused)), > >>>>>> + unsigned w __attribute__ ((unused))) {} > >>>>>> #endif > >>>>>> > >>>>>> #ifdef L_gcov_merge_delta > >>>>>> void __gcov_merge_delta (gcov_type *counters __attribute__ ((unused)), > >>>>>> - unsigned n_counters __attribute__ ((unused))) {} > >>>>>> + unsigned n_counters __attribute__ ((unused)), > >>>>>> + unsigned gcov_type *src __attribute__ ((unused)), > >>>>>> + unsigned w __attribute__ ((unused))) {} > >>>>>> #endif > >>>>>> > >>>>>> #else > >>>>>> > >>>>>> #ifdef L_gcov_merge_add > >>>>>> /* The profile merging function that just adds the counters. It is given > >>>>>> - an array COUNTERS of N_COUNTERS old counters and it reads the same number > >>>>>> - of counters from the gcov file. */ > >>>>>> + an array COUNTERS of N_COUNTERS old counters. > >>>>>> + When SRC==NULL, it reads the same number of counters from the gcov file. > >>>>>> + Otherwise, it reads from SRC array. These read values will be multiplied > >>>>>> + by weight W before adding to the old counters. */ > >>>>>> void > >>>>>> -__gcov_merge_add (gcov_type *counters, unsigned n_counters) > >>>>>> +__gcov_merge_add (gcov_type *counters, unsigned n_counters, > >>>>>> + gcov_type *src, unsigned w) > >>>>>> { > >>>>>> + int in_mem = (src != 0); > >>>>>> + > >>>>>> for (; n_counters; counters++, n_counters--) > >>>>>> - *counters += gcov_read_counter (); > >>>>>> + { > >>>>>> + gcov_type value; > >>>>>> + > >>>>>> + if (in_mem) > >>>>>> + value = *(src++); > >>>>>> + else > >>>>>> + value = gcov_read_counter (); > >>>>>> + > >>>>>> + *counters += w * value; > >>>>>> + } > >>>>>> } > >>>>>> #endif /* L_gcov_merge_add */ > >>>>>> > >>>>>> #ifdef L_gcov_merge_ior > >>>>>> /* The profile merging function that just adds the counters. It is given > >>>>>> - an array COUNTERS of N_COUNTERS old counters and it reads the same number > >>>>>> - of counters from the gcov file. */ > >>>>>> + an array COUNTERS of N_COUNTERS old counters. > >>>>>> + When SRC==NULL, it reads the same number of counters from the gcov file. > >>>>>> + Otherwise, it reads from SRC array. */ > >>>>>> void > >>>>>> -__gcov_merge_ior (gcov_type *counters, unsigned n_counters) > >>>>>> +__gcov_merge_ior (gcov_type *counters, unsigned n_counters, > >>>>>> + gcov_type *src, unsigned w __attribute__ ((unused))) > >>>>> > >>>>> So the new in-memory variants are introduced for merging tool, while libgcc use gcov_read_counter > >>>>> interface? > >>>>> Perhaps we can actually just duplicate the functions to avoid runtime to do all the scalling > >>>>> and in_mem tests it won't need? > >>>>> > >>>>> I would suggest going with libgcov.h changes and clenaups first, with interface changes next > >>>>> and the gcov-tool is probably quite obvious at the end? > >>>>> Do you think you can split the patch this way? > >>>>> > >>>>> Thanks and sorry for taking long to review. I should have more time again now. > >>>>> Honza > >>>> > >>>> The libgcov.h related changes are in the attached patch. I think it > >>>> addresses your concerns. Bootstrapped and tested on > >>>> x86-64-unknown-linux-gnu. A profiledbootstrap is in progress. > >>>> > >>>> Ok for trunk if profiledbootstrap passes? > >>> > >>> Both a profiledbootstrap and LTO profiledbootstrap pass. > >>> > >>> Teresa > >>> > >>>> > >>>> Thanks, > >>>> Teresa > >>>> > >>>> > >>>> -- > >>>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 > >>> > >>> > >>> > >>> -- > >>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 > > > > > > > > -- > > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
Ok -- gcov_write_counter and gcov_write_tag_length are qualified as low level primitives for basic gcov format and probably should be kept in gcov-io.c. gcov_rewrite is petty much libgcov runtime implementation details so I think it should be moved out. gcov_write_summary is not related to gcov low level format either, neither is gcov_seek. Ok for them to be moved? thanks, David On Mon, Dec 16, 2013 at 2:34 PM, Jan Hubicka <hubicka@ucw.cz> wrote: >> I think so -- they are private to libgcov. Honza, what do you think? > > Hmm, the purpose of gcov-io was to be low level IO library for the basic > gcov file format. I am not sure if gcov_write_tag_length should really resist > in other file than gcov_write_tag. > > I see a desire to isolate actual stdio calls so one can have replacement driver > for i.e. Linux kernel. For that reason things like gcov_seek and friends probably > should be separated, but what is reason for splitting the file handling itself? > > Honza >> >> thanks, >> >> David >> >> On Mon, Dec 16, 2013 at 1:17 PM, Teresa Johnson <tejohnson@google.com> wrote: >> > On Mon, Dec 16, 2013 at 12:55 PM, Xinliang David Li <davidxl@google.com> wrote: >> >> gcov_rewrite function is only needed (and defined) with IN_LIBGCOV. >> >> Should it be moved from common file gcov-io.c to libgcov.c? >> > >> > Possibly. I just looked through gcov-io.c and there are several >> > additional functions that are only defined under "#ifdef IN_LIBGCOV" >> > and only used in libgcov*c (or each other): >> > >> > gcov_write_counter >> > gcov_write_tag_length >> > gcov_write_summary >> > gcov_seek >> > >> > Should they all, plus gcov_rewrite, be moved to libgcov-driver.c? >> > >> > Teresa >> > >> >> >> >> >> >> David >> >> >> >> On Thu, Dec 12, 2013 at 12:11 PM, Teresa Johnson <tejohnson@google.com> wrote: >> >>> On Wed, Dec 11, 2013 at 10:05 PM, Teresa Johnson <tejohnson@google.com> wrote: >> >>>> On Fri, Dec 6, 2013 at 6:23 AM, Jan Hubicka <hubicka@ucw.cz> wrote: >> >>>>>> Hi, all >> >>>>>> >> >>>>>> This is the new patch for gcov-tool (previously profile-tool). >> >>>>>> >> >>>>>> Honza: can you comment on the new merge interface? David posted some >> >>>>>> comments in an earlier email and we want to know what's your opinion. >> >>>>>> >> >>>>>> Test patch has been tested with boostrap, regresssion, >> >>>>>> profiledbootstrap and SPEC2006. >> >>>>>> >> >>>>>> Noticeable changes from the earlier version: >> >>>>>> >> >>>>>> 1. create a new file libgcov.h and move libgcov-*.h headers to libgcov.h >> >>>>>> So we can included multiple libgcov-*.c without adding new macros. >> >>>>>> >> >>>>>> 2. split libgcov.h specific code in gcvo-io.h to libcc/libgcov.h >> >>>>>> Avoid multiple-page of code under IN_LIBGCOV macro -- this >> >>>>>> improves the readability. >> >>>>>> >> >>>>>> 3. make gcov_var static, and move the definition from gcov-io.h to >> >>>>>> gcov-io.c. Also >> >>>>>> move some static functions accessing gcov_var to gcvo-io.c >> >>>>>> Current code rely on GCOV_LINKAGE tricks to avoid multi-definition. I don't see >> >>>>>> a reason that gcov_var needs to exposed as a global. >> >>>>>> >> >>>>>> 4. expose gcov_write_strings() and gcov_sync() to gcov_tool usage >> >>>>>> >> >>>>>> 5. rename profile-tool to gcov-tool per Honza's suggestion. >> >>>>>> >> >>>>>> Thanks, >> >>>>> >> >>>>> Hi, >> >>>>> I did not read in deatil the gcov-tool source itself, but lets first make the interface changes >> >>>>> needed. >> >>>>> >> >>>>>> 2013-11-18 Rong Xu <xur@google.com> >> >>>>>> >> >>>>>> * gcc/gcov-io.c (gcov_var): Moved from gcov-io.h and make it static. >> >>>>>> (gcov_position): Move from gcov-io.h >> >>>>>> (gcov_is_error): Ditto. >> >>>>>> (gcov_rewrite): Ditto. >> >>>>>> * gcc/gcov-io.h: Re-factoring. Move gcov_var to gcov-io.h and >> >>>>>> move the libgcov only part of libgcc/libgcov.h. >> >>>>>> * libgcc/libgcov.h: New common header files for libgcov-*.h >> >>>>>> * libgcc/Makefile.in: Add dependence to libgcov.h >> >>>>>> * libgcc/libgcov-profiler.c: Use libgcov.h >> >>>>>> * libgcc/libgcov-driver.c: Ditto. >> >>>>>> * libgcc/libgcov-interface.c: Ditto. >> >>>>>> * libgcc/libgcov-driver-system.c (allocate_filename_struct): use >> >>>>>> xmalloc instread of malloc. >> >>>>>> * libgcc/libgcov-merge.c (void __gcov_merge_delta): Add more >> >>>>>> parameters to merge function. >> >>>>>> (__gcov_merge_add): Ditto. >> >>>>>> (__gcov_merge_ior): Ditto. >> >>>>>> (__gcov_merge_time_profile): Ditto. >> >>>>>> (__gcov_merge_single): Ditto. >> >>>>>> (__gcov_merge_delta): Ditto. >> >>>>>> * libgcc/libgcov-tool.c (void gcov_tool_set_verbose): New for >> >>>>>> gcov-tool support. >> >>>>>> (set_fn_ctrs): Ditto. >> >>>>>> (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_merge): Ditto. >> >>>>>> (find_match_gcov_inf Ditto.o): >> >>>>>> (gcov_profile_merge): Ditto. >> >>>>>> (__gcov_scale_add): Ditto. >> >>>>>> (__gcov_scale_ior): Ditto. >> >>>>>> (__gcov_scale_delta): Ditto. >> >>>>>> (__gcov_scale_single): Ditto. >> >>>>>> (gcov_profile_scale): Ditto. >> >>>>>> (gcov_profile_normalize): Ditto. >> >>>>>> (__gcov_scale2_add): Ditto. >> >>>>>> (__gcov_scale2_ior): Ditto. >> >>>>>> (__gcov_scale2_delta): Ditto. >> >>>>>> (__gcov_scale2_single): Ditto. >> >>>>>> (gcov_profile_scale2): Ditto. >> >>>>>> * gcc/gcov-tool.c (unlink_file): Gcov-tool driver support. >> >>>>>> (unlink_dir): Ditto. >> >>>>>> (profile_merge): Ditto. >> >>>>>> (print_merge_usage_message): Ditto. >> >>>>>> (merge_usage): Ditto. >> >>>>>> (do_merge): Ditto. >> >>>>>> (profile_rewrite2): Ditto. >> >>>>>> (profile_rewrite): Ditto. >> >>>>>> (print_rewrite_usage_message): Ditto. >> >>>>>> (rewrite_usage): Ditto. >> >>>>>> (do_rewrite): Ditto. >> >>>>>> (print_usage): Ditto. >> >>>>>> (print_version): Ditto. >> >>>>>> (process_args): Ditto. >> >>>>>> (main): Ditto. >> >>>>>> * gcc/Makefile.in: Build and install gcov-tool. >> >>>>> >> >>>>>> Index: gcc/gcov-io.c >> >>>>>> =================================================================== >> >>>>>> --- gcc/gcov-io.c (revision 204895) >> >>>>>> +++ gcc/gcov-io.c (working copy) >> >>>>>> @@ -36,6 +36,37 @@ static const gcov_unsigned_t *gcov_read_words (uns >> >>>>>> static void gcov_allocate (unsigned); >> >>>>>> #endif >> >>>>>> >> >>>>>> +/* Moved for gcov-io.h and make it static. */ >> >>>>>> +static struct gcov_var gcov_var; >> >>>>> >> >>>>> This is more an changelog message than a comment in source file. >> >>>>> Just describe what gcov_var is. >> >>>> >> >>>> I changed this so gcov_var is no longer static, but global as before. >> >>>> >> >>>>> >> >>>>> Do you know how the size of libgcov changed with your patch? >> >>>>> Quick check of current mainline on compiling empty main gives: >> >>>>> >> >>>>> jh@gcc10:~/trunk/build/gcc$ cat t.c >> >>>>> main() >> >>>>> { >> >>>>> } >> >>>>> jh@gcc10:~/trunk/build/gcc$ ./xgcc -B ./ -O2 -fprofile-generate -o a.out-new --static t.c >> >>>>> jh@gcc10:~/trunk/build/gcc$ gcc -O2 -fprofile-generate -o a.out-old --static t.c >> >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-old >> >>>>> text data bss dec hex filename >> >>>>> 608141 3560 16728 628429 996cd a.out-old >> >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-new >> >>>>> text data bss dec hex filename >> >>>>> 612621 3688 22880 639189 9c0d5 a.out-new >> >>>>> >> >>>>> Without profiling I get: >> >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-new-no >> >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-old-no >> >>>>> text data bss dec hex filename >> >>>>> 599719 3448 12568 615735 96537 a.out-old-no >> >>>>> text data bss dec hex filename >> >>>>> 600247 3448 12568 616263 96747 a.out-new-no >> >>>>> >> >>>>> Quite big for empty program, but mostly glibc fault, I suppose >> >>>>> (that won't be an issue for embedded platforms). But anyway >> >>>>> we increased text size overhead from 8k to 12k, BSS size >> >>>>> overhead from 4k to 10k and data by another 1k. >> >>>>> >> >>>>> text data bss dec hex filename >> >>>>> 126 0 0 126 7e _gcov_merge_add.o (ex libgcov.a) >> >>>>> 251 0 0 251 fb _gcov_merge_single.o (ex libgcov.a) >> >>>>> 242 0 0 242 f2 _gcov_merge_delta.o (ex libgcov.a) >> >>>>> 126 0 0 126 7e _gcov_merge_ior.o (ex libgcov.a) >> >>>>> 156 0 0 156 9c _gcov_merge_time_profile.o (ex libgcov.a) >> >>>>> 89 0 0 89 59 _gcov_interval_profiler.o (ex libgcov.a) >> >>>>> 69 0 0 69 45 _gcov_pow2_profiler.o (ex libgcov.a) >> >>>>> 115 0 0 115 73 _gcov_one_value_profiler.o (ex libgcov.a) >> >>>>> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex libgcov.a) >> >>>>> 57 0 0 57 39 _gcov_average_profiler.o (ex libgcov.a) >> >>>>> 52 0 0 52 34 _gcov_ior_profiler.o (ex libgcov.a) >> >>>>> 178 0 16 194 c2 _gcov_indirect_call_profiler_v2.o (ex libgcov.a) >> >>>>> 77 0 8 85 55 _gcov_time_profiler.o (ex libgcov.a) >> >>>>> 126 0 40 166 a6 _gcov_flush.o (ex libgcov.a) >> >>>>> 101 0 0 101 65 _gcov_fork.o (ex libgcov.a) >> >>>>> 471 0 0 471 1d7 _gcov_execl.o (ex libgcov.a) >> >>>>> 471 0 0 471 1d7 _gcov_execlp.o (ex libgcov.a) >> >>>>> 524 0 0 524 20c _gcov_execle.o (ex libgcov.a) >> >>>>> 98 0 0 98 62 _gcov_execv.o (ex libgcov.a) >> >>>>> 98 0 0 98 62 _gcov_execvp.o (ex libgcov.a) >> >>>>> 108 0 0 108 6c _gcov_execve.o (ex libgcov.a) >> >>>>> 66 0 0 66 42 _gcov_reset.o (ex libgcov.a) >> >>>>> 66 0 0 66 42 _gcov_dump.o (ex libgcov.a) >> >>>>> 9505 0 6144 15649 3d21 _gcov.o (ex libgcov.a) >> >>>>> >> >>>>> I think we definitely need to move those 6k of bss space out. I think those are new >> >>>>> static vars you introduced that I think are unsafe anyway because multiple streaming >> >>>>> may run at once in threaded program where locking mechanizm fails. >> >>>>> (it will probably do other bad things, but definitely we do not want to >> >>>>> conflict on things like filename to write into). >> >>>>> >> >>>>> Compared to my system gcov: >> >>>>> text data bss dec hex filename >> >>>>> 9765 0 64 9829 2665 _gcov.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 125 0 0 125 7d _gcov_merge_add.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 251 0 0 251 fb _gcov_merge_single.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 242 0 0 242 f2 _gcov_merge_delta.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 101 0 0 101 65 _gcov_fork.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 471 0 0 471 1d7 _gcov_execl.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 471 0 0 471 1d7 _gcov_execlp.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 524 0 0 524 20c _gcov_execle.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 98 0 0 98 62 _gcov_execv.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 98 0 0 98 62 _gcov_execvp.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 108 0 0 108 6c _gcov_execve.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 72 0 0 72 48 _gcov_reset.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 72 0 0 72 48 _gcov_dump.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 89 0 0 89 59 _gcov_interval_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 69 0 0 69 45 _gcov_pow2_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 115 0 0 115 73 _gcov_one_value_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 57 0 0 57 39 _gcov_average_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 52 0 0 52 34 _gcov_ior_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> 125 0 0 125 7d _gcov_merge_ior.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >> >>>>> >> >>>>>> Index: gcc/gcov-io.h >> >>>>>> =================================================================== >> >>>>>> --- gcc/gcov-io.h (revision 204895) >> >>>>>> +++ gcc/gcov-io.h (working copy) >> >>>>>> @@ -164,51 +164,7 @@ see the files COPYING3 and COPYING.RUNTIME respect >> >>>>>> #ifndef GCC_GCOV_IO_H >> >>>>>> #define GCC_GCOV_IO_H >> >>>>>> >> >>>>>> -#if IN_LIBGCOV >> >>>>>> -/* About the target */ >> >>>>>> - >> >>>>>> -#if BITS_PER_UNIT == 8 >> >>>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (SI))); >> >>>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (SI))); >> >>>>>> -#if LONG_LONG_TYPE_SIZE > 32 >> >>>>>> -typedef signed gcov_type __attribute__ ((mode (DI))); >> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (DI))); >> >>>>>> -#else >> >>>>>> -typedef signed gcov_type __attribute__ ((mode (SI))); >> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); >> >>>>>> -#endif >> >>>>>> -#else >> >>>>>> -#if BITS_PER_UNIT == 16 >> >>>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (HI))); >> >>>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (HI))); >> >>>>>> -#if LONG_LONG_TYPE_SIZE > 32 >> >>>>>> -typedef signed gcov_type __attribute__ ((mode (SI))); >> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); >> >>>>>> -#else >> >>>>>> -typedef signed gcov_type __attribute__ ((mode (HI))); >> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); >> >>>>>> -#endif >> >>>>>> -#else >> >>>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (QI))); >> >>>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (QI))); >> >>>>>> -#if LONG_LONG_TYPE_SIZE > 32 >> >>>>>> -typedef signed gcov_type __attribute__ ((mode (HI))); >> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); >> >>>>>> -#else >> >>>>>> -typedef signed gcov_type __attribute__ ((mode (QI))); >> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (QI))); >> >>>>>> -#endif >> >>>>>> -#endif >> >>>>>> -#endif >> >>>>> >> >>>>> So this part basically moves libgcov specific bits into libgcov.h? That OK fine by >> >>>>> itself. >> >>>>>> Index: libgcc/libgcov-profiler.c >> >>>>>> =================================================================== >> >>>>>> --- libgcc/libgcov-profiler.c (revision 204895) >> >>>>>> +++ libgcc/libgcov-profiler.c (working copy) >> >>>>>> @@ -23,15 +23,8 @@ a copy of the GCC Runtime Library Exception along >> >>>>>> see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >> >>>>>> <http://www.gnu.org/licenses/>. */ >> >>>>>> >> >>>>>> -#include "tconfig.h" >> >>>>>> -#include "tsystem.h" >> >>>>>> -#include "coretypes.h" >> >>>>>> -#include "tm.h" >> >>>>>> -#include "libgcc_tm.h" >> >>>>>> - >> >>>>>> +#include "libgcov.h" >> >>>>>> #if !defined(inhibit_libc) >> >>>>>> -#define IN_LIBGCOV 1 >> >>>>>> -#include "gcov-io.h" >> >>>>> >> >>>>> I did not pay that much attention into the current include file changes, but wasn't >> >>>>> idea to avoid #include file to include random other #includes? >> >>>>> So perhaps the first block of includes should stay, followed by libgcov.h and gcov-io.h >> >>>>> last? >> >>>> >> >>>> I'm not sure I understand the issue here? The patch basically moves >> >>>> the same includes into libgcov.h, and includes that instead. I see >> >>>> many other header files in gcc that include other headers. >> >>>> >> >>>>>> @@ -325,6 +311,9 @@ static struct gcov_summary all_prg; >> >>>>>> #endif >> >>>>>> /* crc32 for this program. */ >> >>>>>> static gcov_unsigned_t crc32; >> >>>>>> +/* Use this summary checksum rather the computed one if the value is >> >>>>>> + * non-zero. */ >> >>>>>> +static gcov_unsigned_t saved_summary_checksum; >> >>>>> >> >>>>> Why do you need to save the checksum? Won't it reset summary back with multiple streaming? >> >>>> >> >>>> This has been removed. >> >>>> >> >>>>> >> >>>>> I would really like to avoid introducing those static vars that are used exclusively >> >>>>> by gcov_exit. What about putting them into an gcov_context structure that >> >>>>> is passed around the functions that was broken out? >> >>>>>> Index: libgcc/libgcov-merge.c >> >>>>>> =================================================================== >> >>>>>> --- libgcc/libgcov-merge.c (revision 204895) >> >>>>>> +++ libgcc/libgcov-merge.c (working copy) >> >>>>>> @@ -23,107 +23,150 @@ a copy of the GCC Runtime Library Exception along >> >>>>>> see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >> >>>>>> <http://www.gnu.org/licenses/>. */ >> >>>>>> >> >>>>>> -#include "tconfig.h" >> >>>>>> -#include "tsystem.h" >> >>>>>> -#include "coretypes.h" >> >>>>>> -#include "tm.h" >> >>>>>> -#include "libgcc_tm.h" >> >>>>>> +#include "libgcov.h" >> >>>>>> >> >>>>>> -#if defined(inhibit_libc) >> >>>>>> -#define IN_LIBGCOV (-1) >> >>>>>> -#else >> >>>>>> -#define IN_LIBGCOV 1 >> >>>>>> +#include "gcov-io-libgcov.h" >> >>>>>> #endif >> >>>>>> >> >>>>>> -#include "gcov-io.h" >> >>>>>> - >> >>>>>> #if defined(inhibit_libc) >> >>>>>> /* If libc and its header files are not available, provide dummy functions. */ >> >>>>>> >> >>>>>> #ifdef L_gcov_merge_add >> >>>>>> void __gcov_merge_add (gcov_type *counters __attribute__ ((unused)), >> >>>>>> - unsigned n_counters __attribute__ ((unused))) {} >> >>>>>> + unsigned n_counters __attribute__ ((unused)), >> >>>>>> + unsigned gcov_type *src __attribute__ ((unused)), >> >>>>>> + unsigned w __attribute__ ((unused))) {} >> >>>>>> #endif >> >>>>>> >> >>>>>> #ifdef L_gcov_merge_single >> >>>>>> void __gcov_merge_single (gcov_type *counters __attribute__ ((unused)), >> >>>>>> - unsigned n_counters __attribute__ ((unused))) {} >> >>>>>> + unsigned n_counters __attribute__ ((unused)), >> >>>>>> + unsigned gcov_type *src __attribute__ ((unused)), >> >>>>>> + unsigned w __attribute__ ((unused))) {} >> >>>>>> #endif >> >>>>>> >> >>>>>> #ifdef L_gcov_merge_delta >> >>>>>> void __gcov_merge_delta (gcov_type *counters __attribute__ ((unused)), >> >>>>>> - unsigned n_counters __attribute__ ((unused))) {} >> >>>>>> + unsigned n_counters __attribute__ ((unused)), >> >>>>>> + unsigned gcov_type *src __attribute__ ((unused)), >> >>>>>> + unsigned w __attribute__ ((unused))) {} >> >>>>>> #endif >> >>>>>> >> >>>>>> #else >> >>>>>> >> >>>>>> #ifdef L_gcov_merge_add >> >>>>>> /* The profile merging function that just adds the counters. It is given >> >>>>>> - an array COUNTERS of N_COUNTERS old counters and it reads the same number >> >>>>>> - of counters from the gcov file. */ >> >>>>>> + an array COUNTERS of N_COUNTERS old counters. >> >>>>>> + When SRC==NULL, it reads the same number of counters from the gcov file. >> >>>>>> + Otherwise, it reads from SRC array. These read values will be multiplied >> >>>>>> + by weight W before adding to the old counters. */ >> >>>>>> void >> >>>>>> -__gcov_merge_add (gcov_type *counters, unsigned n_counters) >> >>>>>> +__gcov_merge_add (gcov_type *counters, unsigned n_counters, >> >>>>>> + gcov_type *src, unsigned w) >> >>>>>> { >> >>>>>> + int in_mem = (src != 0); >> >>>>>> + >> >>>>>> for (; n_counters; counters++, n_counters--) >> >>>>>> - *counters += gcov_read_counter (); >> >>>>>> + { >> >>>>>> + gcov_type value; >> >>>>>> + >> >>>>>> + if (in_mem) >> >>>>>> + value = *(src++); >> >>>>>> + else >> >>>>>> + value = gcov_read_counter (); >> >>>>>> + >> >>>>>> + *counters += w * value; >> >>>>>> + } >> >>>>>> } >> >>>>>> #endif /* L_gcov_merge_add */ >> >>>>>> >> >>>>>> #ifdef L_gcov_merge_ior >> >>>>>> /* The profile merging function that just adds the counters. It is given >> >>>>>> - an array COUNTERS of N_COUNTERS old counters and it reads the same number >> >>>>>> - of counters from the gcov file. */ >> >>>>>> + an array COUNTERS of N_COUNTERS old counters. >> >>>>>> + When SRC==NULL, it reads the same number of counters from the gcov file. >> >>>>>> + Otherwise, it reads from SRC array. */ >> >>>>>> void >> >>>>>> -__gcov_merge_ior (gcov_type *counters, unsigned n_counters) >> >>>>>> +__gcov_merge_ior (gcov_type *counters, unsigned n_counters, >> >>>>>> + gcov_type *src, unsigned w __attribute__ ((unused))) >> >>>>> >> >>>>> So the new in-memory variants are introduced for merging tool, while libgcc use gcov_read_counter >> >>>>> interface? >> >>>>> Perhaps we can actually just duplicate the functions to avoid runtime to do all the scalling >> >>>>> and in_mem tests it won't need? >> >>>>> >> >>>>> I would suggest going with libgcov.h changes and clenaups first, with interface changes next >> >>>>> and the gcov-tool is probably quite obvious at the end? >> >>>>> Do you think you can split the patch this way? >> >>>>> >> >>>>> Thanks and sorry for taking long to review. I should have more time again now. >> >>>>> Honza >> >>>> >> >>>> The libgcov.h related changes are in the attached patch. I think it >> >>>> addresses your concerns. Bootstrapped and tested on >> >>>> x86-64-unknown-linux-gnu. A profiledbootstrap is in progress. >> >>>> >> >>>> Ok for trunk if profiledbootstrap passes? >> >>> >> >>> Both a profiledbootstrap and LTO profiledbootstrap pass. >> >>> >> >>> Teresa >> >>> >> >>>> >> >>>> Thanks, >> >>>> Teresa >> >>>> >> >>>> >> >>>> -- >> >>>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 >> >>> >> >>> >> >>> >> >>> -- >> >>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 >> > >> > >> > >> > -- >> > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
On Mon, Dec 16, 2013 at 2:48 PM, Xinliang David Li <davidxl@google.com> wrote: > Ok -- gcov_write_counter and gcov_write_tag_length are qualified as > low level primitives for basic gcov format and probably should be kept > in gcov-io.c. > > gcov_rewrite is petty much libgcov runtime implementation details so I > think it should be moved out. gcov_write_summary is not related to > gcov low level format either, neither is gcov_seek. Ok for them to be > moved? After looking at these some more, with the idea that gcov-io.c should encapsulate the lower level IO routines, then I think all of these (including gcov_rewrite) should remain in gcov-io.c. I think gcov_write_summary belongs there since all of the other gcov_write_* are there. And gcov_seek and gcov_rewrite are both adjusting gcov_var fields to affect the file IO operations. And there are currently no references to gcov_var within libgcc/libgcov* files. So I think we should leave the patch as-is. Honza, is the current patch ok for trunk? Thanks, Teresa > > thanks, > > David > > > On Mon, Dec 16, 2013 at 2:34 PM, Jan Hubicka <hubicka@ucw.cz> wrote: >>> I think so -- they are private to libgcov. Honza, what do you think? >> >> Hmm, the purpose of gcov-io was to be low level IO library for the basic >> gcov file format. I am not sure if gcov_write_tag_length should really resist >> in other file than gcov_write_tag. >> >> I see a desire to isolate actual stdio calls so one can have replacement driver >> for i.e. Linux kernel. For that reason things like gcov_seek and friends probably >> should be separated, but what is reason for splitting the file handling itself? >> >> Honza >>> >>> thanks, >>> >>> David >>> >>> On Mon, Dec 16, 2013 at 1:17 PM, Teresa Johnson <tejohnson@google.com> wrote: >>> > On Mon, Dec 16, 2013 at 12:55 PM, Xinliang David Li <davidxl@google.com> wrote: >>> >> gcov_rewrite function is only needed (and defined) with IN_LIBGCOV. >>> >> Should it be moved from common file gcov-io.c to libgcov.c? >>> > >>> > Possibly. I just looked through gcov-io.c and there are several >>> > additional functions that are only defined under "#ifdef IN_LIBGCOV" >>> > and only used in libgcov*c (or each other): >>> > >>> > gcov_write_counter >>> > gcov_write_tag_length >>> > gcov_write_summary >>> > gcov_seek >>> > >>> > Should they all, plus gcov_rewrite, be moved to libgcov-driver.c? >>> > >>> > Teresa >>> > >>> >> >>> >> >>> >> David >>> >> >>> >> On Thu, Dec 12, 2013 at 12:11 PM, Teresa Johnson <tejohnson@google.com> wrote: >>> >>> On Wed, Dec 11, 2013 at 10:05 PM, Teresa Johnson <tejohnson@google.com> wrote: >>> >>>> On Fri, Dec 6, 2013 at 6:23 AM, Jan Hubicka <hubicka@ucw.cz> wrote: >>> >>>>>> Hi, all >>> >>>>>> >>> >>>>>> This is the new patch for gcov-tool (previously profile-tool). >>> >>>>>> >>> >>>>>> Honza: can you comment on the new merge interface? David posted some >>> >>>>>> comments in an earlier email and we want to know what's your opinion. >>> >>>>>> >>> >>>>>> Test patch has been tested with boostrap, regresssion, >>> >>>>>> profiledbootstrap and SPEC2006. >>> >>>>>> >>> >>>>>> Noticeable changes from the earlier version: >>> >>>>>> >>> >>>>>> 1. create a new file libgcov.h and move libgcov-*.h headers to libgcov.h >>> >>>>>> So we can included multiple libgcov-*.c without adding new macros. >>> >>>>>> >>> >>>>>> 2. split libgcov.h specific code in gcvo-io.h to libcc/libgcov.h >>> >>>>>> Avoid multiple-page of code under IN_LIBGCOV macro -- this >>> >>>>>> improves the readability. >>> >>>>>> >>> >>>>>> 3. make gcov_var static, and move the definition from gcov-io.h to >>> >>>>>> gcov-io.c. Also >>> >>>>>> move some static functions accessing gcov_var to gcvo-io.c >>> >>>>>> Current code rely on GCOV_LINKAGE tricks to avoid multi-definition. I don't see >>> >>>>>> a reason that gcov_var needs to exposed as a global. >>> >>>>>> >>> >>>>>> 4. expose gcov_write_strings() and gcov_sync() to gcov_tool usage >>> >>>>>> >>> >>>>>> 5. rename profile-tool to gcov-tool per Honza's suggestion. >>> >>>>>> >>> >>>>>> Thanks, >>> >>>>> >>> >>>>> Hi, >>> >>>>> I did not read in deatil the gcov-tool source itself, but lets first make the interface changes >>> >>>>> needed. >>> >>>>> >>> >>>>>> 2013-11-18 Rong Xu <xur@google.com> >>> >>>>>> >>> >>>>>> * gcc/gcov-io.c (gcov_var): Moved from gcov-io.h and make it static. >>> >>>>>> (gcov_position): Move from gcov-io.h >>> >>>>>> (gcov_is_error): Ditto. >>> >>>>>> (gcov_rewrite): Ditto. >>> >>>>>> * gcc/gcov-io.h: Re-factoring. Move gcov_var to gcov-io.h and >>> >>>>>> move the libgcov only part of libgcc/libgcov.h. >>> >>>>>> * libgcc/libgcov.h: New common header files for libgcov-*.h >>> >>>>>> * libgcc/Makefile.in: Add dependence to libgcov.h >>> >>>>>> * libgcc/libgcov-profiler.c: Use libgcov.h >>> >>>>>> * libgcc/libgcov-driver.c: Ditto. >>> >>>>>> * libgcc/libgcov-interface.c: Ditto. >>> >>>>>> * libgcc/libgcov-driver-system.c (allocate_filename_struct): use >>> >>>>>> xmalloc instread of malloc. >>> >>>>>> * libgcc/libgcov-merge.c (void __gcov_merge_delta): Add more >>> >>>>>> parameters to merge function. >>> >>>>>> (__gcov_merge_add): Ditto. >>> >>>>>> (__gcov_merge_ior): Ditto. >>> >>>>>> (__gcov_merge_time_profile): Ditto. >>> >>>>>> (__gcov_merge_single): Ditto. >>> >>>>>> (__gcov_merge_delta): Ditto. >>> >>>>>> * libgcc/libgcov-tool.c (void gcov_tool_set_verbose): New for >>> >>>>>> gcov-tool support. >>> >>>>>> (set_fn_ctrs): Ditto. >>> >>>>>> (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_merge): Ditto. >>> >>>>>> (find_match_gcov_inf Ditto.o): >>> >>>>>> (gcov_profile_merge): Ditto. >>> >>>>>> (__gcov_scale_add): Ditto. >>> >>>>>> (__gcov_scale_ior): Ditto. >>> >>>>>> (__gcov_scale_delta): Ditto. >>> >>>>>> (__gcov_scale_single): Ditto. >>> >>>>>> (gcov_profile_scale): Ditto. >>> >>>>>> (gcov_profile_normalize): Ditto. >>> >>>>>> (__gcov_scale2_add): Ditto. >>> >>>>>> (__gcov_scale2_ior): Ditto. >>> >>>>>> (__gcov_scale2_delta): Ditto. >>> >>>>>> (__gcov_scale2_single): Ditto. >>> >>>>>> (gcov_profile_scale2): Ditto. >>> >>>>>> * gcc/gcov-tool.c (unlink_file): Gcov-tool driver support. >>> >>>>>> (unlink_dir): Ditto. >>> >>>>>> (profile_merge): Ditto. >>> >>>>>> (print_merge_usage_message): Ditto. >>> >>>>>> (merge_usage): Ditto. >>> >>>>>> (do_merge): Ditto. >>> >>>>>> (profile_rewrite2): Ditto. >>> >>>>>> (profile_rewrite): Ditto. >>> >>>>>> (print_rewrite_usage_message): Ditto. >>> >>>>>> (rewrite_usage): Ditto. >>> >>>>>> (do_rewrite): Ditto. >>> >>>>>> (print_usage): Ditto. >>> >>>>>> (print_version): Ditto. >>> >>>>>> (process_args): Ditto. >>> >>>>>> (main): Ditto. >>> >>>>>> * gcc/Makefile.in: Build and install gcov-tool. >>> >>>>> >>> >>>>>> Index: gcc/gcov-io.c >>> >>>>>> =================================================================== >>> >>>>>> --- gcc/gcov-io.c (revision 204895) >>> >>>>>> +++ gcc/gcov-io.c (working copy) >>> >>>>>> @@ -36,6 +36,37 @@ static const gcov_unsigned_t *gcov_read_words (uns >>> >>>>>> static void gcov_allocate (unsigned); >>> >>>>>> #endif >>> >>>>>> >>> >>>>>> +/* Moved for gcov-io.h and make it static. */ >>> >>>>>> +static struct gcov_var gcov_var; >>> >>>>> >>> >>>>> This is more an changelog message than a comment in source file. >>> >>>>> Just describe what gcov_var is. >>> >>>> >>> >>>> I changed this so gcov_var is no longer static, but global as before. >>> >>>> >>> >>>>> >>> >>>>> Do you know how the size of libgcov changed with your patch? >>> >>>>> Quick check of current mainline on compiling empty main gives: >>> >>>>> >>> >>>>> jh@gcc10:~/trunk/build/gcc$ cat t.c >>> >>>>> main() >>> >>>>> { >>> >>>>> } >>> >>>>> jh@gcc10:~/trunk/build/gcc$ ./xgcc -B ./ -O2 -fprofile-generate -o a.out-new --static t.c >>> >>>>> jh@gcc10:~/trunk/build/gcc$ gcc -O2 -fprofile-generate -o a.out-old --static t.c >>> >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-old >>> >>>>> text data bss dec hex filename >>> >>>>> 608141 3560 16728 628429 996cd a.out-old >>> >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-new >>> >>>>> text data bss dec hex filename >>> >>>>> 612621 3688 22880 639189 9c0d5 a.out-new >>> >>>>> >>> >>>>> Without profiling I get: >>> >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-new-no >>> >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-old-no >>> >>>>> text data bss dec hex filename >>> >>>>> 599719 3448 12568 615735 96537 a.out-old-no >>> >>>>> text data bss dec hex filename >>> >>>>> 600247 3448 12568 616263 96747 a.out-new-no >>> >>>>> >>> >>>>> Quite big for empty program, but mostly glibc fault, I suppose >>> >>>>> (that won't be an issue for embedded platforms). But anyway >>> >>>>> we increased text size overhead from 8k to 12k, BSS size >>> >>>>> overhead from 4k to 10k and data by another 1k. >>> >>>>> >>> >>>>> text data bss dec hex filename >>> >>>>> 126 0 0 126 7e _gcov_merge_add.o (ex libgcov.a) >>> >>>>> 251 0 0 251 fb _gcov_merge_single.o (ex libgcov.a) >>> >>>>> 242 0 0 242 f2 _gcov_merge_delta.o (ex libgcov.a) >>> >>>>> 126 0 0 126 7e _gcov_merge_ior.o (ex libgcov.a) >>> >>>>> 156 0 0 156 9c _gcov_merge_time_profile.o (ex libgcov.a) >>> >>>>> 89 0 0 89 59 _gcov_interval_profiler.o (ex libgcov.a) >>> >>>>> 69 0 0 69 45 _gcov_pow2_profiler.o (ex libgcov.a) >>> >>>>> 115 0 0 115 73 _gcov_one_value_profiler.o (ex libgcov.a) >>> >>>>> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex libgcov.a) >>> >>>>> 57 0 0 57 39 _gcov_average_profiler.o (ex libgcov.a) >>> >>>>> 52 0 0 52 34 _gcov_ior_profiler.o (ex libgcov.a) >>> >>>>> 178 0 16 194 c2 _gcov_indirect_call_profiler_v2.o (ex libgcov.a) >>> >>>>> 77 0 8 85 55 _gcov_time_profiler.o (ex libgcov.a) >>> >>>>> 126 0 40 166 a6 _gcov_flush.o (ex libgcov.a) >>> >>>>> 101 0 0 101 65 _gcov_fork.o (ex libgcov.a) >>> >>>>> 471 0 0 471 1d7 _gcov_execl.o (ex libgcov.a) >>> >>>>> 471 0 0 471 1d7 _gcov_execlp.o (ex libgcov.a) >>> >>>>> 524 0 0 524 20c _gcov_execle.o (ex libgcov.a) >>> >>>>> 98 0 0 98 62 _gcov_execv.o (ex libgcov.a) >>> >>>>> 98 0 0 98 62 _gcov_execvp.o (ex libgcov.a) >>> >>>>> 108 0 0 108 6c _gcov_execve.o (ex libgcov.a) >>> >>>>> 66 0 0 66 42 _gcov_reset.o (ex libgcov.a) >>> >>>>> 66 0 0 66 42 _gcov_dump.o (ex libgcov.a) >>> >>>>> 9505 0 6144 15649 3d21 _gcov.o (ex libgcov.a) >>> >>>>> >>> >>>>> I think we definitely need to move those 6k of bss space out. I think those are new >>> >>>>> static vars you introduced that I think are unsafe anyway because multiple streaming >>> >>>>> may run at once in threaded program where locking mechanizm fails. >>> >>>>> (it will probably do other bad things, but definitely we do not want to >>> >>>>> conflict on things like filename to write into). >>> >>>>> >>> >>>>> Compared to my system gcov: >>> >>>>> text data bss dec hex filename >>> >>>>> 9765 0 64 9829 2665 _gcov.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 125 0 0 125 7d _gcov_merge_add.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 251 0 0 251 fb _gcov_merge_single.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 242 0 0 242 f2 _gcov_merge_delta.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 101 0 0 101 65 _gcov_fork.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 471 0 0 471 1d7 _gcov_execl.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 471 0 0 471 1d7 _gcov_execlp.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 524 0 0 524 20c _gcov_execle.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 98 0 0 98 62 _gcov_execv.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 98 0 0 98 62 _gcov_execvp.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 108 0 0 108 6c _gcov_execve.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 72 0 0 72 48 _gcov_reset.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 72 0 0 72 48 _gcov_dump.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 89 0 0 89 59 _gcov_interval_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 69 0 0 69 45 _gcov_pow2_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 115 0 0 115 73 _gcov_one_value_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 57 0 0 57 39 _gcov_average_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 52 0 0 52 34 _gcov_ior_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> 125 0 0 125 7d _gcov_merge_ior.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>> >>>>> >>> >>>>>> Index: gcc/gcov-io.h >>> >>>>>> =================================================================== >>> >>>>>> --- gcc/gcov-io.h (revision 204895) >>> >>>>>> +++ gcc/gcov-io.h (working copy) >>> >>>>>> @@ -164,51 +164,7 @@ see the files COPYING3 and COPYING.RUNTIME respect >>> >>>>>> #ifndef GCC_GCOV_IO_H >>> >>>>>> #define GCC_GCOV_IO_H >>> >>>>>> >>> >>>>>> -#if IN_LIBGCOV >>> >>>>>> -/* About the target */ >>> >>>>>> - >>> >>>>>> -#if BITS_PER_UNIT == 8 >>> >>>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (SI))); >>> >>>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (SI))); >>> >>>>>> -#if LONG_LONG_TYPE_SIZE > 32 >>> >>>>>> -typedef signed gcov_type __attribute__ ((mode (DI))); >>> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (DI))); >>> >>>>>> -#else >>> >>>>>> -typedef signed gcov_type __attribute__ ((mode (SI))); >>> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); >>> >>>>>> -#endif >>> >>>>>> -#else >>> >>>>>> -#if BITS_PER_UNIT == 16 >>> >>>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (HI))); >>> >>>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (HI))); >>> >>>>>> -#if LONG_LONG_TYPE_SIZE > 32 >>> >>>>>> -typedef signed gcov_type __attribute__ ((mode (SI))); >>> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); >>> >>>>>> -#else >>> >>>>>> -typedef signed gcov_type __attribute__ ((mode (HI))); >>> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); >>> >>>>>> -#endif >>> >>>>>> -#else >>> >>>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (QI))); >>> >>>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (QI))); >>> >>>>>> -#if LONG_LONG_TYPE_SIZE > 32 >>> >>>>>> -typedef signed gcov_type __attribute__ ((mode (HI))); >>> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); >>> >>>>>> -#else >>> >>>>>> -typedef signed gcov_type __attribute__ ((mode (QI))); >>> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (QI))); >>> >>>>>> -#endif >>> >>>>>> -#endif >>> >>>>>> -#endif >>> >>>>> >>> >>>>> So this part basically moves libgcov specific bits into libgcov.h? That OK fine by >>> >>>>> itself. >>> >>>>>> Index: libgcc/libgcov-profiler.c >>> >>>>>> =================================================================== >>> >>>>>> --- libgcc/libgcov-profiler.c (revision 204895) >>> >>>>>> +++ libgcc/libgcov-profiler.c (working copy) >>> >>>>>> @@ -23,15 +23,8 @@ a copy of the GCC Runtime Library Exception along >>> >>>>>> see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >>> >>>>>> <http://www.gnu.org/licenses/>. */ >>> >>>>>> >>> >>>>>> -#include "tconfig.h" >>> >>>>>> -#include "tsystem.h" >>> >>>>>> -#include "coretypes.h" >>> >>>>>> -#include "tm.h" >>> >>>>>> -#include "libgcc_tm.h" >>> >>>>>> - >>> >>>>>> +#include "libgcov.h" >>> >>>>>> #if !defined(inhibit_libc) >>> >>>>>> -#define IN_LIBGCOV 1 >>> >>>>>> -#include "gcov-io.h" >>> >>>>> >>> >>>>> I did not pay that much attention into the current include file changes, but wasn't >>> >>>>> idea to avoid #include file to include random other #includes? >>> >>>>> So perhaps the first block of includes should stay, followed by libgcov.h and gcov-io.h >>> >>>>> last? >>> >>>> >>> >>>> I'm not sure I understand the issue here? The patch basically moves >>> >>>> the same includes into libgcov.h, and includes that instead. I see >>> >>>> many other header files in gcc that include other headers. >>> >>>> >>> >>>>>> @@ -325,6 +311,9 @@ static struct gcov_summary all_prg; >>> >>>>>> #endif >>> >>>>>> /* crc32 for this program. */ >>> >>>>>> static gcov_unsigned_t crc32; >>> >>>>>> +/* Use this summary checksum rather the computed one if the value is >>> >>>>>> + * non-zero. */ >>> >>>>>> +static gcov_unsigned_t saved_summary_checksum; >>> >>>>> >>> >>>>> Why do you need to save the checksum? Won't it reset summary back with multiple streaming? >>> >>>> >>> >>>> This has been removed. >>> >>>> >>> >>>>> >>> >>>>> I would really like to avoid introducing those static vars that are used exclusively >>> >>>>> by gcov_exit. What about putting them into an gcov_context structure that >>> >>>>> is passed around the functions that was broken out? >>> >>>>>> Index: libgcc/libgcov-merge.c >>> >>>>>> =================================================================== >>> >>>>>> --- libgcc/libgcov-merge.c (revision 204895) >>> >>>>>> +++ libgcc/libgcov-merge.c (working copy) >>> >>>>>> @@ -23,107 +23,150 @@ a copy of the GCC Runtime Library Exception along >>> >>>>>> see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >>> >>>>>> <http://www.gnu.org/licenses/>. */ >>> >>>>>> >>> >>>>>> -#include "tconfig.h" >>> >>>>>> -#include "tsystem.h" >>> >>>>>> -#include "coretypes.h" >>> >>>>>> -#include "tm.h" >>> >>>>>> -#include "libgcc_tm.h" >>> >>>>>> +#include "libgcov.h" >>> >>>>>> >>> >>>>>> -#if defined(inhibit_libc) >>> >>>>>> -#define IN_LIBGCOV (-1) >>> >>>>>> -#else >>> >>>>>> -#define IN_LIBGCOV 1 >>> >>>>>> +#include "gcov-io-libgcov.h" >>> >>>>>> #endif >>> >>>>>> >>> >>>>>> -#include "gcov-io.h" >>> >>>>>> - >>> >>>>>> #if defined(inhibit_libc) >>> >>>>>> /* If libc and its header files are not available, provide dummy functions. */ >>> >>>>>> >>> >>>>>> #ifdef L_gcov_merge_add >>> >>>>>> void __gcov_merge_add (gcov_type *counters __attribute__ ((unused)), >>> >>>>>> - unsigned n_counters __attribute__ ((unused))) {} >>> >>>>>> + unsigned n_counters __attribute__ ((unused)), >>> >>>>>> + unsigned gcov_type *src __attribute__ ((unused)), >>> >>>>>> + unsigned w __attribute__ ((unused))) {} >>> >>>>>> #endif >>> >>>>>> >>> >>>>>> #ifdef L_gcov_merge_single >>> >>>>>> void __gcov_merge_single (gcov_type *counters __attribute__ ((unused)), >>> >>>>>> - unsigned n_counters __attribute__ ((unused))) {} >>> >>>>>> + unsigned n_counters __attribute__ ((unused)), >>> >>>>>> + unsigned gcov_type *src __attribute__ ((unused)), >>> >>>>>> + unsigned w __attribute__ ((unused))) {} >>> >>>>>> #endif >>> >>>>>> >>> >>>>>> #ifdef L_gcov_merge_delta >>> >>>>>> void __gcov_merge_delta (gcov_type *counters __attribute__ ((unused)), >>> >>>>>> - unsigned n_counters __attribute__ ((unused))) {} >>> >>>>>> + unsigned n_counters __attribute__ ((unused)), >>> >>>>>> + unsigned gcov_type *src __attribute__ ((unused)), >>> >>>>>> + unsigned w __attribute__ ((unused))) {} >>> >>>>>> #endif >>> >>>>>> >>> >>>>>> #else >>> >>>>>> >>> >>>>>> #ifdef L_gcov_merge_add >>> >>>>>> /* The profile merging function that just adds the counters. It is given >>> >>>>>> - an array COUNTERS of N_COUNTERS old counters and it reads the same number >>> >>>>>> - of counters from the gcov file. */ >>> >>>>>> + an array COUNTERS of N_COUNTERS old counters. >>> >>>>>> + When SRC==NULL, it reads the same number of counters from the gcov file. >>> >>>>>> + Otherwise, it reads from SRC array. These read values will be multiplied >>> >>>>>> + by weight W before adding to the old counters. */ >>> >>>>>> void >>> >>>>>> -__gcov_merge_add (gcov_type *counters, unsigned n_counters) >>> >>>>>> +__gcov_merge_add (gcov_type *counters, unsigned n_counters, >>> >>>>>> + gcov_type *src, unsigned w) >>> >>>>>> { >>> >>>>>> + int in_mem = (src != 0); >>> >>>>>> + >>> >>>>>> for (; n_counters; counters++, n_counters--) >>> >>>>>> - *counters += gcov_read_counter (); >>> >>>>>> + { >>> >>>>>> + gcov_type value; >>> >>>>>> + >>> >>>>>> + if (in_mem) >>> >>>>>> + value = *(src++); >>> >>>>>> + else >>> >>>>>> + value = gcov_read_counter (); >>> >>>>>> + >>> >>>>>> + *counters += w * value; >>> >>>>>> + } >>> >>>>>> } >>> >>>>>> #endif /* L_gcov_merge_add */ >>> >>>>>> >>> >>>>>> #ifdef L_gcov_merge_ior >>> >>>>>> /* The profile merging function that just adds the counters. It is given >>> >>>>>> - an array COUNTERS of N_COUNTERS old counters and it reads the same number >>> >>>>>> - of counters from the gcov file. */ >>> >>>>>> + an array COUNTERS of N_COUNTERS old counters. >>> >>>>>> + When SRC==NULL, it reads the same number of counters from the gcov file. >>> >>>>>> + Otherwise, it reads from SRC array. */ >>> >>>>>> void >>> >>>>>> -__gcov_merge_ior (gcov_type *counters, unsigned n_counters) >>> >>>>>> +__gcov_merge_ior (gcov_type *counters, unsigned n_counters, >>> >>>>>> + gcov_type *src, unsigned w __attribute__ ((unused))) >>> >>>>> >>> >>>>> So the new in-memory variants are introduced for merging tool, while libgcc use gcov_read_counter >>> >>>>> interface? >>> >>>>> Perhaps we can actually just duplicate the functions to avoid runtime to do all the scalling >>> >>>>> and in_mem tests it won't need? >>> >>>>> >>> >>>>> I would suggest going with libgcov.h changes and clenaups first, with interface changes next >>> >>>>> and the gcov-tool is probably quite obvious at the end? >>> >>>>> Do you think you can split the patch this way? >>> >>>>> >>> >>>>> Thanks and sorry for taking long to review. I should have more time again now. >>> >>>>> Honza >>> >>>> >>> >>>> The libgcov.h related changes are in the attached patch. I think it >>> >>>> addresses your concerns. Bootstrapped and tested on >>> >>>> x86-64-unknown-linux-gnu. A profiledbootstrap is in progress. >>> >>>> >>> >>>> Ok for trunk if profiledbootstrap passes? >>> >>> >>> >>> Both a profiledbootstrap and LTO profiledbootstrap pass. >>> >>> >>> >>> Teresa >>> >>> >>> >>>> >>> >>>> Thanks, >>> >>>> Teresa >>> >>>> >>> >>>> >>> >>>> -- >>> >>>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 >>> >>> >>> >>> >>> >>> >>> >>> -- >>> >>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 >>> > >>> > >>> > >>> > -- >>> > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
On Tue, Dec 17, 2013 at 7:48 AM, Teresa Johnson <tejohnson@google.com> wrote: > On Mon, Dec 16, 2013 at 2:48 PM, Xinliang David Li <davidxl@google.com> wrote: >> Ok -- gcov_write_counter and gcov_write_tag_length are qualified as >> low level primitives for basic gcov format and probably should be kept >> in gcov-io.c. >> >> gcov_rewrite is petty much libgcov runtime implementation details so I >> think it should be moved out. gcov_write_summary is not related to >> gcov low level format either, neither is gcov_seek. Ok for them to be >> moved? > > After looking at these some more, with the idea that gcov-io.c should > encapsulate the lower level IO routines, then I think all of these > (including gcov_rewrite) should remain in gcov-io.c. I think > gcov_write_summary belongs there since all of the other gcov_write_* > are there. And gcov_seek and gcov_rewrite are both adjusting gcov_var > fields to affect the file IO operations. And there are currently no > references to gcov_var within libgcc/libgcov* files. > > So I think we should leave the patch as-is. Sounds fine to me. David > Honza, is the current > patch ok for trunk? > > Thanks, > Teresa > >> >> thanks, >> >> David >> >> >> On Mon, Dec 16, 2013 at 2:34 PM, Jan Hubicka <hubicka@ucw.cz> wrote: >>>> I think so -- they are private to libgcov. Honza, what do you think? >>> >>> Hmm, the purpose of gcov-io was to be low level IO library for the basic >>> gcov file format. I am not sure if gcov_write_tag_length should really resist >>> in other file than gcov_write_tag. >>> >>> I see a desire to isolate actual stdio calls so one can have replacement driver >>> for i.e. Linux kernel. For that reason things like gcov_seek and friends probably >>> should be separated, but what is reason for splitting the file handling itself? >>> >>> Honza >>>> >>>> thanks, >>>> >>>> David >>>> >>>> On Mon, Dec 16, 2013 at 1:17 PM, Teresa Johnson <tejohnson@google.com> wrote: >>>> > On Mon, Dec 16, 2013 at 12:55 PM, Xinliang David Li <davidxl@google.com> wrote: >>>> >> gcov_rewrite function is only needed (and defined) with IN_LIBGCOV. >>>> >> Should it be moved from common file gcov-io.c to libgcov.c? >>>> > >>>> > Possibly. I just looked through gcov-io.c and there are several >>>> > additional functions that are only defined under "#ifdef IN_LIBGCOV" >>>> > and only used in libgcov*c (or each other): >>>> > >>>> > gcov_write_counter >>>> > gcov_write_tag_length >>>> > gcov_write_summary >>>> > gcov_seek >>>> > >>>> > Should they all, plus gcov_rewrite, be moved to libgcov-driver.c? >>>> > >>>> > Teresa >>>> > >>>> >> >>>> >> >>>> >> David >>>> >> >>>> >> On Thu, Dec 12, 2013 at 12:11 PM, Teresa Johnson <tejohnson@google.com> wrote: >>>> >>> On Wed, Dec 11, 2013 at 10:05 PM, Teresa Johnson <tejohnson@google.com> wrote: >>>> >>>> On Fri, Dec 6, 2013 at 6:23 AM, Jan Hubicka <hubicka@ucw.cz> wrote: >>>> >>>>>> Hi, all >>>> >>>>>> >>>> >>>>>> This is the new patch for gcov-tool (previously profile-tool). >>>> >>>>>> >>>> >>>>>> Honza: can you comment on the new merge interface? David posted some >>>> >>>>>> comments in an earlier email and we want to know what's your opinion. >>>> >>>>>> >>>> >>>>>> Test patch has been tested with boostrap, regresssion, >>>> >>>>>> profiledbootstrap and SPEC2006. >>>> >>>>>> >>>> >>>>>> Noticeable changes from the earlier version: >>>> >>>>>> >>>> >>>>>> 1. create a new file libgcov.h and move libgcov-*.h headers to libgcov.h >>>> >>>>>> So we can included multiple libgcov-*.c without adding new macros. >>>> >>>>>> >>>> >>>>>> 2. split libgcov.h specific code in gcvo-io.h to libcc/libgcov.h >>>> >>>>>> Avoid multiple-page of code under IN_LIBGCOV macro -- this >>>> >>>>>> improves the readability. >>>> >>>>>> >>>> >>>>>> 3. make gcov_var static, and move the definition from gcov-io.h to >>>> >>>>>> gcov-io.c. Also >>>> >>>>>> move some static functions accessing gcov_var to gcvo-io.c >>>> >>>>>> Current code rely on GCOV_LINKAGE tricks to avoid multi-definition. I don't see >>>> >>>>>> a reason that gcov_var needs to exposed as a global. >>>> >>>>>> >>>> >>>>>> 4. expose gcov_write_strings() and gcov_sync() to gcov_tool usage >>>> >>>>>> >>>> >>>>>> 5. rename profile-tool to gcov-tool per Honza's suggestion. >>>> >>>>>> >>>> >>>>>> Thanks, >>>> >>>>> >>>> >>>>> Hi, >>>> >>>>> I did not read in deatil the gcov-tool source itself, but lets first make the interface changes >>>> >>>>> needed. >>>> >>>>> >>>> >>>>>> 2013-11-18 Rong Xu <xur@google.com> >>>> >>>>>> >>>> >>>>>> * gcc/gcov-io.c (gcov_var): Moved from gcov-io.h and make it static. >>>> >>>>>> (gcov_position): Move from gcov-io.h >>>> >>>>>> (gcov_is_error): Ditto. >>>> >>>>>> (gcov_rewrite): Ditto. >>>> >>>>>> * gcc/gcov-io.h: Re-factoring. Move gcov_var to gcov-io.h and >>>> >>>>>> move the libgcov only part of libgcc/libgcov.h. >>>> >>>>>> * libgcc/libgcov.h: New common header files for libgcov-*.h >>>> >>>>>> * libgcc/Makefile.in: Add dependence to libgcov.h >>>> >>>>>> * libgcc/libgcov-profiler.c: Use libgcov.h >>>> >>>>>> * libgcc/libgcov-driver.c: Ditto. >>>> >>>>>> * libgcc/libgcov-interface.c: Ditto. >>>> >>>>>> * libgcc/libgcov-driver-system.c (allocate_filename_struct): use >>>> >>>>>> xmalloc instread of malloc. >>>> >>>>>> * libgcc/libgcov-merge.c (void __gcov_merge_delta): Add more >>>> >>>>>> parameters to merge function. >>>> >>>>>> (__gcov_merge_add): Ditto. >>>> >>>>>> (__gcov_merge_ior): Ditto. >>>> >>>>>> (__gcov_merge_time_profile): Ditto. >>>> >>>>>> (__gcov_merge_single): Ditto. >>>> >>>>>> (__gcov_merge_delta): Ditto. >>>> >>>>>> * libgcc/libgcov-tool.c (void gcov_tool_set_verbose): New for >>>> >>>>>> gcov-tool support. >>>> >>>>>> (set_fn_ctrs): Ditto. >>>> >>>>>> (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_merge): Ditto. >>>> >>>>>> (find_match_gcov_inf Ditto.o): >>>> >>>>>> (gcov_profile_merge): Ditto. >>>> >>>>>> (__gcov_scale_add): Ditto. >>>> >>>>>> (__gcov_scale_ior): Ditto. >>>> >>>>>> (__gcov_scale_delta): Ditto. >>>> >>>>>> (__gcov_scale_single): Ditto. >>>> >>>>>> (gcov_profile_scale): Ditto. >>>> >>>>>> (gcov_profile_normalize): Ditto. >>>> >>>>>> (__gcov_scale2_add): Ditto. >>>> >>>>>> (__gcov_scale2_ior): Ditto. >>>> >>>>>> (__gcov_scale2_delta): Ditto. >>>> >>>>>> (__gcov_scale2_single): Ditto. >>>> >>>>>> (gcov_profile_scale2): Ditto. >>>> >>>>>> * gcc/gcov-tool.c (unlink_file): Gcov-tool driver support. >>>> >>>>>> (unlink_dir): Ditto. >>>> >>>>>> (profile_merge): Ditto. >>>> >>>>>> (print_merge_usage_message): Ditto. >>>> >>>>>> (merge_usage): Ditto. >>>> >>>>>> (do_merge): Ditto. >>>> >>>>>> (profile_rewrite2): Ditto. >>>> >>>>>> (profile_rewrite): Ditto. >>>> >>>>>> (print_rewrite_usage_message): Ditto. >>>> >>>>>> (rewrite_usage): Ditto. >>>> >>>>>> (do_rewrite): Ditto. >>>> >>>>>> (print_usage): Ditto. >>>> >>>>>> (print_version): Ditto. >>>> >>>>>> (process_args): Ditto. >>>> >>>>>> (main): Ditto. >>>> >>>>>> * gcc/Makefile.in: Build and install gcov-tool. >>>> >>>>> >>>> >>>>>> Index: gcc/gcov-io.c >>>> >>>>>> =================================================================== >>>> >>>>>> --- gcc/gcov-io.c (revision 204895) >>>> >>>>>> +++ gcc/gcov-io.c (working copy) >>>> >>>>>> @@ -36,6 +36,37 @@ static const gcov_unsigned_t *gcov_read_words (uns >>>> >>>>>> static void gcov_allocate (unsigned); >>>> >>>>>> #endif >>>> >>>>>> >>>> >>>>>> +/* Moved for gcov-io.h and make it static. */ >>>> >>>>>> +static struct gcov_var gcov_var; >>>> >>>>> >>>> >>>>> This is more an changelog message than a comment in source file. >>>> >>>>> Just describe what gcov_var is. >>>> >>>> >>>> >>>> I changed this so gcov_var is no longer static, but global as before. >>>> >>>> >>>> >>>>> >>>> >>>>> Do you know how the size of libgcov changed with your patch? >>>> >>>>> Quick check of current mainline on compiling empty main gives: >>>> >>>>> >>>> >>>>> jh@gcc10:~/trunk/build/gcc$ cat t.c >>>> >>>>> main() >>>> >>>>> { >>>> >>>>> } >>>> >>>>> jh@gcc10:~/trunk/build/gcc$ ./xgcc -B ./ -O2 -fprofile-generate -o a.out-new --static t.c >>>> >>>>> jh@gcc10:~/trunk/build/gcc$ gcc -O2 -fprofile-generate -o a.out-old --static t.c >>>> >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-old >>>> >>>>> text data bss dec hex filename >>>> >>>>> 608141 3560 16728 628429 996cd a.out-old >>>> >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-new >>>> >>>>> text data bss dec hex filename >>>> >>>>> 612621 3688 22880 639189 9c0d5 a.out-new >>>> >>>>> >>>> >>>>> Without profiling I get: >>>> >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-new-no >>>> >>>>> jh@gcc10:~/trunk/build/gcc$ size a.out-old-no >>>> >>>>> text data bss dec hex filename >>>> >>>>> 599719 3448 12568 615735 96537 a.out-old-no >>>> >>>>> text data bss dec hex filename >>>> >>>>> 600247 3448 12568 616263 96747 a.out-new-no >>>> >>>>> >>>> >>>>> Quite big for empty program, but mostly glibc fault, I suppose >>>> >>>>> (that won't be an issue for embedded platforms). But anyway >>>> >>>>> we increased text size overhead from 8k to 12k, BSS size >>>> >>>>> overhead from 4k to 10k and data by another 1k. >>>> >>>>> >>>> >>>>> text data bss dec hex filename >>>> >>>>> 126 0 0 126 7e _gcov_merge_add.o (ex libgcov.a) >>>> >>>>> 251 0 0 251 fb _gcov_merge_single.o (ex libgcov.a) >>>> >>>>> 242 0 0 242 f2 _gcov_merge_delta.o (ex libgcov.a) >>>> >>>>> 126 0 0 126 7e _gcov_merge_ior.o (ex libgcov.a) >>>> >>>>> 156 0 0 156 9c _gcov_merge_time_profile.o (ex libgcov.a) >>>> >>>>> 89 0 0 89 59 _gcov_interval_profiler.o (ex libgcov.a) >>>> >>>>> 69 0 0 69 45 _gcov_pow2_profiler.o (ex libgcov.a) >>>> >>>>> 115 0 0 115 73 _gcov_one_value_profiler.o (ex libgcov.a) >>>> >>>>> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex libgcov.a) >>>> >>>>> 57 0 0 57 39 _gcov_average_profiler.o (ex libgcov.a) >>>> >>>>> 52 0 0 52 34 _gcov_ior_profiler.o (ex libgcov.a) >>>> >>>>> 178 0 16 194 c2 _gcov_indirect_call_profiler_v2.o (ex libgcov.a) >>>> >>>>> 77 0 8 85 55 _gcov_time_profiler.o (ex libgcov.a) >>>> >>>>> 126 0 40 166 a6 _gcov_flush.o (ex libgcov.a) >>>> >>>>> 101 0 0 101 65 _gcov_fork.o (ex libgcov.a) >>>> >>>>> 471 0 0 471 1d7 _gcov_execl.o (ex libgcov.a) >>>> >>>>> 471 0 0 471 1d7 _gcov_execlp.o (ex libgcov.a) >>>> >>>>> 524 0 0 524 20c _gcov_execle.o (ex libgcov.a) >>>> >>>>> 98 0 0 98 62 _gcov_execv.o (ex libgcov.a) >>>> >>>>> 98 0 0 98 62 _gcov_execvp.o (ex libgcov.a) >>>> >>>>> 108 0 0 108 6c _gcov_execve.o (ex libgcov.a) >>>> >>>>> 66 0 0 66 42 _gcov_reset.o (ex libgcov.a) >>>> >>>>> 66 0 0 66 42 _gcov_dump.o (ex libgcov.a) >>>> >>>>> 9505 0 6144 15649 3d21 _gcov.o (ex libgcov.a) >>>> >>>>> >>>> >>>>> I think we definitely need to move those 6k of bss space out. I think those are new >>>> >>>>> static vars you introduced that I think are unsafe anyway because multiple streaming >>>> >>>>> may run at once in threaded program where locking mechanizm fails. >>>> >>>>> (it will probably do other bad things, but definitely we do not want to >>>> >>>>> conflict on things like filename to write into). >>>> >>>>> >>>> >>>>> Compared to my system gcov: >>>> >>>>> text data bss dec hex filename >>>> >>>>> 9765 0 64 9829 2665 _gcov.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 125 0 0 125 7d _gcov_merge_add.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 251 0 0 251 fb _gcov_merge_single.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 242 0 0 242 f2 _gcov_merge_delta.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 101 0 0 101 65 _gcov_fork.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 471 0 0 471 1d7 _gcov_execl.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 471 0 0 471 1d7 _gcov_execlp.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 524 0 0 524 20c _gcov_execle.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 98 0 0 98 62 _gcov_execv.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 98 0 0 98 62 _gcov_execvp.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 108 0 0 108 6c _gcov_execve.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 72 0 0 72 48 _gcov_reset.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 72 0 0 72 48 _gcov_dump.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 89 0 0 89 59 _gcov_interval_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 69 0 0 69 45 _gcov_pow2_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 115 0 0 115 73 _gcov_one_value_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 122 0 0 122 7a _gcov_indirect_call_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 57 0 0 57 39 _gcov_average_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 52 0 0 52 34 _gcov_ior_profiler.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> 125 0 0 125 7d _gcov_merge_ior.o (ex ./lib64/gcc/x86_64-suse-linux/4.8/libgcov.a) >>>> >>>>> >>>> >>>>>> Index: gcc/gcov-io.h >>>> >>>>>> =================================================================== >>>> >>>>>> --- gcc/gcov-io.h (revision 204895) >>>> >>>>>> +++ gcc/gcov-io.h (working copy) >>>> >>>>>> @@ -164,51 +164,7 @@ see the files COPYING3 and COPYING.RUNTIME respect >>>> >>>>>> #ifndef GCC_GCOV_IO_H >>>> >>>>>> #define GCC_GCOV_IO_H >>>> >>>>>> >>>> >>>>>> -#if IN_LIBGCOV >>>> >>>>>> -/* About the target */ >>>> >>>>>> - >>>> >>>>>> -#if BITS_PER_UNIT == 8 >>>> >>>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (SI))); >>>> >>>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (SI))); >>>> >>>>>> -#if LONG_LONG_TYPE_SIZE > 32 >>>> >>>>>> -typedef signed gcov_type __attribute__ ((mode (DI))); >>>> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (DI))); >>>> >>>>>> -#else >>>> >>>>>> -typedef signed gcov_type __attribute__ ((mode (SI))); >>>> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); >>>> >>>>>> -#endif >>>> >>>>>> -#else >>>> >>>>>> -#if BITS_PER_UNIT == 16 >>>> >>>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (HI))); >>>> >>>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (HI))); >>>> >>>>>> -#if LONG_LONG_TYPE_SIZE > 32 >>>> >>>>>> -typedef signed gcov_type __attribute__ ((mode (SI))); >>>> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); >>>> >>>>>> -#else >>>> >>>>>> -typedef signed gcov_type __attribute__ ((mode (HI))); >>>> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); >>>> >>>>>> -#endif >>>> >>>>>> -#else >>>> >>>>>> -typedef unsigned gcov_unsigned_t __attribute__ ((mode (QI))); >>>> >>>>>> -typedef unsigned gcov_position_t __attribute__ ((mode (QI))); >>>> >>>>>> -#if LONG_LONG_TYPE_SIZE > 32 >>>> >>>>>> -typedef signed gcov_type __attribute__ ((mode (HI))); >>>> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); >>>> >>>>>> -#else >>>> >>>>>> -typedef signed gcov_type __attribute__ ((mode (QI))); >>>> >>>>>> -typedef unsigned gcov_type_unsigned __attribute__ ((mode (QI))); >>>> >>>>>> -#endif >>>> >>>>>> -#endif >>>> >>>>>> -#endif >>>> >>>>> >>>> >>>>> So this part basically moves libgcov specific bits into libgcov.h? That OK fine by >>>> >>>>> itself. >>>> >>>>>> Index: libgcc/libgcov-profiler.c >>>> >>>>>> =================================================================== >>>> >>>>>> --- libgcc/libgcov-profiler.c (revision 204895) >>>> >>>>>> +++ libgcc/libgcov-profiler.c (working copy) >>>> >>>>>> @@ -23,15 +23,8 @@ a copy of the GCC Runtime Library Exception along >>>> >>>>>> see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >>>> >>>>>> <http://www.gnu.org/licenses/>. */ >>>> >>>>>> >>>> >>>>>> -#include "tconfig.h" >>>> >>>>>> -#include "tsystem.h" >>>> >>>>>> -#include "coretypes.h" >>>> >>>>>> -#include "tm.h" >>>> >>>>>> -#include "libgcc_tm.h" >>>> >>>>>> - >>>> >>>>>> +#include "libgcov.h" >>>> >>>>>> #if !defined(inhibit_libc) >>>> >>>>>> -#define IN_LIBGCOV 1 >>>> >>>>>> -#include "gcov-io.h" >>>> >>>>> >>>> >>>>> I did not pay that much attention into the current include file changes, but wasn't >>>> >>>>> idea to avoid #include file to include random other #includes? >>>> >>>>> So perhaps the first block of includes should stay, followed by libgcov.h and gcov-io.h >>>> >>>>> last? >>>> >>>> >>>> >>>> I'm not sure I understand the issue here? The patch basically moves >>>> >>>> the same includes into libgcov.h, and includes that instead. I see >>>> >>>> many other header files in gcc that include other headers. >>>> >>>> >>>> >>>>>> @@ -325,6 +311,9 @@ static struct gcov_summary all_prg; >>>> >>>>>> #endif >>>> >>>>>> /* crc32 for this program. */ >>>> >>>>>> static gcov_unsigned_t crc32; >>>> >>>>>> +/* Use this summary checksum rather the computed one if the value is >>>> >>>>>> + * non-zero. */ >>>> >>>>>> +static gcov_unsigned_t saved_summary_checksum; >>>> >>>>> >>>> >>>>> Why do you need to save the checksum? Won't it reset summary back with multiple streaming? >>>> >>>> >>>> >>>> This has been removed. >>>> >>>> >>>> >>>>> >>>> >>>>> I would really like to avoid introducing those static vars that are used exclusively >>>> >>>>> by gcov_exit. What about putting them into an gcov_context structure that >>>> >>>>> is passed around the functions that was broken out? >>>> >>>>>> Index: libgcc/libgcov-merge.c >>>> >>>>>> =================================================================== >>>> >>>>>> --- libgcc/libgcov-merge.c (revision 204895) >>>> >>>>>> +++ libgcc/libgcov-merge.c (working copy) >>>> >>>>>> @@ -23,107 +23,150 @@ a copy of the GCC Runtime Library Exception along >>>> >>>>>> see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >>>> >>>>>> <http://www.gnu.org/licenses/>. */ >>>> >>>>>> >>>> >>>>>> -#include "tconfig.h" >>>> >>>>>> -#include "tsystem.h" >>>> >>>>>> -#include "coretypes.h" >>>> >>>>>> -#include "tm.h" >>>> >>>>>> -#include "libgcc_tm.h" >>>> >>>>>> +#include "libgcov.h" >>>> >>>>>> >>>> >>>>>> -#if defined(inhibit_libc) >>>> >>>>>> -#define IN_LIBGCOV (-1) >>>> >>>>>> -#else >>>> >>>>>> -#define IN_LIBGCOV 1 >>>> >>>>>> +#include "gcov-io-libgcov.h" >>>> >>>>>> #endif >>>> >>>>>> >>>> >>>>>> -#include "gcov-io.h" >>>> >>>>>> - >>>> >>>>>> #if defined(inhibit_libc) >>>> >>>>>> /* If libc and its header files are not available, provide dummy functions. */ >>>> >>>>>> >>>> >>>>>> #ifdef L_gcov_merge_add >>>> >>>>>> void __gcov_merge_add (gcov_type *counters __attribute__ ((unused)), >>>> >>>>>> - unsigned n_counters __attribute__ ((unused))) {} >>>> >>>>>> + unsigned n_counters __attribute__ ((unused)), >>>> >>>>>> + unsigned gcov_type *src __attribute__ ((unused)), >>>> >>>>>> + unsigned w __attribute__ ((unused))) {} >>>> >>>>>> #endif >>>> >>>>>> >>>> >>>>>> #ifdef L_gcov_merge_single >>>> >>>>>> void __gcov_merge_single (gcov_type *counters __attribute__ ((unused)), >>>> >>>>>> - unsigned n_counters __attribute__ ((unused))) {} >>>> >>>>>> + unsigned n_counters __attribute__ ((unused)), >>>> >>>>>> + unsigned gcov_type *src __attribute__ ((unused)), >>>> >>>>>> + unsigned w __attribute__ ((unused))) {} >>>> >>>>>> #endif >>>> >>>>>> >>>> >>>>>> #ifdef L_gcov_merge_delta >>>> >>>>>> void __gcov_merge_delta (gcov_type *counters __attribute__ ((unused)), >>>> >>>>>> - unsigned n_counters __attribute__ ((unused))) {} >>>> >>>>>> + unsigned n_counters __attribute__ ((unused)), >>>> >>>>>> + unsigned gcov_type *src __attribute__ ((unused)), >>>> >>>>>> + unsigned w __attribute__ ((unused))) {} >>>> >>>>>> #endif >>>> >>>>>> >>>> >>>>>> #else >>>> >>>>>> >>>> >>>>>> #ifdef L_gcov_merge_add >>>> >>>>>> /* The profile merging function that just adds the counters. It is given >>>> >>>>>> - an array COUNTERS of N_COUNTERS old counters and it reads the same number >>>> >>>>>> - of counters from the gcov file. */ >>>> >>>>>> + an array COUNTERS of N_COUNTERS old counters. >>>> >>>>>> + When SRC==NULL, it reads the same number of counters from the gcov file. >>>> >>>>>> + Otherwise, it reads from SRC array. These read values will be multiplied >>>> >>>>>> + by weight W before adding to the old counters. */ >>>> >>>>>> void >>>> >>>>>> -__gcov_merge_add (gcov_type *counters, unsigned n_counters) >>>> >>>>>> +__gcov_merge_add (gcov_type *counters, unsigned n_counters, >>>> >>>>>> + gcov_type *src, unsigned w) >>>> >>>>>> { >>>> >>>>>> + int in_mem = (src != 0); >>>> >>>>>> + >>>> >>>>>> for (; n_counters; counters++, n_counters--) >>>> >>>>>> - *counters += gcov_read_counter (); >>>> >>>>>> + { >>>> >>>>>> + gcov_type value; >>>> >>>>>> + >>>> >>>>>> + if (in_mem) >>>> >>>>>> + value = *(src++); >>>> >>>>>> + else >>>> >>>>>> + value = gcov_read_counter (); >>>> >>>>>> + >>>> >>>>>> + *counters += w * value; >>>> >>>>>> + } >>>> >>>>>> } >>>> >>>>>> #endif /* L_gcov_merge_add */ >>>> >>>>>> >>>> >>>>>> #ifdef L_gcov_merge_ior >>>> >>>>>> /* The profile merging function that just adds the counters. It is given >>>> >>>>>> - an array COUNTERS of N_COUNTERS old counters and it reads the same number >>>> >>>>>> - of counters from the gcov file. */ >>>> >>>>>> + an array COUNTERS of N_COUNTERS old counters. >>>> >>>>>> + When SRC==NULL, it reads the same number of counters from the gcov file. >>>> >>>>>> + Otherwise, it reads from SRC array. */ >>>> >>>>>> void >>>> >>>>>> -__gcov_merge_ior (gcov_type *counters, unsigned n_counters) >>>> >>>>>> +__gcov_merge_ior (gcov_type *counters, unsigned n_counters, >>>> >>>>>> + gcov_type *src, unsigned w __attribute__ ((unused))) >>>> >>>>> >>>> >>>>> So the new in-memory variants are introduced for merging tool, while libgcc use gcov_read_counter >>>> >>>>> interface? >>>> >>>>> Perhaps we can actually just duplicate the functions to avoid runtime to do all the scalling >>>> >>>>> and in_mem tests it won't need? >>>> >>>>> >>>> >>>>> I would suggest going with libgcov.h changes and clenaups first, with interface changes next >>>> >>>>> and the gcov-tool is probably quite obvious at the end? >>>> >>>>> Do you think you can split the patch this way? >>>> >>>>> >>>> >>>>> Thanks and sorry for taking long to review. I should have more time again now. >>>> >>>>> Honza >>>> >>>> >>>> >>>> The libgcov.h related changes are in the attached patch. I think it >>>> >>>> addresses your concerns. Bootstrapped and tested on >>>> >>>> x86-64-unknown-linux-gnu. A profiledbootstrap is in progress. >>>> >>>> >>>> >>>> Ok for trunk if profiledbootstrap passes? >>>> >>> >>>> >>> Both a profiledbootstrap and LTO profiledbootstrap pass. >>>> >>> >>>> >>> Teresa >>>> >>> >>>> >>>> >>>> >>>> Thanks, >>>> >>>> Teresa >>>> >>>> >>>> >>>> >>>> >>>> -- >>>> >>>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 >>>> >>> >>>> >>> >>>> >>> >>>> >>> -- >>>> >>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 >>>> > >>>> > >>>> > >>>> > -- >>>> > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 > > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
Index: gcc/gcov-io.c =================================================================== --- gcc/gcov-io.c (revision 205895) +++ gcc/gcov-io.c (working copy) @@ -36,6 +36,36 @@ static const gcov_unsigned_t *gcov_read_words (uns static void gcov_allocate (unsigned); #endif +GCOV_LINKAGE struct gcov_var gcov_var; + +/* Save the current position in the gcov file. */ +static inline gcov_position_t +gcov_position (void) +{ + gcc_assert (gcov_var.mode > 0); + return gcov_var.start + gcov_var.offset; +} + +/* Return nonzero if the error flag is set. */ +static inline int +gcov_is_error (void) +{ + return gcov_var.file ? gcov_var.error : 1; +} + +#if IN_LIBGCOV +/* Move to beginning of file and initialize for writing. */ +GCOV_LINKAGE inline void +gcov_rewrite (void) +{ + gcc_assert (gcov_var.mode > 0); + gcov_var.mode = -1; + gcov_var.start = 0; + gcov_var.offset = 0; + fseek (gcov_var.file, 0L, SEEK_SET); +} +#endif + static inline gcov_unsigned_t from_file (gcov_unsigned_t value) { #if !IN_LIBGCOV Index: gcc/gcov-io.h =================================================================== --- gcc/gcov-io.h (revision 205895) +++ gcc/gcov-io.h (working copy) @@ -164,51 +164,7 @@ see the files COPYING3 and COPYING.RUNTIME respect #ifndef GCC_GCOV_IO_H #define GCC_GCOV_IO_H -#if IN_LIBGCOV -/* About the target */ - -#if BITS_PER_UNIT == 8 -typedef unsigned gcov_unsigned_t __attribute__ ((mode (SI))); -typedef unsigned gcov_position_t __attribute__ ((mode (SI))); -#if LONG_LONG_TYPE_SIZE > 32 -typedef signed gcov_type __attribute__ ((mode (DI))); -typedef unsigned gcov_type_unsigned __attribute__ ((mode (DI))); -#else -typedef signed gcov_type __attribute__ ((mode (SI))); -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); -#endif -#else -#if BITS_PER_UNIT == 16 -typedef unsigned gcov_unsigned_t __attribute__ ((mode (HI))); -typedef unsigned gcov_position_t __attribute__ ((mode (HI))); -#if LONG_LONG_TYPE_SIZE > 32 -typedef signed gcov_type __attribute__ ((mode (SI))); -typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); -#else -typedef signed gcov_type __attribute__ ((mode (HI))); -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); -#endif -#else -typedef unsigned gcov_unsigned_t __attribute__ ((mode (QI))); -typedef unsigned gcov_position_t __attribute__ ((mode (QI))); -#if LONG_LONG_TYPE_SIZE > 32 -typedef signed gcov_type __attribute__ ((mode (HI))); -typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); -#else -typedef signed gcov_type __attribute__ ((mode (QI))); -typedef unsigned gcov_type_unsigned __attribute__ ((mode (QI))); -#endif -#endif -#endif - - -#if defined (TARGET_POSIX_IO) -#define GCOV_LOCKED 1 -#else -#define GCOV_LOCKED 0 -#endif - -#else /* !IN_LIBGCOV */ +#ifndef IN_LIBGCOV /* About the host */ typedef unsigned gcov_unsigned_t; @@ -231,48 +187,10 @@ typedef unsigned HOST_WIDEST_INT gcov_type_unsigne #define GCOV_LOCKED 0 #endif -#endif /* !IN_LIBGCOV */ - -/* In gcov we want function linkage to be static. In the compiler we want - it extern, so that they can be accessed from elsewhere. In libgcov we - need these functions to be extern, so prefix them with __gcov. In - libgcov they must also be hidden so that the instance in the executable - is not also used in a DSO. */ -#if IN_LIBGCOV - -#include "tconfig.h" - -#define gcov_var __gcov_var -#define gcov_open __gcov_open -#define gcov_close __gcov_close -#define gcov_write_tag_length __gcov_write_tag_length -#define gcov_position __gcov_position -#define gcov_seek __gcov_seek -#define gcov_rewrite __gcov_rewrite -#define gcov_is_error __gcov_is_error -#define gcov_write_unsigned __gcov_write_unsigned -#define gcov_write_counter __gcov_write_counter -#define gcov_write_summary __gcov_write_summary -#define gcov_read_unsigned __gcov_read_unsigned -#define gcov_read_counter __gcov_read_counter -#define gcov_read_summary __gcov_read_summary - -/* Poison these, so they don't accidentally slip in. */ -#pragma GCC poison gcov_write_string gcov_write_tag gcov_write_length -#pragma GCC poison gcov_read_string gcov_sync gcov_time gcov_magic - -#ifdef HAVE_GAS_HIDDEN -#define ATTRIBUTE_HIDDEN __attribute__ ((__visibility__ ("hidden"))) -#else #define ATTRIBUTE_HIDDEN -#endif -#else +#endif /* !IN_LIBGOCV */ -#define ATTRIBUTE_HIDDEN - -#endif - #ifndef GCOV_LINKAGE #define GCOV_LINKAGE extern #endif @@ -442,110 +360,12 @@ struct gcov_summary struct gcov_ctr_summary ctrs[GCOV_COUNTERS_SUMMABLE]; }; -/* Structures embedded in coveraged program. The structures generated - by write_profile must match these. */ +#if !defined(inhibit_libc) -#if IN_LIBGCOV -/* Information about counters for a single function. */ -struct gcov_ctr_info -{ - gcov_unsigned_t num; /* number of counters. */ - gcov_type *values; /* their values. */ -}; - -/* Information about a single function. This uses the trailing array - idiom. The number of counters is determined from the merge pointer - array in gcov_info. The key is used to detect which of a set of - comdat functions was selected -- it points to the gcov_info object - of the object file containing the selected comdat function. */ - -struct gcov_fn_info -{ - const struct gcov_info *key; /* comdat key */ - gcov_unsigned_t ident; /* unique ident of function */ - gcov_unsigned_t lineno_checksum; /* function lineo_checksum */ - gcov_unsigned_t cfg_checksum; /* function cfg checksum */ - struct gcov_ctr_info ctrs[0]; /* instrumented counters */ -}; - -/* Type of function used to merge counters. */ -typedef void (*gcov_merge_fn) (gcov_type *, gcov_unsigned_t); - -/* Information about a single object file. */ -struct gcov_info -{ - gcov_unsigned_t version; /* expected version number */ - struct gcov_info *next; /* link to next, used by libgcov */ - - gcov_unsigned_t stamp; /* uniquifying time stamp */ - const char *filename; /* output file name */ - - gcov_merge_fn merge[GCOV_COUNTERS]; /* merge functions (null for - unused) */ - - unsigned n_functions; /* number of functions */ - const struct gcov_fn_info *const *functions; /* pointer to pointers - to function information */ -}; - -/* Register a new object file module. */ -extern void __gcov_init (struct gcov_info *) ATTRIBUTE_HIDDEN; - -/* Called before fork, to avoid double counting. */ -extern void __gcov_flush (void) ATTRIBUTE_HIDDEN; - -/* Function to reset all counters to 0. */ -extern void __gcov_reset (void); - -/* Function to enable early write of profile information so far. */ -extern void __gcov_dump (void); - -/* The merge function that just sums the counters. */ -extern void __gcov_merge_add (gcov_type *, unsigned) ATTRIBUTE_HIDDEN; - -/* The merge function to choose the most common value. */ -extern void __gcov_merge_single (gcov_type *, unsigned) ATTRIBUTE_HIDDEN; - -/* The merge function to choose the most common difference between - consecutive values. */ -extern void __gcov_merge_delta (gcov_type *, unsigned) ATTRIBUTE_HIDDEN; - -/* The merge function that just ors the counters together. */ -extern void __gcov_merge_ior (gcov_type *, unsigned) ATTRIBUTE_HIDDEN; - -extern void __gcov_merge_time_profile (gcov_type *, unsigned) ATTRIBUTE_HIDDEN; - -/* The profiler functions. */ -extern void __gcov_interval_profiler (gcov_type *, gcov_type, int, unsigned); -extern void __gcov_pow2_profiler (gcov_type *, gcov_type); -extern void __gcov_one_value_profiler (gcov_type *, gcov_type); -extern void __gcov_indirect_call_profiler (gcov_type*, gcov_type, - void*, void*); -extern void __gcov_indirect_call_profiler_v2 (gcov_type, void *); -extern void __gcov_average_profiler (gcov_type *, gcov_type); -extern void __gcov_ior_profiler (gcov_type *, gcov_type); -extern void __gcov_time_profiler (gcov_type *); - -#ifndef inhibit_libc -/* The wrappers around some library functions.. */ -extern pid_t __gcov_fork (void) ATTRIBUTE_HIDDEN; -extern int __gcov_execl (const char *, char *, ...) ATTRIBUTE_HIDDEN; -extern int __gcov_execlp (const char *, char *, ...) ATTRIBUTE_HIDDEN; -extern int __gcov_execle (const char *, char *, ...) ATTRIBUTE_HIDDEN; -extern int __gcov_execv (const char *, char *const []) ATTRIBUTE_HIDDEN; -extern int __gcov_execvp (const char *, char *const []) ATTRIBUTE_HIDDEN; -extern int __gcov_execve (const char *, char *const [], char *const []) - ATTRIBUTE_HIDDEN; -#endif - -#endif /* IN_LIBGCOV */ - -#if IN_LIBGCOV >= 0 - /* Optimum number of gcov_unsigned_t's read from or written to disk. */ #define GCOV_BLOCK_SIZE (1 << 10) -GCOV_LINKAGE struct gcov_var +struct gcov_var { FILE *file; gcov_position_t start; /* Position of first byte of block */ @@ -567,7 +387,7 @@ struct gcov_summary size_t alloc; gcov_unsigned_t *buffer; #endif -} gcov_var ATTRIBUTE_HIDDEN; +}; /* Functions for reading and writing gcov files. In libgcov you can open the file for reading then writing. Elsewhere you can open the @@ -578,38 +398,20 @@ struct gcov_summary you use the functions for reading, then gcov_rewrite then the functions for writing. Your file may become corrupted if you break these invariants. */ -#if IN_LIBGCOV -GCOV_LINKAGE int gcov_open (const char */*name*/) ATTRIBUTE_HIDDEN; -#else + +#if !IN_LIBGCOV GCOV_LINKAGE int gcov_open (const char */*name*/, int /*direction*/); GCOV_LINKAGE int gcov_magic (gcov_unsigned_t, gcov_unsigned_t); #endif -GCOV_LINKAGE int gcov_close (void) ATTRIBUTE_HIDDEN; /* Available everywhere. */ -static gcov_position_t gcov_position (void); -static int gcov_is_error (void); - +GCOV_LINKAGE int gcov_close (void) ATTRIBUTE_HIDDEN; GCOV_LINKAGE gcov_unsigned_t gcov_read_unsigned (void) ATTRIBUTE_HIDDEN; GCOV_LINKAGE gcov_type gcov_read_counter (void) ATTRIBUTE_HIDDEN; GCOV_LINKAGE void gcov_read_summary (struct gcov_summary *) ATTRIBUTE_HIDDEN; - -#if IN_LIBGCOV -/* Available only in libgcov */ -GCOV_LINKAGE void gcov_write_counter (gcov_type) ATTRIBUTE_HIDDEN; -GCOV_LINKAGE void gcov_write_tag_length (gcov_unsigned_t, gcov_unsigned_t) - ATTRIBUTE_HIDDEN; -GCOV_LINKAGE void gcov_write_summary (gcov_unsigned_t /*tag*/, - const struct gcov_summary *) - ATTRIBUTE_HIDDEN; -static void gcov_rewrite (void); -GCOV_LINKAGE void gcov_seek (gcov_position_t /*position*/) ATTRIBUTE_HIDDEN; -#else -/* Available outside libgcov */ GCOV_LINKAGE const char *gcov_read_string (void); GCOV_LINKAGE void gcov_sync (gcov_position_t /*base*/, gcov_unsigned_t /*length */); -#endif #if !IN_GCOV /* Available outside gcov */ @@ -651,37 +453,6 @@ GCOV_LINKAGE void compute_working_sets (const stru GCOV_LINKAGE time_t gcov_time (void); #endif -/* Save the current position in the gcov file. */ +#endif /* !inhibit_libc */ -static inline gcov_position_t -gcov_position (void) -{ - gcc_assert (gcov_var.mode > 0); - return gcov_var.start + gcov_var.offset; -} - -/* Return nonzero if the error flag is set. */ - -static inline int -gcov_is_error (void) -{ - return gcov_var.file ? gcov_var.error : 1; -} - -#if IN_LIBGCOV -/* Move to beginning of file and initialize for writing. */ - -static inline void -gcov_rewrite (void) -{ - gcc_assert (gcov_var.mode > 0); - gcov_var.mode = -1; - gcov_var.start = 0; - gcov_var.offset = 0; - fseek (gcov_var.file, 0L, SEEK_SET); -} -#endif - -#endif /* IN_LIBGCOV >= 0 */ - #endif /* GCC_GCOV_IO_H */ Index: libgcc/libgcov-driver.c =================================================================== --- libgcc/libgcov-driver.c (revision 205895) +++ libgcc/libgcov-driver.c (working copy) @@ -23,23 +23,9 @@ a copy of the GCC Runtime Library Exception along see the files COPYING3 and COPYING.RUNTIME respectively. If not, see <http://www.gnu.org/licenses/>. */ -#include "tconfig.h" -#include "tsystem.h" -#include "coretypes.h" -#include "tm.h" -#include "libgcc_tm.h" +#include "libgcov.h" #if defined(inhibit_libc) -#define IN_LIBGCOV (-1) -#else -#define IN_LIBGCOV 1 -#if defined(L_gcov) -#define GCOV_LINKAGE /* nothing */ -#endif -#endif -#include "gcov-io.h" - -#if defined(inhibit_libc) /* If libc and its header files are not available, provide dummy functions. */ #if defined(L_gcov) @@ -156,7 +142,7 @@ buffer_fn_data (const char *filename, const struct n_ctrs++; len = sizeof (*fn_buffer) + sizeof (fn_buffer->info.ctrs[0]) * n_ctrs; - fn_buffer = (struct gcov_fn_buffer *)malloc (len); + fn_buffer = (struct gcov_fn_buffer *) xmalloc (len); if (!fn_buffer) goto fail; @@ -183,7 +169,7 @@ buffer_fn_data (const char *filename, const struct length = GCOV_TAG_COUNTER_NUM (gcov_read_unsigned ()); len = length * sizeof (gcov_type); - values = (gcov_type *)malloc (len); + values = (gcov_type *) xmalloc (len); if (!values) goto fail; @@ -450,7 +436,7 @@ gcov_exit_merge_gcda (struct gcov_info *gi_ptr, histogram entries that will be emitted, and thus the size of the merged summary. */ (*sum_tail) = (struct gcov_summary_buffer *) - malloc (sizeof(struct gcov_summary_buffer)); + xmalloc (sizeof(struct gcov_summary_buffer)); (*sum_tail)->summary = tmp; (*sum_tail)->next = 0; sum_tail = &((*sum_tail)->next); @@ -718,7 +704,7 @@ gcov_exit_merge_summary (const struct gcov_info *g } #endif } - + prg->checksum = crc32; return 0; Index: libgcc/libgcov-driver-system.c =================================================================== --- libgcc/libgcov-driver-system.c (revision 205895) +++ libgcc/libgcov-driver-system.c (working copy) @@ -124,7 +124,7 @@ allocate_filename_struct (struct gcov_filename_aux prefix_length = 1; } /* Allocate and initialize the filename scratch space plus one. */ - gi_filename = (char *) malloc (prefix_length + gcov_max_filename + 2); + gi_filename = (char *) xmalloc (prefix_length + gcov_max_filename + 2); if (prefix_length) memcpy (gi_filename, gcov_prefix, prefix_length); gi_filename_up = gi_filename + prefix_length; Index: libgcc/libgcov.h =================================================================== --- libgcc/libgcov.h (revision 0) +++ libgcc/libgcov.h (revision 0) @@ -0,0 +1,225 @@ +/* Header file for libgcov-*.c. + Contributed by Rong Xu <xur@google.com>. + Copyright (C) 2013 Free Software Foundation, Inc. + + This file is part of GCC. + + GCC is free software; you can redistribute it and/or modify it under + the terms of the GNU General Public License as published by the Free + Software Foundation; either version 3, or (at your option) any later + version. + + GCC is distributed in the hope that it will be useful, but WITHOUT ANY + WARRANTY; without even the implied warranty of MERCHANTABILITY or + FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + for more details. + + You should have received a copy of the GNU General Public License + along with GCC; see the file COPYING3. If not see + <http://www.gnu.org/licenses/>. */ + +#ifndef GCC_LIBGCOV_H +#define GCC_LIBGCOV_H + +/* work around the poisoned malloc/calloc in system.h. */ +#ifndef xmalloc +#define xmalloc malloc +#endif +#ifndef xcalloc +#define xcalloc calloc +#endif + +#include "tconfig.h" +#include "tsystem.h" +#include "coretypes.h" +#include "tm.h" +#include "libgcc_tm.h" + +#if BITS_PER_UNIT == 8 +typedef unsigned gcov_unsigned_t __attribute__ ((mode (SI))); +typedef unsigned gcov_position_t __attribute__ ((mode (SI))); +#if LONG_LONG_TYPE_SIZE > 32 +typedef signed gcov_type __attribute__ ((mode (DI))); +typedef unsigned gcov_type_unsigned __attribute__ ((mode (DI))); +#else +typedef signed gcov_type __attribute__ ((mode (SI))); +typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); +#endif +#else +#if BITS_PER_UNIT == 16 +typedef unsigned gcov_unsigned_t __attribute__ ((mode (HI))); +typedef unsigned gcov_position_t __attribute__ ((mode (HI))); +#if LONG_LONG_TYPE_SIZE > 32 +typedef signed gcov_type __attribute__ ((mode (SI))); +typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI))); +#else +typedef signed gcov_type __attribute__ ((mode (HI))); +typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); +#endif +#else +typedef unsigned gcov_unsigned_t __attribute__ ((mode (QI))); +typedef unsigned gcov_position_t __attribute__ ((mode (QI))); +#if LONG_LONG_TYPE_SIZE > 32 +typedef signed gcov_type __attribute__ ((mode (HI))); +typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI))); +#else +typedef signed gcov_type __attribute__ ((mode (QI))); +typedef unsigned gcov_type_unsigned __attribute__ ((mode (QI))); +#endif +#endif +#endif + +#if defined (TARGET_POSIX_IO) +#define GCOV_LOCKED 1 +#else +#define GCOV_LOCKED 0 +#endif + +#if defined(inhibit_libc) +#define IN_LIBGCOV (-1) +#else +#define IN_LIBGCOV 1 +#if defined(L_gcov) +#define GCOV_LINKAGE /* nothing */ +#endif +#endif + +/* In libgcov we need these functions to be extern, so prefix them with + __gcov. In libgcov they must also be hidden so that the instance in + the executable is not also used in a DSO. */ +#define gcov_var __gcov_var +#define gcov_open __gcov_open +#define gcov_close __gcov_close +#define gcov_write_tag_length __gcov_write_tag_length +#define gcov_position __gcov_position +#define gcov_seek __gcov_seek +#define gcov_rewrite __gcov_rewrite +#define gcov_is_error __gcov_is_error +#define gcov_write_unsigned __gcov_write_unsigned +#define gcov_write_counter __gcov_write_counter +#define gcov_write_summary __gcov_write_summary +#define gcov_read_unsigned __gcov_read_unsigned +#define gcov_read_counter __gcov_read_counter +#define gcov_read_summary __gcov_read_summary + +/* Poison these, so they don't accidentally slip in. */ +#pragma GCC poison gcov_write_string gcov_write_tag gcov_write_length +#pragma GCC poison gcov_time gcov_magic + +#ifdef HAVE_GAS_HIDDEN +#define ATTRIBUTE_HIDDEN __attribute__ ((__visibility__ ("hidden"))) +#else +#define ATTRIBUTE_HIDDEN +#endif + +#include "gcov-io.h" + +/* Structures embedded in coveraged program. The structures generated + by write_profile must match these. */ + +/* Information about counters for a single function. */ +struct gcov_ctr_info +{ + gcov_unsigned_t num; /* number of counters. */ + gcov_type *values; /* their values. */ +}; + +/* Information about a single function. This uses the trailing array + idiom. The number of counters is determined from the merge pointer + array in gcov_info. The key is used to detect which of a set of + comdat functions was selected -- it points to the gcov_info object + of the object file containing the selected comdat function. */ + +struct gcov_fn_info +{ + const struct gcov_info *key; /* comdat key */ + gcov_unsigned_t ident; /* unique ident of function */ + gcov_unsigned_t lineno_checksum; /* function lineo_checksum */ + gcov_unsigned_t cfg_checksum; /* function cfg checksum */ + struct gcov_ctr_info ctrs[0]; /* instrumented counters */ +}; + +/* Type of function used to merge counters. */ +typedef void (*gcov_merge_fn) (gcov_type *, gcov_unsigned_t); + +/* Information about a single object file. */ +struct gcov_info +{ + gcov_unsigned_t version; /* expected version number */ + struct gcov_info *next; /* link to next, used by libgcov */ + + gcov_unsigned_t stamp; /* uniquifying time stamp */ + const char *filename; /* output file name */ + + gcov_merge_fn merge[GCOV_COUNTERS]; /* merge functions (null for + unused) */ + + unsigned n_functions; /* number of functions */ + const struct gcov_fn_info *const *functions; /* pointer to pointers + to function information */ +}; + +/* Register a new object file module. */ +extern void __gcov_init (struct gcov_info *) ATTRIBUTE_HIDDEN; + +/* Called before fork, to avoid double counting. */ +extern void __gcov_flush (void) ATTRIBUTE_HIDDEN; + +/* Function to reset all counters to 0. */ +extern void __gcov_reset (void); + +/* Function to enable early write of profile information so far. */ +extern void __gcov_dump (void); + +/* The merge function that just sums the counters. */ +extern void __gcov_merge_add (gcov_type *, unsigned) ATTRIBUTE_HIDDEN; + +/* The merge function to select the minimum valid counter value. */ +extern void __gcov_merge_time_profile (gcov_type *, unsigned) ATTRIBUTE_HIDDEN; + +/* The merge function to choose the most common value. */ +extern void __gcov_merge_single (gcov_type *, unsigned) ATTRIBUTE_HIDDEN; + +/* The merge function to choose the most common difference between + consecutive values. */ +extern void __gcov_merge_delta (gcov_type *, unsigned) ATTRIBUTE_HIDDEN; + +/* The merge function that just ors the counters together. */ +extern void __gcov_merge_ior (gcov_type *, unsigned) ATTRIBUTE_HIDDEN; + +/* The profiler functions. */ +extern void __gcov_interval_profiler (gcov_type *, gcov_type, int, unsigned); +extern void __gcov_pow2_profiler (gcov_type *, gcov_type); +extern void __gcov_one_value_profiler (gcov_type *, gcov_type); +extern void __gcov_indirect_call_profiler (gcov_type*, gcov_type, + void*, void*); +extern void __gcov_indirect_call_profiler_v2 (gcov_type, void *); +extern void __gcov_time_profiler (gcov_type *); +extern void __gcov_average_profiler (gcov_type *, gcov_type); +extern void __gcov_ior_profiler (gcov_type *, gcov_type); + +#ifndef inhibit_libc +/* The wrappers around some library functions.. */ +extern pid_t __gcov_fork (void) ATTRIBUTE_HIDDEN; +extern int __gcov_execl (const char *, char *, ...) ATTRIBUTE_HIDDEN; +extern int __gcov_execlp (const char *, char *, ...) ATTRIBUTE_HIDDEN; +extern int __gcov_execle (const char *, char *, ...) ATTRIBUTE_HIDDEN; +extern int __gcov_execv (const char *, char *const []) ATTRIBUTE_HIDDEN; +extern int __gcov_execvp (const char *, char *const []) ATTRIBUTE_HIDDEN; +extern int __gcov_execve (const char *, char *const [], char *const []) + ATTRIBUTE_HIDDEN; + +/* Functions that only available in libgcov. */ +GCOV_LINKAGE int gcov_open (const char */*name*/) ATTRIBUTE_HIDDEN; +GCOV_LINKAGE void gcov_write_counter (gcov_type) ATTRIBUTE_HIDDEN; +GCOV_LINKAGE void gcov_write_tag_length (gcov_unsigned_t, gcov_unsigned_t) + ATTRIBUTE_HIDDEN; +GCOV_LINKAGE void gcov_write_summary (gcov_unsigned_t /*tag*/, + const struct gcov_summary *) + ATTRIBUTE_HIDDEN; +GCOV_LINKAGE void gcov_seek (gcov_position_t /*position*/) ATTRIBUTE_HIDDEN; +GCOV_LINKAGE inline void gcov_rewrite (void); + +#endif /* !inhibit_libc */ + +#endif /* GCC_LIBGCOV_H */ Index: libgcc/libgcov-interface.c =================================================================== --- libgcc/libgcov-interface.c (revision 205895) +++ libgcc/libgcov-interface.c (working copy) @@ -23,22 +23,11 @@ a copy of the GCC Runtime Library Exception along see the files COPYING3 and COPYING.RUNTIME respectively. If not, see <http://www.gnu.org/licenses/>. */ -#include "tconfig.h" -#include "tsystem.h" -#include "coretypes.h" -#include "tm.h" -#include "libgcc_tm.h" +#include "libgcov.h" #include "gthr.h" #if defined(inhibit_libc) -#define IN_LIBGCOV (-1) -#else -#define IN_LIBGCOV 1 -#endif -#include "gcov-io.h" -#if defined(inhibit_libc) - #ifdef L_gcov_flush void __gcov_flush (void) {} #endif Index: libgcc/libgcov-merge.c =================================================================== --- libgcc/libgcov-merge.c (revision 205895) +++ libgcc/libgcov-merge.c (working copy) @@ -23,21 +23,9 @@ a copy of the GCC Runtime Library Exception along see the files COPYING3 and COPYING.RUNTIME respectively. If not, see <http://www.gnu.org/licenses/>. */ -#include "tconfig.h" -#include "tsystem.h" -#include "coretypes.h" -#include "tm.h" -#include "libgcc_tm.h" +#include "libgcov.h" #if defined(inhibit_libc) -#define IN_LIBGCOV (-1) -#else -#define IN_LIBGCOV 1 -#endif - -#include "gcov-io.h" - -#if defined(inhibit_libc) /* If libc and its header files are not available, provide dummy functions. */ #ifdef L_gcov_merge_add Index: libgcc/libgcov-profiler.c =================================================================== --- libgcc/libgcov-profiler.c (revision 205895) +++ libgcc/libgcov-profiler.c (working copy) @@ -23,15 +23,8 @@ a copy of the GCC Runtime Library Exception along see the files COPYING3 and COPYING.RUNTIME respectively. If not, see <http://www.gnu.org/licenses/>. */ -#include "tconfig.h" -#include "tsystem.h" -#include "coretypes.h" -#include "tm.h" -#include "libgcc_tm.h" - +#include "libgcov.h" #if !defined(inhibit_libc) -#define IN_LIBGCOV 1 -#include "gcov-io.h" #ifdef L_gcov_interval_profiler /* If VALUE is in interval <START, START + STEPS - 1>, then increases the Index: libgcc/Makefile.in =================================================================== --- libgcc/Makefile.in (revision 205895) +++ libgcc/Makefile.in (working copy) @@ -868,14 +868,14 @@ libgcov-driver-objects = $(patsubst %,%$(objext),$ libgcov-objects = $(libgcov-merge-objects) $(libgcov-profiler-objects) \ $(libgcov-interface-objects) $(libgcov-driver-objects) -$(libgcov-merge-objects): %$(objext): $(srcdir)/libgcov-merge.c +$(libgcov-merge-objects): %$(objext): $(srcdir)/libgcov-merge.c $(srcdir)/libgcov.h $(gcc_compile) -DL$* -c $(srcdir)/libgcov-merge.c -$(libgcov-profiler-objects): %$(objext): $(srcdir)/libgcov-profiler.c +$(libgcov-profiler-objects): %$(objext): $(srcdir)/libgcov-profiler.c $(srcdir)/libgcov.h $(gcc_compile) -DL$* -c $(srcdir)/libgcov-profiler.c -$(libgcov-interface-objects): %$(objext): $(srcdir)/libgcov-interface.c +$(libgcov-interface-objects): %$(objext): $(srcdir)/libgcov-interface.c $(srcdir)/libgcov.h $(gcc_compile) -DL$* -c $(srcdir)/libgcov-interface.c $(libgcov-driver-objects): %$(objext): $(srcdir)/libgcov-driver.c \ - $(srcdir)/libgcov-driver-system.c + $(srcdir)/libgcov-driver-system.c $(srcdir)/libgcov.h $(gcc_compile) -DL$* -c $(srcdir)/libgcov-driver.c