[llvm] 2c7610c - [nfc]Make InstrProfSymtab non-copyable and non-movable (#86882)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 27 20:40:05 PDT 2024
Author: Mingming Liu
Date: 2024-03-27T20:40:01-07:00
New Revision: 2c7610cc43cd70192a0ed5eac58471c50045c6de
URL: https://github.com/llvm/llvm-project/commit/2c7610cc43cd70192a0ed5eac58471c50045c6de
DIFF: https://github.com/llvm/llvm-project/commit/2c7610cc43cd70192a0ed5eac58471c50045c6de.diff
LOG: [nfc]Make InstrProfSymtab non-copyable and non-movable (#86882)
- The direct use case (in [1]) is to add `llvm::IntervalMap` [2] and the allocator required by IntervalMap ctor [3]
to class `InstrProfSymtab` as owned members. The allocator class doesn't have a move-assignment operator;
and it's going to take much effort to implement move-assignment operator for the allocator class such that the
enclosing class is movable.
- There is only one use of compiler-generated move-assignment operator in the repo, which is in
CoverageMappingReader.cpp. Luckily it's possible to use std::unique_ptr<InstrProfSymtab> instead, so did the change.
[1] https://github.com/llvm/llvm-project/pull/66825
[2] https://github.com/llvm/llvm-project/blob/4c2f68840e984b0f111779c46845ac00e3a7547d/llvm/include/llvm/ADT/IntervalMap.h#L936
[3] https://github.com/llvm/llvm-project/blob/4c2f68840e984b0f111779c46845ac00e3a7547d/llvm/include/llvm/ADT/IntervalMap.h#L1041
Added:
Modified:
llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
llvm/include/llvm/ProfileData/InstrProf.h
llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
index 346ca4ad2eb314..f05b90114d75a6 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
@@ -184,7 +184,7 @@ class BinaryCoverageReader : public CoverageMappingReader {
private:
std::vector<std::string> Filenames;
std::vector<ProfileMappingRecord> MappingRecords;
- InstrProfSymtab ProfileNames;
+ std::unique_ptr<InstrProfSymtab> ProfileNames;
size_t CurrentRecord = 0;
std::vector<StringRef> FunctionsFilenames;
std::vector<CounterExpression> Expressions;
@@ -195,8 +195,9 @@ class BinaryCoverageReader : public CoverageMappingReader {
// D69471, which can split up function records into multiple sections on ELF.
FuncRecordsStorage FuncRecords;
- BinaryCoverageReader(FuncRecordsStorage &&FuncRecords)
- : FuncRecords(std::move(FuncRecords)) {}
+ BinaryCoverageReader(std::unique_ptr<InstrProfSymtab> Symtab,
+ FuncRecordsStorage &&FuncRecords)
+ : ProfileNames(std::move(Symtab)), FuncRecords(std::move(FuncRecords)) {}
public:
BinaryCoverageReader(const BinaryCoverageReader &) = delete;
@@ -209,12 +210,10 @@ class BinaryCoverageReader : public CoverageMappingReader {
SmallVectorImpl<object::BuildIDRef> *BinaryIDs = nullptr);
static Expected<std::unique_ptr<BinaryCoverageReader>>
- createCoverageReaderFromBuffer(StringRef Coverage,
- FuncRecordsStorage &&FuncRecords,
- InstrProfSymtab &&ProfileNames,
- uint8_t BytesInAddress,
- llvm::endianness Endian,
- StringRef CompilationDir = "");
+ createCoverageReaderFromBuffer(
+ StringRef Coverage, FuncRecordsStorage &&FuncRecords,
+ std::unique_ptr<InstrProfSymtab> ProfileNamesPtr, uint8_t BytesInAddress,
+ llvm::endianness Endian, StringRef CompilationDir = "");
Error readNextRecord(CoverageMappingRecord &Record) override;
};
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index 25ec06a7392027..612c444faec648 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -471,6 +471,13 @@ class InstrProfSymtab {
public:
InstrProfSymtab() = default;
+ // Not copyable or movable.
+ // Consider std::unique_ptr for move.
+ InstrProfSymtab(const InstrProfSymtab &) = delete;
+ InstrProfSymtab &operator=(const InstrProfSymtab &) = delete;
+ InstrProfSymtab(InstrProfSymtab &&) = delete;
+ InstrProfSymtab &operator=(InstrProfSymtab &&) = delete;
+
/// Create InstrProfSymtab from an object file section which
/// contains function PGO names. When section may contain raw
/// string data or string data in compressed form. This method
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
index d328460510830a..445b48067a9755 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -894,31 +894,34 @@ static Error readCoverageMappingData(
Expected<std::unique_ptr<BinaryCoverageReader>>
BinaryCoverageReader::createCoverageReaderFromBuffer(
StringRef Coverage, FuncRecordsStorage &&FuncRecords,
- InstrProfSymtab &&ProfileNames, uint8_t BytesInAddress,
+ std::unique_ptr<InstrProfSymtab> ProfileNamesPtr, uint8_t BytesInAddress,
llvm::endianness Endian, StringRef CompilationDir) {
- std::unique_ptr<BinaryCoverageReader> Reader(
- new BinaryCoverageReader(std::move(FuncRecords)));
- Reader->ProfileNames = std::move(ProfileNames);
+ if (ProfileNamesPtr == nullptr)
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "Caller must provide ProfileNames");
+ std::unique_ptr<BinaryCoverageReader> Reader(new BinaryCoverageReader(
+ std::move(ProfileNamesPtr), std::move(FuncRecords)));
+ InstrProfSymtab &ProfileNames = *Reader->ProfileNames;
StringRef FuncRecordsRef = Reader->FuncRecords->getBuffer();
if (BytesInAddress == 4 && Endian == llvm::endianness::little) {
if (Error E = readCoverageMappingData<uint32_t, llvm::endianness::little>(
- Reader->ProfileNames, Coverage, FuncRecordsRef,
- Reader->MappingRecords, CompilationDir, Reader->Filenames))
+ ProfileNames, Coverage, FuncRecordsRef, Reader->MappingRecords,
+ CompilationDir, Reader->Filenames))
return std::move(E);
} else if (BytesInAddress == 4 && Endian == llvm::endianness::big) {
if (Error E = readCoverageMappingData<uint32_t, llvm::endianness::big>(
- Reader->ProfileNames, Coverage, FuncRecordsRef,
- Reader->MappingRecords, CompilationDir, Reader->Filenames))
+ ProfileNames, Coverage, FuncRecordsRef, Reader->MappingRecords,
+ CompilationDir, Reader->Filenames))
return std::move(E);
} else if (BytesInAddress == 8 && Endian == llvm::endianness::little) {
if (Error E = readCoverageMappingData<uint64_t, llvm::endianness::little>(
- Reader->ProfileNames, Coverage, FuncRecordsRef,
- Reader->MappingRecords, CompilationDir, Reader->Filenames))
+ ProfileNames, Coverage, FuncRecordsRef, Reader->MappingRecords,
+ CompilationDir, Reader->Filenames))
return std::move(E);
} else if (BytesInAddress == 8 && Endian == llvm::endianness::big) {
if (Error E = readCoverageMappingData<uint64_t, llvm::endianness::big>(
- Reader->ProfileNames, Coverage, FuncRecordsRef,
- Reader->MappingRecords, CompilationDir, Reader->Filenames))
+ ProfileNames, Coverage, FuncRecordsRef, Reader->MappingRecords,
+ CompilationDir, Reader->Filenames))
return std::move(E);
} else
return make_error<CoverageMapError>(
@@ -963,8 +966,8 @@ loadTestingFormat(StringRef Data, StringRef CompilationDir) {
if (Data.size() < ProfileNamesSize)
return make_error<CoverageMapError>(coveragemap_error::malformed,
"the size of ProfileNames is too big");
- InstrProfSymtab ProfileNames;
- if (Error E = ProfileNames.create(Data.substr(0, ProfileNamesSize), Address))
+ auto ProfileNames = std::make_unique<InstrProfSymtab>();
+ if (Error E = ProfileNames->create(Data.substr(0, ProfileNamesSize), Address))
return std::move(E);
Data = Data.substr(ProfileNamesSize);
@@ -1099,7 +1102,7 @@ loadBinaryFormat(std::unique_ptr<Binary> Bin, StringRef Arch,
OF->isLittleEndian() ? llvm::endianness::little : llvm::endianness::big;
// Look for the sections that we are interested in.
- InstrProfSymtab ProfileNames;
+ auto ProfileNames = std::make_unique<InstrProfSymtab>();
std::vector<SectionRef> NamesSectionRefs;
// If IPSK_name is not found, fallback to search for IPK_covname, which is
// used when binary correlation is enabled.
@@ -1116,7 +1119,7 @@ loadBinaryFormat(std::unique_ptr<Binary> Bin, StringRef Arch,
return make_error<CoverageMapError>(
coveragemap_error::malformed,
"the size of coverage mapping section is not one");
- if (Error E = ProfileNames.create(NamesSectionRefs.back()))
+ if (Error E = ProfileNames->create(NamesSectionRefs.back()))
return std::move(E);
auto CoverageSection = lookupSections(*OF, IPSK_covmap);
More information about the llvm-commits
mailing list