[llvm] 93953d4 - [NFC][SampleFDO] Move some common stuff from SampleProfileReaderExtBinary/WriterExtBinary

Wei Mi via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 22 15:57:17 PDT 2020


Author: Wei Mi
Date: 2020-10-22T15:56:55-07:00
New Revision: 93953d411a0fc240e10237fea34aaff43e8f0ff5

URL: https://github.com/llvm/llvm-project/commit/93953d411a0fc240e10237fea34aaff43e8f0ff5
DIFF: https://github.com/llvm/llvm-project/commit/93953d411a0fc240e10237fea34aaff43e8f0ff5.diff

LOG: [NFC][SampleFDO] Move some common stuff from SampleProfileReaderExtBinary/WriterExtBinary
to their parent classes.

SampleProfileReaderExtBinary/SampleProfileWriterExtBinary specify the typical
section layout currently used by SampleFDO. Currently a lot of section
reader/writer stay in the two classes. However, as we expect to have more
types of SampleFDO profiles, we hope those new types of profiles can share
the common sections while configuring their own sections easily with minimal
change. That is why I move some common stuff from
SampleProfileReaderExtBinary/SampleProfileWriterExtBinary to
SampleProfileReaderExtBinaryBase/SampleProfileWriterExtBinaryBase so new
profiles class inheriting from the base class can reuse them.

Differential Revision: https://reviews.llvm.org/D89524

Added: 
    

Modified: 
    llvm/include/llvm/ProfileData/SampleProfReader.h
    llvm/include/llvm/ProfileData/SampleProfWriter.h
    llvm/lib/ProfileData/SampleProfReader.cpp
    llvm/lib/ProfileData/SampleProfWriter.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ProfileData/SampleProfReader.h b/llvm/include/llvm/ProfileData/SampleProfReader.h
index 385ac820f5b5..ef1550f99fef 100644
--- a/llvm/include/llvm/ProfileData/SampleProfReader.h
+++ b/llvm/include/llvm/ProfileData/SampleProfReader.h
@@ -598,40 +598,23 @@ class SampleProfileReaderExtBinaryBase : public SampleProfileReaderBinary {
 
 protected:
   std::vector<SecHdrTableEntry> SecHdrTable;
-  std::unique_ptr<ProfileSymbolList> ProfSymList;
   std::error_code readSecHdrTableEntry();
   std::error_code readSecHdrTable();
-  virtual std::error_code readHeader() override;
-  virtual std::error_code verifySPMagic(uint64_t Magic) override = 0;
-  virtual std::error_code readOneSection(const uint8_t *Start, uint64_t Size,
-                                         const SecHdrTableEntry &Entry) = 0;
-
-public:
-  SampleProfileReaderExtBinaryBase(std::unique_ptr<MemoryBuffer> B,
-                                   LLVMContext &C, SampleProfileFormat Format)
-      : SampleProfileReaderBinary(std::move(B), C, Format) {}
 
-  /// Read sample profiles in extensible format from the associated file.
-  std::error_code readImpl() override;
-
-  /// Get the total size of all \p Type sections.
-  uint64_t getSectionSize(SecType Type);
-  /// Get the total size of header and all sections.
-  uint64_t getFileSize();
-  virtual bool dumpSectionInfo(raw_ostream &OS = dbgs()) override;
-};
-
-class SampleProfileReaderExtBinary : public SampleProfileReaderExtBinaryBase {
-private:
-  virtual std::error_code verifySPMagic(uint64_t Magic) override;
-  virtual std::error_code
-  readOneSection(const uint8_t *Start, uint64_t Size,
-                 const SecHdrTableEntry &Entry) override;
-  std::error_code readProfileSymbolList();
   std::error_code readFuncOffsetTable();
   std::error_code readFuncProfiles();
   std::error_code readMD5NameTable();
   std::error_code readNameTableSec(bool IsMD5);
+  std::error_code readProfileSymbolList();
+
+  virtual std::error_code readHeader() override;
+  virtual std::error_code verifySPMagic(uint64_t Magic) override = 0;
+  virtual std::error_code readOneSection(const uint8_t *Start, uint64_t Size,
+                                         const SecHdrTableEntry &Entry);
+  // placeholder for subclasses to dispatch their own section readers.
+  virtual std::error_code readCustomSection(const SecHdrTableEntry &Entry) = 0;
+
+  std::unique_ptr<ProfileSymbolList> ProfSymList;
 
   /// The table mapping from function name to the offset of its FunctionSample
   /// towards file start.
@@ -651,16 +634,18 @@ class SampleProfileReaderExtBinary : public SampleProfileReaderExtBinaryBase {
   std::unique_ptr<std::vector<std::string>> MD5StringBuf;
 
 public:
-  SampleProfileReaderExtBinary(std::unique_ptr<MemoryBuffer> B, LLVMContext &C,
-                               SampleProfileFormat Format = SPF_Ext_Binary)
-      : SampleProfileReaderExtBinaryBase(std::move(B), C, Format) {}
+  SampleProfileReaderExtBinaryBase(std::unique_ptr<MemoryBuffer> B,
+                                   LLVMContext &C, SampleProfileFormat Format)
+      : SampleProfileReaderBinary(std::move(B), C, Format) {}
 
-  /// \brief Return true if \p Buffer is in the format supported by this class.
-  static bool hasFormat(const MemoryBuffer &Buffer);
+  /// Read sample profiles in extensible format from the associated file.
+  std::error_code readImpl() override;
 
-  virtual std::unique_ptr<ProfileSymbolList> getProfileSymbolList() override {
-    return std::move(ProfSymList);
-  };
+  /// Get the total size of all \p Type sections.
+  uint64_t getSectionSize(SecType Type);
+  /// Get the total size of header and all sections.
+  uint64_t getFileSize();
+  virtual bool dumpSectionInfo(raw_ostream &OS = dbgs()) override;
 
   /// Collect functions with definitions in Module \p M.
   void collectFuncsFrom(const Module &M) override;
@@ -670,6 +655,27 @@ class SampleProfileReaderExtBinary : public SampleProfileReaderExtBinaryBase {
     assert(!NameTable.empty() && "NameTable should have been initialized");
     return MD5StringBuf && !MD5StringBuf->empty();
   }
+
+  virtual std::unique_ptr<ProfileSymbolList> getProfileSymbolList() override {
+    return std::move(ProfSymList);
+  };
+};
+
+class SampleProfileReaderExtBinary : public SampleProfileReaderExtBinaryBase {
+private:
+  virtual std::error_code verifySPMagic(uint64_t Magic) override;
+  virtual std::error_code
+  readCustomSection(const SecHdrTableEntry &Entry) override {
+    return sampleprof_error::success;
+  };
+
+public:
+  SampleProfileReaderExtBinary(std::unique_ptr<MemoryBuffer> B, LLVMContext &C,
+                               SampleProfileFormat Format = SPF_Ext_Binary)
+      : SampleProfileReaderExtBinaryBase(std::move(B), C, Format) {}
+
+  /// \brief Return true if \p Buffer is in the format supported by this class.
+  static bool hasFormat(const MemoryBuffer &Buffer);
 };
 
 class SampleProfileReaderCompactBinary : public SampleProfileReaderBinary {

diff  --git a/llvm/include/llvm/ProfileData/SampleProfWriter.h b/llvm/include/llvm/ProfileData/SampleProfWriter.h
index 7d0df9e44f58..24269c5fbbf5 100644
--- a/llvm/include/llvm/ProfileData/SampleProfWriter.h
+++ b/llvm/include/llvm/ProfileData/SampleProfWriter.h
@@ -152,6 +152,24 @@ class SampleProfileWriterExtBinaryBase : public SampleProfileWriterBinary {
 
   virtual void setToCompressAllSections() override;
   void setToCompressSection(SecType Type);
+  virtual std::error_code writeSample(const FunctionSamples &S) override;
+
+  // Set to use MD5 to represent string in NameTable.
+  virtual void setUseMD5() override {
+    UseMD5 = true;
+    addSectionFlag(SecNameTable, SecNameTableFlags::SecFlagMD5Name);
+  }
+
+  // Set the profile to be partial. It means the profile is for
+  // common/shared code. The common profile is usually merged from
+  // profiles collected from running other targets.
+  virtual void setPartialProfile() override {
+    addSectionFlag(SecProfSummary, SecProfSummaryFlags::SecFlagPartial);
+  }
+
+  virtual void setProfileSymbolList(ProfileSymbolList *PSL) override {
+    ProfSymList = PSL;
+  };
 
 protected:
   uint64_t markSectionStart(SecType Type);
@@ -164,16 +182,38 @@ class SampleProfileWriterExtBinaryBase : public SampleProfileWriterBinary {
     }
   }
 
+  // placeholder for subclasses to dispatch their own section writers.
+  virtual std::error_code writeCustomSection(SecType Type) = 0;
+
   virtual void initSectionHdrLayout() = 0;
+  // specify the order to write sections.
   virtual std::error_code
   writeSections(const StringMap<FunctionSamples> &ProfileMap) = 0;
 
+  // Dispatch section writer for each section.
+  virtual std::error_code
+  writeOneSection(SecType Type, const StringMap<FunctionSamples> &ProfileMap);
+
+  // Helper function to write name table.
+  virtual std::error_code writeNameTable() override;
+
+  // Functions to write various kinds of sections.
+  std::error_code
+  writeNameTableSection(const StringMap<FunctionSamples> &ProfileMap);
+  std::error_code writeFuncOffsetTable();
+  std::error_code writeProfileSymbolListSection();
+
   // Specifiy the order of sections in section header table. Note
   // the order of sections in the profile may be 
diff erent that the
   // order in SectionHdrLayout. sample Reader will follow the order
   // in SectionHdrLayout to read each section.
   SmallVector<SecHdrTableEntry, 8> SectionHdrLayout;
 
+  // Save the start of SecLBRProfile so we can compute the offset to the
+  // start of SecLBRProfile for each Function's Profile and will keep it
+  // in FuncOffsetTable.
+  uint64_t SecLBRProfileStart = 0;
+
 private:
   void allocSecHdrTable();
   std::error_code writeSecHdrTable();
@@ -198,6 +238,14 @@ class SampleProfileWriterExtBinaryBase : public SampleProfileWriterBinary {
   uint64_t SecHdrTableOffset;
   // Initial Section Flags setting.
   std::vector<SecHdrTableEntry> SecHdrTable;
+
+  // FuncOffsetTable maps function name to its profile offset in SecLBRProfile
+  // section. It is used to load function profile on demand.
+  MapVector<StringRef, uint64_t> FuncOffsetTable;
+  // Whether to use MD5 to represent string.
+  bool UseMD5 = false;
+
+  ProfileSymbolList *ProfSymList = nullptr;
 };
 
 class SampleProfileWriterExtBinary : public SampleProfileWriterExtBinaryBase {
@@ -207,24 +255,6 @@ class SampleProfileWriterExtBinary : public SampleProfileWriterExtBinaryBase {
     initSectionHdrLayout();
   }
 
-  virtual std::error_code writeSample(const FunctionSamples &S) override;
-  virtual void setProfileSymbolList(ProfileSymbolList *PSL) override {
-    ProfSymList = PSL;
-  };
-
-  // Set to use MD5 to represent string in NameTable.
-  virtual void setUseMD5() override {
-    UseMD5 = true;
-    addSectionFlag(SecNameTable, SecNameTableFlags::SecFlagMD5Name);
-  }
-
-  // Set the profile to be partial. It means the profile is for
-  // common/shared code. The common profile is usually merged from
-  // profiles collected from running other targets.
-  virtual void setPartialProfile() override {
-    addSectionFlag(SecProfSummary, SecProfSummaryFlags::SecFlagPartial);
-  }
-
 private:
   virtual void initSectionHdrLayout() override {
     // Note that SecFuncOffsetTable section is written after SecLBRProfile
@@ -246,20 +276,9 @@ class SampleProfileWriterExtBinary : public SampleProfileWriterExtBinaryBase {
   virtual std::error_code
   writeSections(const StringMap<FunctionSamples> &ProfileMap) override;
 
-  std::error_code writeFuncOffsetTable();
-  virtual std::error_code writeNameTable() override;
-
-  ProfileSymbolList *ProfSymList = nullptr;
-
-  // Save the start of SecLBRProfile so we can compute the offset to the
-  // start of SecLBRProfile for each Function's Profile and will keep it
-  // in FuncOffsetTable.
-  uint64_t SecLBRProfileStart = 0;
-  // FuncOffsetTable maps function name to its profile offset in SecLBRProfile
-  // section. It is used to load function profile on demand.
-  MapVector<StringRef, uint64_t> FuncOffsetTable;
-  // Whether to use MD5 to represent string.
-  bool UseMD5 = false;
+  virtual std::error_code writeCustomSection(SecType Type) override {
+    return sampleprof_error::success;
+  };
 };
 
 // CompactBinary is a compact format of binary profile which both reduces

diff  --git a/llvm/lib/ProfileData/SampleProfReader.cpp b/llvm/lib/ProfileData/SampleProfReader.cpp
index 59fae9e236f3..caff9e7d7d25 100644
--- a/llvm/lib/ProfileData/SampleProfReader.cpp
+++ b/llvm/lib/ProfileData/SampleProfReader.cpp
@@ -470,7 +470,7 @@ std::error_code SampleProfileReaderBinary::readImpl() {
   return sampleprof_error::success;
 }
 
-std::error_code SampleProfileReaderExtBinary::readOneSection(
+std::error_code SampleProfileReaderExtBinaryBase::readOneSection(
     const uint8_t *Start, uint64_t Size, const SecHdrTableEntry &Entry) {
   Data = Start;
   End = Start + Size;
@@ -490,28 +490,30 @@ std::error_code SampleProfileReaderExtBinary::readOneSection(
     if (std::error_code EC = readFuncProfiles())
       return EC;
     break;
-  case SecProfileSymbolList:
-    if (std::error_code EC = readProfileSymbolList())
-      return EC;
-    break;
   case SecFuncOffsetTable:
     if (std::error_code EC = readFuncOffsetTable())
       return EC;
     break;
+  case SecProfileSymbolList:
+    if (std::error_code EC = readProfileSymbolList())
+      return EC;
+    break;
   default:
+    if (std::error_code EC = readCustomSection(Entry))
+      return EC;
     break;
   }
   return sampleprof_error::success;
 }
 
-void SampleProfileReaderExtBinary::collectFuncsFrom(const Module &M) {
+void SampleProfileReaderExtBinaryBase::collectFuncsFrom(const Module &M) {
   UseAllFuncs = false;
   FuncsToUse.clear();
   for (auto &F : M)
     FuncsToUse.insert(FunctionSamples::getCanonicalFnName(F));
 }
 
-std::error_code SampleProfileReaderExtBinary::readFuncOffsetTable() {
+std::error_code SampleProfileReaderExtBinaryBase::readFuncOffsetTable() {
   auto Size = readNumber<uint64_t>();
   if (std::error_code EC = Size.getError())
     return EC;
@@ -531,7 +533,7 @@ std::error_code SampleProfileReaderExtBinary::readFuncOffsetTable() {
   return sampleprof_error::success;
 }
 
-std::error_code SampleProfileReaderExtBinary::readFuncProfiles() {
+std::error_code SampleProfileReaderExtBinaryBase::readFuncProfiles() {
   const uint8_t *Start = Data;
   if (UseAllFuncs) {
     while (Data < End) {
@@ -576,7 +578,7 @@ std::error_code SampleProfileReaderExtBinary::readFuncProfiles() {
   return sampleprof_error::success;
 }
 
-std::error_code SampleProfileReaderExtBinary::readProfileSymbolList() {
+std::error_code SampleProfileReaderExtBinaryBase::readProfileSymbolList() {
   if (!ProfSymList)
     ProfSymList = std::make_unique<ProfileSymbolList>();
 
@@ -720,7 +722,7 @@ std::error_code SampleProfileReaderBinary::readNameTable() {
   return sampleprof_error::success;
 }
 
-std::error_code SampleProfileReaderExtBinary::readMD5NameTable() {
+std::error_code SampleProfileReaderExtBinaryBase::readMD5NameTable() {
   auto Size = readNumber<uint64_t>();
   if (std::error_code EC = Size.getError())
     return EC;
@@ -739,7 +741,7 @@ std::error_code SampleProfileReaderExtBinary::readMD5NameTable() {
   return sampleprof_error::success;
 }
 
-std::error_code SampleProfileReaderExtBinary::readNameTableSec(bool IsMD5) {
+std::error_code SampleProfileReaderExtBinaryBase::readNameTableSec(bool IsMD5) {
   if (IsMD5)
     return readMD5NameTable();
   return SampleProfileReaderBinary::readNameTable();

diff  --git a/llvm/lib/ProfileData/SampleProfWriter.cpp b/llvm/lib/ProfileData/SampleProfWriter.cpp
index 48d3faa6cd2f..8233b38c3c21 100644
--- a/llvm/lib/ProfileData/SampleProfWriter.cpp
+++ b/llvm/lib/ProfileData/SampleProfWriter.cpp
@@ -144,7 +144,7 @@ std::error_code SampleProfileWriterExtBinaryBase::write(
 }
 
 std::error_code
-SampleProfileWriterExtBinary::writeSample(const FunctionSamples &S) {
+SampleProfileWriterExtBinaryBase::writeSample(const FunctionSamples &S) {
   uint64_t Offset = OutputStream->tell();
   StringRef Name = S.getName();
   FuncOffsetTable[Name] = Offset - SecLBRProfileStart;
@@ -152,7 +152,7 @@ SampleProfileWriterExtBinary::writeSample(const FunctionSamples &S) {
   return writeBody(S);
 }
 
-std::error_code SampleProfileWriterExtBinary::writeFuncOffsetTable() {
+std::error_code SampleProfileWriterExtBinaryBase::writeFuncOffsetTable() {
   auto &OS = *OutputStream;
 
   // Write out the table size.
@@ -166,7 +166,7 @@ std::error_code SampleProfileWriterExtBinary::writeFuncOffsetTable() {
   return sampleprof_error::success;
 }
 
-std::error_code SampleProfileWriterExtBinary::writeNameTable() {
+std::error_code SampleProfileWriterExtBinaryBase::writeNameTable() {
   if (!UseMD5)
     return SampleProfileWriterBinary::writeNameTable();
 
@@ -182,48 +182,78 @@ std::error_code SampleProfileWriterExtBinary::writeNameTable() {
   return sampleprof_error::success;
 }
 
-std::error_code SampleProfileWriterExtBinary::writeSections(
+std::error_code SampleProfileWriterExtBinaryBase::writeNameTableSection(
     const StringMap<FunctionSamples> &ProfileMap) {
-  uint64_t SectionStart = markSectionStart(SecProfSummary);
-  computeSummary(ProfileMap);
-  if (auto EC = writeSummary())
-    return EC;
-  if (std::error_code EC = addNewSection(SecProfSummary, SectionStart))
-    return EC;
-
-  // Generate the name table for all the functions referenced in the profile.
-  SectionStart = markSectionStart(SecNameTable);
   for (const auto &I : ProfileMap) {
     addName(I.first());
     addNames(I.second);
   }
-  writeNameTable();
-  if (std::error_code EC = addNewSection(SecNameTable, SectionStart))
+  if (auto EC = writeNameTable())
     return EC;
+  return sampleprof_error::success;
+}
 
-  SectionStart = markSectionStart(SecLBRProfile);
-  SecLBRProfileStart = OutputStream->tell();
-  if (std::error_code EC = writeFuncProfiles(ProfileMap))
-    return EC;
-  if (std::error_code EC = addNewSection(SecLBRProfile, SectionStart))
-    return EC;
+std::error_code
+SampleProfileWriterExtBinaryBase::writeProfileSymbolListSection() {
+  if (ProfSymList && ProfSymList->size() > 0)
+    if (std::error_code EC = ProfSymList->write(*OutputStream))
+      return EC;
 
-  if (ProfSymList && ProfSymList->toCompress())
+  return sampleprof_error::success;
+}
+
+std::error_code SampleProfileWriterExtBinaryBase::writeOneSection(
+    SecType Type, const StringMap<FunctionSamples> &ProfileMap) {
+  // The setting of SecFlagCompress should happen before markSectionStart.
+  if (Type == SecProfileSymbolList && ProfSymList && ProfSymList->toCompress())
     setToCompressSection(SecProfileSymbolList);
 
-  SectionStart = markSectionStart(SecProfileSymbolList);
-  if (ProfSymList && ProfSymList->size() > 0)
-    if (std::error_code EC = ProfSymList->write(*OutputStream))
+  uint64_t SectionStart = markSectionStart(Type);
+  switch (Type) {
+  case SecProfSummary:
+    computeSummary(ProfileMap);
+    if (auto EC = writeSummary())
+      return EC;
+    break;
+  case SecNameTable:
+    if (auto EC = writeNameTableSection(ProfileMap))
+      return EC;
+    break;
+  case SecLBRProfile:
+    SecLBRProfileStart = OutputStream->tell();
+    if (std::error_code EC = writeFuncProfiles(ProfileMap))
       return EC;
-  if (std::error_code EC = addNewSection(SecProfileSymbolList, SectionStart))
+    break;
+  case SecFuncOffsetTable:
+    if (auto EC = writeFuncOffsetTable())
+      return EC;
+    break;
+  case SecProfileSymbolList:
+    if (auto EC = writeProfileSymbolListSection())
+      return EC;
+    break;
+  default:
+    if (auto EC = writeCustomSection(Type))
+      return EC;
+    break;
+  }
+  if (std::error_code EC = addNewSection(Type, SectionStart))
     return EC;
+  return sampleprof_error::success;
+}
 
-  SectionStart = markSectionStart(SecFuncOffsetTable);
-  if (std::error_code EC = writeFuncOffsetTable())
+std::error_code SampleProfileWriterExtBinary::writeSections(
+    const StringMap<FunctionSamples> &ProfileMap) {
+  if (auto EC = writeOneSection(SecProfSummary, ProfileMap))
     return EC;
-  if (std::error_code EC = addNewSection(SecFuncOffsetTable, SectionStart))
+  if (auto EC = writeOneSection(SecNameTable, ProfileMap))
+    return EC;
+  if (auto EC = writeOneSection(SecLBRProfile, ProfileMap))
+    return EC;
+  if (auto EC = writeOneSection(SecProfileSymbolList, ProfileMap))
+    return EC;
+  if (auto EC = writeOneSection(SecFuncOffsetTable, ProfileMap))
     return EC;
-
   return sampleprof_error::success;
 }
 


        


More information about the llvm-commits mailing list