bruno created this revision.
bruno added reviewers: vsk, zequanwu.
Herald added subscribers: hoy, modimo, wenlei.
Herald added a project: All.
bruno requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
D83592 <https://reviews.llvm.org/D83592> added comments to be part of skipped
regions, and as part of that, it
also shrinks a skipped range if it spans a line that contains a non-comment
token. This is done by `adjustSkippedRange`.
The `adjustSkippedRange` currently runs on skipped regions that are not
comments, causing a 5min regression while building a big C++ files without any
comments.
Fix the compile time introduced in D83592 <https://reviews.llvm.org/D83592> by
tagging SkippedRange with kind
information and use that to decide what needs additional processing. There are
no good way to add a testcase for this that I'm aware.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D127338
Files:
clang/lib/CodeGen/CoverageMappingGen.cpp
clang/lib/CodeGen/CoverageMappingGen.h
Index: clang/lib/CodeGen/CoverageMappingGen.h
===================================================================
--- clang/lib/CodeGen/CoverageMappingGen.h
+++ clang/lib/CodeGen/CoverageMappingGen.h
@@ -31,15 +31,29 @@
class Stmt;
struct SkippedRange {
+ enum Kind {
+ PPIfElse, // Preprocessor #if/#else ...
+ EmptyLine,
+ Comment,
+ };
+
SourceRange Range;
// The location of token before the skipped source range.
SourceLocation PrevTokLoc;
// The location of token after the skipped source range.
SourceLocation NextTokLoc;
+ // The nature of this skipped range
+ Kind RangeKind;
+
+ bool isComment() { return RangeKind == Comment; }
+ bool isEmptyLine() { return RangeKind == EmptyLine; }
+ bool isPPIfElse() { return RangeKind == PPIfElse; }
- SkippedRange(SourceRange Range, SourceLocation PrevTokLoc = SourceLocation(),
+ SkippedRange(SourceRange Range, Kind K,
+ SourceLocation PrevTokLoc = SourceLocation(),
SourceLocation NextTokLoc = SourceLocation())
- : Range(Range), PrevTokLoc(PrevTokLoc), NextTokLoc(NextTokLoc) {}
+ : Range(Range), PrevTokLoc(PrevTokLoc), NextTokLoc(NextTokLoc),
+ RangeKind(K) {}
};
/// Stores additional source code information like skipped ranges which
@@ -62,7 +76,7 @@
std::vector<SkippedRange> &getSkippedRanges() { return SkippedRanges; }
- void AddSkippedRange(SourceRange Range);
+ void AddSkippedRange(SourceRange Range, SkippedRange::Kind RangeKind);
void SourceRangeSkipped(SourceRange Range, SourceLocation EndifLoc) override;
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===================================================================
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -60,26 +60,27 @@
return CoverageInfo;
}
-void CoverageSourceInfo::AddSkippedRange(SourceRange Range) {
+void CoverageSourceInfo::AddSkippedRange(SourceRange Range,
+ SkippedRange::Kind RangeKind) {
if (EmptyLineCommentCoverage && !SkippedRanges.empty() &&
PrevTokLoc == SkippedRanges.back().PrevTokLoc &&
SourceMgr.isWrittenInSameFile(SkippedRanges.back().Range.getEnd(),
Range.getBegin()))
SkippedRanges.back().Range.setEnd(Range.getEnd());
else
- SkippedRanges.push_back({Range, PrevTokLoc});
+ SkippedRanges.push_back({Range, RangeKind, PrevTokLoc});
}
void CoverageSourceInfo::SourceRangeSkipped(SourceRange Range, SourceLocation) {
- AddSkippedRange(Range);
+ AddSkippedRange(Range, SkippedRange::PPIfElse);
}
void CoverageSourceInfo::HandleEmptyline(SourceRange Range) {
- AddSkippedRange(Range);
+ AddSkippedRange(Range, SkippedRange::EmptyLine);
}
bool CoverageSourceInfo::HandleComment(Preprocessor &PP, SourceRange Range) {
- AddSkippedRange(Range);
+ AddSkippedRange(Range, SkippedRange::Comment);
return false;
}
@@ -335,6 +336,8 @@
/// This shrinks the skipped range if it spans a line that contains a
/// non-comment token. If shrinking the skipped range would make it empty,
/// this returns None.
+ /// Note this function can potentially be expensive because
+ /// getSpellingLineNumber uses getLineNumber, which is expensive.
Optional<SpellingRegion> adjustSkippedRange(SourceManager &SM,
SourceLocation LocStart,
SourceLocation LocEnd,
@@ -382,9 +385,14 @@
auto CovFileID = getCoverageFileID(LocStart);
if (!CovFileID)
continue;
- Optional<SpellingRegion> SR =
- adjustSkippedRange(SM, LocStart, LocEnd, I.PrevTokLoc, I.NextTokLoc);
- if (!SR.hasValue())
+ Optional<SpellingRegion> SR;
+ if (I.isComment())
+ SR = adjustSkippedRange(SM, LocStart, LocEnd, I.PrevTokLoc,
+ I.NextTokLoc);
+ else if (I.isPPIfElse())
+ SR = {SM, LocStart, LocEnd};
+
+ if (I.isEmptyLine() || !SR.hasValue())
continue;
auto Region = CounterMappingRegion::makeSkipped(
*CovFileID, SR->LineStart, SR->ColumnStart, SR->LineEnd,
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
