[llvm] r331801 - [Coverage] Take filenames into account when loading function records.

[Max Moroz via llvm-commits]

Author: dor1s
Date: Tue May  8 12:26:51 2018
New Revision: 331801

URL: http://llvm.org/viewvc/llvm-project?rev=331801&view=rev
Log:
[Coverage] Take filenames into account when loading function records.

Summary:
Don't skip functions with the same name but from different files.

That change makes it possible to generate code coverage reports from
different binaries compiled from different sources even if there are functions
with non-unique names. Without that change, code coverage for such functions is
missing except of the first function processed.

Reviewers: vsk, morehouse

Reviewed By: vsk

Subscribers: llvm-commits, kcc

Differential Revision: https://reviews.llvm.org/D46478

Modified:
    llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMapping.h
    llvm/trunk/lib/ProfileData/Coverage/CoverageMapping.cpp
    llvm/trunk/test/tools/llvm-cov/multiple-objects.test
    llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp

Modified: llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMapping.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMapping.h?rev=331801&r1=331800&r2=331801&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMapping.h (original)
+++ llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMapping.h Tue May  8 12:26:51 2018
@@ -17,6 +17,7 @@
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/StringRef.h"
@@ -506,7 +507,7 @@ public:
 /// This is the main interface to get coverage information, using a profile to
 /// fill out execution counts.
 class CoverageMapping {
-  StringSet<> FunctionNames;
+  DenseMap<size_t, DenseSet<size_t>> RecordProvenance;
   std::vector<FunctionRecord> Functions;
   std::vector<std::pair<std::string, uint64_t>> FuncHashMismatches;
   std::vector<std::pair<std::string, uint64_t>> FuncCounterMismatches;

Modified: llvm/trunk/lib/ProfileData/Coverage/CoverageMapping.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/Coverage/CoverageMapping.cpp?rev=331801&r1=331800&r2=331801&view=diff
==============================================================================
--- llvm/trunk/lib/ProfileData/Coverage/CoverageMapping.cpp (original)
+++ llvm/trunk/lib/ProfileData/Coverage/CoverageMapping.cpp Tue May  8 12:26:51 2018
@@ -207,8 +207,10 @@ Error CoverageMapping::loadFunctionRecor
   else
     OrigFuncName = getFuncNameWithoutPrefix(OrigFuncName, Record.Filenames[0]);
 
-  // Don't load records for functions we've already seen.
-  if (!FunctionNames.insert(OrigFuncName).second)
+  // Don't load records for (filenames, function) pairs we've already seen.
+  auto FilenamesHash = hash_combine_range(Record.Filenames.begin(),
+                                          Record.Filenames.end());
+  if (!RecordProvenance[FilenamesHash].insert(hash_value(OrigFuncName)).second)
     return Error::success();
 
   CounterMappingContext Ctx(Record.Expressions);

Modified: llvm/trunk/test/tools/llvm-cov/multiple-objects.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cov/multiple-objects.test?rev=331801&r1=331800&r2=331801&view=diff
==============================================================================
--- llvm/trunk/test/tools/llvm-cov/multiple-objects.test (original)
+++ llvm/trunk/test/tools/llvm-cov/multiple-objects.test Tue May  8 12:26:51 2018
@@ -6,6 +6,11 @@ REPORT: Filename{{ +}}Regions{{ +}}Misse
 REPORT-NEXT: ---
 REPORT-NEXT: header.h{{ +}}25{{ +}}14{{ +}}44.00%
 
+# Make sure that both use_1.cc and use_2.cc have coverage reported.
+# Before https://reviews.llvm.org/D46478, only one of them used to be reported.
+REPORT-NEXT: use_1.cc{{ +}}1{{ +}}0{{ +}}100.00%
+REPORT-NEXT: use_2.cc{{ +}}2{{ +}}0{{ +}}100.00%
+
 Instructions for regenerating the test:
 
 clang -std=c++11 -mllvm -enable-name-compression=false -fprofile-instr-generate -fcoverage-mapping use_1.cc -o use_1

Modified: llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp?rev=331801&r1=331800&r2=331801&view=diff
==============================================================================
--- llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp (original)
+++ llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp Tue May  8 12:26:51 2018
@@ -859,17 +859,34 @@ TEST_P(CoverageMappingTest, load_coverag
 TEST_P(CoverageMappingTest, skip_duplicate_function_record) {
   ProfileWriter.addRecord({"func", 0x1234, {1}}, Err);
 
+  // This record should be loaded.
   startFunction("func", 0x1234);
   addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
 
+  // This record should be loaded.
   startFunction("func", 0x1234);
   addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
+  addCMR(Counter::getCounter(0), "file2", 1, 1, 9, 9);
+
+  // This record should be skipped.
+  startFunction("func", 0x1234);
+  addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
+
+  // This record should be loaded.
+  startFunction("func", 0x1234);
+  addCMR(Counter::getCounter(0), "file2", 1, 1, 9, 9);
+  addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
+
+  // This record should be skipped.
+  startFunction("func", 0x1234);
+  addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
+  addCMR(Counter::getCounter(0), "file2", 1, 1, 9, 9);
 
   EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
 
   auto Funcs = LoadedCoverage->getCoveredFunctions();
   unsigned NumFuncs = std::distance(Funcs.begin(), Funcs.end());
-  ASSERT_EQ(1U, NumFuncs);
+  ASSERT_EQ(3U, NumFuncs);
 }
 
 // FIXME: Use ::testing::Combine() when llvm updates its copy of googletest.