[llvm] [memprof] Add MemProf version (PR #86414)
Kazu Hirata via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 28 13:37:15 PDT 2024
https://github.com/kazutakahirata updated https://github.com/llvm/llvm-project/pull/86414
>From f90707fe6dbf1a38899dd44cbd8a588499248a72 Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Thu, 21 Mar 2024 13:41:55 -0700
Subject: [PATCH 1/3] [memprof] Add MemProf version
This patch adds a version field to the MemProf section of the indexed
profile format, calling the new version "version 1". The existing
version is called "version 0".
The writer supports both versions via a command-line option:
llvm-profdata merge --memprof-version=1 ...
The reader supports both versions by automatically detecting the
version from the header.
---
.../llvm/ProfileData/InstrProfWriter.h | 7 +++-
llvm/include/llvm/ProfileData/MemProf.h | 9 +++++
llvm/lib/ProfileData/InstrProfReader.cpp | 33 +++++++++++++++++--
llvm/lib/ProfileData/InstrProfWriter.cpp | 32 +++++++++++-------
llvm/tools/llvm-profdata/llvm-profdata.cpp | 10 +++++-
5 files changed, 76 insertions(+), 15 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/InstrProfWriter.h b/llvm/include/llvm/ProfileData/InstrProfWriter.h
index f70574d1f75632..eede0464b7073c 100644
--- a/llvm/include/llvm/ProfileData/InstrProfWriter.h
+++ b/llvm/include/llvm/ProfileData/InstrProfWriter.h
@@ -75,11 +75,16 @@ class InstrProfWriter {
// deployment of newer versions of llvm-profdata.
bool WritePrevVersion = false;
+ // The version we should use to write MemProf in.
+ memprof::MemProfVersion MemProfVersionRequested;
+
public:
InstrProfWriter(bool Sparse = false,
uint64_t TemporalProfTraceReservoirSize = 0,
uint64_t MaxTemporalProfTraceLength = 0,
- bool WritePrevVersion = false);
+ bool WritePrevVersion = false,
+ memprof::MemProfVersion MemProfVersionRequested =
+ memprof::MemProfVersion0);
~InstrProfWriter();
StringMap<ProfilingData> &getProfileData() { return FunctionData; }
diff --git a/llvm/include/llvm/ProfileData/MemProf.h b/llvm/include/llvm/ProfileData/MemProf.h
index 37c19094bc2a63..81bf0445ef746e 100644
--- a/llvm/include/llvm/ProfileData/MemProf.h
+++ b/llvm/include/llvm/ProfileData/MemProf.h
@@ -15,6 +15,15 @@
namespace llvm {
namespace memprof {
+// The versions of the indexed MemProf format
+enum MemProfVersion {
+ // Version 0: This version didn't have a version field have a version in the
+ // header.
+ MemProfVersion0 = 0,
+ // Version 1: Added a version field to the header.
+ MemProfVersion1 = 1,
+};
+
enum class Meta : uint64_t {
Start = 0,
#define MIBEntryDef(NameTag, Name, Type) NameTag,
diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
index 31b742bca14d6f..224b5f18735257 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -24,6 +24,7 @@
#include "llvm/Support/Endian.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/SwapByteOrder.h"
#include "llvm/Support/VirtualFileSystem.h"
@@ -1230,10 +1231,38 @@ Error IndexedInstrProfReader::readHeader() {
Header->MemProfOffset);
const unsigned char *Ptr = Start + MemProfOffset;
- // The value returned from RecordTableGenerator.Emit.
- const uint64_t RecordTableOffset =
+
+ // Read the first 64-bit word, which may be RecordTableOffset in
+ // memprof::MemProfVersion0 or the MemProf version number in
+ // memprof::MemProfVersion1.
+ const uint64_t FirstWord =
support::endian::readNext<uint64_t, llvm::endianness::little,
unaligned>(Ptr);
+
+ memprof::MemProfVersion Version = memprof::MemProfVersion0;
+ if (static_cast<memprof::MemProfVersion>(FirstWord) ==
+ memprof::MemProfVersion1) {
+ // Everything is good. We can proceed to deserialize the rest.
+ Version = memprof::MemProfVersion1;
+ } 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 should be RecordTableOffset,
+ // which should be at least 24 because of the MemProf header size.
+ Version = memprof::MemProfVersion0;
+ } else {
+ return make_error<InstrProfError>(
+ instrprof_error::unsupported_version,
+ formatv("MemProf version {} not supported; version 0 or 1 required",
+ FirstWord));
+ }
+
+ // The value returned from RecordTableGenerator.Emit.
+ const uint64_t RecordTableOffset =
+ Version == memprof::MemProfVersion0
+ ? FirstWord
+ : support::endian::readNext<uint64_t, llvm::endianness::little,
+ unaligned>(Ptr);
// The offset in the stream right before invoking
// FrameTableGenerator.Emit.
const uint64_t FramePayloadOffset =
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index d9fe88a00bdfc4..7e3da730f54443 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -179,14 +179,15 @@ class InstrProfRecordWriterTrait {
} // end namespace llvm
-InstrProfWriter::InstrProfWriter(bool Sparse,
- uint64_t TemporalProfTraceReservoirSize,
- uint64_t MaxTemporalProfTraceLength,
- bool WritePrevVersion)
+InstrProfWriter::InstrProfWriter(
+ bool Sparse, uint64_t TemporalProfTraceReservoirSize,
+ uint64_t MaxTemporalProfTraceLength, bool WritePrevVersion,
+ memprof::MemProfVersion MemProfVersionRequested)
: Sparse(Sparse), MaxTemporalProfTraceLength(MaxTemporalProfTraceLength),
TemporalProfTraceReservoirSize(TemporalProfTraceReservoirSize),
InfoObj(new InstrProfRecordWriterTrait()),
- WritePrevVersion(WritePrevVersion) {}
+ WritePrevVersion(WritePrevVersion),
+ MemProfVersionRequested(MemProfVersionRequested) {}
InstrProfWriter::~InstrProfWriter() { delete InfoObj; }
@@ -528,7 +529,15 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
// OnDiskChainedHashTable MemProfFrameData
uint64_t MemProfSectionStart = 0;
if (static_cast<bool>(ProfileKind & InstrProfKind::MemProf)) {
+ assert((MemProfVersionRequested == memprof::MemProfVersion0 ||
+ MemProfVersionRequested == memprof::MemProfVersion1) &&
+ "Requested MemProf version not supported");
+
MemProfSectionStart = OS.tell();
+
+ if (MemProfVersionRequested == memprof::MemProfVersion1)
+ OS.write(memprof::MemProfVersion1);
+
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.
@@ -570,12 +579,13 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
uint64_t FrameTableOffset = FrameTableGenerator.Emit(OS.OS, *FrameWriter);
- PatchItem PatchItems[] = {
- {MemProfSectionStart, &RecordTableOffset, 1},
- {MemProfSectionStart + sizeof(uint64_t), &FramePayloadOffset, 1},
- {MemProfSectionStart + 2 * sizeof(uint64_t), &FrameTableOffset, 1},
- };
- OS.patch(PatchItems);
+ uint64_t Header[] = {RecordTableOffset, FramePayloadOffset,
+ FrameTableOffset};
+ uint64_t HeaderUpdatePos = MemProfSectionStart;
+ if (MemProfVersionRequested == memprof::MemProfVersion1)
+ // The updates goes just after the version field.
+ HeaderUpdatePos += sizeof(uint64_t);
+ OS.patch({{HeaderUpdatePos, Header, std::size(Header)}});
}
// BinaryIdSection has two parts:
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 3a7bd061d3d23a..942a48814f14b4 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -300,6 +300,13 @@ cl::opt<bool> DoWritePrevVersion(
cl::desc("Write the previous version of indexed format, to enable "
"some forward compatibility."));
+cl::opt<memprof::MemProfVersion> MemProfVersionRequested(
+ "memprof-version", cl::Hidden, cl::sub(MergeSubcommand),
+ cl::desc("Specify the version of the memprof format to use"),
+ cl::init(memprof::MemProfVersion0),
+ cl::values(clEnumValN(memprof::MemProfVersion0, "0", "version 0"),
+ clEnumValN(memprof::MemProfVersion1, "1", "version 1")));
+
// Options specific to overlap subcommand.
cl::opt<std::string> BaseFilename(cl::Positional, cl::Required,
cl::desc("<base profile file>"),
@@ -588,7 +595,8 @@ struct WriterContext {
WriterContext(bool IsSparse, std::mutex &ErrLock,
SmallSet<instrprof_error, 4> &WriterErrorCodes,
uint64_t ReservoirSize = 0, uint64_t MaxTraceLength = 0)
- : Writer(IsSparse, ReservoirSize, MaxTraceLength, DoWritePrevVersion),
+ : Writer(IsSparse, ReservoirSize, MaxTraceLength, DoWritePrevVersion,
+ MemProfVersionRequested),
ErrLock(ErrLock), WriterErrorCodes(WriterErrorCodes) {}
};
>From 011deb14b5d53ce27c5cbc0da7d904aa07ecfed3 Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Thu, 28 Mar 2024 12:20:06 -0700
Subject: [PATCH 2/3] Address comments.
---
.../llvm/ProfileData/InstrProfWriter.h | 14 +++++------
llvm/include/llvm/ProfileData/MemProf.h | 15 ++++++++----
llvm/lib/ProfileData/InstrProfReader.cpp | 19 ++++++++-------
llvm/lib/ProfileData/InstrProfWriter.cpp | 24 ++++++++++++-------
llvm/tools/llvm-profdata/llvm-profdata.cpp | 8 +++----
5 files changed, 46 insertions(+), 34 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/InstrProfWriter.h b/llvm/include/llvm/ProfileData/InstrProfWriter.h
index eede0464b7073c..4e7c5a9ee13cc9 100644
--- a/llvm/include/llvm/ProfileData/InstrProfWriter.h
+++ b/llvm/include/llvm/ProfileData/InstrProfWriter.h
@@ -75,16 +75,14 @@ class InstrProfWriter {
// deployment of newer versions of llvm-profdata.
bool WritePrevVersion = false;
- // The version we should use to write MemProf in.
- memprof::MemProfVersion MemProfVersionRequested;
+ // The MemProf version we should use to write in.
+ memprof::IndexedVersion MemProfVersionRequested;
public:
- InstrProfWriter(bool Sparse = false,
- uint64_t TemporalProfTraceReservoirSize = 0,
- uint64_t MaxTemporalProfTraceLength = 0,
- bool WritePrevVersion = false,
- memprof::MemProfVersion MemProfVersionRequested =
- memprof::MemProfVersion0);
+ InstrProfWriter(
+ bool Sparse = false, uint64_t TemporalProfTraceReservoirSize = 0,
+ uint64_t MaxTemporalProfTraceLength = 0, bool WritePrevVersion = false,
+ memprof::IndexedVersion MemProfVersionRequested = memprof::Version0);
~InstrProfWriter();
StringMap<ProfilingData> &getProfileData() { return FunctionData; }
diff --git a/llvm/include/llvm/ProfileData/MemProf.h b/llvm/include/llvm/ProfileData/MemProf.h
index 81bf0445ef746e..486af73461762d 100644
--- a/llvm/include/llvm/ProfileData/MemProf.h
+++ b/llvm/include/llvm/ProfileData/MemProf.h
@@ -16,14 +16,19 @@ namespace llvm {
namespace memprof {
// The versions of the indexed MemProf format
-enum MemProfVersion {
- // Version 0: This version didn't have a version field have a version in the
- // header.
- MemProfVersion0 = 0,
+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.
- MemProfVersion1 = 1,
+ Version1 = 1,
};
+constexpr uint64_t MinimumSupportedVersion = Version0;
+constexpr uint64_t MaximumSupportedVersion = Version1;
+
+// Verify that the minimum and maximum satisfy the obvious constraint.
+static_assert(MinimumSupportedVersion <= MaximumSupportedVersion);
+
enum class Meta : uint64_t {
Start = 0,
#define MIBEntryDef(NameTag, Name, Type) NameTag,
diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
index 224b5f18735257..ab86558e9fff8f 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -1239,27 +1239,28 @@ Error IndexedInstrProfReader::readHeader() {
support::endian::readNext<uint64_t, llvm::endianness::little,
unaligned>(Ptr);
- memprof::MemProfVersion Version = memprof::MemProfVersion0;
- if (static_cast<memprof::MemProfVersion>(FirstWord) ==
- memprof::MemProfVersion1) {
+ memprof::IndexedVersion Version = memprof::Version0;
+ if (FirstWord == memprof::Version1) {
// Everything is good. We can proceed to deserialize the rest.
- Version = memprof::MemProfVersion1;
+ Version = memprof::Version1;
} 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 should be RecordTableOffset,
+ // In memprof::MemProfVersion0, FirstWord will be RecordTableOffset,
// which should be at least 24 because of the MemProf header size.
- Version = memprof::MemProfVersion0;
+ Version = memprof::Version0;
} else {
return make_error<InstrProfError>(
instrprof_error::unsupported_version,
- formatv("MemProf version {} not supported; version 0 or 1 required",
- FirstWord));
+ formatv("MemProf version {} not supported; "
+ "requires version between {} and {}, inclusive",
+ FirstWord, memprof::MinimumSupportedVersion,
+ memprof::MaximumSupportedVersion));
}
// The value returned from RecordTableGenerator.Emit.
const uint64_t RecordTableOffset =
- Version == memprof::MemProfVersion0
+ Version == memprof::Version0
? FirstWord
: support::endian::readNext<uint64_t, llvm::endianness::little,
unaligned>(Ptr);
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index 7e3da730f54443..8f067f8d05e2b9 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -22,6 +22,7 @@
#include "llvm/Support/Endian.h"
#include "llvm/Support/EndianStream.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/OnDiskHashTable.h"
#include "llvm/Support/raw_ostream.h"
@@ -182,7 +183,7 @@ class InstrProfRecordWriterTrait {
InstrProfWriter::InstrProfWriter(
bool Sparse, uint64_t TemporalProfTraceReservoirSize,
uint64_t MaxTemporalProfTraceLength, bool WritePrevVersion,
- memprof::MemProfVersion MemProfVersionRequested)
+ memprof::IndexedVersion MemProfVersionRequested)
: Sparse(Sparse), MaxTemporalProfTraceLength(MaxTemporalProfTraceLength),
TemporalProfTraceReservoirSize(TemporalProfTraceReservoirSize),
InfoObj(new InstrProfRecordWriterTrait()),
@@ -517,6 +518,7 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
// Write the MemProf profile data if we have it. This includes a simple schema
// with the format described below followed by the hashtable:
+ // uint64_t Version
// uint64_t RecordTableOffset = RecordTableGenerator.Emit
// uint64_t FramePayloadOffset = Stream offset before emitting the frame table
// uint64_t FrameTableOffset = FrameTableGenerator.Emit
@@ -529,14 +531,20 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
// OnDiskChainedHashTable MemProfFrameData
uint64_t MemProfSectionStart = 0;
if (static_cast<bool>(ProfileKind & InstrProfKind::MemProf)) {
- assert((MemProfVersionRequested == memprof::MemProfVersion0 ||
- MemProfVersionRequested == memprof::MemProfVersion1) &&
- "Requested MemProf version not supported");
+ if (MemProfVersionRequested < memprof::MinimumSupportedVersion ||
+ MemProfVersionRequested > memprof::MaximumSupportedVersion) {
+ return make_error<InstrProfError>(
+ instrprof_error::unsupported_version,
+ formatv("MemProf version {} not supported; "
+ "requires version between {} and {}, inclusive",
+ MemProfVersionRequested, memprof::MinimumSupportedVersion,
+ memprof::MaximumSupportedVersion));
+ }
MemProfSectionStart = OS.tell();
- if (MemProfVersionRequested == memprof::MemProfVersion1)
- OS.write(memprof::MemProfVersion1);
+ if (MemProfVersionRequested >= memprof::Version1)
+ OS.write(MemProfVersionRequested);
OS.write(0ULL); // Reserve space for the memprof record table offset.
OS.write(0ULL); // Reserve space for the memprof frame payload offset.
@@ -582,8 +590,8 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
uint64_t Header[] = {RecordTableOffset, FramePayloadOffset,
FrameTableOffset};
uint64_t HeaderUpdatePos = MemProfSectionStart;
- if (MemProfVersionRequested == memprof::MemProfVersion1)
- // The updates goes just after the version field.
+ if (MemProfVersionRequested >= memprof::Version1)
+ // The updates go just after the version field.
HeaderUpdatePos += sizeof(uint64_t);
OS.patch({{HeaderUpdatePos, Header, std::size(Header)}});
}
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 942a48814f14b4..e8ee3c238194e8 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -300,12 +300,12 @@ cl::opt<bool> DoWritePrevVersion(
cl::desc("Write the previous version of indexed format, to enable "
"some forward compatibility."));
-cl::opt<memprof::MemProfVersion> MemProfVersionRequested(
+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::MemProfVersion0),
- cl::values(clEnumValN(memprof::MemProfVersion0, "0", "version 0"),
- clEnumValN(memprof::MemProfVersion1, "1", "version 1")));
+ cl::init(memprof::Version0),
+ cl::values(clEnumValN(memprof::Version0, "0", "version 0"),
+ clEnumValN(memprof::Version1, "1", "version 1")));
// Options specific to overlap subcommand.
cl::opt<std::string> BaseFilename(cl::Positional, cl::Required,
>From 2e77a05cf36878c9724eda0210fd3744e139e36e Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Thu, 28 Mar 2024 13:36:19 -0700
Subject: [PATCH 3/3] Address the latest comment.
---
llvm/include/llvm/ProfileData/InstrProfWriter.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/include/llvm/ProfileData/InstrProfWriter.h b/llvm/include/llvm/ProfileData/InstrProfWriter.h
index 4e7c5a9ee13cc9..d2156c86787273 100644
--- a/llvm/include/llvm/ProfileData/InstrProfWriter.h
+++ b/llvm/include/llvm/ProfileData/InstrProfWriter.h
@@ -75,7 +75,7 @@ class InstrProfWriter {
// deployment of newer versions of llvm-profdata.
bool WritePrevVersion = false;
- // The MemProf version we should use to write in.
+ // The MemProf version we should write.
memprof::IndexedVersion MemProfVersionRequested;
public:
More information about the llvm-commits
mailing list