[PATCH] D32688: [Coverage] Comdat section name should be same as the variable name in COFF format

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Fri May 12 16:06:10 PDT 2017


On Fri, May 12, 2017 at 3:17 PM, Reid Kleckner via Phabricator <
reviews at reviews.llvm.org> wrote:

> rnk added a comment.
>
> Are we sure we want it like this? Doesn't this retain instrprof data
> structures from both A.obj and B.obj for inline functions present in both
> objs? Won't that be a problem?
>
> >> From @davidxl :
> >
> > It was comdat associated with the function before -- but it was wrong.
> The problem is that different comdat functions may have different CFGs (due
> to different early inlining), leading to incompatible profile counter data.
> Note that a comdat function can be later inlined, so using a shared copy of
> profile counter will be bad (out of bound access, not to mention profile
> use phase can not use it).
>
> I agree, if instrprof runs before inlining, then we don't want these to be
> in a comdat group with the function. It's not safe for other functions to
> reference the data after inlining.
>
> However, if instrprof runs after "early inlining", which I didn't think we
> had any of, then how have we solved the problem of different comdat
> functions having different CFGs at instrumentation time?
>

We have instrprof running after early inlining for IR based
instrumentation.  The comdat key is created using cfg hashing so that
comdat counters from the same comdat function are partially merged --
comdat instances sharing the same cfg will share the same counter.

David

>
>
> https://reviews.llvm.org/D32688
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170512/900c149d/attachment.html>


More information about the llvm-commits mailing list