ikudrin created this revision.
ikudrin added reviewers: davidxl, bogner, vsk.
ikudrin added a subscriber: cfe-commits.

In many cases, these code regions are just redundant, but sometimes they could 
be assigned to the counter of the parent code region instead of the counter of 
the nested block.

https://reviews.llvm.org/D23987

Files:
  lib/CodeGen/CoverageMappingGen.cpp
  test/CoverageMapping/macroception.c
  test/CoverageMapping/macros.c

Index: test/CoverageMapping/macros.c
===================================================================
--- test/CoverageMapping/macros.c
+++ test/CoverageMapping/macros.c
@@ -3,6 +3,7 @@
 #define MACRO return; bar()
 #define MACRO_2 bar()
 #define MACRO_1 return; MACRO_2
+#define MACRO_3 MACRO_2
 
 void bar() {}
 
@@ -24,7 +25,6 @@
   i = 2;
 }
 // CHECK-NEXT: File 1, 5:17 -> 5:32 = #0
-// CHECK-NEXT: File 1, 5:25 -> 5:32 = 0
 // CHECK-NEXT: Expansion,File 1, 5:25 -> 5:32 = 0
 // CHECK-NEXT: File 2, 4:17 -> 4:22 = 0
 
@@ -47,6 +47,15 @@
 // CHECK-NEXT: File 1, 4:17 -> 4:22 = #2
 // CHECK-NOT: File 1
 
+// CHECK-NEXT: func5
+void func5() { // CHECK-NEXT: File 0, [[@LINE]]:14 -> [[@LINE+4]]:2 = #0
+  int i = 0;
+  if (i > 5) // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:12 = #0
+    MACRO_3; // CHECK-NEXT: Expansion,File 0, [[@LINE]]:5 -> [[@LINE]]:12 = #1
+}
+// CHECK-NEXT: Expansion,File 1, 6:17 -> 6:24 = #1
+// CHECK-NEXT: File 2, 4:17 -> 4:22 = #1
+
 int main(int argc, const char *argv[]) {
   func();
   func2();
Index: test/CoverageMapping/macroception.c
===================================================================
--- test/CoverageMapping/macroception.c
+++ test/CoverageMapping/macroception.c
@@ -11,7 +11,6 @@
 int main() M1
   return 0;
 }
-// CHECK-NEXT: File 1, 4:12 -> 4:14 = #0
 // CHECK-NEXT: Expansion,File 1, 4:12 -> 4:14 = #0
 // CHECK-NEXT: File 2, 3:12 -> 3:13 = #0
 
