public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Rong Xu <xur@google.com>
To: Xinliang David Li <davidxl@google.com>
Cc: Christophe Lyon <christophe.lyon@linaro.org>,
	Richard Biener <richard.guenther@gmail.com>,
		Jan Hubicka <hubicka@ucw.cz>,
	GCC Patches <gcc-patches@gcc.gnu.org>,
		Xinliang David Li <xinliangli@gmail.com>,
	Teresa Johnson <tejohnson@google.com>,
		Dehao Chen <dehao@google.com>
Subject: Re: [PATCH] offline gcda profile processing tool
Date: Fri, 11 Jul 2014 16:44:00 -0000	[thread overview]
Message-ID: <CAF1bQ=ReiuefoPM4_q1LbJ4SUh+2uwUqjxyX79XB4wdRH2q7kg@mail.gmail.com> (raw)
In-Reply-To: <CAAkRFZ+QNyWHwA1DuSnswes3HX1-H=A6i6qed66gqgN00kDPxQ@mail.gmail.com>

I should use _WIN32 macro. This code is for windows mkdir api.
I'll commit a patch for this shortly (approved by honza).

-Rong

On Fri, Jul 11, 2014 at 8:49 AM, Xinliang David Li <davidxl@google.com> wrote:
> What is the macro to test POSIX IO on host? The guard uses
> TARGET_POSIX_IO which is not right.
>
> David
>
> On Fri, Jul 11, 2014 at 1:12 AM, Christophe Lyon
> <christophe.lyon@linaro.org> wrote:
>> On 11 July 2014 10:07, Richard Biener <richard.guenther@gmail.com> wrote:
>>> 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.
>>>
>>
>> On my side, commit 212448 breaks the cross-build of GCC for targets
>> using newlib:
>> * arm-none-eabi
>> * aarch64-none-elf
>> /usr/include/sys/stat.h: In function <80><98>void
>> gcov_output_files(const char*, gcov_info*)<80><99>:
>> /usr/include/sys/stat.h:317: error: too few arguments to function
>> <80><98>int mkdir(const char*, __mode_t)<80><99>
>> /tmp/1392958_1.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/gcov-tool.c:96:
>> error: at this point in file
>> make[2]: *** [gcov-tool.o] Error 1
>>
>> Christophe.
>>
>>>> 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

  reply	other threads:[~2014-07-11 16:44 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-13 20:43 Rong Xu
2014-01-16 17:30 ` Rong Xu
2014-03-03 22:02   ` Rong Xu
2014-04-15 22:05     ` Jan Hubicka
2014-04-16 17:28       ` Rong Xu
2014-04-17  5:19         ` Jan Hubicka
2014-05-01 22:37           ` Rong Xu
2014-05-05 17:17             ` Rong Xu
2014-07-11  8:07               ` Richard Biener
2014-07-11  8:12                 ` Christophe Lyon
2014-07-11 15:50                   ` Xinliang David Li
2014-07-11 16:44                     ` Rong Xu [this message]
2014-07-11 15:42                 ` Xinliang David Li
2014-07-11 16:03                   ` Jakub Jelinek
2014-07-11 16:13                     ` Xinliang David Li
2014-07-11 16:48                       ` Rong Xu
2014-07-11 18:30                         ` Rong Xu
2014-07-11 18:46                           ` Jan Hubicka
2014-07-11 18:53                             ` Rong Xu
2014-07-13  7:49                           ` Andreas Schwab
2014-05-15 20:37             ` Jan Hubicka
2014-05-20 22:59               ` Rong Xu
2014-05-25 23:43                 ` Jan Hubicka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAF1bQ=ReiuefoPM4_q1LbJ4SUh+2uwUqjxyX79XB4wdRH2q7kg@mail.gmail.com' \
    --to=xur@google.com \
    --cc=christophe.lyon@linaro.org \
    --cc=davidxl@google.com \
    --cc=dehao@google.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=richard.guenther@gmail.com \
    --cc=tejohnson@google.com \
    --cc=xinliangli@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).