Index: llvm/trunk/lib/ProfileData/Coverage/CoverageMapping.cpp =================================================================== --- llvm/trunk/lib/ProfileData/Coverage/CoverageMapping.cpp +++ llvm/trunk/lib/ProfileData/Coverage/CoverageMapping.cpp @@ -334,13 +334,25 @@ /// Sort a nested sequence of regions from a single file. static void sortNestedRegions(MutableArrayRef 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(); - }); + std::sort(Regions.begin(), Regions.end(), [](const CountedRegion &LHS, + const CountedRegion &RHS) { + if (LHS.startLoc() != RHS.startLoc()) + return LHS.startLoc() < RHS.startLoc(); + if (LHS.endLoc() != RHS.endLoc()) + // When LHS completely contains RHS, we sort LHS first. + return RHS.endLoc() < LHS.endLoc(); + // If LHS and RHS cover the same area, we need to sort them according + // to their kinds so that the most suitable region will become "active" + // in combineRegions(). Because we accumulate counter values only from + // regions of the same kind as the first region of the area, prefer + // CodeRegion to ExpansionRegion and ExpansionRegion to SkippedRegion. + static_assert(coverage::CounterMappingRegion::CodeRegion < + coverage::CounterMappingRegion::ExpansionRegion && + coverage::CounterMappingRegion::ExpansionRegion < + coverage::CounterMappingRegion::SkippedRegion, + "Unexpected order of region kind values"); + return LHS.Kind < RHS.Kind; + }); } /// Combine counts of regions which cover the same area. @@ -360,15 +372,18 @@ 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. + // If CodeRegions and ExpansionRegions cover the same area, it's probably + // a macro which is fully expanded to another macro. In that case, we need + // to accumulate counts only from CodeRegions, or else the area will be + // counted twice. + // On the other hand, a macro may have a nested macro in its body. If the + // outer macro is used several times, the ExpansionRegion for the nested + // macro will also be added several times. These ExpansionRegions cover + // the same source locations and have to be combined to reach the correct + // value for that area. + // We add counts of the regions of the same kind as the active region + // to handle the both situations. + if (I->Kind == Active->Kind) Active->ExecutionCount += I->ExecutionCount; } return Regions.drop_back(std::distance(++Active, End)); Index: llvm/trunk/test/tools/llvm-cov/Inputs/combine_expansions.proftext =================================================================== --- llvm/trunk/test/tools/llvm-cov/Inputs/combine_expansions.proftext +++ llvm/trunk/test/tools/llvm-cov/Inputs/combine_expansions.proftext @@ -0,0 +1,8 @@ +main +# Func Hash: +0 +# Num Counters: +1 +# Counter Values: +1 + Index: llvm/trunk/test/tools/llvm-cov/combine_expansions.cpp =================================================================== --- llvm/trunk/test/tools/llvm-cov/combine_expansions.cpp +++ llvm/trunk/test/tools/llvm-cov/combine_expansions.cpp @@ -0,0 +1,26 @@ +// Check that we combine expansion regions. + +// RUN: llvm-profdata merge %S/Inputs/combine_expansions.proftext -o %t.profdata +// RUN: llvm-cov show %S/Inputs/combine_expansions.covmapping -instr-profile %t.profdata -filename-equivalence %s | FileCheck %s + +#define SIMPLE_OP \ + ++x +// CHECK: | [[@LINE-2]]|#define SIMPLE_OP +// CHECK-NEXT: 2| [[@LINE-2]]| ++x + +#define DO_SOMETHING \ + { \ + int x = 0; \ + SIMPLE_OP; \ + } +// CHECK: | [[@LINE-5]]|#define DO_SOMETHING +// CHECK-NEXT: 2| [[@LINE-5]]| { +// CHECK-NEXT: 2| [[@LINE-5]]| int x = 0; +// CHECK-NEXT: 2| [[@LINE-5]]| SIMPLE_OP; +// CHECK-NEXT: 2| [[@LINE-5]]| } + +int main() { // CHECK: 1| [[@LINE]]|int main() { + DO_SOMETHING; // CHECK-NEXT: 1| [[@LINE]]| DO_SOMETHING; + DO_SOMETHING; // CHECK-NEXT: 1| [[@LINE]]| DO_SOMETHING; + return 0; // CHECK-NEXT: 1| [[@LINE]]| return 0; +} // CHECK-NEXT: 1| [[@LINE]]|} Index: llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp =================================================================== --- llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp +++ llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp @@ -422,6 +422,8 @@ EXPECT_EQ(CoverageSegment(9, 9, false), Segments[3]); } +// If CodeRegions and ExpansionRegions cover the same area, +// only counts of CodeRegions should be used. TEST_P(MaybeSparseCoverageMappingTest, dont_combine_expansions) { InstrProfRecord Record1("func", 0x1234, {10, 20}); InstrProfRecord Record2("func", 0x1234, {0, 0}); @@ -444,6 +446,29 @@ ASSERT_EQ(CoverageSegment(9, 9, false), Segments[3]); } +// If an area is covered only by ExpansionRegions, they should be combinated. +TEST_P(MaybeSparseCoverageMappingTest, combine_expansions) { + InstrProfRecord Record("func", 0x1234, {2, 3, 7}); + NoError(ProfileWriter.addRecord(std::move(Record))); + + startFunction("func", 0x1234); + addCMR(Counter::getCounter(1), "include1", 1, 1, 1, 10); + addCMR(Counter::getCounter(2), "include2", 1, 1, 1, 10); + addCMR(Counter::getCounter(0), "file", 1, 1, 5, 5); + addExpansionCMR("file", "include1", 3, 1, 3, 5); + addExpansionCMR("file", "include2", 3, 1, 3, 5); + + loadCoverageMapping(); + + CoverageData Data = LoadedCoverage->getCoverageForFile("file"); + std::vector Segments(Data.begin(), Data.end()); + ASSERT_EQ(4U, Segments.size()); + EXPECT_EQ(CoverageSegment(1, 1, 2, true), Segments[0]); + EXPECT_EQ(CoverageSegment(3, 1, 10, true), Segments[1]); + EXPECT_EQ(CoverageSegment(3, 5, 2, false), Segments[2]); + EXPECT_EQ(CoverageSegment(5, 5, false), Segments[3]); +} + TEST_P(MaybeSparseCoverageMappingTest, strip_filename_prefix) { InstrProfRecord Record("file1:func", 0x1234, {0}); NoError(ProfileWriter.addRecord(std::move(Record)));