Skip to content

LLVMCoverage: Unify getCoverageForFile and getCoverageForFunction. NFC #120842

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 37 commits into
base: main
Choose a base branch
from

Conversation

chapuni
Copy link
Contributor

@chapuni chapuni commented Dec 21, 2024

Since #119952, getCoverageForFile and getCoverageForFunction have similar structure each other. Ther merged method addFunctionRegions has two lambda subfunctions.

  • getCoverageForFile
    • MainFileID may be nullopt.
    • shouldProcess picks up relevant records along FileIDs that is scanned based on MainFileID. They may have expanded source files.
    • shouldExpand takes the presense of MainFileID into account.
  • getCoverageForFunction
    • This assumes the presense of MainFileID.
    • shouldProcess picks up records that belong only to MainFileID.
    • shouldExpand assumes the presense of MainFileID.

This change introduces a wrapper class MergeableCoverageData for further merging instances. At the moment, this returns CoverageData including buildSegments().

This change itself is NFC.

chapuni added 20 commits October 3, 2024 15:47
`SingleByteCoverage` is not per-region attribute at least.
At the moment, this change moves it into `FunctionRecord`.
- Round `Counts` as 1/0
- Confirm both `ExecutionCount` and `AltExecutionCount` are in range.
Conflicts:
	llvm/test/tools/llvm-cov/branch-macros.cpp
…ingle/merge

Conflicts:
	llvm/test/tools/llvm-cov/Inputs/showLineExecutionCounts.cpp
…v/binary' into users/chapuni/cov/single/refactor-base

Conflicts:
	llvm/test/tools/llvm-cov/branch-macros.test
	llvm/test/tools/llvm-cov/showLineExecutionCounts.test
@chapuni chapuni requested review from ornata and evodius96 December 21, 2024 16:46
@llvmbot llvmbot added the PGO Profile Guided Optimizations label Dec 21, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 21, 2024

@llvm/pr-subscribers-pgo

Author: NAKAMURA Takumi (chapuni)

Changes

Preparation for #119282 and #119299


Full diff: https://github.com/llvm/llvm-project/pull/120842.diff

