Skip to content

[llvm-cov] Add support for baseline coverage #117910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fmeum
Copy link
Contributor

@fmeum fmeum commented Nov 27, 2024

When no profile is provided via --instr-profile, the export/report/show commands now emit coverage data equivalent to that obtained from a profile with all zero counters ("baseline coverage").

This is useful for build systems (e.g. Bazel) that can track coverage information for each build target, even those that are never linked into tests and thus don't have runtime coverage data recorded. By merging in baseline coverage, lines in files that aren't linked into tests are correctly reported as uncovered.

@llvmbot llvmbot added PGO Profile Guided Optimizations llvm:binary-utilities labels Nov 27, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 27, 2024

@llvm/pr-subscribers-pgo

@llvm/pr-subscribers-llvm-binary-utilities

Author: Fabian Meumertzheim (fmeum)

Changes

When no profile is provided via --instr-profile, the export/report/show commands now emit coverage data equivalent to that obtained from a profile with all zero counters ("baseline coverage").

This is useful for build systems (e.g. Bazel) that can track coverage information for each build target, even those that are never linked into tests and thus don't have runtime coverage data recorded. By merging in baseline coverage, lines in files that aren't linked into tests are correctly reported as uncovered.


Patch is 22.47 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/117910.diff

6 Files Affected:

  • (modified) llvm/docs/CommandGuide/llvm-cov.rst (+12-3)
  • (modified) llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h (+15-9)
  • (modified) llvm/lib/ProfileData/Coverage/CoverageMapping.cpp (+73-47)
  • (added) llvm/test/tools/llvm-cov/showLineExecutionCounts-lcov-baseline.test (+37)
  • (modified) llvm/tools/llvm-cov/CodeCoverage.cpp (+35-21)
  • (modified) llvm/unittests/ProfileData/CoverageMappingTest.cpp (+3-1)
diff --git a/llvm/docs/CommandGuide/llvm-cov.rst b/llvm/docs/CommandGuide/llvm-cov.rst
index 968f3c452f5584..5f19eb67e92fea 100644
--- a/llvm/docs/CommandGuide/llvm-cov.rst
+++ b/llvm/docs/CommandGuide/llvm-cov.rst
@@ -188,7 +188,7 @@ SHOW COMMAND
 SYNOPSIS
 ^^^^^^^^
 
-:program:`llvm-cov show` [*options*] -instr-profile *PROFILE* [*BIN*] [*-object BIN*]... [*-sources*] [*SOURCE*]...
+:program:`llvm-cov show` [*options*] [-instr-profile *PROFILE*] [*BIN*] [*-object BIN*]... [*-sources*] [*SOURCE*]...
 
 DESCRIPTION
 ^^^^^^^^^^^
@@ -200,6 +200,9 @@ filtered to only show the coverage for the files listed in *SOURCE*....
 *BIN* may be an executable, object file, dynamic library, or archive (thin or
 otherwise).
 
+If *PROFILE* is not provided, the command displays the baseline coverage of the
+binaries with all zero execution counts.
+
 To use :program:`llvm-cov show`, you need a program that is compiled with
 instrumentation to emit profile and coverage data. To build such a program with
 ``clang`` use the ``-fprofile-instr-generate`` and ``-fcoverage-mapping``
@@ -390,7 +393,7 @@ REPORT COMMAND
 SYNOPSIS
 ^^^^^^^^
 
-:program:`llvm-cov report` [*options*] -instr-profile *PROFILE* [*BIN*] [*-object BIN*]... [*-sources*] [*SOURCE*]...
+:program:`llvm-cov report` [*options*] [-instr-profile *PROFILE*] [*BIN*] [*-object BIN*]... [*-sources*] [*SOURCE*]...
 
 DESCRIPTION
 ^^^^^^^^^^^
@@ -402,6 +405,9 @@ filtered to only show the coverage for the files listed in *SOURCE*....
 *BIN* may be an executable, object file, dynamic library, or archive (thin or
 otherwise).
 
