From: "Sharad Singhai (शरद सिंघई)" <singhai@google.com>
To: Xinliang David Li <davidxl@google.com>
Cc: Jan Hubicka <hubicka@ucw.cz>,
reply@codereview.appspotmail.com, gcc-patches@gcc.gnu.org
Subject: Re: Disable early inlining while compiling for coverage (issue5173042)
Date: Sat, 01 Oct 2011 19:49:00 -0000 [thread overview]
Message-ID: <CAKxPW64tfdfEgYwJ_6aRex+p6o3n7b8q+LSZrWq2u-Fc6cu3NQ@mail.gmail.com> (raw)
In-Reply-To: <CAAkRFZLcGDy1sJuQVwSSJnkKM1HYwVV4v1YpcZEr4hRL6YBUAg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3400 bytes --]
I have updated PR/45890 with a test case.
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45890
In any case, I am attaching a test case here as well.
Sharad
On Sat, Oct 1, 2011 at 10:19 AM, Xinliang David Li <davidxl@google.com> wrote:
>
> On Sat, Oct 1, 2011 at 5:17 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> >> Yes, this will improve test coverage option's usability, but please
> >> provide the example to explain the issues.
> >>
> >> David
> >>
> >> On Fri, Sep 30, 2011 at 6:12 PM, Sharad Singhai <singhai@google.com> wrote:
> >> > This patch disables early inlining when --coverage option is
> >> > specified. This improves coverage data in presence of other
> >> > optimizations, specially with -O2 where early inlining changes the
> >> > control flow graph sufficiently enough to generate seemingly very odd
> >> > source coverage.
> >
> > BTW early inlining was introduced to make coverage possible on some C++
> > sources, like tramp3d ;)
>
> Early inline can be important for FDO performance reasons -- as inline
> instances get 'context' sensitive profile data.
>
> >However the problem here is that since we moved
> > coverage to SSA, we do it too late. My longer term plan is to separate
> > coverage and profile feedback passes (so they can't be done both together) and
> > instrument early for coverage & ensure that everything before coverage is done
> > is save WRT line info.
>
> Yes, coverage and FDO are two different animals having different
> requirements -- they happen to share the same instrumentation
> mechanism.
>
> > Inlining alone does not mess up the line info, but most
> > optimizations we have in early optimization queue do.
>
> Inlining can do some damage too but to a less extent. For instance,
> the exit block of the callee instance merged with caller's bb causing
> confusion.
>
> >
> > We discussed it back when Richi implemented SSA profiling but we didn't do that
> > basically due to lack of testcases. Would be possible to take one you have
> > and fill in some PRs? Those are regressions WRT pre-SSA profiling releases (I think 4.5?)
>
> Yes.
>
> Sharad, I did not see the test case attached? Please file a bug about
> this. In the meantime, you can checkin the workaround to google
> banches.
>
> thanks,
>
> David
>
> >
> > Honza
> >> >
> >> > Bootstrapped okay and regression tests passed.
> >> >
> >> > Okay for google/gcc-4_6?
> >> >
> >> > 2011-09-30 Sharad Singhai <singhai@google.com>
> >> >
> >> > * gcc.c (cc1_options): Added -fno-early-inlining for coverage.
> >> >
> >> > Index: gcc.c
> >> > ===================================================================
> >> > --- gcc.c (revision 179402)
> >> > +++ gcc.c (working copy)
> >> > @@ -776,7 +776,7 @@
> >> > %{!fsyntax-only:%{S:%W{o*}%{!o*:-o %b.s}}}\
> >> > %{fsyntax-only:-o %j} %{-param*}\
> >> > %{fmudflap|fmudflapth:-fno-builtin -fno-merge-constants}\
> >> > - %{coverage:-fprofile-arcs -ftest-coverage}";
> >> > + %{coverage:-fprofile-arcs -ftest-coverage -fno-early-inlining}";
> >> >
> >> > /* If an assembler wrapper is used to invoke post-assembly tools
> >> > like MAO, --save-temps need to be passed to save the output of
> >> >
> >> > --
> >> > This patch is available for review at http://codereview.appspot.com/5173042
> >> >
> >
[-- Attachment #2: gcov.tar.gz --]
[-- Type: application/x-gzip, Size: 791 bytes --]
next prev parent reply other threads:[~2011-10-01 19:49 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-01 1:12 Sharad Singhai
2011-10-01 4:15 ` Xinliang David Li
2011-10-01 5:37 ` Sharad Singhai (शरद सिंघई)
2011-10-01 12:17 ` Jan Hubicka
2011-10-01 17:19 ` Xinliang David Li
2011-10-01 19:49 ` Sharad Singhai (शरद सिंघई) [this message]
2011-10-02 10:04 ` Jan Hubicka
2011-10-02 10:09 ` Jan Hubicka
2011-11-07 21:40 ` Sharad Singhai
2011-11-07 22:16 ` Xinliang David Li
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=CAKxPW64tfdfEgYwJ_6aRex+p6o3n7b8q+LSZrWq2u-Fc6cu3NQ@mail.gmail.com \
--to=singhai@google.com \
--cc=davidxl@google.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=hubicka@ucw.cz \
--cc=reply@codereview.appspotmail.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).