Re: [RFC] libgcov.c re-factoring and offline profile-tool

[Rong Xu <xur at google dot com>]

On Fri, Nov 8, 2013 at 10:22 AM, Xinliang David Li <davidxl@google.com> wrote:
> in libgcov-driver.c
>
>> /* Flag when the profile has already been dumped via __gcov_dump().  */
>> static int gcov_dump_complete;
>
>> inline void
>> set_gcov_dump_complete (void)
>>{
>   > gcov_dump_complete = 1;
>>}
>
>>inline void
>>reset_gcov_dump_complete (void)
>>{
>>  gcov_dump_complete = 0;
>>}
>
>
> These should be moved to libgcov-interface.c. Is there reason to not
> mark them as static?

gcov_dump_complete is used in gcov_exit() which is in
libgcov-driver.c, but it's set in __gcov_dump and __gcov_reset, both
in libgcov-interface.c.
I want gcov_dump_complete a static. So I have to add to global
functions that accessible from libgcov-interface.c.
Another choice is to move __gcov_dump and __gcov_reset to
libgcov-driver.c and that will simplify the code.

>
> in libgcov-profiler.c, line 142
>
>> #if defined(HAVE_CC_TLS) && !defined (USE_EMUTLS)
>> __thread
>
> trailing whilte space.
>

Will fix.

>
>>     || (VTABLE_USES_DESCRIPTORS && __gcov_indirect_call_callee
>>         && *(void **) cur_func == *(void **) __gcov_indirect_call_callee))
>
> trailing white space.
>

Will fix.

>
> in libgcov-merge.c
>
> 1) I don't think you need in_mem flag. For in memory merge, the source != NULL.
> 2) document the new source and weight parameter -- especially the weight.

Will do.

> 3) How do you deal with integer overflow ?

This is good question. gcvo_type is (typically) long long. I thought
the count value will not be nearly close to the max of long long.
(We did see overflow in compiler, but that's because of the scaling --
some of the scaling factors are really large.)

>
> David
>
>
>
>
>
>
> On Thu, Nov 7, 2013 at 3:34 PM, Rong Xu <xur@google.com> wrote:
>> On Thu, Nov 7, 2013 at 9:40 AM, Joseph S. Myers <joseph@codesourcery.com> wrote:
>>> On Thu, 7 Nov 2013, Rong Xu wrote:
>>>
>>>> Thanks Joseph for these detailed comments/suggestions.
>>>> The fixed patch is attached to this email.
>>>> The only thing left out is the Texinfo manual. Do you mean this tool
>>>> should have its it's own texi file in gcc/doc?
>>>
>>> Its own texi file, probably included as a chapter by gcc.texi (like
>>> gcov.texi is).
>>
>> OK. will add this in.
>>
>> My last patch that changes the command handling is actually broken.
>> Please ignore that patch and use the one in this email.
>>
>> Also did some minor adjust of the code in libgcov.
>>
>> Thanks,
>>
>> -Rong
>>
>>
>>>
>>> --
>>> Joseph S. Myers
>>> joseph@codesourcery.com