@@ -21,7 +20,6 @@
 void func2() {
   int x = 0;
 M11
-// CHECK-NEXT: File 1, 6:13 -> 6:16 = #0
 // CHECK-NEXT: Expansion,File 1, 6:13 -> 6:16 = #0
 // CHECK-NEXT: File 2, 5:13 -> 5:14 = #0
 
@@ -32,9 +30,7 @@
 void func3() M1
   int x = 0;
 M11
-// CHECK-NEXT: File 1, 4:12 -> 4:14 = #0
 // CHECK-NEXT: Expansion,File 1, 4:12 -> 4:14 = #0
-// CHECK-NEXT: File 2, 6:13 -> 6:16 = #0
 // CHECK-NEXT: Expansion,File 2, 6:13 -> 6:16 = #0
 // CHECK-NEXT: File 3, 3:12 -> 3:13 = #0
 // CHECK-NEXT: File 4, 5:13 -> 5:14 = #0
@@ -44,9 +40,7 @@
 // CHECK-NEXT: File 0, [[@LINE+2]]:16 -> [[@LINE+2]]:20 = #0
 // CHECK-NEXT: Expansion,File 0, [[@LINE+1]]:17 -> [[@LINE+1]]:20 = #0
 void func4() M1 M11
-// CHECK-NEXT: File 1, 4:12 -> 4:14 = #0
 // CHECK-NEXT: Expansion,File 1, 4:12 -> 4:14 = #0
-// CHECK-NEXT: File 2, 6:13 -> 6:16 = #0
 // CHECK-NEXT: Expansion,File 2, 6:13 -> 6:16 = #0
 // CHECK-NEXT: File 3, 3:12 -> 3:13 = #0
 // CHECK-NEXT: File 4, 5:13 -> 5:14 = #0
Index: lib/CodeGen/CoverageMappingGen.cpp
===================================================================
--- lib/CodeGen/CoverageMappingGen.cpp
+++ lib/CodeGen/CoverageMappingGen.cpp
@@ -92,6 +92,14 @@
   /// \brief The source mapping regions for this function.
   std::vector<SourceMappingRegion> SourceRegions;
 
+  /// \brief A set of regions which can be used as a filter.
+  ///
+  /// It is produced by emitExpansionRegions() and is used in
+  /// emitSourceRegions() to suppress producing code regions if
+  /// the same area is covered by expansion regions.
+  typedef llvm::SmallSet<std::pair<SourceLocation, SourceLocation>, 8>
+      SourceRegionFilter;
+
   CoverageMappingBuilder(CoverageMappingModuleGen &CVM, SourceManager &SM,
                          const LangOptions &LangOpts)
       : CVM(CVM), SM(SM), LangOpts(LangOpts) {}
@@ -249,7 +257,8 @@
 
   /// \brief Generate the coverage counter mapping regions from collected
   /// source regions.
-  void emitSourceRegions() {
+  void
+  emitSourceRegions(const SourceRegionFilter &Filter = SourceRegionFilter()) {
     for (const auto &Region : SourceRegions) {
       assert(Region.hasEndLoc() && "incomplete region");
 
@@ -269,6 +278,13 @@
       assert(SM.isWrittenInSameFile(LocStart, LocEnd) &&
              "region spans multiple files");
 
+      // Don't add code regions for the area covered by expansion regions.
+      // This not only suppresses redundant regions, but sometimes prevents
+      // creating regions with wrong counters if, for example, a statement's
+      // body ends at the end of a nested macro.
+      if (Filter.count(std::make_pair(LocStart, LocEnd)))
+        continue;
+
       // Find the spilling locations for the mapping region.
       unsigned LineStart = SM.getSpellingLineNumber(LocStart);
       unsigned ColumnStart = SM.getSpellingColumnNumber(LocStart);
@@ -283,7 +299,8 @@
   }
 
   /// \brief Generate expansion regions for each virtual file we've seen.
-  void emitExpansionRegions() {
+  SourceRegionFilter emitExpansionRegions() {
+    SourceRegionFilter Filter;
     for (const auto &FM : FileIDMapping) {
       SourceLocation ExpandedLoc = FM.second.second;
       SourceLocation ParentLoc = getIncludeOrExpansionLoc(ExpandedLoc);
@@ -299,6 +316,7 @@
       SourceLocation LocEnd = getPreciseTokenLocEnd(ParentLoc);
       assert(SM.isWrittenInSameFile(ParentLoc, LocEnd) &&
              "region spans multiple files");
+      Filter.insert(std::make_pair(ParentLoc, LocEnd));
 
       unsigned LineStart = SM.getSpellingLineNumber(ParentLoc);
       unsigned ColumnStart = SM.getSpellingColumnNumber(ParentLoc);
@@ -309,6 +327,7 @@
           *ParentFileID, *ExpandedFileID, LineStart, ColumnStart, LineEnd,
           ColumnEnd));
     }
+    return Filter;
   }
 };
 
@@ -605,8 +624,14 @@
   void write(llvm::raw_ostream &OS) {
     llvm::SmallVector<unsigned, 8> VirtualFileMapping;
     gatherFileIDs(VirtualFileMapping);
-    emitSourceRegions();
-    emitExpansionRegions();
+    SourceRegionFilter Filter = emitExpansionRegions();
+    emitSourceRegions(Filter);
+    // Expansion regions are expected to be added after code regions.
+    std::stable_sort(
+        MappingRegions.begin(), MappingRegions.end(),
+        [](const CounterMappingRegion &LHS, const CounterMappingRegion &RHS) {
+          return LHS.Kind < RHS.Kind;
+        });
     gatherSkippedRegions();
 
     if (MappingRegions.empty())
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to