Re: [PATCH] D20287: [Coverage] Ensure that the hash for a used function is non-zero.

[Igor Kudrin via cfe-commits]

ikudrin abandoned this revision.
ikudrin added a comment.

This change is not needed anymore because the whole issue was fixed in 
http://reviews.llvm.org/D20286.


http://reviews.llvm.org/D20287



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20287: [Coverage] Ensure that the hash for a used function is non-zero.

[David Li via cfe-commits]

davidxl added a comment.

Strictly speaking, this patch requires a version bump of the indexed format. 
The profile reader also needs to adjust the FunctionHash computation (either 
using 0 or simple function hash) based on the version of the profile data.

Check with Justin/vsk to see if it is important to keep backward compatability 
for these simple functions in older profile data.

On the other hand, I wonder what is the real root cause of the problem.  The 
dummy function record does not have its 'own' profile counts, so

if (std::error_code EC = ProfileReader.getFunctionCounts(

  Record.FunctionName, Record.FunctionHash, Counts)) {

call in CoverageMapping::load (..)

method should return the the counts of the used  function even with zero 
functionhash.  What did I miss?


http://reviews.llvm.org/D20287



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20287: [Coverage] Ensure that the hash for a used function is non-zero.

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

Does anyone known, why we need dummy coverage mapping records for unused 
functions? How are they used? Isn't it better to remove these dummy records to 
prevent confusion with the real ones?


http://reviews.llvm.org/D20287



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20287: [Coverage] Ensure that the hash for a used function is non-zero.

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

The motivation sample (using llvm-cov with http://reviews.llvm.org/D20286 
applied):

  $ cat > sample.h << EOF
  inline int sample_func(int A) {
return A;
  }
  EOF
  $ cat > dummy.cpp << EOF
  #include "sample.h"
  EOF
  $ cat > sample.cpp << EOF
  #include "sample.h"
  
  int main() {
return sample_func(5);
  }
  EOF
  $ clang++ -fprofile-instr-generate -fcoverage-mapping dummy.cpp sample.cpp
  $ ./a.out
  $ llvm-profdata merge -o default.profdata default.profraw
  $ llvm-cov show a.out -instr-profile default.profdata
  sample.h:
0|1|inline int sample_func(int A) {
0|2|  return A;
0|3|}
  
  sample.cpp:
 |1|#include "sample.h"
 |2|
1|3|int main() {
1|4|  return sample_func(5);
1|5|}

And if this patch is applied:

  $ clang++ -fprofile-instr-generate -fcoverage-mapping dummy.cpp sample.cpp
  $ ./a.out
  $ llvm-profdata merge -o default.profdata default.profraw
  $ llvm-cov show a.out -instr-profile default.profdata
  sample.h:
1|1|inline int sample_func(int A) {
1|2|  return A;
1|3|}
  
  sample.cpp:
 |1|#include "sample.h"
 |2|
1|3|int main() {
1|4|  return sample_func(5);
1|5|}


http://reviews.llvm.org/D20287



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits