On Sat, Oct 1, 2011 at 5:17 AM, Jan Hubicka <hubi...@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 <sing...@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 <sing...@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 >> > >