Disable early inlining while compiling for coverage (issue5173042)

Xinliang David Li davidxl@google.com
Sat Oct 1 17:19:00 GMT 2011


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
>> >
>



More information about the Gcc-patches mailing list