[Coverage] Restore the correct count value after processing a nested region in case of combined regions.

If several regions cover the same area of code, we have to restore
the combined value for that area when return from a nested region.

This patch achieves that by combining regions before calling buildSegments.

Differential Revision: http://reviews.llvm.org/D18610

llvm-svn: 267390

llvm/include/llvm/ProfileData/CoverageMapping.h

@@ -370,13 +370,6 @@ struct CoverageSegment {
     return std::tie(L.Line, L.Col, L.Count, L.HasCount, L.IsRegionEntry) ==
            std::tie(R.Line, R.Col, R.Count, R.HasCount, R.IsRegionEntry);
   }
-
-  void setCount(uint64_t NewCount) {
-    Count = NewCount;
-    HasCount = true;
-  }
-
-  void addCount(uint64_t NewCount) { setCount(Count + NewCount); }
 };
 
 /// \brief Coverage information to be processed or displayed.

llvm/lib/ProfileData/CoverageMapping.cpp

@@ -286,20 +286,17 @@ class SegmentBuilder {
   /// Start a segment with the given Region's count.
   void startSegment(unsigned Line, unsigned Col, bool IsRegionEntry,
                     const CountedRegion &Region) {
-    if (Segments.empty())
-      Segments.emplace_back(Line, Col, IsRegionEntry);
-    CoverageSegment S = Segments.back();
     // Avoid creating empty regions.
-    if (S.Line != Line || S.Col != Col) {
-      Segments.emplace_back(Line, Col, IsRegionEntry);
-      S = Segments.back();
-    }
+    if (!Segments.empty() && Segments.back().Line == Line &&
+        Segments.back().Col == Col)
+      Segments.pop_back();
     DEBUG(dbgs() << "Segment at " << Line << ":" << Col);
     // Set this region's count.
     if (Region.Kind != coverage::CounterMappingRegion::SkippedRegion) {
       DEBUG(dbgs() << " with count " << Region.ExecutionCount);
-      Segments.back().setCount(Region.ExecutionCount);
-    }
+      Segments.emplace_back(Line, Col, Region.ExecutionCount, IsRegionEntry);
+    } else
+      Segments.emplace_back(Line, Col, IsRegionEntry);
     DEBUG(dbgs() << "\n");
   }
 
@@ -321,35 +318,73 @@ class SegmentBuilder {
   }
 
   void buildSegmentsImpl(ArrayRef<CountedRegion> Regions) {
-    const CountedRegion *PrevRegion = nullptr;
     for (const auto &Region : Regions) {
       // Pop any regions that end before this one starts.
       while (!ActiveRegions.empty() &&
              ActiveRegions.back()->endLoc() <= Region.startLoc())
         popRegion();
-      if (PrevRegion && PrevRegion->startLoc() == Region.startLoc() &&
-          PrevRegion->endLoc() == Region.endLoc()) {
-        if (Region.Kind == coverage::CounterMappingRegion::CodeRegion)
-          Segments.back().addCount(Region.ExecutionCount);
-      } else {
-        // Add this region to the stack.
-        ActiveRegions.push_back(&Region);
-        startSegment(Region);
-      }
-      PrevRegion = &Region;
+      // Add this region to the stack.
+      ActiveRegions.push_back(&Region);
+      startSegment(Region);
     }
     // Pop any regions that are left in the stack.
     while (!ActiveRegions.empty())
       popRegion();
   }
 
+  /// Sort a nested sequence of regions from a single file.
+  static void sortNestedRegions(MutableArrayRef<CountedRegion> Regions) {
+    std::sort(Regions.begin(), Regions.end(),
+              [](const CountedRegion &LHS, const CountedRegion &RHS) {
+                if (LHS.startLoc() == RHS.startLoc())
+                  // When LHS completely contains RHS, we sort LHS first.
+                  return RHS.endLoc() < LHS.endLoc();
+                return LHS.startLoc() < RHS.startLoc();
+              });
+  }
+
+  /// Combine counts of regions which cover the same area.
+  static ArrayRef<CountedRegion>
+  combineRegions(MutableArrayRef<CountedRegion> Regions) {
+    if (Regions.empty())
+      return Regions;
+    auto Active = Regions.begin();
+    auto End = Regions.end();
+    for (auto I = Regions.begin() + 1; I != End; ++I) {
+      if (Active->startLoc() != I->startLoc() ||
+          Active->endLoc() != I->endLoc()) {
+        // Shift to the next region.
+        ++Active;
+        if (Active != I)
+          *Active = *I;
+        continue;
+      }
+      // Merge duplicate region.
+      if (I->Kind != coverage::CounterMappingRegion::CodeRegion)
+        // Add counts only from CodeRegions.
+        continue;
+      if (Active->Kind == coverage::CounterMappingRegion::SkippedRegion)
+        // We have to overwrite SkippedRegions because of special handling
+        // of them in startSegment().
+        *Active = *I;
+      else
+        // Otherwise, just append the count.
+        Active->ExecutionCount += I->ExecutionCount;
+    }
+    return Regions.drop_back(std::distance(++Active, End));
+  }
+
 public:
-  /// Build a list of CoverageSegments from a sorted list of Regions.
+  /// Build a list of CoverageSegments from a list of Regions.
   static std::vector<CoverageSegment>
