[llvm-branch-commits] [llvm] a906e3e - [NFC][SampleFDO] Preparation to support multiple sections with the same type

Wei Mi via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Wed Dec 16 22:33:08 PST 2020


Author: Wei Mi
Date: 2020-12-16T22:28:45-08:00
New Revision: a906e3eccd1ed149ffc8bdf2540c7cff4171e3bd

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

LOG: [NFC][SampleFDO] Preparation to support multiple sections with the same type
in ExtBinary format.

Currently ExtBinary format doesn't support multiple sections with the same type
in the profile. We add the support in this patch. Previously we use the section
type to identify a section uniquely. Now we introduces a LayoutIndex in the
SecHdrTableEntry and use the LayoutIndex to locate the target section. The
allocations of NameTable and FuncOffsetTable are adjusted accordingly.

Currently it works as a NFC because it won't change anything for current layout.
The test for multiple sections support will be included in another patch where a
new type of profile containing multiple sections with the same type is
introduced.

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

Added: 
    

Modified: 
    llvm/include/llvm/ProfileData/SampleProf.h
    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/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h
index 6a454de008ee..5d503f2d2d72 100644
--- a/llvm/include/llvm/ProfileData/SampleProf.h
+++ b/llvm/include/llvm/ProfileData/SampleProf.h
@@ -154,6 +154,9 @@ struct SecHdrTableEntry {
   uint64_t Flags;
   uint64_t Offset;
   uint64_t Size;
+  // The index indicating the location of the current entry in
+  // SectionHdrLayout table.
+  uint32_t LayoutIndex;
 };
 
 // Flags common for all sections are defined here. In SecHdrTableEntry::Flags,

