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: Xinliang David Li <davidxl at google dot com>
- To: Rong Xu <xur at google dot com>
- Cc: Jan Hubicka <hubicka at ucw dot cz>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 8 Jan 2014 22:46:43 -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> <20131206142319 dot GC32664 at kam dot mff dot cuni dot cz> <CAAkRFZLo-yzMBxqdL5d_p9h2XaXkq1F_yBN-6CcMhV8JLAewmA at mail dot gmail dot com> <CAF1bQ=SeLAmjKwOFgjhvs-9GdwDM+SYppJdiZu-0AGPOxDdHbA at mail dot gmail dot com>
On Wed, Jan 8, 2014 at 2:20 PM, Rong Xu <xur@google.com> wrote:
> On Wed, Dec 18, 2013 at 9:28 AM, Xinliang David Li <davidxl@google.com> wrote:
>>>>
>>>> #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 thought about this one a little. How about making the interface
>> change conditionally, but still share the implementation? The merge
>> function bodies mostly remain unchanged and there is no runtime
>> penalty for libgcov. The new macros can be shared across most of the
>> mergers.
>>
>> #ifdef IN_PREOFILE_TOOL
>> #define GCOV_MERGE_EXTRA_ARGS gcov_type *src, unsigned w
>> #define GCOV_READ_COUNTER *(src++) * w
>> #else
>> #define GCOV_MERGE_EXTRA_ARGS
>> #define GCOV_READ_COUNTER gcov_read_counter ()
>> #endif
>>
>> __gcov_merge_add (gcov_type *counters, unsigned n_counters,
>> GCOV_MERGE_EXTRA_ARGS)
>> {
>>
>> for (; n_counters; counters++, n_counters--)
>> {
>> *counters += GCOV_READ_COUNTER ;
>> }
>>
>> }
>>
>> thanks,
>
> Personally I don't think the run time test of in_mem will cause any
> issue. This is in profile dumping, why don't we care a few more cycle
> heres? it won't pollute the profile.
>
> If you really don't like that, we can use the above approach, or I can
> hide the logic in gcov_read_counter(), i.e. overload
> gcov_read_counter() in profile_tool. For that, I will need a new
> global variable SRC and set it before calling the merge function.
> I would prefer to keep weight in _gcov_merge_* argument list.
>
> What do you think?
or perhaps just define an inline wrapper function to read counter.
This wrapper function takes src as input. if src is NULL, call
gcov_read_counter.
David
>
> -Rong
>
>>
>> David
>>
>>>
>>> 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