+If *PROFILE* is not provided, the command displays the baseline coverage of the
+binaries with all zero execution counts.
+
 If no source files are provided, a summary line is printed for each file in the
 coverage data. If any files are provided, summaries can be shown for each
 function in the listed files if the ``-show-functions`` option is enabled.
@@ -480,7 +486,7 @@ EXPORT COMMAND
 SYNOPSIS
 ^^^^^^^^
 
-:program:`llvm-cov export` [*options*] -instr-profile *PROFILE* [*BIN*] [*-object BIN*]... [*-sources*] [*SOURCE*]...
+:program:`llvm-cov export` [*options*] [-instr-profile *PROFILE*] [*BIN*] [*-object BIN*]... [*-sources*] [*SOURCE*]...
 
 DESCRIPTION
 ^^^^^^^^^^^
@@ -489,6 +495,9 @@ The :program:`llvm-cov export` command exports coverage data of the binaries
 *BIN*... using the profile data *PROFILE* in either JSON or lcov trace file
 format.
 
+If *PROFILE* is not provided, the command exports baseline coverage data
+with all zero execution counts.
+
 When exporting JSON, the regions, functions, branches, expansions, and
 summaries of the coverage data will be exported. When exporting an lcov trace
 file, the line-based coverage, branch coverage, and summaries will be exported.
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index e631e3899fd4de..0485b8cbc39d04 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -937,18 +937,23 @@ class CoverageMapping {
   // Load coverage records from readers.
   static Error loadFromReaders(
       ArrayRef<std::unique_ptr<CoverageMappingReader>> CoverageReaders,
-      IndexedInstrProfReader &ProfileReader, CoverageMapping &Coverage);
+      std::optional<std::reference_wrapper<IndexedInstrProfReader>>
+          &ProfileReader,
+      CoverageMapping &Coverage);
 
   // Load coverage records from file.
   static Error
   loadFromFile(StringRef Filename, StringRef Arch, StringRef CompilationDir,
-               IndexedInstrProfReader &ProfileReader, CoverageMapping &Coverage,
-               bool &DataFound,
+               std::optional<std::reference_wrapper<IndexedInstrProfReader>>
+                   &ProfileReader,
+               CoverageMapping &Coverage, bool &DataFound,
                SmallVectorImpl<object::BuildID> *FoundBinaryIDs = nullptr);
 
   /// Add a function record corresponding to \p Record.
-  Error loadFunctionRecord(const CoverageMappingRecord &Record,
-                           IndexedInstrProfReader &ProfileReader);
+  Error loadFunctionRecord(
+      const CoverageMappingRecord &Record,
+      const std::optional<std::reference_wrapper<IndexedInstrProfReader>>
+          &ProfileReader);
 
   /// Look up the indices for function records which are at least partially
   /// defined in the specified file. This is guaranteed to return a superset of
@@ -964,15 +969,16 @@ class CoverageMapping {
   /// Load the coverage mapping using the given readers.
   static Expected<std::unique_ptr<CoverageMapping>>
   load(ArrayRef<std::unique_ptr<CoverageMappingReader>> CoverageReaders,
-       IndexedInstrProfReader &ProfileReader);
+       std::optional<std::reference_wrapper<IndexedInstrProfReader>>
+           &ProfileReader);
 
   /// Load the coverage mapping from the given object files and profile. If
   /// \p Arches is non-empty, it must specify an architecture for each object.
   /// Ignores non-instrumented object files unless all are not instrumented.
   static Expected<std::unique_ptr<CoverageMapping>>
