[llvm] 08ddd2c - [PGO] Add support for writing previous indexed format (#84505)

via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 8 12:27:50 PST 2024


Author: Teresa Johnson
Date: 2024-03-08T12:27:46-08:00
New Revision: 08ddd2ce4048b0adbddc3ab9b770ccf43f81d65d

URL: https://github.com/llvm/llvm-project/commit/08ddd2ce4048b0adbddc3ab9b770ccf43f81d65d
DIFF: https://github.com/llvm/llvm-project/commit/08ddd2ce4048b0adbddc3ab9b770ccf43f81d65d.diff

LOG: [PGO] Add support for writing previous indexed format (#84505)

Enable temporary support to ease use of new llvm-profdata with slightly
older indexed profiles after 16e74fd48988ac95551d0f64e1b36f78a82a89a2,
which bumped the indexed format for type profiling.

Added: 
    llvm/test/tools/llvm-profdata/profile-version.test

Modified: 
    llvm/include/llvm/ProfileData/InstrProfWriter.h
    llvm/lib/ProfileData/InstrProfWriter.cpp
    llvm/tools/llvm-profdata/llvm-profdata.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ProfileData/InstrProfWriter.h b/llvm/include/llvm/ProfileData/InstrProfWriter.h
index 7a806fd7fcf345..f70574d1f75632 100644
--- a/llvm/include/llvm/ProfileData/InstrProfWriter.h
+++ b/llvm/include/llvm/ProfileData/InstrProfWriter.h
@@ -68,10 +68,18 @@ class InstrProfWriter {
   // Use raw pointer here for the incomplete type object.
   InstrProfRecordWriterTrait *InfoObj;
 
+  // Temporary support for writing the previous version of the format, to enable
+  // some forward compatibility. Currently this suppresses the writing of the
+  // new vtable names section and header fields.
+  // TODO: Consider enabling this with future version changes as well, to ease
+  // deployment of newer versions of llvm-profdata.
+  bool WritePrevVersion = false;
+
 public:
   InstrProfWriter(bool Sparse = false,
                   uint64_t TemporalProfTraceReservoirSize = 0,
-                  uint64_t MaxTemporalProfTraceLength = 0);
+                  uint64_t MaxTemporalProfTraceLength = 0,
+                  bool WritePrevVersion = false);
   ~InstrProfWriter();
 
   StringMap<ProfilingData> &getProfileData() { return FunctionData; }

diff  --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index 3e0a0e0d70116d..e757c735719b1f 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -181,10 +181,12 @@ class InstrProfRecordWriterTrait {
 
 InstrProfWriter::InstrProfWriter(bool Sparse,
                                  uint64_t TemporalProfTraceReservoirSize,
-                                 uint64_t MaxTemporalProfTraceLength)
+                                 uint64_t MaxTemporalProfTraceLength,
+                                 bool WritePrevVersion)
     : Sparse(Sparse), MaxTemporalProfTraceLength(MaxTemporalProfTraceLength),
       TemporalProfTraceReservoirSize(TemporalProfTraceReservoirSize),
-      InfoObj(new InstrProfRecordWriterTrait()) {}
+      InfoObj(new InstrProfRecordWriterTrait()),
+      WritePrevVersion(WritePrevVersion) {}
 
 InstrProfWriter::~InstrProfWriter() { delete InfoObj; }
 
@@ -432,7 +434,13 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
   // Write the header.
   IndexedInstrProf::Header Header;
   Header.Magic = IndexedInstrProf::Magic;
-  Header.Version = IndexedInstrProf::ProfVersion::CurrentVersion;
+  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))
@@ -484,7 +492,8 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
   OS.write(0);
 
   uint64_t VTableNamesOffset = OS.tell();
-  OS.write(0);
+  if (!WritePrevVersion)
+    OS.write(0);
 
   // Reserve space to write profile summary data.
   uint32_t NumEntries = ProfileSummaryBuilder::DefaultCutoffs.size();
@@ -608,27 +617,29 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
 
   uint64_t VTableNamesSectionStart = OS.tell();
 
-  // Use a dummy (and uncompressed) string as compressed vtable names and get
-  // the necessary profile format change in place for version 12.
-  // TODO: Store the list of vtable names in InstrProfWriter and use the
-  // real compressed name.
-  std::string CompressedVTableNames = "VTableNames";
+  if (!WritePrevVersion) {
+    // Use a dummy (and uncompressed) string as compressed vtable names and get
+    // the necessary profile format change in place for version 12.
+    // TODO: Store the list of vtable names in InstrProfWriter and use the
+    // real compressed name.
+    std::string CompressedVTableNames = "VTableNames";
 
-  uint64_t CompressedStringLen = CompressedVTableNames.length();
+    uint64_t CompressedStringLen = CompressedVTableNames.length();
 
-  // Record the length of compressed string.
-  OS.write(CompressedStringLen);
+    // Record the length of compressed string.
+    OS.write(CompressedStringLen);
 
-  // Write the chars in compressed strings.
-  for (auto &c : CompressedVTableNames)
-    OS.writeByte(static_cast<uint8_t>(c));
+    // Write the chars in compressed strings.
+    for (auto &c : CompressedVTableNames)
+      OS.writeByte(static_cast<uint8_t>(c));
 
-  // Pad up to a multiple of 8.
-  // InstrProfReader would read bytes according to 'CompressedStringLen'.
-  uint64_t PaddedLength = alignTo(CompressedStringLen, 8);
+    // Pad up to a multiple of 8.
+    // InstrProfReader would read bytes according to 'CompressedStringLen'.
+    uint64_t PaddedLength = alignTo(CompressedStringLen, 8);
 
-  for (uint64_t K = CompressedStringLen; K < PaddedLength; K++) {
-    OS.writeByte(0);
+    for (uint64_t K = CompressedStringLen; K < PaddedLength; K++) {
+      OS.writeByte(0);
+    }
   }
 
   uint64_t TemporalProfTracesSectionStart = 0;
@@ -662,26 +673,48 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
   }
   InfoObj->CSSummaryBuilder = nullptr;
 
-  // Now do the final patch:
-  PatchItem PatchItems[] = {
-      // Patch the Header.HashOffset field.
-      {HashTableStartFieldOffset, &HashTableStart, 1},
-      // Patch the Header.MemProfOffset (=0 for profiles without MemProf
-      // data).
-      {MemProfSectionOffset, &MemProfSectionStart, 1},
-      // Patch the Header.BinaryIdSectionOffset.
-      {BinaryIdSectionOffset, &BinaryIdSectionStart, 1},
-      // Patch the Header.TemporalProfTracesOffset (=0 for profiles without
-      // traces).
-      {TemporalProfTracesOffset, &TemporalProfTracesSectionStart, 1},
-      {VTableNamesOffset, &VTableNamesSectionStart, 1},
-      // Patch the summary data.
-      {SummaryOffset, reinterpret_cast<uint64_t *>(TheSummary.get()),
-       (int)(SummarySize / sizeof(uint64_t))},
-      {CSSummaryOffset, reinterpret_cast<uint64_t *>(TheCSSummary.get()),
-       (int)CSSummarySize}};
-
-  OS.patch(PatchItems, std::size(PatchItems));
+  if (!WritePrevVersion) {
+    // Now do the final patch:
+    PatchItem PatchItems[] = {
+        // Patch the Header.HashOffset field.
+        {HashTableStartFieldOffset, &HashTableStart, 1},
+        // Patch the Header.MemProfOffset (=0 for profiles without MemProf
+        // data).
+        {MemProfSectionOffset, &MemProfSectionStart, 1},
+        // Patch the Header.BinaryIdSectionOffset.
+        {BinaryIdSectionOffset, &BinaryIdSectionStart, 1},
+        // Patch the Header.TemporalProfTracesOffset (=0 for profiles without
+        // traces).
+        {TemporalProfTracesOffset, &TemporalProfTracesSectionStart, 1},
+        {VTableNamesOffset, &VTableNamesSectionStart, 1},
+        // Patch the summary data.
+        {SummaryOffset, reinterpret_cast<uint64_t *>(TheSummary.get()),
+         (int)(SummarySize / sizeof(uint64_t))},
+        {CSSummaryOffset, reinterpret_cast<uint64_t *>(TheCSSummary.get()),
+         (int)CSSummarySize}};
+
+    OS.patch(PatchItems, std::size(PatchItems));
+  } else {
+    // Now do the final patch:
+    PatchItem PatchItems[] = {
+        // Patch the Header.HashOffset field.
+        {HashTableStartFieldOffset, &HashTableStart, 1},
+        // Patch the Header.MemProfOffset (=0 for profiles without MemProf
+        // data).
+        {MemProfSectionOffset, &MemProfSectionStart, 1},
+        // Patch the Header.BinaryIdSectionOffset.
+        {BinaryIdSectionOffset, &BinaryIdSectionStart, 1},
+        // Patch the Header.TemporalProfTracesOffset (=0 for profiles without
+        // traces).
+        {TemporalProfTracesOffset, &TemporalProfTracesSectionStart, 1},
+        // Patch the summary data.
+        {SummaryOffset, reinterpret_cast<uint64_t *>(TheSummary.get()),
+         (int)(SummarySize / sizeof(uint64_t))},
+        {CSSummaryOffset, reinterpret_cast<uint64_t *>(TheCSSummary.get()),
+         (int)CSSummarySize}};
+
+    OS.patch(PatchItems, std::size(PatchItems));
+  }
 
   for (const auto &I : FunctionData)
     for (const auto &F : I.getValue())

