From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23930 invoked by alias); 1 Oct 2011 17:19:29 -0000 Received: (qmail 23920 invoked by uid 22791); 1 Oct 2011 17:19:27 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (74.125.121.67) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 01 Oct 2011 17:19:12 +0000 Received: from hpaq1.eem.corp.google.com (hpaq1.eem.corp.google.com [172.25.149.1]) by smtp-out.google.com with ESMTP id p91HJB8P019774 for ; Sat, 1 Oct 2011 10:19:11 -0700 Received: from iabz7 (iabz7.prod.google.com [10.12.102.7]) by hpaq1.eem.corp.google.com with ESMTP id p91HJ9w9001248 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Sat, 1 Oct 2011 10:19:10 -0700 Received: by iabz7 with SMTP id z7so4709142iab.35 for ; Sat, 01 Oct 2011 10:19:09 -0700 (PDT) Received: by 10.231.29.79 with SMTP id p15mr6992359ibc.16.1317489548832; Sat, 01 Oct 2011 10:19:08 -0700 (PDT) MIME-Version: 1.0 Received: by 10.231.29.79 with SMTP id p15mr6992338ibc.16.1317489548242; Sat, 01 Oct 2011 10:19:08 -0700 (PDT) Received: by 10.231.17.136 with HTTP; Sat, 1 Oct 2011 10:19:08 -0700 (PDT) In-Reply-To: <20111001121717.GB25985@atrey.karlin.mff.cuni.cz> References: <20111001011237.1CF69A6832@nabu.mtv.corp.google.com> <20111001121717.GB25985@atrey.karlin.mff.cuni.cz> Date: Sat, 01 Oct 2011 17:19:00 -0000 Message-ID: Subject: Re: Disable early inlining while compiling for coverage (issue5173042) From: Xinliang David Li To: Jan Hubicka Cc: Sharad Singhai , reply@codereview.appspotmail.com, gcc-patches@gcc.gnu.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-10/txt/msg00025.txt.bz2 On Sat, Oct 1, 2011 at 5:17 AM, Jan Hubicka 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 wro= te: >> > 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. =A0My 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 d= o that > basically due to lack of testcases. =A0Would be possible to take one you = have > and fill in some PRs? Those are regressions WRT pre-SSA profiling release= s (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 =A0 Sharad Singhai =A0 >> > >> > =A0 =A0 =A0 =A0* gcc.c (cc1_options): Added -fno-early-inlining for co= verage. >> > >> > Index: gcc.c >> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> > --- gcc.c =A0 =A0 =A0 (revision 179402) >> > +++ gcc.c =A0 =A0 =A0 (working copy) >> > @@ -776,7 +776,7 @@ >> > =A0%{!fsyntax-only:%{S:%W{o*}%{!o*:-o %b.s}}}\ >> > =A0%{fsyntax-only:-o %j} %{-param*}\ >> > =A0%{fmudflap|fmudflapth:-fno-builtin -fno-merge-constants}\ >> > - %{coverage:-fprofile-arcs -ftest-coverage}"; >> > + %{coverage:-fprofile-arcs -ftest-coverage -fno-early-inlining}"; >> > >> > =A0/* If an assembler wrapper is used to invoke post-assembly tools >> > =A0 =A0like MAO, --save-temps need to be passed to save the output of >> > >> > -- >> > This patch is available for review at http://codereview.appspot.com/51= 73042 >> > >