* [Bug gcov-profile/45890] Coverage may be weakened by r164986
2010-10-05 10:44 [Bug gcov-profile/45890] New: Coverage may be weakened by r164986 rguenth at gcc dot gnu.org
@ 2011-10-01 19:40 ` singhai at gcc dot gnu.org
2011-10-01 19:43 ` singhai at gcc dot gnu.org
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: singhai at gcc dot gnu.org @ 2011-10-01 19:40 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45890
--- Comment #1 from Sharad Singhai <singhai at gcc dot gnu.org> 2011-10-01 19:39:32 UTC ---
Created attachment 25392
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25392
tar of source and coverage files
^ permalink raw reply [flat|nested] 8+ messages in thread* [Bug gcov-profile/45890] Coverage may be weakened by r164986
2010-10-05 10:44 [Bug gcov-profile/45890] New: Coverage may be weakened by r164986 rguenth at gcc dot gnu.org
2011-10-01 19:40 ` [Bug gcov-profile/45890] " singhai at gcc dot gnu.org
@ 2011-10-01 19:43 ` singhai at gcc dot gnu.org
2011-10-04 9:15 ` rguenther at suse dot de
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: singhai at gcc dot gnu.org @ 2011-10-01 19:43 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45890
--- Comment #2 from Sharad Singhai <singhai at gcc dot gnu.org> 2011-10-01 19:42:48 UTC ---
I don't know if it is related. But the coverage is imprecise in case of
attached source file foo.c. Build it with O2, as
gcc --coverage -O2 foo.c
./a.out
gcov -b foo.c
Now the coverage data for the 'if' condition in 'sum' looks like this:
(in attached file foo.c.gcov)
8: 8: if (v[i]) total += 1;
branch 0 never executed
branch 1 never executed
branch 2 taken 75% (fallthrough)
branch 3 taken 25%
Thus a simple two-way conditional looks like a four-way branch. It is due to
early inlining of 'sum' into 'main where a couple of bb's get eliminated/merged
so that the branch coverage incorrectly gets attributed to the conditional.
Similarly the coverage data for the for-loop in 'sum' looks like this
9: 7: for (i = 0; i < N; ++i) {
branch 0 never executed
branch 1 never executed
branch 2 taken 89%
branch 3 taken 11% (fallthrough)
After disabling early inlining, the coverage data looks saner. (Attached in
foo.c.gcov.no-early-inlining.)
gcc --coverage -O2 -fno-early-inlining foo.c
./a.out
gcov -b foo.c
Adding the following patch mitigates the issue.
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
^ permalink raw reply [flat|nested] 8+ messages in thread* [Bug gcov-profile/45890] Coverage may be weakened by r164986
2010-10-05 10:44 [Bug gcov-profile/45890] New: Coverage may be weakened by r164986 rguenth at gcc dot gnu.org
2011-10-01 19:40 ` [Bug gcov-profile/45890] " singhai at gcc dot gnu.org
2011-10-01 19:43 ` singhai at gcc dot gnu.org
@ 2011-10-04 9:15 ` rguenther at suse dot de
2011-10-04 9:19 ` hubicka at ucw dot cz
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: rguenther at suse dot de @ 2011-10-04 9:15 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45890
--- Comment #3 from rguenther at suse dot de <rguenther at suse dot de> 2011-10-04 09:14:42 UTC ---
On Sat, 1 Oct 2011, singhai at gcc dot gnu.org wrote:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45890
>
> --- Comment #2 from Sharad Singhai <singhai at gcc dot gnu.org> 2011-10-01 19:42:48 UTC ---
> I don't know if it is related. But the coverage is imprecise in case of
> attached source file foo.c. Build it with O2, as
>
> gcc --coverage -O2 foo.c
> ./a.out
> gcov -b foo.c
>
> Now the coverage data for the 'if' condition in 'sum' looks like this:
> (in attached file foo.c.gcov)
>
> 8: 8: if (v[i]) total += 1;
> branch 0 never executed
> branch 1 never executed
> branch 2 taken 75% (fallthrough)
> branch 3 taken 25%
>
> Thus a simple two-way conditional looks like a four-way branch. It is due to
> early inlining of 'sum' into 'main where a couple of bb's get eliminated/merged
> so that the branch coverage incorrectly gets attributed to the conditional.
> Similarly the coverage data for the for-loop in 'sum' looks like this
>
> 9: 7: for (i = 0; i < N; ++i) {
> branch 0 never executed
> branch 1 never executed
> branch 2 taken 89%
> branch 3 taken 11% (fallthrough)
>
> After disabling early inlining, the coverage data looks saner. (Attached in
> foo.c.gcov.no-early-inlining.)
>
> gcc --coverage -O2 -fno-early-inlining foo.c
> ./a.out
> gcov -b foo.c
>
> Adding the following patch mitigates the issue.
That's surely not the way to go. Why do you want precise coverage
with -O2?
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread* [Bug gcov-profile/45890] Coverage may be weakened by r164986
2010-10-05 10:44 [Bug gcov-profile/45890] New: Coverage may be weakened by r164986 rguenth at gcc dot gnu.org
` (2 preceding siblings ...)
2011-10-04 9:15 ` rguenther at suse dot de
@ 2011-10-04 9:19 ` hubicka at ucw dot cz
2011-10-04 9:51 ` rguenther at suse dot de
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: hubicka at ucw dot cz @ 2011-10-04 9:19 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45890
--- Comment #4 from Jan Hubicka <hubicka at ucw dot cz> 2011-10-04 09:18:56 UTC ---
> > gcc --coverage -O2 -fno-early-inlining foo.c
> > ./a.out
> > gcov -b foo.c
> >
> > Adding the following patch mitigates the issue.
>
> That's surely not the way to go. Why do you want precise coverage
> with -O2?
I guess for performance reasons.
I would vote for adding coverage pass just after into-SSA.
Honza
^ permalink raw reply [flat|nested] 8+ messages in thread* [Bug gcov-profile/45890] Coverage may be weakened by r164986
2010-10-05 10:44 [Bug gcov-profile/45890] New: Coverage may be weakened by r164986 rguenth at gcc dot gnu.org
` (3 preceding siblings ...)
2011-10-04 9:19 ` hubicka at ucw dot cz
@ 2011-10-04 9:51 ` rguenther at suse dot de
2011-10-04 10:05 ` hubicka at ucw dot cz
2011-10-04 10:18 ` rguenther at suse dot de
6 siblings, 0 replies; 8+ messages in thread
From: rguenther at suse dot de @ 2011-10-04 9:51 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45890
--- Comment #5 from rguenther at suse dot de <rguenther at suse dot de> 2011-10-04 09:50:55 UTC ---
On Tue, 4 Oct 2011, hubicka at ucw dot cz wrote:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45890
>
> --- Comment #4 from Jan Hubicka <hubicka at ucw dot cz> 2011-10-04 09:18:56 UTC ---
> > > gcc --coverage -O2 -fno-early-inlining foo.c
> > > ./a.out
> > > gcov -b foo.c
> > >
> > > Adding the following patch mitigates the issue.
> >
> > That's surely not the way to go. Why do you want precise coverage
> > with -O2?
> I guess for performance reasons.
> I would vote for adding coverage pass just after into-SSA.
Where do we do it right now? At profile instrumentation stage, right?
I'd say if we don't need SSA form we should do coverage instrumentation
right after CFG construction, no? Before into-SSA we already have
stuff like OMP lowering. But yes, just after into-SSA would be
an improvement.
Richard.
^ permalink raw reply [flat|nested] 8+ messages in thread* [Bug gcov-profile/45890] Coverage may be weakened by r164986
2010-10-05 10:44 [Bug gcov-profile/45890] New: Coverage may be weakened by r164986 rguenth at gcc dot gnu.org
` (4 preceding siblings ...)
2011-10-04 9:51 ` rguenther at suse dot de
@ 2011-10-04 10:05 ` hubicka at ucw dot cz
2011-10-04 10:18 ` rguenther at suse dot de
6 siblings, 0 replies; 8+ messages in thread
From: hubicka at ucw dot cz @ 2011-10-04 10:05 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45890
--- Comment #6 from Jan Hubicka <hubicka at ucw dot cz> 2011-10-04 10:04:37 UTC ---
> Where do we do it right now? At profile instrumentation stage, right?
> I'd say if we don't need SSA form we should do coverage instrumentation
> right after CFG construction, no? Before into-SSA we already have
> stuff like OMP lowering. But yes, just after into-SSA would be
> an improvement.
We could also re-instantiate the former profiling pass before SSA for coverage,
but it seems to me that there is no real reason to destroy line info before
we get into-SSA and things will be a bit smoother if we do instrumentation
at SSA form only. But I don't really have any strong opinions here.
^ permalink raw reply [flat|nested] 8+ messages in thread* [Bug gcov-profile/45890] Coverage may be weakened by r164986
2010-10-05 10:44 [Bug gcov-profile/45890] New: Coverage may be weakened by r164986 rguenth at gcc dot gnu.org
` (5 preceding siblings ...)
2011-10-04 10:05 ` hubicka at ucw dot cz
@ 2011-10-04 10:18 ` rguenther at suse dot de
6 siblings, 0 replies; 8+ messages in thread
From: rguenther at suse dot de @ 2011-10-04 10:18 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45890
--- Comment #7 from rguenther at suse dot de <rguenther at suse dot de> 2011-10-04 10:18:18 UTC ---
On Tue, 4 Oct 2011, hubicka at ucw dot cz wrote:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45890
>
> --- Comment #6 from Jan Hubicka <hubicka at ucw dot cz> 2011-10-04 10:04:37 UTC ---
> > Where do we do it right now? At profile instrumentation stage, right?
> > I'd say if we don't need SSA form we should do coverage instrumentation
> > right after CFG construction, no? Before into-SSA we already have
> > stuff like OMP lowering. But yes, just after into-SSA would be
> > an improvement.
>
> We could also re-instantiate the former profiling pass before SSA for coverage,
> but it seems to me that there is no real reason to destroy line info before
> we get into-SSA and things will be a bit smoother if we do instrumentation
> at SSA form only. But I don't really have any strong opinions here.
Yeah, I suppose you are right.
^ permalink raw reply [flat|nested] 8+ messages in thread