[llvm] [memprof] Remove MemProf format Version 0 (PR #116442)
Kazu Hirata via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 15 13:38:19 PST 2024
https://github.com/kazutakahirata created https://github.com/llvm/llvm-project/pull/116442
This patch removes MemProf format Version 0 now that version 2 and 3
seem to be working well.
I'm not touching version 1 for now because some tests still rely on
version 1.
Note that Version 0 is identical to Version 1 except that the MemProf
section of the indexed format has a MemProf version field.
>From f548fd7943d609f19ad6c35b561d2f4069fd4171 Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Fri, 20 Sep 2024 19:00:55 -0700
Subject: [PATCH] [memprof] Remove MemProf format Version 0
This patch removes MemProf format Version 0 now that version 2 and 3
seem to be working well.
I'm not touching version 1 for now because some tests still rely on
version 1.
Note that Version 0 is identical to Version 1 except that the MemProf
section of the indexed format has a MemProf version field.
---
.../llvm/ProfileData/InstrProfReader.h | 3 +-
llvm/include/llvm/ProfileData/MemProf.h | 4 +--
llvm/lib/ProfileData/InstrProfReader.cpp | 23 +++---------
llvm/lib/ProfileData/InstrProfWriter.cpp | 35 -------------------
llvm/lib/ProfileData/MemProf.cpp | 18 ++++------
.../llvm-profdata/memprof-merge-versions.test | 3 --
llvm/tools/llvm-profdata/llvm-profdata.cpp | 3 +-
llvm/unittests/ProfileData/MemProfTest.cpp | 8 ++---
8 files changed, 18 insertions(+), 79 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/InstrProfReader.h b/llvm/include/llvm/ProfileData/InstrProfReader.h
index 1930cc3f5c2c30..058b9a1ce02e0b 100644
--- a/llvm/include/llvm/ProfileData/InstrProfReader.h
+++ b/llvm/include/llvm/ProfileData/InstrProfReader.h
@@ -686,8 +686,7 @@ class IndexedMemProfReader {
// The number of elements in the radix tree array.
unsigned RadixTreeSize = 0;
- Error deserializeV012(const unsigned char *Start, const unsigned char *Ptr,
- uint64_t FirstWord);
+ Error deserializeV12(const unsigned char *Start, const unsigned char *Ptr);
Error deserializeV3(const unsigned char *Start, const unsigned char *Ptr);
public:
diff --git a/llvm/include/llvm/ProfileData/MemProf.h b/llvm/include/llvm/ProfileData/MemProf.h
index ae262060718a7c..7fa476c61e44f4 100644
--- a/llvm/include/llvm/ProfileData/MemProf.h
+++ b/llvm/include/llvm/ProfileData/MemProf.h
@@ -24,8 +24,6 @@ struct MemProfRecord;
// The versions of the indexed MemProf format
enum IndexedVersion : uint64_t {
- // Version 0: This version didn't have a version field.
- Version0 = 0,
// Version 1: Added a version field to the header.
Version1 = 1,
// Version 2: Added a call stack table.
@@ -35,7 +33,7 @@ enum IndexedVersion : uint64_t {
Version3 = 3,
};
-constexpr uint64_t MinimumSupportedVersion = Version0;
+constexpr uint64_t MinimumSupportedVersion = Version1;
constexpr uint64_t MaximumSupportedVersion = Version3;
// Verify that the minimum and maximum satisfy the obvious constraint.
diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
index 5a2a3352c4b07d..2c18f5d4d5b186 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -1226,14 +1226,11 @@ IndexedInstrProfReader::readSummary(IndexedInstrProf::ProfVersion Version,
}
}
-Error IndexedMemProfReader::deserializeV012(const unsigned char *Start,
- const unsigned char *Ptr,
- uint64_t FirstWord) {
+Error IndexedMemProfReader::deserializeV12(const unsigned char *Start,
+ const unsigned char *Ptr) {
// The value returned from RecordTableGenerator.Emit.
const uint64_t RecordTableOffset =
- Version == memprof::Version0
- ? FirstWord
- : support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
+ support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
// The offset in the stream right before invoking
// FrameTableGenerator.Emit.
const uint64_t FramePayloadOffset =
@@ -1322,9 +1319,7 @@ Error IndexedMemProfReader::deserialize(const unsigned char *Start,
uint64_t MemProfOffset) {
const unsigned char *Ptr = Start + MemProfOffset;
- // Read the first 64-bit word, which may be RecordTableOffset in
- // memprof::MemProfVersion0 or the MemProf version number in
- // memprof::MemProfVersion1 and above.
+ // Read the MemProf version number.
const uint64_t FirstWord =
support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
@@ -1332,12 +1327,6 @@ Error IndexedMemProfReader::deserialize(const unsigned char *Start,
FirstWord == memprof::Version3) {
// Everything is good. We can proceed to deserialize the rest.
Version = static_cast<memprof::IndexedVersion>(FirstWord);
- } else if (FirstWord >= 24) {
- // This is a heuristic/hack to detect memprof::MemProfVersion0,
- // which does not have a version field in the header.
- // In memprof::MemProfVersion0, FirstWord will be RecordTableOffset,
- // which should be at least 24 because of the MemProf header size.
- Version = memprof::Version0;
} else {
return make_error<InstrProfError>(
instrprof_error::unsupported_version,
@@ -1348,10 +1337,9 @@ Error IndexedMemProfReader::deserialize(const unsigned char *Start,
}
switch (Version) {
- case memprof::Version0:
case memprof::Version1:
case memprof::Version2:
- if (Error E = deserializeV012(Start, Ptr, FirstWord))
+ if (Error E = deserializeV12(Start, Ptr))
return E;
break;
case memprof::Version3:
@@ -1644,7 +1632,6 @@ IndexedMemProfReader::getMemProfRecord(const uint64_t FuncNameHash) const {
const memprof::IndexedMemProfRecord &IndexedRecord = *Iter;
switch (Version) {
- case memprof::Version0:
case memprof::Version1:
assert(MemProfFrameTable && "MemProfFrameTable must be available");
assert(!MemProfCallStackTable &&
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index 456014741d93f7..47f463541d8ef4 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -620,39 +620,6 @@ writeMemProfCallStackArray(
return MemProfCallStackIndexes;
}
-// Write out MemProf Version0 as follows:
-// uint64_t RecordTableOffset = RecordTableGenerator.Emit
-// uint64_t FramePayloadOffset = Offset for the frame payload
-// uint64_t FrameTableOffset = FrameTableGenerator.Emit
-// uint64_t Num schema entries
-// uint64_t Schema entry 0
-// uint64_t Schema entry 1
-// ....
-// uint64_t Schema entry N - 1
-// OnDiskChainedHashTable MemProfRecordData
-// OnDiskChainedHashTable MemProfFrameData
-static Error writeMemProfV0(ProfOStream &OS,
- memprof::IndexedMemProfData &MemProfData) {
- uint64_t HeaderUpdatePos = OS.tell();
- OS.write(0ULL); // Reserve space for the memprof record table offset.
- OS.write(0ULL); // Reserve space for the memprof frame payload offset.
- OS.write(0ULL); // Reserve space for the memprof frame table offset.
-
- auto Schema = memprof::getFullSchema();
- writeMemProfSchema(OS, Schema);
-
- uint64_t RecordTableOffset =
- writeMemProfRecords(OS, MemProfData.Records, &Schema, memprof::Version0);
-
- uint64_t FramePayloadOffset = OS.tell();
- uint64_t FrameTableOffset = writeMemProfFrames(OS, MemProfData.Frames);
-
- uint64_t Header[] = {RecordTableOffset, FramePayloadOffset, FrameTableOffset};
- OS.patch({{HeaderUpdatePos, Header}});
-
- return Error::success();
-}
-
// Write out MemProf Version1 as follows:
// uint64_t Version (NEW in V1)
// uint64_t RecordTableOffset = RecordTableGenerator.Emit
@@ -809,8 +776,6 @@ static Error writeMemProf(ProfOStream &OS,
memprof::IndexedVersion MemProfVersionRequested,
bool MemProfFullSchema) {
switch (MemProfVersionRequested) {
- case memprof::Version0:
- return writeMemProfV0(OS, MemProfData);
case memprof::Version1:
return writeMemProfV1(OS, MemProfData);
case memprof::Version2:
diff --git a/llvm/lib/ProfileData/MemProf.cpp b/llvm/lib/ProfileData/MemProf.cpp
index 6d784053f877d4..6c4bda2d9264ff 100644
--- a/llvm/lib/ProfileData/MemProf.cpp
+++ b/llvm/lib/ProfileData/MemProf.cpp
@@ -58,7 +58,6 @@ static size_t serializedSizeV3(const IndexedAllocationInfo &IAI,
size_t IndexedAllocationInfo::serializedSize(const MemProfSchema &Schema,
IndexedVersion Version) const {
switch (Version) {
- case Version0:
case Version1:
return serializedSizeV0(*this, Schema);
case Version2:
@@ -69,12 +68,12 @@ size_t IndexedAllocationInfo::serializedSize(const MemProfSchema &Schema,
llvm_unreachable("unsupported MemProf version");
}
-static size_t serializedSizeV0(const IndexedMemProfRecord &Record,
+static size_t serializedSizeV1(const IndexedMemProfRecord &Record,
const MemProfSchema &Schema) {
// The number of alloc sites to serialize.
size_t Result = sizeof(uint64_t);
for (const IndexedAllocationInfo &N : Record.AllocSites)
- Result += N.serializedSize(Schema, Version0);
+ Result += N.serializedSize(Schema, Version1);
// The number of callsites we have information for.
Result += sizeof(uint64_t);
@@ -117,9 +116,8 @@ static size_t serializedSizeV3(const IndexedMemProfRecord &Record,
size_t IndexedMemProfRecord::serializedSize(const MemProfSchema &Schema,
IndexedVersion Version) const {
switch (Version) {
- case Version0:
case Version1:
- return serializedSizeV0(*this, Schema);
+ return serializedSizeV1(*this, Schema);
case Version2:
return serializedSizeV2(*this, Schema);
case Version3:
@@ -128,7 +126,7 @@ size_t IndexedMemProfRecord::serializedSize(const MemProfSchema &Schema,
llvm_unreachable("unsupported MemProf version");
}
-static void serializeV0(const IndexedMemProfRecord &Record,
+static void serializeV1(const IndexedMemProfRecord &Record,
const MemProfSchema &Schema, raw_ostream &OS) {
using namespace support;
@@ -197,9 +195,8 @@ void IndexedMemProfRecord::serialize(
llvm::DenseMap<CallStackId, LinearCallStackId> *MemProfCallStackIndexes)
const {
switch (Version) {
- case Version0:
case Version1:
- serializeV0(*this, Schema, OS);
+ serializeV1(*this, Schema, OS);
return;
case Version2:
serializeV2(*this, Schema, OS);
@@ -211,7 +208,7 @@ void IndexedMemProfRecord::serialize(
llvm_unreachable("unsupported MemProf version");
}
-static IndexedMemProfRecord deserializeV0(const MemProfSchema &Schema,
+static IndexedMemProfRecord deserializeV1(const MemProfSchema &Schema,
const unsigned char *Ptr) {
using namespace support;
@@ -327,9 +324,8 @@ IndexedMemProfRecord::deserialize(const MemProfSchema &Schema,
const unsigned char *Ptr,
IndexedVersion Version) {
switch (Version) {
- case Version0:
case Version1:
- return deserializeV0(Schema, Ptr);
+ return deserializeV1(Schema, Ptr);
case Version2:
return deserializeV2(Schema, Ptr);
case Version3:
diff --git a/llvm/test/tools/llvm-profdata/memprof-merge-versions.test b/llvm/test/tools/llvm-profdata/memprof-merge-versions.test
index 144afdd92a1149..722ef43484e7e9 100644
--- a/llvm/test/tools/llvm-profdata/memprof-merge-versions.test
+++ b/llvm/test/tools/llvm-profdata/memprof-merge-versions.test
@@ -7,9 +7,6 @@ RUN: echo "1" >> %t.proftext
RUN: echo "1" >> %t.proftext
To update the inputs used below run Inputs/update_memprof_inputs.sh /path/to/updated/clang
-RUN: llvm-profdata merge %t.proftext %p/Inputs/basic.memprofraw --memprof-version=0 --profiled-binary %p/Inputs/basic.memprofexe -o %t.prof.v0
-RUN: llvm-profdata show %t.prof.v0 | FileCheck %s
-
RUN: llvm-profdata merge %t.proftext %p/Inputs/basic.memprofraw --memprof-version=1 --profiled-binary %p/Inputs/basic.memprofexe -o %t.prof.v1
RUN: llvm-profdata show %t.prof.v1 | FileCheck %s
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index f7023aa966adf6..8a42e430fb54e8 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -333,8 +333,7 @@ cl::opt<memprof::IndexedVersion> MemProfVersionRequested(
"memprof-version", cl::Hidden, cl::sub(MergeSubcommand),
cl::desc("Specify the version of the memprof format to use"),
cl::init(memprof::Version3),
- cl::values(clEnumValN(memprof::Version0, "0", "version 0"),
- clEnumValN(memprof::Version1, "1", "version 1"),
+ cl::values(clEnumValN(memprof::Version1, "1", "version 1"),
clEnumValN(memprof::Version2, "2", "version 2"),
clEnumValN(memprof::Version3, "3", "version 3")));
diff --git a/llvm/unittests/ProfileData/MemProfTest.cpp b/llvm/unittests/ProfileData/MemProfTest.cpp
index b7a9170642f2ba..c90669811e60ae 100644
--- a/llvm/unittests/ProfileData/MemProfTest.cpp
+++ b/llvm/unittests/ProfileData/MemProfTest.cpp
@@ -268,9 +268,7 @@ TEST(MemProf, PortableWrapper) {
EXPECT_EQ(3UL, ReadBlock.getAllocCpuId());
}
-// Version0 and Version1 serialize IndexedMemProfRecord in the same format, so
-// we share one test.
-TEST(MemProf, RecordSerializationRoundTripVersion0And1) {
+TEST(MemProf, RecordSerializationRoundTripVersion1) {
const auto Schema = llvm::memprof::getFullSchema();
MemInfoBlock Info(/*size=*/16, /*access_count=*/7, /*alloc_timestamp=*/1000,
@@ -294,11 +292,11 @@ TEST(MemProf, RecordSerializationRoundTripVersion0And1) {
std::string Buffer;
llvm::raw_string_ostream OS(Buffer);
- Record.serialize(Schema, OS, llvm::memprof::Version0);
+ Record.serialize(Schema, OS, llvm::memprof::Version1);
const IndexedMemProfRecord GotRecord = IndexedMemProfRecord::deserialize(
Schema, reinterpret_cast<const unsigned char *>(Buffer.data()),
- llvm::memprof::Version0);
+ llvm::memprof::Version1);
EXPECT_EQ(Record, GotRecord);
}
More information about the llvm-commits
mailing list