-  buildSegments(ArrayRef<CountedRegion> Regions) {
+  buildSegments(MutableArrayRef<CountedRegion> Regions) {
     std::vector<CoverageSegment> Segments;
     SegmentBuilder Builder(Segments);
-    Builder.buildSegmentsImpl(Regions);
+
+    sortNestedRegions(Regions);
+    ArrayRef<CountedRegion> CombinedRegions = combineRegions(Regions);
+
+    Builder.buildSegmentsImpl(CombinedRegions);
     return Segments;
   }
 };
@@ -397,17 +432,6 @@ static Optional<unsigned> findMainViewFileID(StringRef SourceFile,
   return None;
 }
 
-/// Sort a nested sequence of regions from a single file.
-template <class It> static void sortNestedRegions(It First, It Last) {
-  std::sort(First, Last,
-            [](const CountedRegion &LHS, const CountedRegion &RHS) {
-    if (LHS.startLoc() == RHS.startLoc())
-      // When LHS completely contains RHS, we sort LHS first.
-      return RHS.endLoc() < LHS.endLoc();
-    return LHS.startLoc() < RHS.startLoc();
-  });
-}
-
 static bool isExpansion(const CountedRegion &R, unsigned FileID) {
   return R.Kind == CounterMappingRegion::ExpansionRegion && R.FileID == FileID;
 }
@@ -427,7 +451,6 @@ CoverageData CoverageMapping::getCoverageForFile(StringRef Filename) {
       }
   }
 
-  sortNestedRegions(Regions.begin(), Regions.end());
   DEBUG(dbgs() << "Emitting segments for file: " << Filename << "\n");
   FileCoverage.Segments = SegmentBuilder::buildSegments(Regions);
 
@@ -469,7 +492,6 @@ CoverageMapping::getCoverageForFunction(const FunctionRecord &Function) {
         FunctionCoverage.Expansions.emplace_back(CR, Function);
     }
 
-  sortNestedRegions(Regions.begin(), Regions.end());
   DEBUG(dbgs() << "Emitting segments for function: " << Function.Name << "\n");
   FunctionCoverage.Segments = SegmentBuilder::buildSegments(Regions);
 
@@ -488,7 +510,6 @@ CoverageMapping::getCoverageForExpansion(const ExpansionRecord &Expansion) {
         ExpansionCoverage.Expansions.emplace_back(CR, Expansion.Function);
     }
 
-  sortNestedRegions(Regions.begin(), Regions.end());
   DEBUG(dbgs() << "Emitting segments for expansion of file " << Expansion.FileID
                << "\n");
   ExpansionCoverage.Segments = SegmentBuilder::buildSegments(Regions);

llvm/test/tools/llvm-cov/showTemplateInstantiations.cpp

@@ -7,10 +7,10 @@ template<typename T> // ALL:          | [[@LINE]]|template<typename T>
 int func(T x) {      // ALL-NEXT:    2| [[@LINE]]|int func(T x) {
   if(x)              // ALL-NEXT:    2| [[@LINE]]|  if(x)
     return 0;        // ALL-NEXT:    1| [[@LINE]]|    return 0;
-  else               // ALL-NEXT:    1| [[@LINE]]|  else
+  else               // ALL-NEXT:    2| [[@LINE]]|  else
     return 1;        // ALL-NEXT:    1| [[@LINE]]|    return 1;
   int j = 1;         // ALL-NEXT:    0| [[@LINE]]|  int j = 1;
-}                    // ALL-NEXT:    1| [[@LINE]]|}
+}                    // ALL-NEXT:    2| [[@LINE]]|}
 
                      // CHECK:       {{^ *(\| )?}}_Z4funcIbEiT_:
                      // CHECK-NEXT:  1| [[@LINE-9]]|int func(T x) {

llvm/unittests/ProfileData/CoverageMappingTest.cpp

@@ -402,6 +402,26 @@ TEST_P(MaybeSparseCoverageMappingTest, combine_regions) {
   ASSERT_EQ(CoverageSegment(9, 9, false), Segments[3]);
 }
 
+TEST_P(MaybeSparseCoverageMappingTest,
+       restore_combined_counter_after_nested_region) {
+  InstrProfRecord Record("func", 0x1234, {10, 20, 40});
+  ProfileWriter.addRecord(std::move(Record));
+
+  startFunction("func", 0x1234);
+  addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
+  addCMR(Counter::getCounter(1), "file1", 1, 1, 9, 9);
+  addCMR(Counter::getCounter(2), "file1", 3, 3, 5, 5);
+  loadCoverageMapping();
+
+  CoverageData Data = LoadedCoverage->getCoverageForFile("file1");
+  std::vector<CoverageSegment> Segments(Data.begin(), Data.end());
+  ASSERT_EQ(4U, Segments.size());
+  EXPECT_EQ(CoverageSegment(1, 1, 30, true), Segments[0]);
+  EXPECT_EQ(CoverageSegment(3, 3, 40, true), Segments[1]);
+  EXPECT_EQ(CoverageSegment(5, 5, 30, false), Segments[2]);
+  EXPECT_EQ(CoverageSegment(9, 9, false), Segments[3]);
+}
+
 TEST_P(MaybeSparseCoverageMappingTest, dont_combine_expansions) {
   InstrProfRecord Record1("func", 0x1234, {10, 20});
   InstrProfRecord Record2("func", 0x1234, {0, 0});