Index: include/llvm/ProfileData/Coverage/CoverageMappingReader.h =================================================================== --- include/llvm/ProfileData/Coverage/CoverageMappingReader.h +++ include/llvm/ProfileData/Coverage/CoverageMappingReader.h @@ -103,6 +103,16 @@ Error read(); }; +/// \brief Checks if the given coverage mapping data is exported for +/// an unused function. +class RawCoverageMappingDummyChecker : public RawCoverageReader { +public: + RawCoverageMappingDummyChecker(StringRef MappingData) + : RawCoverageReader(MappingData) {} + + Error isDummy(bool &Result); +}; + /// \brief Reader for the raw coverage mapping data. class RawCoverageMappingReader : public RawCoverageReader { ArrayRef TranslationUnitFilenames; Index: lib/ProfileData/Coverage/CoverageMappingReader.cpp =================================================================== --- lib/ProfileData/Coverage/CoverageMappingReader.cpp +++ lib/ProfileData/Coverage/CoverageMappingReader.cpp @@ -13,7 +13,7 @@ //===----------------------------------------------------------------------===// #include "llvm/ProfileData/Coverage/CoverageMappingReader.h" -#include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/DenseMap.h" #include "llvm/Object/MachOUniversal.h" #include "llvm/Object/ObjectFile.h" #include "llvm/Support/Debug.h" @@ -294,6 +294,39 @@ return Error::success(); } +Error RawCoverageMappingDummyChecker::isDummy(bool &Result) { + // A dummy coverage mapping data consists of just one region with zero count. + Result = false; + uint64_t NumFileMappings; + if (auto Err = readSize(NumFileMappings)) + return Err; + if (NumFileMappings != 1) + return Error::success(); + // We don't expect any specific value for the filename index, just skip it. + uint64_t FilenameIndex; + if (auto Err = + readIntMax(FilenameIndex, std::numeric_limits::max())) + return Err; + uint64_t NumExpressions; + if (auto Err = readSize(NumExpressions)) + return Err; + if (NumExpressions != 0) + return Error::success(); + uint64_t NumRegions; + if (auto Err = readSize(NumRegions)) + return Err; + if (NumRegions != 1) + return Error::success(); + uint64_t EncodedCounterAndRegion; + if (auto Err = readIntMax(EncodedCounterAndRegion, + std::numeric_limits::max())) + return Err; + unsigned Tag = EncodedCounterAndRegion & Counter::EncodingTagMask; + if (Tag == Counter::Zero) + Result = true; + return Error::success(); +} + Error InstrProfSymtab::create(SectionRef &Section) { if (auto EC = Section.getContents(Data)) return errorCodeToError(EC); @@ -310,6 +343,17 @@ return Data.substr(Pointer - Address, Size); } +// Check if the mapping data is a dummy, i.e. is emitted for an unused function. +static Error isCoverageMappingDummy(uint64_t Hash, StringRef Mapping, + bool &Result) { + // The hash value of dummy mapping records is always zero. + if (Hash) { + Result = false; + return Error::success(); + } + return RawCoverageMappingDummyChecker(Mapping).isDummy(Result); +} + namespace { struct CovMapFuncRecordReader { // The interface to read coverage mapping function records for @@ -334,11 +378,55 @@ typedef typename coverage::CovMapTraits::NameRefType NameRefType; - llvm::DenseSet UniqueFunctionMappingData; + // Maps function's name references to the indexes of their records + // in \c Records. + llvm::DenseMap FunctionRecords; InstrProfSymtab &ProfileNames; std::vector &Filenames; std::vector &Records; + // Add the record to the collection if we don't already have a record that + // points to the same function name. This is useful to ignore the redundant + // records for the functions with ODR linkage. + // In addition, prefer records with real coverage mapping data to dummy + // records, which were emitted for inline functions which were seen but + // not used in the corresponding translation unit. + Error insertFunctionRecordIfNeeded(const FuncRecordType *CFR, + StringRef Mapping, size_t FilenamesBegin) { + uint64_t FuncHash = CFR->template getFuncHash(); + NameRefType NameRef = CFR->template getFuncNameRef(); + auto InsertResult = + FunctionRecords.insert(std::make_pair(NameRef, Records.size())); + if (InsertResult.second) { + StringRef FuncName; + if (Error Err = CFR->template getFuncName(ProfileNames, FuncName)) + return Err; + Records.emplace_back(Version, FuncName, FuncHash, Mapping, FilenamesBegin, + Filenames.size() - FilenamesBegin); + return Error::success(); + } + // Update the existent record if it's a dummy and the new record is real. + size_t OldRecordIndex = InsertResult.first->second; + BinaryCoverageReader::ProfileMappingRecord &OldRecord = + Records[OldRecordIndex]; + bool OldIsDummy; + if (Error Err = isCoverageMappingDummy( + OldRecord.FunctionHash, OldRecord.CoverageMapping, OldIsDummy)) + return Err; + if (!OldIsDummy) + return Error::success(); + bool NewIsDummy; + if (Error Err = isCoverageMappingDummy(FuncHash, Mapping, NewIsDummy)) + return Err; + if (NewIsDummy) + return Error::success(); + OldRecord.FunctionHash = FuncHash; + OldRecord.CoverageMapping = Mapping; + OldRecord.FilenamesBegin = FilenamesBegin; + OldRecord.FilenamesSize = Filenames.size() - FilenamesBegin; + return Error::success(); + } + public: VersionedCovMapFuncRecordReader( InstrProfSymtab &P, @@ -387,7 +475,6 @@ while ((const char *)CFR < FunEnd) { // Read the function information uint32_t DataSize = CFR->template getDataSize(); - uint64_t FuncHash = CFR->template getFuncHash(); // Now use that to read the coverage data. if (CovBuf + DataSize > CovEnd) @@ -395,21 +482,9 @@ auto Mapping = StringRef(CovBuf, DataSize); CovBuf += DataSize; - // Ignore this record if we already have a record that points to the same - // function name. This is useful to ignore the redundant records for the - // functions with ODR linkage. - NameRefType NameRef = CFR->template getFuncNameRef(); - if (!UniqueFunctionMappingData.insert(NameRef).second) { - CFR++; - continue; - } - - StringRef FuncName; - if (Error E = CFR->template getFuncName(ProfileNames, FuncName)) - return E; - Records.push_back(BinaryCoverageReader::ProfileMappingRecord( - Version, FuncName, FuncHash, Mapping, FilenamesBegin, - Filenames.size() - FilenamesBegin)); + if (Error Err = + insertFunctionRecordIfNeeded(CFR, Mapping, FilenamesBegin)) + return Err; CFR++; } return Error::success(); Index: test/tools/llvm-cov/Inputs/prefer_used_to_unused.cpp =================================================================== --- /dev/null +++ test/tools/llvm-cov/Inputs/prefer_used_to_unused.cpp @@ -0,0 +1,5 @@ +#include "prefer_used_to_unused.h" + +int main() { + return sampleFunc(5) + simpleFunc(5); +} Index: test/tools/llvm-cov/Inputs/prefer_used_to_unused.proftext =================================================================== --- /dev/null +++ test/tools/llvm-cov/Inputs/prefer_used_to_unused.proftext @@ -0,0 +1,25 @@ +_Z10sampleFunci +# Func Hash: +10 +# Num Counters: +2 +# Counter Values: +1 +1 + +main +# Func Hash: +0 +# Num Counters: +1 +# Counter Values: +1 + +_Z10simpleFunci +# Func Hash: +0 +# Num Counters: +1 +# Counter Values: +1 + Index: test/tools/llvm-cov/prefer_used_to_unused.h =================================================================== --- /dev/null +++ test/tools/llvm-cov/prefer_used_to_unused.h @@ -0,0 +1,22 @@ +// Check that llvm-cov loads coverage mapping data for real function even though +// an unused function comes first. + +// If you need to rebuild the 'covmapping' file for this test, please use +// the following commands: +// clang++ -fprofile-instr-generate -fcoverage-mapping -o tmp -x c++ prefer_used_to_unused.h prefer_used_to_unused.cpp +// llvm-cov convert-for-testing -o prefer_used_to_unused.covmapping tmp + +// RUN: llvm-profdata merge %S/Inputs/prefer_used_to_unused.proftext -o %t.profdata +// RUN: llvm-cov show %S/Inputs/prefer_used_to_unused.covmapping -instr-profile %t.profdata -filename-equivalence %s | FileCheck %s + +// Coverage data for this function has a non-zero hash value if it is used in the translation unit. +inline int sampleFunc(int A) { // CHECK: 1| [[@LINE]]|inline int sampleFunc(int A) { + if (A > 0) // CHECK-NEXT: 1| [[@LINE]]| if (A > 0) + return A; // CHECK-NEXT: 1| [[@LINE]]| return A; + return 0; // CHECK-NEXT: 0| [[@LINE]]| return 0; +} // CHECK-NEXT: 1| [[@LINE]]|} + +// The hash for this function is zero in both cases, either it is used in the translation unit or not. +inline int simpleFunc(int A) { // CHECK: 1| [[@LINE]]|inline int simpleFunc(int A) { + return A; // CHECK-NEXT: 1| [[@LINE]]| return A; +} // CHECK-NEXT: 1| [[@LINE]]|}