On 01/20/12 22:41, Xinliang David Li wrote:
Nathan, I just noticed this path. This is a good improvement over the
existing scheme.
I see one potential problem with the patch -- different instances of
the same comdat function can potentially have different control flows
(due to differences in early inline, early optimizations in different
module contexts) before profile counter allocation -- which means
making the profile counters 'comdat' can later cause coverage mismatch
problems during profile-use phase.
Right, I have an new patch that reenables the comdat-awareness, however there is
one case I theorized about but couldn't generate a testcase for. Your email
convinced me to try harder, and I now have such a testcase that needs
addressing. Anyway, the tree-based coverage counters treat the per-function
counters as-if they are function-scope static variables. Hence inlining Foo
into Baz will not add new counters to Baz but increment Foo's counters. So you
can't distinguish between different inlinings of Foo. Of course optimizing
Foo's inline instance in Baz may delete various basic blocks and so lead to
non-increments. However, for the purposes of profiling, the average behaviour
over all inlinings in that translation unit is used. As I recall the rtl-based
counters didn't do this.
Anyway, with comdat-awareness, the case I need to address is a function Foo that
is inlined in some places but not in others. So there's a comdat Foo instance
emitted. That instance may or may not be selected by the linker. If it is not
selected, we still want to track the counter values for its inlinings in that
TU. If it is selected, we don't want to double count its counter values. I
need to think about this a bit, but have some ideas.
In this patch, fn data buffer is introduced. What is that for and how
does it work?
The intention is that the fn_info structure is in the same comdat group as the
comdat function to which it refers. Therefore, if the instance of fn_info in
the executable will match the instance of the function to which it refers.
However, each object file needs to generate coverage information for all the
functions in that object file and correctly mark whether a comdat function in it
was in the final link or not. So its array of pointers to fn_info will name the
fn_infos of all its functions. For a comdat function, the slot will select the
fn_info of some other TU. The gcov_info back-pointer in each fn_info allows the
gcov runtime to detect this case.
Does that help?
nathan