[PATCH] D18831: [Coverage] Use the count value of the outer region for an expansion region.
Justin Bogner via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 25 16:00:51 PDT 2016
Igor Kudrin <ikudrin.dev at gmail.com> writes:
> ikudrin updated this revision to Diff 54836.
> ikudrin added a comment.
>
> - Added a test for llvm-cov.
LGTM, but I noticed a typo.
>
> http://reviews.llvm.org/D18831
>
> Files:
> lib/ProfileData/CoverageMapping.cpp
> test/tools/llvm-cov/Inputs/expansion-counts.covmapping
> test/tools/llvm-cov/Inputs/expansion-counts.proftext
> test/tools/llvm-cov/expansion-counts.cpp
> unittests/ProfileData/CoverageMappingTest.cpp
>
> Index: unittests/ProfileData/CoverageMappingTest.cpp
> ===================================================================
> --- unittests/ProfileData/CoverageMappingTest.cpp
> +++ unittests/ProfileData/CoverageMappingTest.cpp
> @@ -512,6 +512,36 @@
> EXPECT_EQ(CoverageSegment(1, 10, false), Segments[1]);
> }
>
> +TEST_P(MaybeSparseCoverageMappingTest,
> + expansion_region_uses_count_of_outer_region) {
> + InstrProfRecord Record("func", 0x1234, {10, 20});
> + ProfileWriter.addRecord(std::move(Record));
> +
> + startFunction("func", 0x1234);
> + addCMR(Counter::getCounter(1), "include", 1, 1, 1, 10);
> + addCMR(Counter::getCounter(0), "file", 1, 1, 5, 5);
> + addExpansionCMR("file", "include", 3, 1, 3, 5);
> +
> + loadCoverageMapping();
> +
> + CoverageData DataI = LoadedCoverage->getCoverageForFile("include");
> + std::vector<CoverageSegment> SegmentsI(DataI.begin(), DataI.end());
> + ASSERT_EQ(2U, SegmentsI.size());
> + EXPECT_EQ(CoverageSegment(1, 1, 20, true), SegmentsI[0]);
> + EXPECT_EQ(CoverageSegment(1, 10, false), SegmentsI[1]);
> +
> + // Despite the fact that the count value for the segment in "include" is "20",
> + // the corresponding segment in "file" should have value "10", inheriting
> + // it from its outer region.
> + CoverageData DataF = LoadedCoverage->getCoverageForFile("file");
> + std::vector<CoverageSegment> SegmentsF(DataF.begin(), DataF.end());
> + ASSERT_EQ(4U, SegmentsF.size());
> + EXPECT_EQ(CoverageSegment(1, 1, 10, true), SegmentsF[0]);
> + EXPECT_EQ(CoverageSegment(3, 1, 10, true), SegmentsF[1]);
> + EXPECT_EQ(CoverageSegment(3, 5, 10, false), SegmentsF[2]);
> + EXPECT_EQ(CoverageSegment(5, 5, false), SegmentsF[3]);
> +}
> +
> INSTANTIATE_TEST_CASE_P(MaybeSparse, MaybeSparseCoverageMappingTest,
> ::testing::Bool());
>
> Index: test/tools/llvm-cov/expansion-counts.cpp
> ===================================================================
> --- /dev/null
> +++ test/tools/llvm-cov/expansion-counts.cpp
> @@ -0,0 +1,39 @@
> +// Check that expansion regions use counts of their outer regions.
> +
> +// RUN: llvm-profdata merge %S/Inputs/expansion-counts.proftext -o %t.profdata
> +// RUN: llvm-cov show %S/Inputs/expansion-counts.covmapping -instr-profile %t.profdata -filename-equivalence %s | FileCheck %s
> +
> +#define DO_LOOP1() \
> + for (int K = 0; K < 2; ++K) {}
> +// CHECK: | [[@LINE-2]]|#define DO_LOOP1()
> +// CHECK-NEXT: 3| [[@LINE-2]]| for (int K = 0; K < 2; ++K) {}
> +
> +#define DO_LOOP2() \
> + for (int K = 0; K < 2; ++K) {}
> +// CHECK: | [[@LINE-2]]|#define DO_LOOP2()
> +// CHECK-NEXT: 3| [[@LINE-2]]| for (int K = 0; K < 2; ++K) {}
> +
> +#define DO_LOOP3() \
> + for (int K = 0; K < 2; ++K) {}
> +// CHECK: | [[@LINE-2]]|#define DO_LOOP3()
> +// CHECK-NEXT: 6| [[@LINE-2]]| for (int K = 0; K < 2; ++K) {}
> +
> +#define JUST_EXPAND_LOOP2() DO_LOOP2()
> +// CHECK: 1| [[@LINE-1]]|#define JUST_EXPAND_LOOP2() DO_LOOP2()
> +
> +#define DO_SOMETHING() \
> + { \
> + DO_LOOP3(); \
> + }
> +// CHECK: | [[@LINE-4]]|#define DO_SOMETHING()
> +// CHECK-NEXT: 2| [[@LINE-4]]| {
> +// CHECK-NEXT: 2| [[@LINE-4]]| DO_LOOP3();
> +// CHECK-NEXT: 2| [[@LINE-4]]| }
> +
> +int main() { // CHECK: 1| [[@LINE]]|int main() {
> + DO_LOOP1(); // CHECK-NEXT: 1| [[@LINE]]| DO_LOOP1();
> + JUST_EXPAND_LOOP2(); // CHECK-NEXT: 1| [[@LINE]]| JUST_EXPAND_LOOP2();
> + 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: test/tools/llvm-cov/Inputs/expansion-counts.proftext
> ===================================================================
> --- /dev/null
> +++ test/tools/llvm-cov/Inputs/expansion-counts.proftext
> @@ -0,0 +1,12 @@
> +main
> +# Func Hash:
> +1065220
> +# Num Counters:
> +5
> +# Counter Values:
> +1
> +2
> +2
> +2
> +2
> +
> Index: lib/ProfileData/CoverageMapping.cpp
> ===================================================================
> --- lib/ProfileData/CoverageMapping.cpp
> +++ lib/ProfileData/CoverageMapping.cpp
> @@ -334,17 +334,28 @@
>
> /// 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();
> - });
> + 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 region, prefer CodeRegion to
> + // ExpansionRegion, and ExpansionRegion to SkippedRegion.
> + // That feature is required for correct working of
> + // combineRegions() and fixExpansionRegions().
> + 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.
> - static ArrayRef<CountedRegion>
> + static MutableArrayRef<CountedRegion>
> combineRegions(MutableArrayRef<CountedRegion> Regions) {
> if (Regions.empty())
> return Regions;
> @@ -360,31 +371,52 @@
> 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.
> + // As long as Regions are sorted with sortNestedRegions(),
> + // we expect that the following conditions are met:
> + // * If *I is CodeRegion, *Active is CodeRegion;
> + // * If *Active is ExpansionRegion, *I cannot be CodeRegion;
> + // * If *Active is SkippedRegion, *I can only be SkippedRegion.
> + // We add counts only for CodeRegions because:
> + // * ExapnsionRegions will inherit their counts from their outer regions;
s/ExapnsionRegions/ExpansionRegions/
> + // * SkippedRegions do not need to be combined at all.
> + if (I->Kind == coverage::CounterMappingRegion::CodeRegion) {
> + assert(Active->Kind == coverage::CounterMappingRegion::CodeRegion);
> Active->ExecutionCount += I->ExecutionCount;
> + }
> }
> return Regions.drop_back(std::distance(++Active, End));
> }
>
> + /// Set counts of ExpansionRegions to the value of the outer region.
> + static void fixExpansionRegions(MutableArrayRef<CountedRegion> Regions) {
> + for (auto Begin = Regions.begin(), I = Regions.begin(), End = Regions.end();
> + I != End; ++I) {
> + if (I->Kind != coverage::CounterMappingRegion::ExpansionRegion)
> + continue;
> + // Find the nearest region which contains the current.
> + for (auto Outer = I; Outer != Begin;) {
> + --Outer;
> + assert(Outer->startLoc() <= I->startLoc());
> + if (Outer->endLoc() >= I->endLoc()) {
> + I->ExecutionCount = Outer->ExecutionCount;
> + break;
> + }
> + }
> + }
> + }
> +
> public:
> /// Build a list of CoverageSegments from a list of Regions.
> static std::vector<CoverageSegment>
> buildSegments(MutableArrayRef<CountedRegion> Regions) {
> std::vector<CoverageSegment> Segments;
> SegmentBuilder Builder(Segments);
>
> sortNestedRegions(Regions);
> - ArrayRef<CountedRegion> CombinedRegions = combineRegions(Regions);
> + Regions = combineRegions(Regions);
> + fixExpansionRegions(Regions);
>
> - Builder.buildSegmentsImpl(CombinedRegions);
> + Builder.buildSegmentsImpl(Regions);
> return Segments;
> }
> };
More information about the llvm-commits
mailing list