llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-codegen Author: Fabian Meumertzheim (fmeum) <details> <summary>Changes</summary> For every used function, `gatherSkippedRegions` adjusted every skipped range (comment, empty line, `#if 0` block) seen so far in the translation unit, which is quadratic. Instead, index the skipped ranges by file and location and only look at those near the function's lines. The mappings don't change, except that comments in `_Pragma` strings no longer hit the "region spans multiple files" assertion. CPU time with `-O0 -fprofile-instr-generate -fcoverage-mapping -emit-llvm`: | File | Before | After | |---|---:|---:| | SemaExpr.cpp | 24.3 s | 8.1 s | | DAGCombiner.cpp | 6.0 s | 3.8 s | | InstCombineCalls.cpp | 3.7 s | 2.3 s | | Instructions.cpp | 2.4 s | 1.8 s | Assisted-by: Claude Code, OpenAI Codex --- Full diff: https://github.com/llvm/llvm-project/pull/233136.diff 3 Files Affected: - (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+67-27) - (modified) clang/lib/CodeGen/CoverageMappingGen.h (+13-4) - (added) clang/test/CoverageMapping/pragma-comment.c (+11) ``````````diff diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 970812f7b0f75..34d6732237fc5 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -103,6 +103,38 @@ void CoverageSourceInfo::updateNextTokLoc(SourceLocation Loc) { SkippedRanges.back().NextTokLoc = Loc; } +SmallVector<SkippedRange, 0> +CoverageSourceInfo::getSkippedRangesIn(SourceRange Range) { + // Index the skipped ranges added since the last call. As they are mostly + // added in source order, they are usually appended. + for (unsigned I = NumIndexedSkippedRanges; I < SkippedRanges.size(); ++I) { + SourceLocation Begin = SkippedRanges[I].Range.getBegin(); + auto &Indices = SkippedRangesByFile[SourceMgr.getFileID(Begin)]; + Indices.insert(llvm::upper_bound(Indices, Begin, + [&](SourceLocation Loc, unsigned J) { + return Loc < + SkippedRanges[J].Range.getBegin(); + }), + I); + } + NumIndexedSkippedRanges = SkippedRanges.size(); + + SmallVector<SkippedRange, 0> Result; + auto It = SkippedRangesByFile.find(SourceMgr.getFileID(Range.getBegin())); + if (It == SkippedRangesByFile.end()) + return Result; + ArrayRef<unsigned> Indices = It->second; + const unsigned *First = llvm::partition_point(Indices, [&](unsigned I) { + return SkippedRanges[I].Range.getBegin() < Range.getBegin(); + }); + for (unsigned I : llvm::make_range(First, Indices.end())) { + if (Range.getEnd() < SkippedRanges[I].Range.getBegin()) + break; + Result.push_back(SkippedRanges[I]); + } + return Result; +} + namespace { /// A region of source code that can be mapped to a counter. class SourceMappingRegion { @@ -479,34 +511,42 @@ class CoverageMappingBuilder { std::max(FileLineRanges[R.FileID].second, R.LineEnd); } - auto SkippedRanges = CVM.getSourceInfo().getSkippedRanges(); - for (auto &I : SkippedRanges) { - SourceRange Range = I.Range; - auto LocStart = Range.getBegin(); - auto LocEnd = Range.getEnd(); - assert(SM.isWrittenInSameFile(LocStart, LocEnd) && - "region spans multiple files"); - - auto CovFileID = getCoverageFileID(LocStart); - if (!CovFileID) - continue; - std::optional<SpellingRegion> SR; - if (I.isComment()) - SR = adjustSkippedRange(SM, LocStart, LocEnd, I.PrevTokLoc, - I.NextTokLoc); - else if (I.isPPIfElse() || I.isEmptyLine()) - SR = {SM, LocStart, LocEnd}; - - if (!SR) + for (const auto &[File, Mapping] : FileIDMapping) { + unsigned CovFileID = Mapping.first; + auto [FirstLine, LastLine] = FileLineRanges[CovFileID]; + // Ignore files without regions and macro expansions, whose skipped + // ranges (e.g. comments in _Pragma) don't correspond to lines. + if (!SM.getSLocEntry(File).isFile() || FirstLine > LastLine) continue; - auto Region = CounterMappingRegion::makeSkipped( - *CovFileID, SR->LineStart, SR->ColumnStart, SR->LineEnd, - SR->ColumnEnd); - // Make sure that we only collect the regions that are inside - // the source code of this function. - if (Region.LineStart >= FileLineRanges[*CovFileID].first && - Region.LineEnd <= FileLineRanges[*CovFileID].second) - MappingRegions.push_back(Region); + // Only collect the skipped ranges inside the source code of this + // function. As adjusting a skipped range may move its start to the next + // line, also consider the ones that begin on the line before. + SourceLocation Begin = + SM.translateLineCol(File, std::max(FirstLine, 2u) - 1, 1); + SourceLocation End = SM.translateLineCol(File, LastLine + 1, 1); + for (const SkippedRange &I : + CVM.getSourceInfo().getSkippedRangesIn({Begin, End})) { + SourceRange Range = I.Range; + auto LocStart = Range.getBegin(); + auto LocEnd = Range.getEnd(); + assert(SM.isWrittenInSameFile(LocStart, LocEnd) && + "region spans multiple files"); + + std::optional<SpellingRegion> SR; + if (I.isComment()) + SR = adjustSkippedRange(SM, LocStart, LocEnd, I.PrevTokLoc, + I.NextTokLoc); + else if (I.isPPIfElse() || I.isEmptyLine()) + SR = {SM, LocStart, LocEnd}; + + if (!SR) + continue; + auto Region = CounterMappingRegion::makeSkipped( + CovFileID, SR->LineStart, SR->ColumnStart, SR->LineEnd, + SR->ColumnEnd); + if (Region.LineStart >= FirstLine && Region.LineEnd <= LastLine) + MappingRegions.push_back(Region); + } } } diff --git a/clang/lib/CodeGen/CoverageMappingGen.h b/clang/lib/CodeGen/CoverageMappingGen.h index 0ed50597e1dc3..aa2c2efa81158 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.h +++ b/clang/lib/CodeGen/CoverageMappingGen.h @@ -50,9 +50,9 @@ struct SkippedRange { // The nature of this skipped range Kind RangeKind; - bool isComment() { return RangeKind == Comment; } - bool isEmptyLine() { return RangeKind == EmptyLine; } - bool isPPIfElse() { return RangeKind == PPIfElse; } + bool isComment() const { return RangeKind == Comment; } + bool isEmptyLine() const { return RangeKind == EmptyLine; } + bool isPPIfElse() const { return RangeKind == PPIfElse; } SkippedRange(SourceRange Range, Kind K, SourceLocation PrevTokLoc = SourceLocation(), @@ -70,6 +70,13 @@ class CoverageSourceInfo : public PPCallbacks, // A vector of skipped source ranges and PrevTokLoc with NextTokLoc. std::vector<SkippedRange> SkippedRanges; + // Indices into SkippedRanges by file, sorted by begin location. This is + // updated lazily by getSkippedRangesIn. + llvm::DenseMap<FileID, SmallVector<unsigned, 0>> SkippedRangesByFile; + + // The number of SkippedRanges in SkippedRangesByFile. + unsigned NumIndexedSkippedRanges = 0; + SourceManager &SourceMgr; public: @@ -79,7 +86,9 @@ class CoverageSourceInfo : public PPCallbacks, CoverageSourceInfo(SourceManager &SourceMgr) : SourceMgr(SourceMgr) {} - std::vector<SkippedRange> &getSkippedRanges() { return SkippedRanges; } + /// Return the skipped ranges that begin within \p Range, which must be in a + /// single file, sorted by their begin locations. + SmallVector<SkippedRange, 0> getSkippedRangesIn(SourceRange Range); void AddSkippedRange(SourceRange Range, SkippedRange::Kind RangeKind); diff --git a/clang/test/CoverageMapping/pragma-comment.c b/clang/test/CoverageMapping/pragma-comment.c new file mode 100644 index 0000000000000..3c868b1ac5dcb --- /dev/null +++ b/clang/test/CoverageMapping/pragma-comment.c @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s | FileCheck %s + +// Comments in _Pragma strings are lexed in macro expansions. + +void f(void) { + _Pragma("clang diagnostic push /* comment */") +} + +// CHECK-LABEL: f: +// CHECK-NEXT: File 0, [[@LINE-5]]:14 -> [[@LINE-3]]:2 = #0 +// CHECK-NOT: Skipped `````````` </details> https://github.com/llvm/llvm-project/pull/233136 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
