[llvm] [nfc][InstrFDO]Encapsulate header writes in a class member function (PR #90142)
Mingming Liu via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 29 18:07:54 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 1/3] [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 4b42392bc1e061..adf47925798f46 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 7c56cde3e6cedd..c6971d2e152ed7 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 2/3] '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 c6971d2e152ed7..c531d37aaa8379 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 3/3] 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 adf47925798f46..463b7e733da864 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 c531d37aaa8379..4c9474dd5a4c93 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);
More information about the llvm-commits
mailing list