diff  --git a/llvm/test/tools/llvm-profdata/profile-version.test b/llvm/test/tools/llvm-profdata/profile-version.test
new file mode 100644
index 00000000000000..cb68a648d5e5aa
--- /dev/null
+++ b/llvm/test/tools/llvm-profdata/profile-version.test
@@ -0,0 +1,9 @@
+Test the profile version.
+
+RUN: llvm-profdata merge -o %t.profdata %p/Inputs/basic.proftext
+RUN: llvm-profdata show --profile-version %t.profdata | FileCheck %s
+CHECK: Profile version: 12
+
+RUN: llvm-profdata merge -o %t.prev.profdata %p/Inputs/basic.proftext --write-prev-version
+RUN: llvm-profdata show --profile-version %t.prev.profdata | FileCheck %s --check-prefix=PREV
+PREV: Profile version: 11

diff  --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 8400b0769944cf..3a7bd061d3d23a 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -291,6 +291,15 @@ cl::opt<bool> DropProfileSymbolList(
     cl::desc("Drop the profile symbol list when merging AutoFDO profiles "
              "(only meaningful for -sample)"));
 
+// Temporary support for writing the previous version of the format, to enable
+// some forward compatibility.
+// TODO: Consider enabling this with future version changes as well, to ease
+// deployment of newer versions of llvm-profdata.
+cl::opt<bool> DoWritePrevVersion(
+    "write-prev-version", cl::init(false), cl::Hidden,
+    cl::desc("Write the previous version of indexed format, to enable "
+             "some forward compatibility."));
+
 // Options specific to overlap subcommand.
 cl::opt<std::string> BaseFilename(cl::Positional, cl::Required,
                                   cl::desc("<base profile file>"),
@@ -579,8 +588,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), ErrLock(ErrLock),
-        WriterErrorCodes(WriterErrorCodes) {}
+      : Writer(IsSparse, ReservoirSize, MaxTraceLength, DoWritePrevVersion),
+        ErrLock(ErrLock), WriterErrorCodes(WriterErrorCodes) {}
 };
 
 /// Computer the overlap b/w profile BaseFilename and TestFileName,


        


More information about the llvm-commits mailing list