diff  --git a/llvm/include/llvm/ProfileData/SampleProfReader.h b/llvm/include/llvm/ProfileData/SampleProfReader.h
index ce1b1f436357..35e71f336c27 100644
--- a/llvm/include/llvm/ProfileData/SampleProfReader.h
+++ b/llvm/include/llvm/ProfileData/SampleProfReader.h
@@ -623,7 +623,7 @@ class SampleProfileReaderExtBinaryBase : public SampleProfileReaderBinary {
 
 protected:
   std::vector<SecHdrTableEntry> SecHdrTable;
-  std::error_code readSecHdrTableEntry();
+  std::error_code readSecHdrTableEntry(uint32_t Idx);
   std::error_code readSecHdrTable();
 
   std::error_code readFuncMetadata();

diff  --git a/llvm/include/llvm/ProfileData/SampleProfWriter.h b/llvm/include/llvm/ProfileData/SampleProfWriter.h
index cfb2602f6784..5bb1446acb0b 100644
--- a/llvm/include/llvm/ProfileData/SampleProfWriter.h
+++ b/llvm/include/llvm/ProfileData/SampleProfWriter.h
@@ -175,8 +175,9 @@ class SampleProfileWriterExtBinaryBase : public SampleProfileWriterBinary {
   };
 
 protected:
-  uint64_t markSectionStart(SecType Type);
-  std::error_code addNewSection(SecType Sec, uint64_t SectionStart);
+  uint64_t markSectionStart(SecType Type, uint32_t LayoutIdx);
+  std::error_code addNewSection(SecType Sec, uint32_t LayoutIdx,
+                                uint64_t SectionStart);
   template <class SecFlagType>
   void addSectionFlag(SecType Type, SecFlagType Flag) {
     for (auto &Entry : SectionHdrLayout) {
@@ -193,9 +194,11 @@ class SampleProfileWriterExtBinaryBase : public SampleProfileWriterBinary {
   virtual std::error_code
   writeSections(const StringMap<FunctionSamples> &ProfileMap) = 0;
 
-  // Dispatch section writer for each section.
+  // Dispatch section writer for each section. \p LayoutIdx is the sequence
+  // number indicating where the section is located in SectionHdrLayout.
   virtual std::error_code
-  writeOneSection(SecType Type, const StringMap<FunctionSamples> &ProfileMap);
+  writeOneSection(SecType Type, uint32_t LayoutIdx,
+                  const StringMap<FunctionSamples> &ProfileMap);
 
   // Helper function to write name table.
   virtual std::error_code writeNameTable() override;
@@ -209,7 +212,7 @@ class SampleProfileWriterExtBinaryBase : public SampleProfileWriterBinary {
   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
+  // the order of sections in SecHdrTable 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;
@@ -224,7 +227,6 @@ class SampleProfileWriterExtBinaryBase : public SampleProfileWriterBinary {
   std::error_code writeSecHdrTable();
   virtual std::error_code
   writeHeader(const StringMap<FunctionSamples> &ProfileMap) override;
-  SecHdrTableEntry &getEntryInLayout(SecType Type);
   std::error_code compressAndOutput();
 
   // We will swap the raw_ostream held by LocalBufStream and that
@@ -241,7 +243,10 @@ class SampleProfileWriterExtBinaryBase : public SampleProfileWriterBinary {
   // The location in the output stream where the SecHdrTable should be
   // written to.
   uint64_t SecHdrTableOffset;
-  // Initial Section Flags setting.
+  // The table contains SecHdrTableEntry entries in order of how they are
+  // populated in the writer. It may be 
diff erent from the order in
+  // SectionHdrLayout which specifies the sequence in which sections will
+  // be read.
   std::vector<SecHdrTableEntry> SecHdrTable;
 
   // FuncOffsetTable maps function name to its profile offset in SecLBRProfile

diff  --git a/llvm/lib/ProfileData/SampleProfReader.cpp b/llvm/lib/ProfileData/SampleProfReader.cpp
index 18f9ec75c03e..ebae9cee955d 100644
--- a/llvm/lib/ProfileData/SampleProfReader.cpp
+++ b/llvm/lib/ProfileData/SampleProfReader.cpp
@@ -622,6 +622,11 @@ void SampleProfileReaderExtBinaryBase::collectFuncsFrom(const Module &M) {
 }
 
 std::error_code SampleProfileReaderExtBinaryBase::readFuncOffsetTable() {
+  // If there are more than one FuncOffsetTable, the profile read associated
+  // with previous FuncOffsetTable has to be done before next FuncOffsetTable
+  // is read.
+  FuncOffsetTable.clear();
+
   auto Size = readNumber<uint64_t>();
   if (std::error_code EC = Size.getError())
     return EC;
@@ -819,7 +824,7 @@ std::error_code SampleProfileReaderBinary::readNameTable() {
   auto Size = readNumber<uint32_t>();
   if (std::error_code EC = Size.getError())
     return EC;
-  NameTable.reserve(*Size);
+  NameTable.reserve(*Size + NameTable.size());
   for (uint32_t I = 0; I < *Size; ++I) {
     auto Name(readString());
     if (std::error_code EC = Name.getError())
@@ -897,7 +902,8 @@ std::error_code SampleProfileReaderCompactBinary::readNameTable() {
   return sampleprof_error::success;
 }
 
-std::error_code SampleProfileReaderExtBinaryBase::readSecHdrTableEntry() {
+std::error_code
+SampleProfileReaderExtBinaryBase::readSecHdrTableEntry(uint32_t Idx) {
   SecHdrTableEntry Entry;
   auto Type = readUnencodedNumber<uint64_t>();
   if (std::error_code EC = Type.getError())
@@ -919,6 +925,7 @@ std::error_code SampleProfileReaderExtBinaryBase::readSecHdrTableEntry() {
     return EC;
   Entry.Size = *Size;
 
+  Entry.LayoutIndex = Idx;
   SecHdrTable.push_back(std::move(Entry));
   return sampleprof_error::success;
 }
@@ -929,7 +936,7 @@ std::error_code SampleProfileReaderExtBinaryBase::readSecHdrTable() {
     return EC;
 
   for (uint32_t i = 0; i < (*EntryNum); i++)
-    if (std::error_code EC = readSecHdrTableEntry())
+    if (std::error_code EC = readSecHdrTableEntry(i))
       return EC;
 
   return sampleprof_error::success;
@@ -951,11 +958,12 @@ std::error_code SampleProfileReaderExtBinaryBase::readHeader() {
 }
 
 uint64_t SampleProfileReaderExtBinaryBase::getSectionSize(SecType Type) {
+  uint64_t Size = 0;
   for (auto &Entry : SecHdrTable) {
     if (Entry.Type == Type)
-      return Entry.Size;
+      Size += Entry.Size;
   }
-  return 0;
+  return Size;
 }
 
 uint64_t SampleProfileReaderExtBinaryBase::getFileSize() {
@@ -1007,7 +1015,7 @@ bool SampleProfileReaderExtBinaryBase::dumpSectionInfo(raw_ostream &OS) {
        << ", Size: " << Entry.Size << ", Flags: " << getSecFlagsStr(Entry)
        << "\n";
     ;
-    TotalSecsSize += getSectionSize(Entry.Type);
+    TotalSecsSize += Entry.Size;
   }
   uint64_t HeaderSize = SecHdrTable.front().Offset;
   assert(HeaderSize + TotalSecsSize == getFileSize() &&

diff  --git a/llvm/lib/ProfileData/SampleProfWriter.cpp b/llvm/lib/ProfileData/SampleProfWriter.cpp
index 47f94093d061..60cfe505d19b 100644
--- a/llvm/lib/ProfileData/SampleProfWriter.cpp
+++ b/llvm/lib/ProfileData/SampleProfWriter.cpp
@@ -73,19 +73,16 @@ SampleProfileWriter::write(const StringMap<FunctionSamples> &ProfileMap) {
   return sampleprof_error::success;
 }
 
-SecHdrTableEntry &
-SampleProfileWriterExtBinaryBase::getEntryInLayout(SecType Type) {
-  auto SecIt = std::find_if(
-      SectionHdrLayout.begin(), SectionHdrLayout.end(),
-      [=](const auto &Entry) -> bool { return Entry.Type == Type; });
-  return *SecIt;
-}
-
 /// Return the current position and prepare to use it as the start
-/// position of a section.
-uint64_t SampleProfileWriterExtBinaryBase::markSectionStart(SecType Type) {
+/// position of a section given the section type \p Type and its position
+/// \p LayoutIdx in SectionHdrLayout.
+uint64_t
+SampleProfileWriterExtBinaryBase::markSectionStart(SecType Type,
+                                                   uint32_t LayoutIdx) {
   uint64_t SectionStart = OutputStream->tell();
-  auto &Entry = getEntryInLayout(Type);
+  assert(LayoutIdx < SectionHdrLayout.size() && "LayoutIdx out of range");
+  const auto &Entry = SectionHdrLayout[LayoutIdx];
+  assert(Entry.Type == Type && "Unexpected section type");
   // Use LocalBuf as a temporary output for writting data.
   if (hasSecFlag(Entry, SecCommonFlags::SecFlagCompress))
     LocalBufStream.swap(OutputStream);
@@ -112,18 +109,21 @@ std::error_code SampleProfileWriterExtBinaryBase::compressAndOutput() {
   return sampleprof_error::success;
 }
 
-/// Add a new section into section header table.
-std::error_code
-SampleProfileWriterExtBinaryBase::addNewSection(SecType Type,
-                                                uint64_t SectionStart) {
-  auto Entry = getEntryInLayout(Type);
+/// Add a new section into section header table given the section type
+/// \p Type, its position \p LayoutIdx in SectionHdrLayout and the
+/// location \p SectionStart where the section should be written to.
+std::error_code SampleProfileWriterExtBinaryBase::addNewSection(
+    SecType Type, uint32_t LayoutIdx, uint64_t SectionStart) {
+  assert(LayoutIdx < SectionHdrLayout.size() && "LayoutIdx out of range");
+  const auto &Entry = SectionHdrLayout[LayoutIdx];
+  assert(Entry.Type == Type && "Unexpected section type");
   if (hasSecFlag(Entry, SecCommonFlags::SecFlagCompress)) {
     LocalBufStream.swap(OutputStream);
     if (std::error_code EC = compressAndOutput())
       return EC;
   }
   SecHdrTable.push_back({Type, Entry.Flags, SectionStart - FileStart,
-                         OutputStream->tell() - SectionStart});
+                         OutputStream->tell() - SectionStart, LayoutIdx});
   return sampleprof_error::success;
 }
 
@@ -163,6 +163,7 @@ std::error_code SampleProfileWriterExtBinaryBase::writeFuncOffsetTable() {
     writeNameIdx(entry.first);
     encodeULEB128(entry.second, OS);
   }
+  FuncOffsetTable.clear();
   return sampleprof_error::success;
 }
 
@@ -217,14 +218,15 @@ SampleProfileWriterExtBinaryBase::writeProfileSymbolListSection() {
 }
 
 std::error_code SampleProfileWriterExtBinaryBase::writeOneSection(
-    SecType Type, const StringMap<FunctionSamples> &ProfileMap) {
+    SecType Type, uint32_t LayoutIdx,
+    const StringMap<FunctionSamples> &ProfileMap) {
   // The setting of SecFlagCompress should happen before markSectionStart.
   if (Type == SecProfileSymbolList && ProfSymList && ProfSymList->toCompress())
     setToCompressSection(SecProfileSymbolList);
   if (Type == SecFuncMetadata && FunctionSamples::ProfileIsProbeBased)
     addSectionFlag(SecFuncMetadata, SecFuncMetadataFlags::SecFlagIsProbeBased);
 
-  uint64_t SectionStart = markSectionStart(Type);
+  uint64_t SectionStart = markSectionStart(Type, LayoutIdx);
   switch (Type) {
   case SecProfSummary:
     computeSummary(ProfileMap);
@@ -257,24 +259,28 @@ std::error_code SampleProfileWriterExtBinaryBase::writeOneSection(
       return EC;
     break;
   }
-  if (std::error_code EC = addNewSection(Type, SectionStart))
+  if (std::error_code EC = addNewSection(Type, LayoutIdx, SectionStart))
     return EC;
   return sampleprof_error::success;
 }
 
 std::error_code SampleProfileWriterExtBinary::writeSections(
     const StringMap<FunctionSamples> &ProfileMap) {
-  if (auto EC = writeOneSection(SecProfSummary, ProfileMap))
+  // The const indices passed to writeOneSection below are specifying the
+  // positions of the sections in SectionHdrLayout. Look at
+  // initSectionHdrLayout to find out where each section is located in
+  // SectionHdrLayout.
+  if (auto EC = writeOneSection(SecProfSummary, 0, ProfileMap))
     return EC;
-  if (auto EC = writeOneSection(SecNameTable, ProfileMap))
+  if (auto EC = writeOneSection(SecNameTable, 1, ProfileMap))
     return EC;
-  if (auto EC = writeOneSection(SecLBRProfile, ProfileMap))
+  if (auto EC = writeOneSection(SecLBRProfile, 3, ProfileMap))
     return EC;
-  if (auto EC = writeOneSection(SecProfileSymbolList, ProfileMap))
+  if (auto EC = writeOneSection(SecProfileSymbolList, 4, ProfileMap))
     return EC;
-  if (auto EC = writeOneSection(SecFuncOffsetTable, ProfileMap))
+  if (auto EC = writeOneSection(SecFuncOffsetTable, 2, ProfileMap))
     return EC;
-  if (auto EC = writeOneSection(SecFuncMetadata, ProfileMap))
+  if (auto EC = writeOneSection(SecFuncMetadata, 5, ProfileMap))
     return EC;
   return sampleprof_error::success;
 }
@@ -497,24 +503,31 @@ std::error_code SampleProfileWriterExtBinaryBase::writeSecHdrTable() {
     return sampleprof_error::ostream_seek_unsupported;
   support::endian::Writer Writer(*OutputStream, support::little);
 
-  DenseMap<uint32_t, uint32_t> IndexMap;
-  for (uint32_t i = 0; i < SecHdrTable.size(); i++) {
-    IndexMap.insert({static_cast<uint32_t>(SecHdrTable[i].Type), i});
+  assert(SecHdrTable.size() == SectionHdrLayout.size() &&
+         "SecHdrTable entries doesn't match SectionHdrLayout");
+  SmallVector<uint32_t, 16> IndexMap(SecHdrTable.size(), -1);
+  for (uint32_t TableIdx = 0; TableIdx < SecHdrTable.size(); TableIdx++) {
+    IndexMap[SecHdrTable[TableIdx].LayoutIndex] = TableIdx;
   }
 
   // Write the section header table in the order specified in
-  // SectionHdrLayout. That is the sections order Reader will see.
-  // Note that the sections order in which Reader expects to read
-  // may be 
diff erent from the order in which Writer is able to
-  // write, so we need to adjust the order in SecHdrTable to be
-  // consistent with SectionHdrLayout when we write SecHdrTable
-  // to the memory.
-  for (uint32_t i = 0; i < SectionHdrLayout.size(); i++) {
-    uint32_t idx = IndexMap[static_cast<uint32_t>(SectionHdrLayout[i].Type)];
-    Writer.write(static_cast<uint64_t>(SecHdrTable[idx].Type));
-    Writer.write(static_cast<uint64_t>(SecHdrTable[idx].Flags));
-    Writer.write(static_cast<uint64_t>(SecHdrTable[idx].Offset));
-    Writer.write(static_cast<uint64_t>(SecHdrTable[idx].Size));
+  // SectionHdrLayout. SectionHdrLayout specifies the sections
+  // order in which profile reader expect to read, so the section
+  // header table should be written in the order in SectionHdrLayout.
+  // Note that the section order in SecHdrTable may be 
diff erent
+  // from the order in SectionHdrLayout, for example, SecFuncOffsetTable
+  // needs to be computed after SecLBRProfile (the order in SecHdrTable),
+  // but it needs to be read before SecLBRProfile (the order in
+  // SectionHdrLayout). So we use IndexMap above to switch the order.
+  for (uint32_t LayoutIdx = 0; LayoutIdx < SectionHdrLayout.size();
+       LayoutIdx++) {
+    assert(IndexMap[LayoutIdx] < SecHdrTable.size() &&
+           "Incorrect LayoutIdx in SecHdrTable");
+    auto Entry = SecHdrTable[IndexMap[LayoutIdx]];
+    Writer.write(static_cast<uint64_t>(Entry.Type));
+    Writer.write(static_cast<uint64_t>(Entry.Flags));
+    Writer.write(static_cast<uint64_t>(Entry.Offset));
+    Writer.write(static_cast<uint64_t>(Entry.Size));
   }
 
   // Reset OutputStream.


        


More information about the llvm-branch-commits mailing list