> 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 <tejohn...@google.com> wrote:
> > On Mon, Dec 16, 2013 at 12:55 PM, Xinliang David Li <davi...@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 <tejohn...@google.com> 
> >> wrote:
> >>> On Wed, Dec 11, 2013 at 10:05 PM, Teresa Johnson <tejohn...@google.com> 
> >>> wrote:
> >>>> On Fri, Dec 6, 2013 at 6:23 AM, Jan Hubicka <hubi...@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  <x...@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 | tejohn...@google.com | 408-460-2413
> >>>
> >>>
> >>>
> >>> --
> >>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
> >
> >
> >
> > --
> > Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413

Reply via email to