Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

The patch makes InstrProfWriter::writeImpl less monolithic by adding
InstrProfWriter::writeBinaryIds to serialize binary IDs. This way,
InstrProfWriter::writeImpl can simply call the new function instead of
handling all the details within writeImpl.

The patch makes InstrProfWriter::writeImpl less monolithic by adding
InstrProfWriter::writeBinaryIds to serialize binary IDs.  This way,
InstrProfWriter::writeImpl can simply call the new function instead of
handling all the details within writeImpl.
@llvmbot llvmbot added the PGO Profile Guided Optimizations label Dec 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 5, 2024

@llvm/pr-subscribers-pgo

Author: Kazu Hirata (kazutakahirata)

Changes

The patch makes InstrProfWriter::writeImpl less monolithic by adding
InstrProfWriter::writeBinaryIds to serialize binary IDs. This way,
InstrProfWriter::writeImpl can simply call the new function instead of
handling all the details within writeImpl.


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

2 Files Affected:

  • (modified) llvm/include/llvm/ProfileData/InstrProfWriter.h (+3)
  • (modified) llvm/lib/ProfileData/InstrProfWriter.cpp (+41-34)
diff --git a/llvm/include/llvm/ProfileData/InstrProfWriter.h b/llvm/include/llvm/ProfileData/InstrProfWriter.h
index fdb51c4ab42182..117fddb5729e40 100644
--- a/llvm/include/llvm/ProfileData/InstrProfWriter.h
+++ b/llvm/include/llvm/ProfileData/InstrProfWriter.h
@@ -237,6 +237,9 @@ class InstrProfWriter {
   uint64_t writeHeader(const IndexedInstrProf::Header &header,
                        const bool WritePrevVersion, ProfOStream &OS);
 
+  // Writes binary IDs.
+  Error writeBinaryIds(ProfOStream &OS);
+
   // Writes compressed vtable names to profiles.
   Error writeVTableNames(ProfOStream &OS);
 };
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index 64625dee7701e4..f112ea2efcaa98 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -810,6 +810,45 @@ uint64_t InstrProfWriter::writeHeader(const IndexedInstrProf::Header &Header,
   return BackPatchStartOffset;
 }
 
+Error InstrProfWriter::writeBinaryIds(ProfOStream &OS) {
+  // BinaryIdSection has two parts:
+  // 1. uint64_t BinaryIdsSectionSize
+  // 2. list of binary ids that consist of:
+  //    a. uint64_t BinaryIdLength
+  //    b. uint8_t  BinaryIdData
+  //    c. uint8_t  Padding (if necessary)
+  // Calculate size of binary section.
+  uint64_t BinaryIdsSectionSize = 0;
+
+  // Remove duplicate binary ids.
+  llvm::sort(BinaryIds);
+  BinaryIds.erase(llvm::unique(BinaryIds), BinaryIds.end());
+
+  for (const auto &BI : BinaryIds) {
+    // Increment by binary id length data type size.
+    BinaryIdsSectionSize += sizeof(uint64_t);
+    // Increment by binary id data length, aligned to 8 bytes.
+    BinaryIdsSectionSize += alignToPowerOf2(BI.size(), sizeof(uint64_t));
+  }
+  // Write binary ids section size.
+  OS.write(BinaryIdsSectionSize);
+
+  for (const auto &BI : BinaryIds) {
+    uint64_t BILen = BI.size();
+    // Write binary id length.
+    OS.write(BILen);
+    // Write binary id data.
+    for (unsigned K = 0; K < BILen; K++)
+      OS.writeByte(BI[K]);
+    // Write padding if necessary.
+    uint64_t PaddingSize = alignToPowerOf2(BILen, sizeof(uint64_t)) - BILen;
+    for (unsigned K = 0; K < PaddingSize; K++)
+      OS.writeByte(0);
+  }
+
+  return Error::success();
+}
+
 Error InstrProfWriter::writeVTableNames(ProfOStream &OS) {
   std::vector<std::string> VTableNameStrs;
   for (StringRef VTableName : VTableNames.keys())
@@ -920,41 +959,9 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
       return E;
   }
 
-  // BinaryIdSection has two parts:
-  // 1. uint64_t BinaryIdsSectionSize
-  // 2. list of binary ids that consist of:
-  //    a. uint64_t BinaryIdLength
-  //    b. uint8_t  BinaryIdData
-  //    c. uint8_t  Padding (if necessary)
   uint64_t BinaryIdSectionStart = OS.tell();
-  // Calculate size of binary section.
-  uint64_t BinaryIdsSectionSize = 0;
-
-  // Remove duplicate binary ids.
-  llvm::sort(BinaryIds);
-  BinaryIds.erase(llvm::unique(BinaryIds), BinaryIds.end());
-
-  for (const auto &BI : BinaryIds) {
-    // Increment by binary id length data type size.
-    BinaryIdsSectionSize += sizeof(uint64_t);
-    // Increment by binary id data length, aligned to 8 bytes.
-    BinaryIdsSectionSize += alignToPowerOf2(BI.size(), sizeof(uint64_t));
-  }
-  // Write binary ids section size.
-  OS.write(BinaryIdsSectionSize);
-
-  for (const auto &BI : BinaryIds) {
-    uint64_t BILen = BI.size();
-    // Write binary id length.
-    OS.write(BILen);
-    // Write binary id data.
-    for (unsigned K = 0; K < BILen; K++)
-      OS.writeByte(BI[K]);
-    // Write padding if necessary.
-    uint64_t PaddingSize = alignToPowerOf2(BILen, sizeof(uint64_t)) - BILen;
-    for (unsigned K = 0; K < PaddingSize; K++)
-      OS.writeByte(0);
-  }
+  if (auto E = writeBinaryIds(OS))
+    return E;
 
   uint64_t VTableNamesSectionStart = OS.tell();
 

@kazutakahirata kazutakahirata merged commit bda0209 into llvm:main Dec 5, 2024
10 checks passed
@kazutakahirata kazutakahirata deleted the memprof_ProfWriter_BinaryIds branch December 5, 2024 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PGO Profile Guided Optimizations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants