[memprof] Remove MemProf format Version 0

[kazutakahirata]

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.

[snehasish]

lgtm

[llvmbot]

@llvm/pr-subscribers-pgo

Author: Kazu Hirata (kazutakahirata)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/116442.diff

8 Files Affected:

• (modified) llvm/include/llvm/ProfileData/InstrProfReader.h (+1-2)
• (modified) llvm/include/llvm/ProfileData/MemProf.h (+1-3)
• (modified) llvm/lib/ProfileData/InstrProfReader.cpp (+5-18)
• (modified) llvm/lib/ProfileData/InstrProfWriter.cpp (-35)
• (modified) llvm/lib/ProfileData/MemProf.cpp (+7-11)
• (modified) llvm/test/tools/llvm-profdata/memprof-merge-versions.test (-3)
• (modified) llvm/tools/llvm-profdata/llvm-profdata.cpp (+1-2)
• (modified) llvm/unittests/ProfileData/MemProfTest.cpp (+3-5)

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);
 }