<div dir="ltr">I understand that by itself this patch may not be that meaningful, I just wanted to get something out there asap. I will send the initial patches for clang and llvm-cov in the next couple of days.<br></div><div class="gmail_extra">
<br><br><div class="gmail_quote">2014-06-26 11:40 GMT-07:00 Philip Reames <span dir="ltr"><<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
On 06/26/2014 11:23 AM, Hal Finkel wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
----- Original Message -----<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
From: "Philip Reames" <<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>><br>
To: <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
Sent: Thursday, June 26, 2014 12:51:43 PM<br>
Subject: Re: [PATCH] Initial code coverage mapping data structures, and reader and writers + C interface for<br>
ProfileData library<br>
<br>
<br>
<br>
<br>
On 06/26/2014 09:43 AM, Alex L wrote:<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
2014-06-25 17:57 GMT-07:00 Eric Christopher < <a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a> > :<br>
<br>
<br>
So, any reason why we want the new coverage system? You didn't seem<br>
to<br>
highlight what you saw wrong with gcov or the way it works.<br>
<br>
I saw your original mail and it didn't have a lot of motivation here.<br>
<br>
Thanks.<br>
<br>
-eric<br>
<br>
<br>
<br>
I am not very familiar with GCOV, but the primary motivation for the<br>
new coverage system is to provide very accurate execution counts for<br>
a program. This would enable us to provide modern, high quality<br>
tools for code coverage analysis.<br>
<br>
By very accurate I mean that instead of reasoning about code coverage<br>
for basic blocks, branches and even lines, we would be able to<br>
reason about code coverage for the regions of source code that<br>
resemble the corresponding AST. This would enable the coverage tool<br>
to locate and mark the exact regions of code that weren't executed,<br>
even if the IR for those regions was transformed by the optimizer. I<br>
think that GCOV fails to provide coverage for certain constructs<br>
when optimizations are enabled. Also, I think that GCOV doesn't<br>
really show good coverage for the lines that have multiple regions<br>
with different execution counts, and the new system will enable us<br>
to create a tool which will have a better way to deal with this<br>
particular situation.<br>
<br>
<br>
Also, the GCOV way produces separate mapping files and counter files<br>
for each source file/object file, which can be somewhat<br>
inconvenient. In the new system we pack the mapping data into the<br>
generated IR and allow it to be merged by the linker, and as a<br>
result of that all our mapping information is embedded inside the<br>
program's executable.<br>
<br>
<br>
<br>
The new coverage tool will be able to provide a more interactive<br>
experience as well, by showing reports or code coverage only for<br>
selected items like certain functions, classes, etc.<br>
<br>
<br>
<br>
Also, this new coverage system will provide a library which various<br>
code coverage tools can use to make coverage reports without the<br>
need to parse the output of GCOV.<br>
I find nothing in the above explanation to justify including this<br>
code *in LLVM*. Such a tool may be useful, but this really sounds<br>
like an independent project (i.e. "replace gcov").<br>
<br>
Glancing through the code, I also see no real interaction with LLVM.<br>
This really seems like an independent profiling library which could<br>
be used to provide profiling data to LLVM, but is otherwise<br>
unrelated. Correct me if I'm wrong here - I did a *very* quick scan.<br>
<br>
Your post about this topic on llvm-dev has not generated any<br>
consensus. If anything, there seems to be an active disinterest in<br>
your proposal.<br>
</blockquote>
I'm not sure what you mean by "has not generated any consensus", I saw no discussion at all. That having been said, I know that a number of community members care a lot about coverage quantification (including me), and the gcov zillions-of-files approach clearly does not scale. Really, getting feedback on long RFCs is not easy, and I'd draw no inference from the lack of response to date.<br>
</blockquote></div></div>
To be clear, I am not opposed to supporting profiling. I'm in fact quite in support of the overall objective. It's simply that *this patch* *at the current time* doesn't seem ready.<br>
<br>
I also wonder if things like profile format readers need to be part of LLVM at all. Why isn't this handled entirely by the frontend or a separate tool? We can already represent profiling information in the IR. (Admittedly, in limited ways. But we should fix that!) I could even see having a collection of profile format readers being it's own sub project.<div class="">
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
That having been said, I'm not sure what to think about this patch. I think it will be easier to review once we see the code that uses it.<br>
</blockquote></div>
Agreed.<div class="HOEnZb"><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
-Hal<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Given the above, I would oppose the inclusion of this change set.<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
On Wed, Jun 25, 2014 at 4:32 PM, Alex L < <a href="mailto:arphaman@gmail.com" target="_blank">arphaman@gmail.com</a> > wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi everyone,<br>
This is a first patch that implements the data structures, readers<br>
and<br>
writers used by the new code coverage system. I added the new code<br>
to the<br>
ProfileData library. I also added a very minimal C api for the<br>
ProfileData<br>
library.<br>
<br>
<a href="http://reviews.llvm.org/D4301" target="_blank">http://reviews.llvm.org/D4301</a><br>
<br>
<br>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
<br>
</blockquote>
<br>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
<br>
</blockquote></blockquote>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>