2 Files Affected:

  • (modified) llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h (+3)
  • (modified) llvm/lib/ProfileData/Coverage/CoverageMapping.cpp (+40-37)
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index 42da188fef34ee..be902a20efc47e 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -889,6 +889,7 @@ class InstantiationGroup {
 class CoverageData {
   friend class CoverageMapping;
 
+protected:
   std::string Filename;
   std::vector<CoverageSegment> Segments;
   std::vector<ExpansionRecord> Expansions;
@@ -900,6 +901,8 @@ class CoverageData {
 
   CoverageData(StringRef Filename) : Filename(Filename) {}
 
+  CoverageData(CoverageData &&RHS) = default;
+
   /// Get the name of the file this data covers.
   StringRef getFilename() const { return Filename; }
 
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index d51448567539f2..c6756fafb268a2 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -1343,6 +1343,36 @@ class SegmentBuilder {
   }
 };
 
+struct MergeableCoverageData : public CoverageData {
+  std::vector<CountedRegion> CodeRegions;
+
+  MergeableCoverageData(StringRef Filename) : CoverageData(Filename) {}
+
+  void addFunctionRegions(
+      const FunctionRecord &Function,
+      std::function<bool(const CounterMappingRegion &CR)> shouldProcess,
+      std::function<bool(const CountedRegion &CR)> shouldExpand) {
+    for (const auto &CR : Function.CountedRegions)
+      if (shouldProcess(CR)) {
+        CodeRegions.push_back(CR);
+        if (shouldExpand(CR))
+          Expansions.emplace_back(CR, Function);
+      }
+    // Capture branch regions specific to the function (excluding expansions).
+    for (const auto &CR : Function.CountedBranchRegions)
+      if (shouldProcess(CR))
+        BranchRegions.push_back(CR);
+    // Capture MCDC records specific to the function.
+    for (const auto &MR : Function.MCDCRecords)
+      if (shouldProcess(MR.getDecisionRegion()))
+        MCDCRecords.push_back(MR);
+  }
+
+  CoverageData buildSegments() {
+    Segments = SegmentBuilder::buildSegments(CodeRegions);
+    return CoverageData(std::move(*this));
+  }
+};
 } // end anonymous namespace
 
 std::vector<StringRef> CoverageMapping::getUniqueSourceFiles() const {
@@ -1393,8 +1423,7 @@ static bool isExpansion(const CountedRegion &R, unsigned FileID) {
 }
 
 CoverageData CoverageMapping::getCoverageForFile(StringRef Filename) const {
-  CoverageData FileCoverage(Filename);
-  std::vector<CountedRegion> Regions;
+  MergeableCoverageData FileCoverage(Filename);
 
   // Look up the function records in the given file. Due to hash collisions on
   // the filename, we may get back some records that are not in the file.
@@ -1404,26 +1433,14 @@ CoverageData CoverageMapping::getCoverageForFile(StringRef Filename) const {
     const FunctionRecord &Function = Functions[RecordIndex];
     auto MainFileID = findMainViewFileID(Filename, Function);
     auto FileIDs = gatherFileIDs(Filename, Function);
-    for (const auto &CR : Function.CountedRegions)
-      if (FileIDs.test(CR.FileID)) {
-        Regions.push_back(CR);
-        if (MainFileID && isExpansion(CR, *MainFileID))
-          FileCoverage.Expansions.emplace_back(CR, Function);
-      }
-    // Capture branch regions specific to the function (excluding expansions).
-    for (const auto &CR : Function.CountedBranchRegions)
-      if (FileIDs.test(CR.FileID))
-        FileCoverage.BranchRegions.push_back(CR);
-    // Capture MCDC records specific to the function.
-    for (const auto &MR : Function.MCDCRecords)
-      if (FileIDs.test(MR.getDecisionRegion().FileID))
-        FileCoverage.MCDCRecords.push_back(MR);
+    FileCoverage.addFunctionRegions(
+        Function, [&](auto &CR) { return FileIDs.test(CR.FileID); },
+        [&](auto &CR) { return (MainFileID && isExpansion(CR, *MainFileID)); });
   }
 
   LLVM_DEBUG(dbgs() << "Emitting segments for file: " << Filename << "\n");
-  FileCoverage.Segments = SegmentBuilder::buildSegments(Regions);
 
-  return FileCoverage;
+  return FileCoverage.buildSegments();
 }
 
 std::vector<InstantiationGroup>
@@ -1457,29 +1474,15 @@ CoverageMapping::getCoverageForFunction(const FunctionRecord &Function) const {
   if (!MainFileID)
     return CoverageData();
 
-  CoverageData FunctionCoverage(Function.Filenames[*MainFileID]);
-  std::vector<CountedRegion> Regions;
-  for (const auto &CR : Function.CountedRegions)
-    if (CR.FileID == *MainFileID) {
-      Regions.push_back(CR);
-      if (isExpansion(CR, *MainFileID))
-        FunctionCoverage.Expansions.emplace_back(CR, Function);
-    }
-  // Capture branch regions specific to the function (excluding expansions).
-  for (const auto &CR : Function.CountedBranchRegions)
-    if (CR.FileID == *MainFileID)
-      FunctionCoverage.BranchRegions.push_back(CR);
-
-  // Capture MCDC records specific to the function.
-  for (const auto &MR : Function.MCDCRecords)
-    if (MR.getDecisionRegion().FileID == *MainFileID)
-      FunctionCoverage.MCDCRecords.push_back(MR);
+  MergeableCoverageData FunctionCoverage(Function.Filenames[*MainFileID]);
+  FunctionCoverage.addFunctionRegions(
+      Function, [&](auto &CR) { return (CR.FileID == *MainFileID); },
+      [&](auto &CR) { return isExpansion(CR, *MainFileID); });
 
   LLVM_DEBUG(dbgs() << "Emitting segments for function: " << Function.Name
                     << "\n");
-  FunctionCoverage.Segments = SegmentBuilder::buildSegments(Regions);
 
-  return FunctionCoverage;
+  return FunctionCoverage.buildSegments();
 }
 
 CoverageData CoverageMapping::getCoverageForExpansion(

Conflicts:
	llvm/test/tools/llvm-cov/branch-macros.test
	llvm/test/tools/llvm-cov/showLineExecutionCounts.test
	llvm/tools/llvm-cov/CodeCoverage.cpp
	llvm/tools/llvm-cov/SourceCoverageView.h
@chapuni chapuni requested a review from MaskRay January 6, 2025 02:10
@chapuni chapuni added tools:llvm-cov and removed PGO Profile Guided Optimizations labels Mar 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

llvm-cov show HTML: Inconsistent results between per-file indices and each source file view
2 participants