This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFC] libgcov.c re-factoring and offline profile-tool
- From: Rong Xu <xur at google dot com>
- To: Xinliang David Li <davidxl at google dot com>
- Cc: Jan Hubicka <hubicka at ucw dot cz>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 20 Nov 2013 15:43:25 -0800
- Subject: Re: [RFC] libgcov.c re-factoring and offline profile-tool
- Authentication-results: sourceware.org; auth=none
- References: <CAF1bQ=Qy6--WX-bLYZ8aWXjVVu4pW-H3U+PkATgXwg5c7MZsBg at mail dot gmail dot com> <CAAkRFZ+-tLZqUNQRtu7jN3d3BYG8oi-+q0J6pkduHp5GS3oTkQ at mail dot gmail dot com> <20131104095529 dot GB28992 at kam dot mff dot cuni dot cz> <CAF1bQ=THYY8NqoJ0XDYxnFS9=j9P6-5nLwA6nX1bA+0v1zmUoQ at mail dot gmail dot com> <20131111150245 dot GB6009 at atrey dot karlin dot mff dot cuni dot cz> <CAF1bQ=RSUQdE0tkeqeHnbiNkC=PFPDU6O+zLFNA6jUxqV_aAbg at mail dot gmail dot com> <CAF1bQ=SCtewMrgjcYg4v21jA8uWy_9U=9sB9ULznS=9tdJD8mQ at mail dot gmail dot com> <CAF1bQ=TyNC_rAo1V8Zk9ovOvYAtpqVtep_h7WNLk6v6ZerdcaQ at mail dot gmail dot com> <CAAkRFZK+ypb5H77cg68VbS7gCHvZ=nBdhjXTOAmmc2mRQOswZA at mail dot gmail dot com>
Ping.
On Mon, Nov 18, 2013 at 12:31 PM, Xinliang David Li <davidxl@google.com> wrote:
> gcov-dump tool does raw dump of profile data. In the long run, we
> should have only one gcov profile manipulation tool, so it might be
> better to incorporate gcov-dump into gcov-tool and get rid of
> 'gcov-dump'.
>
> David
>
> On Mon, Nov 18, 2013 at 12:24 PM, Rong Xu <xur@google.com> 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,
>>
>> -Rong
>>
>> On Tue, Nov 12, 2013 at 4:27 PM, Rong Xu <xur@google.com> wrote:
>>> The patch was checked in as r204730.
>>>
>>> Tested with
>>> configure with --enable-languages=all,obj-c++
>>> (ada is currently broken, so I was not be able test)
>>> bootstrap and profiledbootstrap
>>> regression for -m32 and -m64.
>>> spec2006
>>>
>>> The only semantic change is __gcov_flush() used to be under L_gcov.
>>> I put the function to libgcov-interface.c and made it under
>>> L_gcov_flush (newly added).
>>> So if you need this function, you have to enable L_gcov_flush in the
>>> libgcc/Makefile.in.
>>>
>>> Thanks,
>>>
>>> -Rong
>>>
>>> On Mon, Nov 11, 2013 at 10:19 AM, Rong Xu <xur@google.com> wrote:
>>>> On Mon, Nov 11, 2013 at 7:02 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>>>>> 2013-11-04 Rong Xu <xur@google.com>
>>>>>>
>>>>>> * libgcc/libgcov.c: Delete as part of re-factoring.
>>>>>> * libgcc/libgcov-profiler.c (__gcov_interval_profiler): Moved from
>>>>>> libgcov.c
>>>>>> (__gcov_pow2_profiler): Ditto.
>>>>>> (__gcov_one_value_profiler_body): Ditto.
>>>>>> (__gcov_one_value_profiler): Ditto.
>>>>>> (__gcov_indirect_call_profiler): Ditto.
>>>>>> (__gcov_indirect_call_profiler_v2): Ditto.
>>>>>> (__gcov_average_profiler): Ditto.
>>>>>> (__gcov_ior_profiler): Ditto.
>>>>>> * libgcc/libgcov-driver.c (this_prg): Make it file scope
>>>>>> static variable.
>>>>>> (all_prg): Ditto.
>>>>>> (crc32): Ditto.
>>>>>> (gi_filename): Ditto.
>>>>>> (fn_buffer): Ditto.
>>>>>> (sum_buffer): Ditto.
>>>>>> (struct gcov_filename_aux): New types to store auxiliary information
>>>>>> for gi_filename.
>>>>>> (gcov_version): Moved from libgcov.c.
>>>>>> (crc32_unsigned): Ditto.
>>>>>> (gcov_histogram_insert): Ditto.
>>>>>> (gcov_compute_histogram): Ditto.
>>>>>> (gcov_exit): Ditto.
>>>>>> (gcov_clear): Ditto.
>>>>>> (__gcov_init): Ditto.
>>>>>> (gcov_exit_compute_summary): New function split from gcov_exit().
>>>>>> (gcov_exit_merge_gcda): Ditto.
>>>>>> (gcov_exit_write_gcda): Ditto.
>>>>>> (gcov_exit_dump_gcov): Ditto.
>>>>>> * libgcc/libgcov-interface.c (init_mx): Moved from libgcov.c.
>>>>>> (init_mx_once): Ditto.
>>>>>> (__gcov_flush): Ditto.
>>>>>> (__gcov_reset): Ditto.
>>>>>> (__gcov_dump): Ditto.
>>>>>> (__gcov_fork): Ditto.
>>>>>> (__gcov_execl): Ditto.
>>>>>> (__gcov_execlp): Ditto.
>>>>>> (__gcov_execle): Ditto.
>>>>>> (__gcov_execv): Ditto.
>>>>>> (__gcov_execvp): Ditto.
>>>>>> (__gcov_execve): Ditto.
>>>>>> * libgcc/libgcov-driver-system.c (gcov_error): New utility function.
>>>>>> (allocate_filename_struct): New function split from gcov_exit().
>>>>>> (gcov_exit_open_gcda_file): Ditto.
>>>>>> (create_file_directory): Moved from libgcov.c.
>>>>>> * libgcc/libgcov-merge.c:
>>>>>> (__gcov_merge_add): Moved from libgcov.c.
>>>>>> (__gcov_merge_ior): Ditto.
>>>>>> (__gcov_merge_single): Ditto.
>>>>>> (__gcov_merge_delta): Ditto.
>>>>>> * libgcc/Makefile.in: Change to build newly added files.
>>>>>> * gcc/gcov-io.h (__gcov_merge_ior): Add the decl to avoid warning.
>>>>>>
>>>>>
>>>>> Hi,
>>>>> the patch looks resonable. I take your word on the fact that there are no changes
>>>>> in the code, I did not read it all ;)))
>>>>
>>>> We did split gcov_exit() into more functions.
>>>> But, semantically the code is the same.
>>>>
>>>>>> Index: libgcc/Makefile.in
>>>>>> ===================================================================
>>>>>> --- libgcc/Makefile.in (revision 204285)
>>>>>> +++ libgcc/Makefile.in (working copy)
>>>>>> @@ -853,17 +853,37 @@ include $(iterator)
>>>>>> # Build libgcov components.
>>>>>>
>>>>>> # Defined in libgcov.c, included only in gcov library
>>>>>> -LIBGCOV = _gcov _gcov_merge_add _gcov_merge_single _gcov_merge_delta \
>>>>>> - _gcov_fork _gcov_execl _gcov_execlp _gcov_execle \
>>>>>> - _gcov_execv _gcov_execvp _gcov_execve _gcov_reset _gcov_dump \
>>>>>> - _gcov_interval_profiler _gcov_pow2_profiler _gcov_one_value_profiler \
>>>>>> +##LIBGCOV = _gcov _gcov_merge_add _gcov_merge_single _gcov_merge_delta \
>>>>>> +## _gcov_fork _gcov_execl _gcov_execlp _gcov_execle \
>>>>>> +## _gcov_execv _gcov_execvp _gcov_execve _gcov_reset _gcov_dump \
>>>>>> +## _gcov_interval_profiler _gcov_pow2_profiler _gcov_one_value_profiler \
>>>>>> +## _gcov_indirect_call_profiler _gcov_average_profiler _gcov_ior_profiler \
>>>>>> +## _gcov_merge_ior _gcov_indirect_call_profiler_v2
>>>>>
>>>>> Probably no need for this commnet block.
>>>>
>>>> Yes. I ready remove in the most recently patch.
>>>>
>>>>>> +
>>>>>> +LIBGCOV_MERGE = _gcov_merge_add _gcov_merge_single _gcov_merge_delta _gcov_merge_ior
>>>>>> +LIBGCOV_PROFILER = _gcov_interval_profiler _gcov_pow2_profiler _gcov_one_value_profiler \
>>>>>> _gcov_indirect_call_profiler _gcov_average_profiler _gcov_ior_profiler \
>>>>>> - _gcov_merge_ior _gcov_indirect_call_profiler_v2
>>>>>> + _gcov_indirect_call_profiler_v2
>>>>>> +LIBGCOV_INTERFACE = _gcov_flush _gcov_fork _gcov_execl _gcov_execlp _gcov_execle \
>>>>>> + _gcov_execv _gcov_execvp _gcov_execve _gcov_reset _gcov_dump
>>>>>> +LIBGCOV_DRIVER = _gcov
>>>>>>
>>>>>> -libgcov-objects = $(patsubst %,%$(objext),$(LIBGCOV))
>>>>>> +libgcov-merge-objects = $(patsubst %,%$(objext),$(LIBGCOV_MERGE))
>>>>>> +libgcov-profiler-objects = $(patsubst %,%$(objext),$(LIBGCOV_PROFILER))
>>>>>> +libgcov-interface-objects = $(patsubst %,%$(objext),$(LIBGCOV_INTERFACE))
>>>>>> +libgcov-driver-objects = $(patsubst %,%$(objext),$(LIBGCOV_DRIVER))
>>>>>> +libgcov-objects = $(libgcov-merge-objects) $(libgcov-profiler-objects) \
>>>>>> + $(libgcov-interface-objects) $(libgcov-driver-objects)
>>>>>>
>>>>>> -$(libgcov-objects): %$(objext): $(srcdir)/libgcov.c
>>>>>> - $(gcc_compile) -DL$* -c $(srcdir)/libgcov.c
>>>>>> +$(libgcov-merge-objects): %$(objext): $(srcdir)/libgcov-merge.c
>>>>>> + $(gcc_compile) -DL$* -c $(srcdir)/libgcov-merge.c
>>>>>> +$(libgcov-profiler-objects): %$(objext): $(srcdir)/libgcov-profiler.c
>>>>>> + $(gcc_compile) -DL$* -c $(srcdir)/libgcov-profiler.c
>>>>>> +$(libgcov-interface-objects): %$(objext): $(srcdir)/libgcov-interface.c
>>>>>> + $(gcc_compile) -DL$* -c $(srcdir)/libgcov-interface.c
>>>>>> +$(libgcov-driver-objects): %$(objext): $(srcdir)/libgcov-driver.c \
>>>>>> + $(srcdir)/libgcov-driver-system.c
>>>>>> + $(gcc_compile) -DL$* -c $(srcdir)/libgcov-driver.c
>>>>>>
>>>>>>
>>>>>> # Static libraries.
>>>>>> Index: gcc/gcov-io.h
>>>>>> ===================================================================
>>>>>> --- gcc/gcov-io.h (revision 204285)
>>>>>> +++ gcc/gcov-io.h (working copy)
>>>>>> @@ -515,6 +515,8 @@ extern void __gcov_merge_ior (gcov_type *, unsigne
>>>>>> 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);
>>>>>
>>>>> The patch is OK with the change above.
>>>>
>>>> Thanks for the OK. I'll do some final tests on a clean client. Then I'll commit.
>>>>
>>>>> Honza