[llvm] [nfc][InstrFDO]Encapsulate header writes in a class member function (PR #90142)
Mingming Liu via llvm-commits
llvm-commits at lists.llvm.org
Fri May 17 16:32:37 PDT 2024
https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/90142
>From 90109dbe22358645b9e67469f366c3462d15578d Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Thu, 25 Apr 2024 15:45:31 -0700
Subject: [PATCH 01/10] [nfc][InstrFDO]Encapsulate header writes in a class
member function
---
.../llvm/ProfileData/InstrProfWriter.h | 13 +++
llvm/lib/ProfileData/InstrProfWriter.cpp | 79 +++++++++++--------
2 files changed, 58 insertions(+), 34 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/InstrProfWriter.h b/llvm/include/llvm/ProfileData/InstrProfWriter.h
index 4b42392bc1e06..adf47925798f4 100644
--- a/llvm/include/llvm/ProfileData/InstrProfWriter.h
+++ b/llvm/include/llvm/ProfileData/InstrProfWriter.h
@@ -197,6 +197,13 @@ class InstrProfWriter {
const OverlapFuncFilters &FuncFilter);
private:
+ struct HeaderFieldOffsets {
+ uint64_t HashTableStartFieldOffset;
+ uint64_t MemProfSectionOffset;
+ uint64_t BinaryIdSectionOffset;
+ uint64_t TemporalProfTracesOffset;
+ uint64_t VTableNamesOffset;
+ };
void addRecord(StringRef Name, uint64_t Hash, InstrProfRecord &&I,
uint64_t Weight, function_ref<void(Error)> Warn);
bool shouldEncodeData(const ProfilingData &PD);
@@ -204,6 +211,12 @@ class InstrProfWriter {
void addTemporalProfileTrace(TemporalProfTraceTy Trace);
Error writeImpl(ProfOStream &OS);
+
+ // Writes known header fields and preserves space for fields whose value are
+ // known only after payloads are written.
+ // Returns the number of bytes written and outputs the byte offset for back
+ // patching.
+ uint64_t writeHeader(ProfOStream &OS, HeaderFieldOffsets &Offsets);
};
} // end namespace llvm
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index 7c56cde3e6ced..c6971d2e152ed 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -552,26 +552,10 @@ static Error writeMemProf(
memprof::MaximumSupportedVersion));
}
-Error InstrProfWriter::writeImpl(ProfOStream &OS) {
- using namespace IndexedInstrProf;
- using namespace support;
-
- OnDiskChainedHashTableGenerator<InstrProfRecordWriterTrait> Generator;
-
- InstrProfSummaryBuilder ISB(ProfileSummaryBuilder::DefaultCutoffs);
- InfoObj->SummaryBuilder = &ISB;
- InstrProfSummaryBuilder CSISB(ProfileSummaryBuilder::DefaultCutoffs);
- InfoObj->CSSummaryBuilder = &CSISB;
-
- // Populate the hash table generator.
- SmallVector<std::pair<StringRef, const ProfilingData *>, 0> OrderedData;
- for (const auto &I : FunctionData)
- if (shouldEncodeData(I.getValue()))
- OrderedData.emplace_back((I.getKey()), &I.getValue());
- llvm::sort(OrderedData, less_first());
- for (const auto &I : OrderedData)
- Generator.insert(I.first, I.second);
-
+uint64_t InstrProfWriter::writeHeader(ProfOStream &OS,
+ HeaderFieldOffsets &Offsets) {
+ // Records the offset before writing any fields.
+ const uint64_t StartOffset = OS.tell();
// Write the header.
IndexedInstrProf::Header Header;
Header.Magic = IndexedInstrProf::Magic;
@@ -612,30 +596,57 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
OS.write(reinterpret_cast<uint64_t *>(&Header)[I]);
// Save the location of Header.HashOffset field in \c OS.
- uint64_t HashTableStartFieldOffset = OS.tell();
+ Offsets.HashTableStartFieldOffset = OS.tell();
// Reserve the space for HashOffset field.
OS.write(0);
// Save the location of MemProf profile data. This is stored in two parts as
// the schema and as a separate on-disk chained hashtable.
- uint64_t MemProfSectionOffset = OS.tell();
+ Offsets.MemProfSectionOffset = OS.tell();
// Reserve space for the MemProf table field to be patched later if this
// profile contains memory profile information.
OS.write(0);
// Save the location of binary ids section.
- uint64_t BinaryIdSectionOffset = OS.tell();
+ Offsets.BinaryIdSectionOffset = OS.tell();
// Reserve space for the BinaryIdOffset field to be patched later if this
// profile contains binary ids.
OS.write(0);
- uint64_t TemporalProfTracesOffset = OS.tell();
+ Offsets.TemporalProfTracesOffset = OS.tell();
OS.write(0);
- uint64_t VTableNamesOffset = OS.tell();
+ Offsets.VTableNamesOffset = OS.tell();
if (!WritePrevVersion)
OS.write(0);
+ return OS.tell() - StartOffset;
+}
+
+Error InstrProfWriter::writeImpl(ProfOStream &OS) {
+ using namespace IndexedInstrProf;
+ using namespace support;
+
+ OnDiskChainedHashTableGenerator<InstrProfRecordWriterTrait> Generator;
+
+ InstrProfSummaryBuilder ISB(ProfileSummaryBuilder::DefaultCutoffs);
+ InfoObj->SummaryBuilder = &ISB;
+ InstrProfSummaryBuilder CSISB(ProfileSummaryBuilder::DefaultCutoffs);
+ InfoObj->CSSummaryBuilder = &CSISB;
+
+ // Populate the hash table generator.
+ SmallVector<std::pair<StringRef, const ProfilingData *>, 0> OrderedData;
+ for (const auto &I : FunctionData)
+ if (shouldEncodeData(I.getValue()))
+ OrderedData.emplace_back((I.getKey()), &I.getValue());
+ llvm::sort(OrderedData, less_first());
+ for (const auto &I : OrderedData)
+ Generator.insert(I.first, I.second);
+
+ HeaderFieldOffsets Offsets;
+
+ this->writeHeader(OS, Offsets);
+
// Reserve space to write profile summary data.
uint32_t NumEntries = ProfileSummaryBuilder::DefaultCutoffs.size();
uint32_t SummarySize = Summary::getSize(Summary::NumKinds, NumEntries);
@@ -767,16 +778,16 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
// Now do the final patch:
PatchItem PatchItems[] = {
// Patch the Header.HashOffset field.
- {HashTableStartFieldOffset, &HashTableStart, 1},
+ {Offsets.HashTableStartFieldOffset, &HashTableStart, 1},
// Patch the Header.MemProfOffset (=0 for profiles without MemProf
// data).
- {MemProfSectionOffset, &MemProfSectionStart, 1},
+ {Offsets.MemProfSectionOffset, &MemProfSectionStart, 1},
// Patch the Header.BinaryIdSectionOffset.
- {BinaryIdSectionOffset, &BinaryIdSectionStart, 1},
+ {Offsets.BinaryIdSectionOffset, &BinaryIdSectionStart, 1},
// Patch the Header.TemporalProfTracesOffset (=0 for profiles without
// traces).
- {TemporalProfTracesOffset, &TemporalProfTracesSectionStart, 1},
- {VTableNamesOffset, &VTableNamesSectionStart, 1},
+ {Offsets.TemporalProfTracesOffset, &TemporalProfTracesSectionStart, 1},
+ {Offsets.VTableNamesOffset, &VTableNamesSectionStart, 1},
// Patch the summary data.
{SummaryOffset, reinterpret_cast<uint64_t *>(TheSummary.get()),
(int)(SummarySize / sizeof(uint64_t))},
@@ -788,15 +799,15 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
// Now do the final patch:
PatchItem PatchItems[] = {
// Patch the Header.HashOffset field.
- {HashTableStartFieldOffset, &HashTableStart, 1},
+ {Offsets.HashTableStartFieldOffset, &HashTableStart, 1},
// Patch the Header.MemProfOffset (=0 for profiles without MemProf
// data).
- {MemProfSectionOffset, &MemProfSectionStart, 1},
+ {Offsets.MemProfSectionOffset, &MemProfSectionStart, 1},
// Patch the Header.BinaryIdSectionOffset.
- {BinaryIdSectionOffset, &BinaryIdSectionStart, 1},
+ {Offsets.BinaryIdSectionOffset, &BinaryIdSectionStart, 1},
// Patch the Header.TemporalProfTracesOffset (=0 for profiles without
// traces).
- {TemporalProfTracesOffset, &TemporalProfTracesSectionStart, 1},
+ {Offsets.TemporalProfTracesOffset, &TemporalProfTracesSectionStart, 1},
// Patch the summary data.
{SummaryOffset, reinterpret_cast<uint64_t *>(TheSummary.get()),
(int)(SummarySize / sizeof(uint64_t))},
>From 3c108ade5bbb7dfa236e2c63989992fcf40c654b Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Thu, 25 Apr 2024 15:47:32 -0700
Subject: [PATCH 02/10] 'void' unused return value. The value will be used in a
follow-up patch
---
llvm/lib/ProfileData/InstrProfWriter.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index c6971d2e152ed..c531d37aaa837 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -645,7 +645,7 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
HeaderFieldOffsets Offsets;
- this->writeHeader(OS, Offsets);
+ (void)this->writeHeader(OS, Offsets);
// Reserve space to write profile summary data.
uint32_t NumEntries = ProfileSummaryBuilder::DefaultCutoffs.size();
>From db86e882dfc6aaf14477cb8bec7991ab82ce8c46 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Mon, 29 Apr 2024 18:07:33 -0700
Subject: [PATCH 03/10] resolve review feedback
---
llvm/include/llvm/ProfileData/InstrProfWriter.h | 8 ++++++++
llvm/lib/ProfileData/InstrProfWriter.cpp | 11 ++++++-----
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/InstrProfWriter.h b/llvm/include/llvm/ProfileData/InstrProfWriter.h
index adf47925798f4..463b7e733da86 100644
--- a/llvm/include/llvm/ProfileData/InstrProfWriter.h
+++ b/llvm/include/llvm/ProfileData/InstrProfWriter.h
@@ -197,6 +197,14 @@ class InstrProfWriter {
const OverlapFuncFilters &FuncFilter);
private:
+ // A profile (with header and payloads) is written out in one pass; profile
+ // header records the byte offsets of individual payload sections.
+ // When the byte size of a payload section is not known before being written
+ // into the output stream, profile writer reserves space for the byte offset
+ // of this section to allow back patching at the specified offset later.
+ //
+ // This struct is produced by `InstrProfWriter::writeHeader` to store the byte
+ // offset of header fields, and used later for back patching.
struct HeaderFieldOffsets {
uint64_t HashTableStartFieldOffset;
uint64_t MemProfSectionOffset;
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index c531d37aaa837..4c9474dd5a4c9 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -624,6 +624,11 @@ uint64_t InstrProfWriter::writeHeader(ProfOStream &OS,
}
Error InstrProfWriter::writeImpl(ProfOStream &OS) {
+ HeaderFieldOffsets Offsets;
+
+ // FIXME: The return value will be used in a future patch.
+ (void)this->writeHeader(OS, Offsets);
+
using namespace IndexedInstrProf;
using namespace support;
@@ -635,7 +640,7 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
InfoObj->CSSummaryBuilder = &CSISB;
// Populate the hash table generator.
- SmallVector<std::pair<StringRef, const ProfilingData *>, 0> OrderedData;
+ SmallVector<std::pair<StringRef, const ProfilingData *>> OrderedData;
for (const auto &I : FunctionData)
if (shouldEncodeData(I.getValue()))
OrderedData.emplace_back((I.getKey()), &I.getValue());
@@ -643,10 +648,6 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
for (const auto &I : OrderedData)
Generator.insert(I.first, I.second);
- HeaderFieldOffsets Offsets;
-
- (void)this->writeHeader(OS, Offsets);
-
// Reserve space to write profile summary data.
uint32_t NumEntries = ProfileSummaryBuilder::DefaultCutoffs.size();
uint32_t SummarySize = Summary::getSize(Summary::NumKinds, NumEntries);
>From 8b6256dc6596f3368686a5c2871dbdac3eb4f60f Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Thu, 16 May 2024 14:11:19 -0700
Subject: [PATCH 04/10] resolve review feedback
---
.../llvm/ProfileData/InstrProfWriter.h | 8 +-
llvm/lib/ProfileData/InstrProfWriter.cpp | 123 +++++++++---------
2 files changed, 69 insertions(+), 62 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/InstrProfWriter.h b/llvm/include/llvm/ProfileData/InstrProfWriter.h
index 463b7e733da86..adf880e0b8fe3 100644
--- a/llvm/include/llvm/ProfileData/InstrProfWriter.h
+++ b/llvm/include/llvm/ProfileData/InstrProfWriter.h
@@ -222,9 +222,11 @@ class InstrProfWriter {
// Writes known header fields and preserves space for fields whose value are
// known only after payloads are written.
- // Returns the number of bytes written and outputs the byte offset for back
- // patching.
- uint64_t writeHeader(ProfOStream &OS, HeaderFieldOffsets &Offsets);
+ // Returns the number of bytes written and records the start offset for back
+ // patching in `BackPatchStartOffset`.
+ size_t writeHeader(IndexedInstrProf::Header &header,
+ const bool WritePrevVersion, ProfOStream &OS,
+ size_t &BackPatchStartOffset);
};
} // end namespace llvm
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index 4c9474dd5a4c9..3deaa26be80aa 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -552,83 +552,42 @@ static Error writeMemProf(
memprof::MaximumSupportedVersion));
}
-uint64_t InstrProfWriter::writeHeader(ProfOStream &OS,
- HeaderFieldOffsets &Offsets) {
+size_t InstrProfWriter::writeHeader(IndexedInstrProf::Header &Header,
+ const bool WritePrevVersion,
+ ProfOStream &OS,
+ size_t &BackPatchStartOffset) {
// Records the offset before writing any fields.
const uint64_t StartOffset = OS.tell();
- // Write the header.
- IndexedInstrProf::Header Header;
- Header.Magic = IndexedInstrProf::Magic;
- Header.Version = WritePrevVersion
- ? IndexedInstrProf::ProfVersion::Version11
- : IndexedInstrProf::ProfVersion::CurrentVersion;
- // The WritePrevVersion handling will either need to be removed or updated
- // if the version is advanced beyond 12.
- assert(IndexedInstrProf::ProfVersion::CurrentVersion ==
- IndexedInstrProf::ProfVersion::Version12);
- if (static_cast<bool>(ProfileKind & InstrProfKind::IRInstrumentation))
- Header.Version |= VARIANT_MASK_IR_PROF;
- if (static_cast<bool>(ProfileKind & InstrProfKind::ContextSensitive))
- Header.Version |= VARIANT_MASK_CSIR_PROF;
- if (static_cast<bool>(ProfileKind &
- InstrProfKind::FunctionEntryInstrumentation))
- Header.Version |= VARIANT_MASK_INSTR_ENTRY;
- if (static_cast<bool>(ProfileKind & InstrProfKind::SingleByteCoverage))
- Header.Version |= VARIANT_MASK_BYTE_COVERAGE;
- if (static_cast<bool>(ProfileKind & InstrProfKind::FunctionEntryOnly))
- Header.Version |= VARIANT_MASK_FUNCTION_ENTRY_ONLY;
- if (static_cast<bool>(ProfileKind & InstrProfKind::MemProf))
- Header.Version |= VARIANT_MASK_MEMPROF;
- if (static_cast<bool>(ProfileKind & InstrProfKind::TemporalProfile))
- Header.Version |= VARIANT_MASK_TEMPORAL_PROF;
-
- Header.Unused = 0;
- Header.HashType = static_cast<uint64_t>(IndexedInstrProf::HashType);
- Header.HashOffset = 0;
- Header.MemProfOffset = 0;
- Header.BinaryIdOffset = 0;
- Header.TemporalProfTracesOffset = 0;
- Header.VTableNamesOffset = 0;
-
// Only write out the first four fields. We need to remember the offset of the
// remaining fields to allow back patching later.
for (int I = 0; I < 4; I++)
OS.write(reinterpret_cast<uint64_t *>(&Header)[I]);
- // Save the location of Header.HashOffset field in \c OS.
- Offsets.HashTableStartFieldOffset = OS.tell();
+ BackPatchStartOffset = OS.tell();
+
// Reserve the space for HashOffset field.
OS.write(0);
- // Save the location of MemProf profile data. This is stored in two parts as
- // the schema and as a separate on-disk chained hashtable.
- Offsets.MemProfSectionOffset = OS.tell();
// Reserve space for the MemProf table field to be patched later if this
// profile contains memory profile information.
OS.write(0);
- // Save the location of binary ids section.
- Offsets.BinaryIdSectionOffset = OS.tell();
// Reserve space for the BinaryIdOffset field to be patched later if this
// profile contains binary ids.
OS.write(0);
- Offsets.TemporalProfTracesOffset = OS.tell();
+ // Reserve space for temporal profile traces for back patching later.
OS.write(0);
- Offsets.VTableNamesOffset = OS.tell();
+ // Reserve space for vtable names for back patching later.
if (!WritePrevVersion)
OS.write(0);
+ // Returns the number of bytes written in the header section.
return OS.tell() - StartOffset;
}
Error InstrProfWriter::writeImpl(ProfOStream &OS) {
- HeaderFieldOffsets Offsets;
-
- // FIXME: The return value will be used in a future patch.
- (void)this->writeHeader(OS, Offsets);
-
using namespace IndexedInstrProf;
using namespace support;
@@ -648,6 +607,43 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
for (const auto &I : OrderedData)
Generator.insert(I.first, I.second);
+ // Write the header.
+ IndexedInstrProf::Header Header;
+ Header.Magic = IndexedInstrProf::Magic;
+ Header.Version = WritePrevVersion
+ ? IndexedInstrProf::ProfVersion::Version11
+ : IndexedInstrProf::ProfVersion::CurrentVersion;
+ // The WritePrevVersion handling will either need to be removed or updated
+ // if the version is advanced beyond 12.
+ assert(IndexedInstrProf::ProfVersion::CurrentVersion ==
+ IndexedInstrProf::ProfVersion::Version12);
+ if (static_cast<bool>(ProfileKind & InstrProfKind::IRInstrumentation))
+ Header.Version |= VARIANT_MASK_IR_PROF;
+ if (static_cast<bool>(ProfileKind & InstrProfKind::ContextSensitive))
+ Header.Version |= VARIANT_MASK_CSIR_PROF;
+ if (static_cast<bool>(ProfileKind &
+ InstrProfKind::FunctionEntryInstrumentation))
+ Header.Version |= VARIANT_MASK_INSTR_ENTRY;
+ if (static_cast<bool>(ProfileKind & InstrProfKind::SingleByteCoverage))
+ Header.Version |= VARIANT_MASK_BYTE_COVERAGE;
+ if (static_cast<bool>(ProfileKind & InstrProfKind::FunctionEntryOnly))
+ Header.Version |= VARIANT_MASK_FUNCTION_ENTRY_ONLY;
+ if (static_cast<bool>(ProfileKind & InstrProfKind::MemProf))
+ Header.Version |= VARIANT_MASK_MEMPROF;
+ if (static_cast<bool>(ProfileKind & InstrProfKind::TemporalProfile))
+ Header.Version |= VARIANT_MASK_TEMPORAL_PROF;
+
+ Header.Unused = 0;
+ Header.HashType = static_cast<uint64_t>(IndexedInstrProf::HashType);
+ Header.HashOffset = 0;
+ Header.MemProfOffset = 0;
+ Header.BinaryIdOffset = 0;
+ Header.TemporalProfTracesOffset = 0;
+ Header.VTableNamesOffset = 0;
+
+ size_t BackPatchStartOffset = 0;
+ this->writeHeader(Header, WritePrevVersion, OS, BackPatchStartOffset);
+
// Reserve space to write profile summary data.
uint32_t NumEntries = ProfileSummaryBuilder::DefaultCutoffs.size();
uint32_t SummarySize = Summary::getSize(Summary::NumKinds, NumEntries);
@@ -775,20 +771,29 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
}
InfoObj->CSSummaryBuilder = nullptr;
+ const size_t MemProfOffset =
+ BackPatchStartOffset + sizeof(IndexedInstrProf::Header::HashOffset);
+ const size_t BinaryIdOffset =
+ MemProfOffset + sizeof(IndexedInstrProf::Header::MemProfOffset);
+ const size_t TemporalProfTracesOffset =
+ BinaryIdOffset + sizeof(IndexedInstrProf::Header::BinaryIdOffset);
+ const size_t VTableNamesOffset =
+ TemporalProfTracesOffset +
+ sizeof(IndexedInstrProf::Header::TemporalProfTracesOffset);
if (!WritePrevVersion) {
// Now do the final patch:
PatchItem PatchItems[] = {
// Patch the Header.HashOffset field.
- {Offsets.HashTableStartFieldOffset, &HashTableStart, 1},
+ {BackPatchStartOffset, &HashTableStart, 1},
// Patch the Header.MemProfOffset (=0 for profiles without MemProf
// data).
- {Offsets.MemProfSectionOffset, &MemProfSectionStart, 1},
+ {MemProfOffset, &MemProfSectionStart, 1},
// Patch the Header.BinaryIdSectionOffset.
- {Offsets.BinaryIdSectionOffset, &BinaryIdSectionStart, 1},
+ {BinaryIdOffset, &BinaryIdSectionStart, 1},
// Patch the Header.TemporalProfTracesOffset (=0 for profiles without
// traces).
- {Offsets.TemporalProfTracesOffset, &TemporalProfTracesSectionStart, 1},
- {Offsets.VTableNamesOffset, &VTableNamesSectionStart, 1},
+ {TemporalProfTracesOffset, &TemporalProfTracesSectionStart, 1},
+ {VTableNamesOffset, &VTableNamesSectionStart, 1},
// Patch the summary data.
{SummaryOffset, reinterpret_cast<uint64_t *>(TheSummary.get()),
(int)(SummarySize / sizeof(uint64_t))},
@@ -800,15 +805,15 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
// Now do the final patch:
PatchItem PatchItems[] = {
// Patch the Header.HashOffset field.
- {Offsets.HashTableStartFieldOffset, &HashTableStart, 1},
+ {BackPatchStartOffset, &HashTableStart, 1},
// Patch the Header.MemProfOffset (=0 for profiles without MemProf
// data).
- {Offsets.MemProfSectionOffset, &MemProfSectionStart, 1},
+ {MemProfOffset, &MemProfSectionStart, 1},
// Patch the Header.BinaryIdSectionOffset.
- {Offsets.BinaryIdSectionOffset, &BinaryIdSectionStart, 1},
+ {BinaryIdOffset, &BinaryIdSectionStart, 1},
// Patch the Header.TemporalProfTracesOffset (=0 for profiles without
// traces).
- {Offsets.TemporalProfTracesOffset, &TemporalProfTracesSectionStart, 1},
+ {TemporalProfTracesOffset, &TemporalProfTracesSectionStart, 1},
// Patch the summary data.
{SummaryOffset, reinterpret_cast<uint64_t *>(TheSummary.get()),
(int)(SummarySize / sizeof(uint64_t))},
>From 22d066ac1a8d5f951d550421d145a2d3c62f23d7 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Thu, 16 May 2024 14:13:20 -0700
Subject: [PATCH 05/10] clean up
---
.../llvm/ProfileData/InstrProfWriter.h | 20 ++-----------------
1 file changed, 2 insertions(+), 18 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/InstrProfWriter.h b/llvm/include/llvm/ProfileData/InstrProfWriter.h
index adf880e0b8fe3..78b1d67738f81 100644
--- a/llvm/include/llvm/ProfileData/InstrProfWriter.h
+++ b/llvm/include/llvm/ProfileData/InstrProfWriter.h
@@ -197,21 +197,6 @@ class InstrProfWriter {
const OverlapFuncFilters &FuncFilter);
private:
- // A profile (with header and payloads) is written out in one pass; profile
- // header records the byte offsets of individual payload sections.
- // When the byte size of a payload section is not known before being written
- // into the output stream, profile writer reserves space for the byte offset
- // of this section to allow back patching at the specified offset later.
- //
- // This struct is produced by `InstrProfWriter::writeHeader` to store the byte
- // offset of header fields, and used later for back patching.
- struct HeaderFieldOffsets {
- uint64_t HashTableStartFieldOffset;
- uint64_t MemProfSectionOffset;
- uint64_t BinaryIdSectionOffset;
- uint64_t TemporalProfTracesOffset;
- uint64_t VTableNamesOffset;
- };
void addRecord(StringRef Name, uint64_t Hash, InstrProfRecord &&I,
uint64_t Weight, function_ref<void(Error)> Warn);
bool shouldEncodeData(const ProfilingData &PD);
@@ -221,9 +206,8 @@ class InstrProfWriter {
Error writeImpl(ProfOStream &OS);
// Writes known header fields and preserves space for fields whose value are
- // known only after payloads are written.
- // Returns the number of bytes written and records the start offset for back
- // patching in `BackPatchStartOffset`.
+ // known only after payloads are written. Returns the number of bytes written
+ // and records the start offset for back patching in `BackPatchStartOffset`.
size_t writeHeader(IndexedInstrProf::Header &header,
const bool WritePrevVersion, ProfOStream &OS,
size_t &BackPatchStartOffset);
>From 379a03201ac1f555f6e13e576788f3a49c06c753 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Fri, 17 May 2024 11:50:38 -0700
Subject: [PATCH 06/10] resolve review feedback
---
.../llvm/ProfileData/InstrProfWriter.h | 5 +-
llvm/lib/ProfileData/InstrProfWriter.cpp | 53 +++++++------------
2 files changed, 20 insertions(+), 38 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/InstrProfWriter.h b/llvm/include/llvm/ProfileData/InstrProfWriter.h
index 78b1d67738f81..4db4beae9cdb5 100644
--- a/llvm/include/llvm/ProfileData/InstrProfWriter.h
+++ b/llvm/include/llvm/ProfileData/InstrProfWriter.h
@@ -208,9 +208,8 @@ class InstrProfWriter {
// Writes known header fields and preserves space for fields whose value are
// known only after payloads are written. Returns the number of bytes written
// and records the start offset for back patching in `BackPatchStartOffset`.
- size_t writeHeader(IndexedInstrProf::Header &header,
- const bool WritePrevVersion, ProfOStream &OS,
- size_t &BackPatchStartOffset);
+ uint64_t writeHeader(const IndexedInstrProf::Header &header,
+ const bool WritePrevVersion, ProfOStream &OS);
};
} // end namespace llvm
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index 3deaa26be80aa..6ae3793ec8fdc 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -552,39 +552,27 @@ static Error writeMemProf(
memprof::MaximumSupportedVersion));
}
-size_t InstrProfWriter::writeHeader(IndexedInstrProf::Header &Header,
- const bool WritePrevVersion,
- ProfOStream &OS,
- size_t &BackPatchStartOffset) {
+uint64_t InstrProfWriter::writeHeader(const IndexedInstrProf::Header &Header,
+ const bool WritePrevVersion,
+ ProfOStream &OS) {
// Records the offset before writing any fields.
const uint64_t StartOffset = OS.tell();
// Only write out the first four fields. We need to remember the offset of the
// remaining fields to allow back patching later.
for (int I = 0; I < 4; I++)
- OS.write(reinterpret_cast<uint64_t *>(&Header)[I]);
+ OS.write(reinterpret_cast<const uint64_t *>(&Header)[I]);
- BackPatchStartOffset = OS.tell();
+ auto BackPatchStartOffset = OS.tell();
- // Reserve the space for HashOffset field.
- OS.write(0);
-
- // Reserve space for the MemProf table field to be patched later if this
- // profile contains memory profile information.
- OS.write(0);
-
- // Reserve space for the BinaryIdOffset field to be patched later if this
- // profile contains binary ids.
- OS.write(0);
-
- // Reserve space for temporal profile traces for back patching later.
- OS.write(0);
-
- // Reserve space for vtable names for back patching later.
+ // Reserve the space for back patching later.
+ OS.write(0); // HashOffset
+ OS.write(0); // MemProfOffset
+ OS.write(0); // BinaryIdOffset
+ OS.write(0); // TemporalProfTracesOffset
if (!WritePrevVersion)
- OS.write(0);
+ OS.write(0); // VTableNamesOffset
- // Returns the number of bytes written in the header section.
- return OS.tell() - StartOffset;
+ return BackPatchStartOffset;
}
Error InstrProfWriter::writeImpl(ProfOStream &OS) {
@@ -641,8 +629,8 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
Header.TemporalProfTracesOffset = 0;
Header.VTableNamesOffset = 0;
- size_t BackPatchStartOffset = 0;
- this->writeHeader(Header, WritePrevVersion, OS, BackPatchStartOffset);
+ const uint64_t BackPatchStartOffset =
+ writeHeader(Header, WritePrevVersion, OS);
// Reserve space to write profile summary data.
uint32_t NumEntries = ProfileSummaryBuilder::DefaultCutoffs.size();
@@ -771,15 +759,10 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
}
InfoObj->CSSummaryBuilder = nullptr;
- const size_t MemProfOffset =
- BackPatchStartOffset + sizeof(IndexedInstrProf::Header::HashOffset);
- const size_t BinaryIdOffset =
- MemProfOffset + sizeof(IndexedInstrProf::Header::MemProfOffset);
- const size_t TemporalProfTracesOffset =
- BinaryIdOffset + sizeof(IndexedInstrProf::Header::BinaryIdOffset);
- const size_t VTableNamesOffset =
- TemporalProfTracesOffset +
- sizeof(IndexedInstrProf::Header::TemporalProfTracesOffset);
+ const size_t MemProfOffset = BackPatchStartOffset + sizeof(uint64_t);
+ const size_t BinaryIdOffset = MemProfOffset + sizeof(uint64_t);
+ const size_t TemporalProfTracesOffset = BinaryIdOffset + sizeof(uint64_t);
+ const size_t VTableNamesOffset = TemporalProfTracesOffset + sizeof(uint64_t);
if (!WritePrevVersion) {
// Now do the final patch:
PatchItem PatchItems[] = {
>From 464a7173a55aa9c9999fdf60addb1d7350052554 Mon Sep 17 00:00:00 2001
From: Mingming Liu <minglotus6 at gmail.com>
Date: Fri, 17 May 2024 16:24:22 -0700
Subject: [PATCH 07/10] Update llvm/include/llvm/ProfileData/InstrProfWriter.h
Co-authored-by: Kazu Hirata <kazu at google.com>
---
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 4db4beae9cdb5..f256c966ff829 100644
--- a/llvm/include/llvm/ProfileData/InstrProfWriter.h
+++ b/llvm/include/llvm/ProfileData/InstrProfWriter.h
@@ -205,7 +205,7 @@ class InstrProfWriter {
Error writeImpl(ProfOStream &OS);
- // Writes known header fields and preserves space for fields whose value are
+ // Writes known header fields and reserves space for fields whose value are
// known only after payloads are written. Returns the number of bytes written
// and records the start offset for back patching in `BackPatchStartOffset`.
uint64_t writeHeader(const IndexedInstrProf::Header &header,
>From b15bcc204e6fa9a2e8a1206d01c7ebe52b397c51 Mon Sep 17 00:00:00 2001
From: Mingming Liu <minglotus6 at gmail.com>
Date: Fri, 17 May 2024 16:24:36 -0700
Subject: [PATCH 08/10] Update llvm/lib/ProfileData/InstrProfWriter.cpp
Co-authored-by: Kazu Hirata <kazu at google.com>
---
llvm/lib/ProfileData/InstrProfWriter.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index 6ae3793ec8fdc..629eb25b9a74b 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -555,7 +555,7 @@ static Error writeMemProf(
uint64_t InstrProfWriter::writeHeader(const IndexedInstrProf::Header &Header,
const bool WritePrevVersion,
ProfOStream &OS) {
- // Records the offset before writing any fields.
+ // Record the offset before writing any fields.
const uint64_t StartOffset = OS.tell();
// Only write out the first four fields. We need to remember the offset of the
// remaining fields to allow back patching later.
>From 1faf269dfeaf590c9cb5cb2e3c3b66b29c2b2722 Mon Sep 17 00:00:00 2001
From: Mingming Liu <minglotus6 at gmail.com>
Date: Fri, 17 May 2024 16:24:45 -0700
Subject: [PATCH 09/10] Update llvm/lib/ProfileData/InstrProfWriter.cpp
Co-authored-by: Kazu Hirata <kazu at google.com>
---
llvm/lib/ProfileData/InstrProfWriter.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index 629eb25b9a74b..d015da2fc3614 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -557,11 +557,11 @@ uint64_t InstrProfWriter::writeHeader(const IndexedInstrProf::Header &Header,
ProfOStream &OS) {
// Record the offset before writing any fields.
const uint64_t StartOffset = OS.tell();
- // Only write out the first four fields. We need to remember the offset of the
- // remaining fields to allow back patching later.
+ // Only write out the first four fields.
for (int I = 0; I < 4; I++)
OS.write(reinterpret_cast<const uint64_t *>(&Header)[I]);
+ // Remember the offset of the remaining fields to allow back patching later.
auto BackPatchStartOffset = OS.tell();
// Reserve the space for back patching later.
>From a855e0add85ec30d4f690c16ac6268c9bd9748e2 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Fri, 17 May 2024 16:32:10 -0700
Subject: [PATCH 10/10] update comment
---
llvm/include/llvm/ProfileData/InstrProfWriter.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/InstrProfWriter.h b/llvm/include/llvm/ProfileData/InstrProfWriter.h
index f256c966ff829..729a22fec2430 100644
--- a/llvm/include/llvm/ProfileData/InstrProfWriter.h
+++ b/llvm/include/llvm/ProfileData/InstrProfWriter.h
@@ -206,8 +206,8 @@ class InstrProfWriter {
Error writeImpl(ProfOStream &OS);
// Writes known header fields and reserves space for fields whose value are
- // known only after payloads are written. Returns the number of bytes written
- // and records the start offset for back patching in `BackPatchStartOffset`.
+ // known only after payloads are written. Returns the start byte offset for
+ // back patching.
uint64_t writeHeader(const IndexedInstrProf::Header &header,
const bool WritePrevVersion, ProfOStream &OS);
};
More information about the llvm-commits
mailing list