This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] offline gcda profile processing tool
- From: Richard Biener <richard dot guenther at gmail 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>, Xinliang David Li <xinliangli at gmail dot com>, Teresa Johnson <tejohnson at google dot com>, Dehao Chen <dehao at google dot com>
- Date: Fri, 11 Jul 2014 10:07:29 +0200
- Subject: Re: [PATCH] offline gcda profile processing tool
- Authentication-results: sourceware.org; auth=none
- References: <CAF1bQ=T7xuC7O4=n90eN-pKh0me0d1e+50iXFtEcOD4o8s0ArQ at mail dot gmail dot com> <CAF1bQ=TypsiC1Aa3Bz14cxX5sP9Zt02ecVPuU_xnS=2+zmDAQg at mail dot gmail dot com> <CAF1bQ=SPdSH8ffMSEBkGQqH-w7b5ye3mC4n2CwqenH8EbGR+PA at mail dot gmail dot com> <20140415213850 dot GA23141 at atrey dot karlin dot mff dot cuni dot cz> <CAF1bQ=Tsi9JmmZSQo4NJwfes-1EAzzVC_645YEpO5k6PRaeKqA at mail dot gmail dot com> <20140417033448 dot GC3157 at kam dot mff dot cuni dot cz> <CAF1bQ=RD2d1E8PGi9FW5WffPupNcG84TBv53GH62k5vVwu2fGQ at mail dot gmail dot com> <CAF1bQ=SYZfERgODf=Yc=qRtDxhJg-8Dt0OaGKf4GxM0_y+vUbg at mail dot gmail dot com>
On Mon, May 5, 2014 at 7:17 PM, Rong Xu <xur@google.com> wrote:
> Here is the updated patch. The difference with patch set 3 is
> (1) use the gcov-counter.def for scaling operation.
> (2) fix wrong scaling function for time-profiler.
>
> passed with bootstrap, profiledboostrap and SPEC2006.
One of the patches breaks bootstrap for me:
/space/rguenther/src/svn/trunk/gcc/../libgcc/libgcov.h:184: error: ISO
C++ forbids zero-size array âctrsâ
make[3]: *** [libgcov-util.o] Error 1
host compiler is gcc 4.3.4
Richard.
> Thanks,
>
> -Rong
>
> On Thu, May 1, 2014 at 3:37 PM, Rong Xu <xur@google.com> wrote:
>> Hi, Honza,
>>
>> Please find the new patch in the attachment. This patch integrated
>> your earlier suggestions. The noticeable changes are:
>> (1) build specialized object for libgcov-driver.c and libgcov-merge.c
>> and link into gcov-tool, rather including the source file.
>> (2) split some gcov counter definition code to gcov-counter.def to
>> avoid code duplication.
>> (3) use a macro for gcov_read_counter(), so gcov-tool can use existing
>> code in libgcov-merge.c with minimal change.
>>
>> This patch does not address the suggestion of creating a new directory
>> for libgcov. I agree with you that's a much better
>> and cleaner structure we should go for. We can do that in follow-up patches.
>>
>> I tested this patch with boostrap and profiledbootstrap. Other tests
>> are on-going.
>>
>> Thanks,
>>
>> -Rong
>>
>> On Wed, Apr 16, 2014 at 8:34 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>>> GCOT_TOOL needs to use this function to read the string in gcda file
>>>> to memory to construct gcov_info objects.
>>>> As you noticed, gcov runtime does not need this interface. But
>>>> gcov-tool links with gcov runtime and it also uses the function.
>>>> We could make it available in gcov_runtime, but that will slightly
>>>> increase the memory footprint.
>>>
>>> Yep, it is not really pretty. I wrote bellow some plan how things may be
>>> structured in more convenient way. Any work in that direction would be welcome.
>>>>
>>>> The planned new functions for trunk version is: (1) overlap score b/w
>>>> two profiles (2) better dumping (top hot objects/function/counters)
>>>> and statistics.
>>>> Once this basic version is in, we can start to add the new functionality.
>>>
>>> Sounds good. I assume the autoFDO does not go via gcov tool but rather uses
>>> custom reader of profile data at GCC side?
>>> I wonder, are there any recent overview papers/slides/text of design of AutoFDO
>>> and other features to be merged?
>>> I remember the talk from GCC Summit and I did read some of pre-LTO LIPO
>>> sources & papers, but it would be nice to have somethin up to date.
>>>>
>>>> libgcov-util.o is built in gcc/ directory, rather in libgcc.
>>>> It's directly linked to gcov-tool.
>>>> So libgcov-util.o is built for HOST, not TARGET.
>>>> With makefile changes, we can built HOST version of libgcov-driver.o
>>>> and libgcov-merge.o.
>>>> I also need to make some static functions/variables public.
>>>
>>> I suppose that can go with IN_GCOV_TOOL ifdef.
>>>
>>> So we currently have basic gcov io handling in gcc/gcov-io.c that is borrowed
>>> by libgcc/libgcov* code. We also will get libgcov-util.c in libgcc directory
>>> that is actually borrowed by by gcc/gcov-tool.c code.
>>>
>>> We now have one runtime using STDIO for streaming and kernel has custom version
>>> that goes via /proc interface (last time I looked). We added some abstraction
>>> into libgcov-interface that is the part that relies on STDIO, partly via gcov-io.c
>>> code and now we have in-memory handling code in libgcov-util.
>>>
>>> I guess it would make most sentse to put all the gcov code into a new directory
>>> (libgcov) and make it stand-alone library that can be configured
>>> 1) for stdio based runtime as we do not
>>> 2) for runtime missing the interface and relyin on user providing it
>>> 3) for use within gcov file manipulation tools with reorg of
>>> GCC/gcov/gcov-dump/gcov-tool to all use the same low-level interfaces.
>>> In a longer term, I would like to make FDO profiling intstrumentation to happen
>>> at linktime. For that I need to make the instrumentation code (minimal spaning
>>> tree & friends) to work w/o cgraph that would ideally be done in a shared
>>> implementation
>>>> > Won't this get wrong answer when counters[0] is not the same?
>>>> > I would expected the merging code to compare the counters first. Similarly for delta counter.
>>>>
>>>> These *_op functions are for scaling only. So there is only one
>>>> profile involved (thus there is no comparison).
>>>> The merge handles are in libgcov-merge.c which have the code to handle
>>>> mismatched profile targets.
>>>
>>> I see, OK then.
>>>> >
>>>> > Adding correct rounding may actually make difference for Martin's startup time work.
>>>>
>>>> Do you mean to use something like in RDIV macro?
>>>
>>> Yes.
>>>
>>> Honza