[clang] [llvm] [coverage] skipping code coverage for 'if constexpr' and 'if consteval' [WIP] (PR #78033)
Hana Dusíková via cfe-commits
cfe-commits at lists.llvm.org
Sat Jan 13 12:02:39 PST 2024
https://github.com/hanickadot updated https://github.com/llvm/llvm-project/pull/78033
>From 7e09f8a2dc9026047d34e0d6f4f55df9ab196fc2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?= <hanicka at hanicka.net>
Date: Sat, 13 Jan 2024 14:54:21 +0100
Subject: [PATCH 1/3] [coverage] skipping code coverage for 'if constexpr' and
'if consteval'
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 154 ++++++++++++++----
.../ProfileData/Coverage/CoverageMapping.cpp | 15 +-
2 files changed, 138 insertions(+), 31 deletions(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index b245abd16c3f4a..c476ffa6305da2 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -119,6 +119,10 @@ class SourceMappingRegion {
/// as the line execution count if there are no other regions on the line.
bool GapRegion;
+ /// Whetever this region is skipped ('if constexpr' or 'if consteval' untaken
+ /// branch)
+ bool SkippedRegion{false};
+
public:
SourceMappingRegion(Counter Count, std::optional<SourceLocation> LocStart,
std::optional<SourceLocation> LocEnd,
@@ -174,6 +178,10 @@ class SourceMappingRegion {
void setGap(bool Gap) { GapRegion = Gap; }
+ bool isSkipped() const { return SkippedRegion; }
+
+ void setSkipped(bool Skipped) { SkippedRegion = Skipped; }
+
bool isBranch() const { return FalseCount.has_value(); }
bool isMCDCDecision() const { return MCDCParams.NumConditions != 0; }
@@ -468,6 +476,10 @@ class CoverageMappingBuilder {
MappingRegions.push_back(CounterMappingRegion::makeGapRegion(
Region.getCounter(), *CovFileID, SR.LineStart, SR.ColumnStart,
SR.LineEnd, SR.ColumnEnd));
+ } else if (Region.isSkipped()) {
+ MappingRegions.push_back(CounterMappingRegion::makeSkipped(
+ *CovFileID, SR.LineStart, SR.ColumnStart, SR.LineEnd,
+ SR.ColumnEnd));
} else if (Region.isBranch()) {
MappingRegions.push_back(CounterMappingRegion::makeBranchRegion(
Region.getCounter(), Region.getFalseCounter(),
@@ -1252,6 +1264,29 @@ struct CounterCoverageMappingBuilder
popRegions(Index);
}
+ /// Emit a skip region between \p StartLoc and \p EndLoc with the given count.
+ void markSkipArea(SourceLocation StartLoc, SourceLocation EndLoc) {
+ if (StartLoc == EndLoc)
+ return;
+ assert(SpellingRegion(SM, StartLoc, EndLoc).isInSourceOrder());
+ handleFileExit(StartLoc);
+ size_t Index = pushRegion({}, StartLoc, EndLoc);
+ getRegion().setSkipped(true);
+ handleFileExit(EndLoc);
+ popRegions(Index);
+ }
+
+ void findGapAreaBetweenAndMarkSkipArea(SourceLocation AfterLoc,
+ SourceLocation BeforeLoc) {
+ // const std::optional<SourceRange> Gap = findGapAreaBetween(AfterLoc,
+ // BeforeLoc);
+ //
+ // if (Gap) {
+ // markSkipArea(Gap->getBegin(), Gap->getEnd());
+ // }
+ markSkipArea(AfterLoc, BeforeLoc);
+ }
+
/// Keep counts of breaks and continues inside loops.
struct BreakContinue {
Counter BreakCount;
@@ -1701,43 +1736,108 @@ struct CounterCoverageMappingBuilder
Visit(S->getSubStmt());
}
+ void CoverIfConsteval(const IfStmt *S) {
+ assert(S->isConsteval());
+
+ const auto *Then = S->getThen();
+ const auto *Else = S->getElse();
+
+ extendRegion(S);
+
+ if (S->isNegatedConsteval()) {
+ // ignore 'if consteval'
+ findGapAreaBetweenAndMarkSkipArea(S->getIfLoc(), getStart(Then));
+ Visit(Then);
+
+ if (Else) {
+ // ignore 'else <else>'
+ findGapAreaBetweenAndMarkSkipArea(getEnd(Then), getEnd(Else));
+ }
+ } else {
+ assert(S->isNonNegatedConsteval());
+ // ignore 'if consteval <then> [else]'
+ findGapAreaBetweenAndMarkSkipArea(S->getIfLoc(),
+ Else ? getStart(Else) : getEnd(Then));
+
+ if (Else)
+ Visit(Else);
+ }
+ }
+
+ void CoverIfConstexpr(const IfStmt *S) {
+ assert(S->isConstexpr());
+
+ // evaluate constant condition...
+ const auto *E = dyn_cast<ConstantExpr>(S->getCond());
+ assert(E != nullptr);
+ const bool isTrue = E->getResultAsAPSInt().getExtValue();
+
+ extendRegion(S);
+
+ const auto *Then = S->getThen();
+ const auto *Else = S->getElse();
+
+ // ignore 'if constexpr ('
+ SourceLocation startOfSkipped = S->getIfLoc();
+
+ if (S->getInit()) {
+ // don't mark initialisation as ignored
+ findGapAreaBetweenAndMarkSkipArea(startOfSkipped, getStart(S->getInit()));
+ Visit(S->getInit());
+ // ignore after initialisation: '; <condition>)'...
+ startOfSkipped = getEnd(S->getInit());
+ }
+
+ if (isTrue) {
+ // ignore '<condition>)'
+ findGapAreaBetweenAndMarkSkipArea(startOfSkipped, getStart(Then));
+ Visit(Then);
+
+ if (Else)
+ // ignore 'else <else>'
+ findGapAreaBetweenAndMarkSkipArea(getEnd(Then), getEnd(Else));
+ } else {
+ // ignore '<condition>) <then> [else]'
+ findGapAreaBetweenAndMarkSkipArea(startOfSkipped,
+ Else ? getStart(Else) : getEnd(Then));
+
+ if (Else)
+ Visit(Else);
+ }
+ }
+
void VisitIfStmt(const IfStmt *S) {
+ // "if constexpr" and "if consteval" are not normal conditional statements,
+ // they should behave more like a preprocessor conditions
+ if (S->isConsteval())
+ return CoverIfConsteval(S);
+ else if (S->isConstexpr())
+ return CoverIfConstexpr(S);
+
extendRegion(S);
if (S->getInit())
Visit(S->getInit());
// Extend into the condition before we propagate through it below - this is
// needed to handle macros that generate the "if" but not the condition.
- if (!S->isConsteval())
- extendRegion(S->getCond());
+ extendRegion(S->getCond());
Counter ParentCount = getRegion().getCounter();
+ Counter ThenCount = getRegionCounter(S);
- // If this is "if !consteval" the then-branch will never be taken, we don't
- // need to change counter
- Counter ThenCount =
- S->isNegatedConsteval() ? ParentCount : getRegionCounter(S);
+ // Emitting a counter for the condition makes it easier to interpret the
+ // counter for the body when looking at the coverage.
+ propagateCounts(ParentCount, S->getCond());
- if (!S->isConsteval()) {
- // Emitting a counter for the condition makes it easier to interpret the
- // counter for the body when looking at the coverage.
- propagateCounts(ParentCount, S->getCond());
-
- // The 'then' count applies to the area immediately after the condition.
- std::optional<SourceRange> Gap =
- findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
- if (Gap)
- fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
- }
+ // The 'then' count applies to the area immediately after the condition.
+ std::optional<SourceRange> Gap =
+ findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
+ if (Gap)
+ fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
extendRegion(S->getThen());
Counter OutCount = propagateCounts(ThenCount, S->getThen());
-
- // If this is "if consteval" the else-branch will never be taken, we don't
- // need to change counter
- Counter ElseCount = S->isNonNegatedConsteval()
- ? ParentCount
- : subtractCounters(ParentCount, ThenCount);
+ Counter ElseCount = subtractCounters(ParentCount, ThenCount);
if (const Stmt *Else = S->getElse()) {
bool ThenHasTerminateStmt = HasTerminateStmt;
@@ -1760,11 +1860,9 @@ struct CounterCoverageMappingBuilder
GapRegionCounter = OutCount;
}
- if (!S->isConsteval()) {
- // Create Branch Region around condition.
- createBranchRegion(S->getCond(), ThenCount,
- subtractCounters(ParentCount, ThenCount));
- }
+ // Create Branch Region around condition.
+ createBranchRegion(S->getCond(), ThenCount,
+ subtractCounters(ParentCount, ThenCount));
}
void VisitCXXTryStmt(const CXXTryStmt *S) {
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index eece6a2cc71797..9df235d2d748e1 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -1310,9 +1310,18 @@ LineCoverageStats::LineCoverageStats(
if (isStartOfRegion(LineSegments[I]))
++MinRegionCount;
- bool StartOfSkippedRegion = !LineSegments.empty() &&
- !LineSegments.front()->HasCount &&
- LineSegments.front()->IsRegionEntry;
+ // if any of region starting at this line, we need to count this line with it
+ auto IsRegionWithoutCount = [](const CoverageSegment *Segment) {
+ assert(Segment != nullptr);
+ return !(Segment->HasCount) && Segment->IsRegionEntry;
+ };
+
+ // previously skipped region could be only one on line, this changed with
+ // improvement for 'if constexpr' and 'if consteval' code coverage
+ const bool StartOfSkippedRegion =
+ !LineSegments.empty() &&
+ std::all_of(LineSegments.begin(), LineSegments.end(),
+ IsRegionWithoutCount);
HasMultipleRegions = MinRegionCount > 1;
Mapped =
>From b2b91dc1ad9c1be8f17dcd9678e1832296775adb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?= <hanicka at hanicka.net>
Date: Sat, 13 Jan 2024 19:19:35 +0100
Subject: [PATCH 2/3] revert in CoverageMapping.cpp
---
llvm/lib/ProfileData/Coverage/CoverageMapping.cpp | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 9df235d2d748e1..eece6a2cc71797 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -1310,18 +1310,9 @@ LineCoverageStats::LineCoverageStats(
if (isStartOfRegion(LineSegments[I]))
++MinRegionCount;
- // if any of region starting at this line, we need to count this line with it
- auto IsRegionWithoutCount = [](const CoverageSegment *Segment) {
- assert(Segment != nullptr);
- return !(Segment->HasCount) && Segment->IsRegionEntry;
- };
-
- // previously skipped region could be only one on line, this changed with
- // improvement for 'if constexpr' and 'if consteval' code coverage
- const bool StartOfSkippedRegion =
- !LineSegments.empty() &&
- std::all_of(LineSegments.begin(), LineSegments.end(),
- IsRegionWithoutCount);
+ bool StartOfSkippedRegion = !LineSegments.empty() &&
+ !LineSegments.front()->HasCount &&
+ LineSegments.front()->IsRegionEntry;
HasMultipleRegions = MinRegionCount > 1;
Mapped =
>From 90f17a22d3e478309d40a97cb275bd257be11143 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?= <hanicka at hanicka.net>
Date: Sat, 13 Jan 2024 21:02:28 +0100
Subject: [PATCH 3/3] snapshot
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 26 +++++---
.../ProfileData/Coverage/CoverageMapping.cpp | 66 ++++++++++++++++++-
2 files changed, 81 insertions(+), 11 deletions(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index c476ffa6305da2..22104af308de5c 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1741,13 +1741,15 @@ struct CounterCoverageMappingBuilder
const auto *Then = S->getThen();
const auto *Else = S->getElse();
+
+ const Counter ParentCount = getRegion().getCounter();
extendRegion(S);
if (S->isNegatedConsteval()) {
// ignore 'if consteval'
findGapAreaBetweenAndMarkSkipArea(S->getIfLoc(), getStart(Then));
- Visit(Then);
+ propagateCounts(ParentCount, Then);
if (Else) {
// ignore 'else <else>'
@@ -1760,7 +1762,7 @@ struct CounterCoverageMappingBuilder
Else ? getStart(Else) : getEnd(Then));
if (Else)
- Visit(Else);
+ propagateCounts(ParentCount, Else);
}
}
@@ -1774,24 +1776,27 @@ struct CounterCoverageMappingBuilder
extendRegion(S);
+ const auto *Init = S->getInit();
const auto *Then = S->getThen();
const auto *Else = S->getElse();
-
+
+ const Counter ParentCount = getRegion().getCounter();
+
// ignore 'if constexpr ('
SourceLocation startOfSkipped = S->getIfLoc();
- if (S->getInit()) {
+ if (Init) {
// don't mark initialisation as ignored
- findGapAreaBetweenAndMarkSkipArea(startOfSkipped, getStart(S->getInit()));
- Visit(S->getInit());
+ findGapAreaBetweenAndMarkSkipArea(startOfSkipped, getStart(Init));
+ propagateCounts(ParentCount, Init);
// ignore after initialisation: '; <condition>)'...
- startOfSkipped = getEnd(S->getInit());
+ startOfSkipped = getEnd(Init);
}
if (isTrue) {
// ignore '<condition>)'
findGapAreaBetweenAndMarkSkipArea(startOfSkipped, getStart(Then));
- Visit(Then);
+ propagateCounts(ParentCount, Then);
if (Else)
// ignore 'else <else>'
@@ -1801,8 +1806,9 @@ struct CounterCoverageMappingBuilder
findGapAreaBetweenAndMarkSkipArea(startOfSkipped,
Else ? getStart(Else) : getEnd(Then));
- if (Else)
- Visit(Else);
+ if (Else) {
+ propagateCounts(ParentCount, Else);
+ }
}
}
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index eece6a2cc71797..c8243852f7a62f 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -1319,8 +1319,72 @@ LineCoverageStats::LineCoverageStats(
!StartOfSkippedRegion &&
((WrappedSegment && WrappedSegment->HasCount) || (MinRegionCount > 0));
- if (!Mapped)
+ Mapped |= std::any_of(LineSegments.begin(), LineSegments.end(), [](const auto * Seq) {
+ return Seq->IsRegionEntry && Seq->HasCount;
+ });
+
+ Mapped |= WrappedSegment && WrappedSegment->IsRegionEntry && WrappedSegment->HasCount;
+
+ /*
+ printf("line %u: ", Line);
+ printf("segments: %u,", (unsigned)LineSegments.size());
+
+ auto print_seq = [](const auto * seq) {
+ printf("(line: %u, column: %u ", seq->Line, seq->Col);
+ if (seq->HasCount) {
+ printf(" count: %llu", seq->Count);
+ } else {
+ printf(" nocount");
+ }
+
+ if (seq->IsRegionEntry) {
+ printf(" entry");
+ }
+
+ if (seq->IsGapRegion) {
+ printf(" gap");
+ }
+
+ printf(")");
+ };
+
+ for (const auto * seq: LineSegments) {
+ printf(" ");
+ print_seq(seq);
+ }
+
+ if (WrappedSegment) {
+ printf(" wrapped seq: ");
+ print_seq(WrappedSegment);
+ }
+
+
+
+ bool should_map = false;
+ for (const auto * seq: LineSegments) {
+ if (seq->IsRegionEntry && seq->HasCount) {
+ should_map = true;
+ }
+ }
+
+ if (should_map) {
+ Mapped = true;
+ }
+
+ if (WrappedSegment && WrappedSegment->IsRegionEntry && WrappedSegment->HasCount) {
+ if (!Mapped) {
+ Mapped = true;
+ printf(" [ARGH:%u]",Line);
+ }
+ }*/
+
+ if (!Mapped) {
+ //printf(" [skip]\n");
return;
+ }
+
+ //printf(" mapped\n");
+
// Pick the max count from the non-gap, region entry segments and the
// wrapped count.
More information about the cfe-commits
mailing list