-  load(ArrayRef<StringRef> ObjectFilenames, StringRef ProfileFilename,
-       vfs::FileSystem &FS, ArrayRef<StringRef> Arches = {},
-       StringRef CompilationDir = "",
+  load(ArrayRef<StringRef> ObjectFilenames,
+       std::optional<StringRef> ProfileFilename, vfs::FileSystem &FS,
+       ArrayRef<StringRef> Arches = {}, StringRef CompilationDir = "",
        const object::BuildIDFetcher *BIDFetcher = nullptr,
        bool CheckBinaryIDs = false);
 
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 119e09187b9080..6fc0f3e42a9353 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -794,7 +794,8 @@ class MCDCDecisionRecorder {
 
 Error CoverageMapping::loadFunctionRecord(
     const CoverageMappingRecord &Record,
-    IndexedInstrProfReader &ProfileReader) {
+    const std::optional<std::reference_wrapper<IndexedInstrProfReader>>
+        &ProfileReader) {
   StringRef OrigFuncName = Record.FunctionName;
   if (OrigFuncName.empty())
     return make_error<CoverageMapError>(coveragemap_error::malformed,
@@ -808,35 +809,44 @@ Error CoverageMapping::loadFunctionRecord(
   CounterMappingContext Ctx(Record.Expressions);
 
   std::vector<uint64_t> Counts;
-  if (Error E = ProfileReader.getFunctionCounts(Record.FunctionName,
-                                                Record.FunctionHash, Counts)) {
-    instrprof_error IPE = std::get<0>(InstrProfError::take(std::move(E)));
-    if (IPE == instrprof_error::hash_mismatch) {
-      FuncHashMismatches.emplace_back(std::string(Record.FunctionName),
-                                      Record.FunctionHash);
-      return Error::success();
+  if (ProfileReader) {
+    if (Error E = ProfileReader.value().get().getFunctionCounts(
+            Record.FunctionName, Record.FunctionHash, Counts)) {
+      instrprof_error IPE = std::get<0>(InstrProfError::take(std::move(E)));
+      if (IPE == instrprof_error::hash_mismatch) {
+        FuncHashMismatches.emplace_back(std::string(Record.FunctionName),
+                                        Record.FunctionHash);
+        return Error::success();
+      }
+      if (IPE != instrprof_error::unknown_function)
+        return make_error<InstrProfError>(IPE);
+      Counts.assign(getMaxCounterID(Ctx, Record) + 1, 0);
     }
-    if (IPE != instrprof_error::unknown_function)
-      return make_error<InstrProfError>(IPE);
+  } else {
     Counts.assign(getMaxCounterID(Ctx, Record) + 1, 0);
   }
   Ctx.setCounts(Counts);
 
   bool IsVersion11 =
-      ProfileReader.getVersion() < IndexedInstrProf::ProfVersion::Version12;
+      ProfileReader && ProfileReader.value().get().getVersion() <
+                           IndexedInstrProf::ProfVersion::Version12;
 
   BitVector Bitmap;
-  if (Error E = ProfileReader.getFunctionBitmap(Record.FunctionName,
-                                                Record.FunctionHash, Bitmap)) {
-    instrprof_error IPE = std::get<0>(InstrProfError::take(std::move(E)));
-    if (IPE == instrprof_error::hash_mismatch) {
-      FuncHashMismatches.emplace_back(std::string(Record.FunctionName),
-                                      Record.FunctionHash);
-      return Error::success();
+  if (ProfileReader) {
+    if (Error E = ProfileReader.value().get().getFunctionBitmap(
+            Record.FunctionName, Record.FunctionHash, Bitmap)) {
+      instrprof_error IPE = std::get<0>(InstrProfError::take(std::move(E)));
+      if (IPE == instrprof_error::hash_mismatch) {
+        FuncHashMismatches.emplace_back(std::string(Record.FunctionName),
+                                        Record.FunctionHash);
+        return Error::success();
+      }
+      if (IPE != instrprof_error::unknown_function)
+        return make_error<InstrProfError>(IPE);
+      Bitmap = BitVector(getMaxBitmapSize(Record, IsVersion11));
     }
-    if (IPE != instrprof_error::unknown_function)
-      return make_error<InstrProfError>(IPE);
-    Bitmap = BitVector(getMaxBitmapSize(Record, IsVersion11));
+  } else {
+    Bitmap = BitVector(getMaxBitmapSize(Record, false));
   }
   Ctx.setBitmap(std::move(Bitmap));
 
@@ -870,8 +880,9 @@ Error CoverageMapping::loadFunctionRecord(
       consumeError(std::move(E));
       return Error::success();
     }
-    Function.pushRegion(Region, *ExecutionCount, *AltExecutionCount,
-                        ProfileReader.hasSingleByteCoverage());
+    Function.pushRegion(
+        Region, *ExecutionCount, *AltExecutionCount,
+        ProfileReader && ProfileReader.value().get().hasSingleByteCoverage());
 
     // Record ExpansionRegion.
     if (Region.Kind == CounterMappingRegion::ExpansionRegion) {
@@ -932,7 +943,9 @@ Error CoverageMapping::loadFunctionRecord(
 // of CoverageMappingReader instances.
 Error CoverageMapping::loadFromReaders(
     ArrayRef<std::unique_ptr<CoverageMappingReader>> CoverageReaders,
-    IndexedInstrProfReader &ProfileReader, CoverageMapping &Coverage) {
+    std::optional<std::reference_wrapper<IndexedInstrProfReader>>
+        &ProfileReader,
+    CoverageMapping &Coverage) {
   for (const auto &CoverageReader : CoverageReaders) {
     for (auto RecordOrErr : *CoverageReader) {
       if (Error E = RecordOrErr.takeError())
@@ -947,7 +960,8 @@ Error CoverageMapping::loadFromReaders(
 
 Expected<std::unique_ptr<CoverageMapping>> CoverageMapping::load(
     ArrayRef<std::unique_ptr<CoverageMappingReader>> CoverageReaders,
-    IndexedInstrProfReader &ProfileReader) {
+    std::optional<std::reference_wrapper<IndexedInstrProfReader>>
+        &ProfileReader) {
   auto Coverage = std::unique_ptr<CoverageMapping>(new CoverageMapping());
   if (Error E = loadFromReaders(CoverageReaders, ProfileReader, *Coverage))
     return std::move(E);
@@ -956,18 +970,19 @@ Expected<std::unique_ptr<CoverageMapping>> CoverageMapping::load(
 
 // If E is a no_data_found error, returns success. Otherwise returns E.
 static Error handleMaybeNoDataFoundError(Error E) {
-  return handleErrors(
-      std::move(E), [](const CoverageMapError &CME) {
-        if (CME.get() == coveragemap_error::no_data_found)
-          return static_cast<Error>(Error::success());
-        return make_error<CoverageMapError>(CME.get(), CME.getMessage());
-      });
+  return handleErrors(std::move(E), [](const CoverageMapError &CME) {
+    if (CME.get() == coveragemap_error::no_data_found)
+      return static_cast<Error>(Error::success());
+    return make_error<CoverageMapError>(CME.get(), CME.getMessage());
+  });
 }
 
 Error CoverageMapping::loadFromFile(
     StringRef Filename, StringRef Arch, StringRef CompilationDir,
-    IndexedInstrProfReader &ProfileReader, CoverageMapping &Coverage,
-    bool &DataFound, SmallVectorImpl<object::BuildID> *FoundBinaryIDs) {
+    std::optional<std::reference_wrapper<IndexedInstrProfReader>>
+        &ProfileReader,
+    CoverageMapping &Coverage, bool &DataFound,
+    SmallVectorImpl<object::BuildID> *FoundBinaryIDs) {
   auto CovMappingBufOrErr = MemoryBuffer::getFileOrSTDIN(
       Filename, /*IsText=*/false, /*RequiresNullTerminator=*/false);
   if (std::error_code EC = CovMappingBufOrErr.getError())
@@ -1003,13 +1018,23 @@ Error CoverageMapping::loadFromFile(
 }
 
 Expected<std::unique_ptr<CoverageMapping>> CoverageMapping::load(
-    ArrayRef<StringRef> ObjectFilenames, StringRef ProfileFilename,
-    vfs::FileSystem &FS, ArrayRef<StringRef> Arches, StringRef CompilationDir,
+    ArrayRef<StringRef> ObjectFilenames,
+    std::optional<StringRef> ProfileFilename, vfs::FileSystem &FS,
+    ArrayRef<StringRef> Arches, StringRef CompilationDir,
     const object::BuildIDFetcher *BIDFetcher, bool CheckBinaryIDs) {
-  auto ProfileReaderOrErr = IndexedInstrProfReader::create(ProfileFilename, FS);
-  if (Error E = ProfileReaderOrErr.takeError())
-    return createFileError(ProfileFilename, std::move(E));
-  auto ProfileReader = std::move(ProfileReaderOrErr.get());
+  std::unique_ptr<IndexedInstrProfReader> ProfileReader;
+  if (ProfileFilename) {
+    auto ProfileReaderOrErr =
+        IndexedInstrProfReader::create(ProfileFilename.value(), FS);
+    if (Error E = ProfileReaderOrErr.takeError())
+      return createFileError(ProfileFilename.value(), std::move(E));
+    ProfileReader = std::move(ProfileReaderOrErr.get());
+  }
+  auto ProfileReaderRef =
+      ProfileReader
+          ? std::optional<std::reference_wrapper<IndexedInstrProfReader>>(
+                *ProfileReader)
+          : std::nullopt;
   auto Coverage = std::unique_ptr<CoverageMapping>(new CoverageMapping());
   bool DataFound = false;
 
@@ -1023,16 +1048,17 @@ Expected<std::unique_ptr<CoverageMapping>> CoverageMapping::load(
 
   SmallVector<object::BuildID> FoundBinaryIDs;
   for (const auto &File : llvm::enumerate(ObjectFilenames)) {
-    if (Error E =
-            loadFromFile(File.value(), GetArch(File.index()), CompilationDir,
-                         *ProfileReader, *Coverage, DataFound, &FoundBinaryIDs))
+    if (Error E = loadFromFile(File.value(), GetArch(File.index()),
+                               CompilationDir, ProfileReaderRef, *Coverage,
+                               DataFound, &FoundBinaryIDs))
       return std::move(E);
   }
 
   if (BIDFetcher) {
     std::vector<object::BuildID> ProfileBinaryIDs;
-    if (Error E = ProfileReader->readBinaryIds(ProfileBinaryIDs))
-      return createFileError(ProfileFilename, std::move(E));
+    if (ProfileReader)
+      if (Error E = ProfileReader->readBinaryIds(ProfileBinaryIDs))
+        return createFileError(ProfileFilename.value(), std::move(E));
 
     SmallVector<object::BuildIDRef> BinaryIDsToFetch;
     if (!ProfileBinaryIDs.empty()) {
@@ -1052,12 +1078,12 @@ Expected<std::unique_ptr<CoverageMapping>> CoverageMapping::load(
       if (PathOpt) {
         std::string Path = std::move(*PathOpt);
         StringRef Arch = Arches.size() == 1 ? Arches.front() : StringRef();
-        if (Error E = loadFromFile(Path, Arch, CompilationDir, *ProfileReader,
-                                  *Coverage, DataFound))
+        if (Error E = loadFromFile(Path, Arch, CompilationDir, ProfileReaderRef,
+                                   *Coverage, DataFound))
           return std::move(E);
       } else if (CheckBinaryIDs) {
         return createFileError(
-            ProfileFilename,
+            ProfileFilename.value(),
             createStringError(errc::no_such_file_or_directory,
                               "Missing binary ID: " +
                                   llvm::toHex(BinaryID, /*LowerCase=*/true)));
diff --git a/llvm/test/tools/llvm-cov/showLineExecutionCounts-lcov-baseline.test b/llvm/test/tools/llvm-cov/showLineExecutionCounts-lcov-baseline.test
new file mode 100644
index 00000000000000..1b4ecf5fc12d83
--- /dev/null
+++ b/llvm/test/tools/llvm-cov/showLineExecutionCounts-lcov-baseline.test
@@ -0,0 +1,37 @@
+// FULL: SF:{{.*}}showLineExecutionCounts.cpp
+// FULL: FN:6,main
+// FULL: FNDA:0,main
+// FULL: FNF:1
+// FULL: FNH:0
+int main() {                              // FULL: DA:[[@LINE]],0
+  int x = 0;                              // FULL: DA:[[@LINE]],0
+                                          // FULL: DA:[[@LINE]],0
+  if (x) {                                // FULL: DA:[[@LINE]],0
+    x = 0;                                // FULL: DA:[[@LINE]],0
+  } else {                                // FULL: DA:[[@LINE]],0
+    x = 1;                                // FULL: DA:[[@LINE]],0
+  }                                       // FULL: DA:[[@LINE]],0
+                                          // FULL: DA:[[@LINE]],0
+  for (int i = 0; i < 100; ++i) {         // FULL: DA:[[@LINE]],0
+    x = 1;                                // FULL: DA:[[@LINE]],0
+  }                                       // FULL: DA:[[@LINE]],0
+                                          // FULL: DA:[[@LINE]],0
+  x = x < 10 ? x + 1 : x - 1;             // FULL: DA:[[@LINE]],0
+  x = x > 10 ?                            // FULL: DA:[[@LINE]],0
+        x - 1:                            // FULL: DA:[[@LINE]],0
+        x + 1;                            // FULL: DA:[[@LINE]],0
+                                          // FULL: DA:[[@LINE]],0
+  return 0;                               // FULL: DA:[[@LINE]],0
+}                                         // FULL: DA:[[@LINE]],0
+// FULL: LF:20
+// FULL: LH:0
+// FULL: end_of_record
+// RUN: llvm-cov export -format=lcov %S/Inputs/lineExecutionCounts.covmapping %s | FileCheck -check-prefixes=FULL %s
+
+// RUN: llvm-cov export -format=lcov -summary-only %S/Inputs/lineExecutionCounts.covmapping %s | FileCheck -check-prefixes=SUMMARYONLY %s
+// SUMMARYONLY: SF:{{.*}}showLineExecutionCounts.cpp
+// SUMMARYONLY: FNF:1
+// SUMMARYONLY: FNH:0
+// SUMMARYONLY: LF:20
+// SUMMARYONLY: LH:0
+// SUMMARYONLY: end_of_record
diff --git a/llvm/tools/llvm-cov/CodeCoverage.cpp b/llvm/tools/llvm-cov/CodeCoverage.cpp
index d06fd86fe52afb..4454176f34c27a 100644
--- a/llvm/tools/llvm-cov/CodeCoverage.cpp
+++ b/llvm/tools/llvm-cov/CodeCoverage.cpp
@@ -153,7 +153,7 @@ class CodeCoverageTool {
   bool HadSourceFiles = false;
 
   /// The path to the indexed profile.
-  std::string PGOFilename;
+  std::optional<std::string> PGOFilename;
 
   /// A list of input source files.
   std::vector<std::string> SourceFiles;
@@ -455,10 +455,12 @@ static bool modifiedTimeGT(StringRef LHS, StringRef RHS) {
 }
 
 std::unique_ptr<CoverageMapping> CodeCoverageTool::load() {
-  for (StringRef ObjectFilename : ObjectFilenames)
-    if (modifiedTimeGT(ObjectFilename, PGOFilename))
-      warning("profile data may be out of date - object is newer",
-              ObjectFilename);
+  if (PGOFilename) {
+    for (StringRef ObjectFilename : ObjectFilenames)
+      if (modifiedTimeGT(ObjectFilename, PGOFilename.value()))
+        warning("profile data may be out of date - object is newer",
+                ObjectFilename);
+  }
   auto FS = vfs::getRealFileSystem();
   auto CoverageOrErr = CoverageMapping::load(
       ObjectFilenames, PGOFilename, *FS, CoverageArches,
@@ -669,8 +671,8 @@ int CodeCoverageTool::run(Command Cmd, int argc, const char **argv) {
       "dump-collected-paths", cl::Optional, cl::Hidden,
       cl::desc("Show the collected paths to source files"));
 
-  cl::opt<std::string, true> PGOFilename(
-      "instr-profile", cl::Required, cl::location(this->PGOFilename),
+  cl::opt<std::string> PGOFilename(
+      "instr-profile", cl::Optional,
       cl::desc(
           "File with the profile data obtained after an instrumented run"));
 
@@ -806,6 +808,9 @@ int CodeCoverageTool::run(Command Cmd, int argc, const char **argv) {
     }
     this->CheckBinaryIDs = CheckBinaryIDs;
 
+    if (!PGOFilename.empty())
+      this->PGOFilename = std::make_optional(PGOFilename.getValue());
+
     if (!CovFilename.empty())
       ObjectFilenames.emplace_back(CovFilename);
     for (const std::string &Filename : CovFilenames)
@@ -1106,13 +1111,18 @@ int CodeCoverageTool::doShow(int argc, const char **argv,
     }
   }
 
-  sys::fs::file_status Status;
-  if (std::error_code EC = sys::fs::status(PGOFilename, Status)) {
-    error("could not read profile data!" + EC.message(), PGOFilename);
-    return 1;
-  ...
[truncated]

@fmeum
Copy link
Contributor Author

fmeum commented Nov 27, 2024

Notes for reviewers:

  • I could gate the new behavior behind a new flag (e.g. -empty-profile) so that forgetting to pass -instr-profile without the intention to get a baseline coverage report would still be an error.
  • Instead of passing an optional reader everywhere, I could try to extract a more narrowly scoped interface for coverage data out of IndexedInstrProfileReader and provide an implementation that returns all zero counters. Let me know if you would prefer that approach and the more drastic API changes to CoverageMapping this would require are acceptable.

@hiraditya
Copy link
Collaborator

needs rebase with tip of the tree.

@ilovepi ilovepi requested a review from gulfemsavrun March 21, 2025 23:16
@fmeum fmeum force-pushed the baseline-coverage branch from 2ee0a53 to bd087fc Compare March 22, 2025 19:12
Copy link

github-actions bot commented Mar 22, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@fmeum fmeum force-pushed the baseline-coverage branch 2 times, most recently from 9998c5b to 641b697 Compare March 22, 2025 21:58
When no profile is provided via --instr-profile, the export/report/show commands
now emit coverage data equivalent to that obtained from a profile with all zero
counters ("baseline coverage").

This is useful for build systems (e.g. Bazel) that can track coverage
information for each build target, even those that are never linked into tests
and thus don't have runtime coverage data recorded. By merging in baseline
coverage, lines in files that aren't linked into tests are correctly reported
as uncovered.
@fmeum fmeum force-pushed the baseline-coverage branch from 641b697 to fe2841b Compare March 22, 2025 22:02
@fmeum
Copy link
Contributor Author

fmeum commented Mar 22, 2025

@hiraditya I rebased it and resolved the conflicts

@gulfemsavrun
Copy link
Contributor

Notes for reviewers:

  • I could gate the new behavior behind a new flag (e.g. -empty-profile) so that forgetting to pass -instr-profile without the intention to get a baseline coverage report would still be an error.
  • Instead of passing an optional reader everywhere, I could try to extract a more narrowly scoped interface for coverage data out of IndexedInstrProfileReader and provide an implementation that returns all zero counters. Let me know if you would prefer that approach and the more drastic API changes to CoverageMapping this would require are acceptable.

Overall, it looks good to me. But, please put the new behavior behind a flag so that we do not change the default behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:binary-utilities PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants