On Mon, 13 Mar 2017, Martin Liška wrote:
> On 03/13/2017 02:53 PM, Richard Biener wrote:
> > On Mon, 13 Mar 2017, Martin Liška wrote:
> >
> >> On 03/13/2017 02:01 PM, Richard Biener wrote:
> >>> On Mon, 13 Mar 2017, Richard Biener wrote:
> >>>
> >>>> On Fri, 10 Mar 2017, Martin Liška wrote:
> >>>>
> >>>>> Hello.
> >>>>>
> >>>>> As briefly discussed in the PR, there are BB that do not correspond to
> >>>>> a real
> >>>>> line in source
> >>>>> code. profile.c emits locations for all BBs that have a gimple statement
> >>>>> belonging to a line.
> >>>>> I hope these should be marked in gcov utility and not added in
> >>>>> --all-block
> >>>>> mode to counts of lines.
> >>>>>
> >>>>> Patch survives make check RUNTESTFLAGS="gcov.exp".
> >>>>>
> >>>>> Thanks for review and feedback.
> >>>>
> >>>> Humm, the patch doesn't seem to change the BBs associated with a line
> >>>> but rather somehow changes how we compute counts (the patch lacks a
> >>>> description of how you arrived at it). I expected the line-to-BB
> >>>> assignment process to be changed (whereever that is...).
> >>
> >> Currently, each basic block must belong to a source line. It's how gcov
> >> iterates all blocks (via lines).
> >>
> >>>
> >>> Ah, ok, looking at where output_location is called on I see we do not
> >>> assign any line to that block. But still why does
> >>> line->has_block (arc->src) return true?
> >>
> >> Good objection! Problematic that 4->5 edge really comes from the same
> >> line.
> >>
> >> <bb 4> [0.00%]:
> >> ret_7 = 111;
> >> PROF_edge_counter_10 = __gcov0.UuT[0];
> >> PROF_edge_counter_11 = PROF_edge_counter_10 + 1;
> >> __gcov0.UuT[0] = PROF_edge_counter_11;
> >>
> >> <bb 5> [0.00%]:
> >> # ret_1 = PHI <ret_5(3), ret_7(4)>
> >> goto <bb 7>; [0.00%]
> >
> > Yes, but that's basically meaningless, unless not all edges come from the
> > same line? I see nowhere where we'd explicitely assign lines to
> > edges so it must be gcov "reconstructing" this somewhere.
>
> That's why I added the another flag. We stream locations for basic blocks via
> 'output_location' function. And assignment blocks to lines happens here:
>
> static void
> add_line_counts (coverage_t *coverage, function_t *fn)
> {
> ...
> if (!ix || ix + 1 == fn->num_blocks)
> /* Entry or exit block */;
> else if (flag_all_blocks)
> {
> line_t *block_line = line;
>
> if (!block_line)
> block_line = &sources[fn->src].lines[fn->line];
>
> block->chain = block_line->u.blocks;
> block_line->u.blocks = block;
> }
>
> where line is always changes when we reach a BB that has a source line
> assigned. Thus it's changed
> for BB 4 and that's why BB 5 is added to same line.
Ah, so this means we should "clear" the current line for BB 5 in
output_location? At least I don't see how your patch may not regress
some cases where the line wasn't output as an optimization?
OTOH I don't know much about gcov format.
Richard.
> Martin
>
> >
> > Richard.
> >
> >> Martin
> >>
> >>>
> >>> Richard.
> >>>
> >>
> >>
> >
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
