[PATCH] D46478: [Coverage] Take filenames into account when loading function records.

[Max Moroz via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL331801: [Coverage] Take filenames into account when loading function records. (authored by Dor1s, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D46478

Files:
  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


Index: llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMapping.h
===================================================================
--- llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -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 @@
 /// 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;
Index: llvm/trunk/test/tools/llvm-cov/multiple-objects.test
===================================================================
--- llvm/trunk/test/tools/llvm-cov/multiple-objects.test
+++ llvm/trunk/test/tools/llvm-cov/multiple-objects.test
@@ -6,6 +6,11 @@
 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
Index: llvm/trunk/lib/ProfileData/Coverage/CoverageMapping.cpp
===================================================================
--- llvm/trunk/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ llvm/trunk/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -207,8 +207,10 @@
   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);
Index: llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp
===================================================================
--- llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp
+++ llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp
@@ -859,17 +859,34 @@
 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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46478.145759.patch
Type: text/x-patch
Size: 4026 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180508/7534f8cf/attachment.bin>