[llvm] [nfc]Make InstrProfSymtab non-copyable and non-movable (PR #86882)
Mingming Liu via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 27 15:21:21 PDT 2024
https://github.com/minglotus-6 created https://github.com/llvm/llvm-project/pull/86882
- The direct use case (in [pr 66825](https://github.com/llvm/llvm-project/pull/66825)) is to add [llvm::IntervalMap](https://github.com/llvm/llvm-project/blob/4c2f68840e984b0f111779c46845ac00e3a7547d/llvm/include/llvm/ADT/IntervalMap.h#L936) and the allocator [required by IntervalMap ctor](https://github.com/llvm/llvm-project/blob/4c2f68840e984b0f111779c46845ac00e3a7547d/llvm/include/llvm/ADT/IntervalMap.h#L1041) 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 in its surrounding code, so did the change.
>From bc67452fa23392a761dc9c2721f9ea1dba993432 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Wed, 27 Mar 2024 15:08:46 -0700
Subject: [PATCH] [nfc]Make InstrProfSymtab non-copyable and non-movable - The
direct use case (in pr 66825) is to add llvm::IntervalMap and allocator to
InstrProfSymtab as class 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 its 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 in its
surrounding code, so did the change.
---
.../Coverage/CoverageMappingReader.h | 4 +--
llvm/include/llvm/ProfileData/InstrProf.h | 9 +++++++
.../Coverage/CoverageMappingReader.cpp | 27 ++++++++++---------
3 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
index 346ca4ad2eb314..5bb5eb582bbfef 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;
@@ -211,7 +211,7 @@ class BinaryCoverageReader : public CoverageMappingReader {
static Expected<std::unique_ptr<BinaryCoverageReader>>
createCoverageReaderFromBuffer(StringRef Coverage,
FuncRecordsStorage &&FuncRecords,
- InstrProfSymtab &&ProfileNames,
+ std::unique_ptr<InstrProfSymtab> ProfileNames,
uint8_t BytesInAddress,
llvm::endianness Endian,
StringRef CompilationDir = "");
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index 25ec06a7392027..3877a5a546e422 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -471,6 +471,15 @@ class InstrProfSymtab {
public:
InstrProfSymtab() = default;
+ // Not copyable or movable.
+ // Consider std::unique_ptr for move.
+ // InstrProfSymtab has a few containers as class members, so consider
+ // std::shared_ptr for read-only copy.
+ 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..1e3c70e3b6a2e4 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -894,31 +894,32 @@ static Error readCoverageMappingData(
Expected<std::unique_ptr<BinaryCoverageReader>>
BinaryCoverageReader::createCoverageReaderFromBuffer(
StringRef Coverage, FuncRecordsStorage &&FuncRecords,
- InstrProfSymtab &&ProfileNames, uint8_t BytesInAddress,
+ std::unique_ptr<InstrProfSymtab> ProfileNames, uint8_t BytesInAddress,
llvm::endianness Endian, StringRef CompilationDir) {
std::unique_ptr<BinaryCoverageReader> Reader(
new BinaryCoverageReader(std::move(FuncRecords)));
Reader->ProfileNames = std::move(ProfileNames);
+ InstrProfSymtab &ProfileNamesRef = *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))
+ ProfileNamesRef, 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))
+ ProfileNamesRef, 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))
+ ProfileNamesRef, 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))
+ ProfileNamesRef, Coverage, FuncRecordsRef, Reader->MappingRecords,
+ CompilationDir, Reader->Filenames))
return std::move(E);
} else
return make_error<CoverageMapError>(
@@ -963,8 +964,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 +1100,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 +1117,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