<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 2, 2017 at 8:57 AM, Reid Kleckner via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">rnk added a comment.<br>
<br>
I suspect that the appropriate fix is to give the __profd variables internal linkage, not to change the comdat name. The linker will still discard duplicate profc and profd sections with this change.<br>
<br>
Actually, I'm surprised that the profiling data isn't comdat associated with the function itself. Is it possible for instrprofiling to be different across different TUs, or to link PGO instrumented code with non-PGO instrumented code? If that's the case, we probably want to make all profiling metadata associated with the function itself.<br></blockquote><div><br></div><div><br></div><div>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).</div><div><br></div><div>David</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
<a href="https://reviews.llvm.org/D32688" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32688</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>