From a906e3eccd1ed149ffc8bdf2540c7cff4171e3bd Mon Sep 17 00:00:00 2001 From: Wei Mi Date: Mon, 14 Dec 2020 14:49:20 -0800 Subject: [PATCH] [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 --- llvm/include/llvm/ProfileData/SampleProf.h | 3 + llvm/include/llvm/ProfileData/SampleProfReader.h | 2 +- llvm/include/llvm/ProfileData/SampleProfWriter.h | 19 +++-- llvm/lib/ProfileData/SampleProfReader.cpp | 20 +++-- llvm/lib/ProfileData/SampleProfWriter.cpp | 95 ++++++++++++++---------- 5 files changed, 84 insertions(+), 55 deletions(-) diff --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h index 6a454de..5d503f2 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 ce1b1f4..35e71f3 100644 --- a/llvm/include/llvm/ProfileData/SampleProfReader.h +++ b/llvm/include/llvm/ProfileData/SampleProfReader.h @@ -623,7 +623,7 @@ private: protected: std::vector 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 cfb2602..5bb1446 100644 --- a/llvm/include/llvm/ProfileData/SampleProfWriter.h +++ b/llvm/include/llvm/ProfileData/SampleProfWriter.h @@ -175,8 +175,9 @@ public: }; 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 void addSectionFlag(SecType Type, SecFlagType Flag) { for (auto &Entry : SectionHdrLayout) { @@ -193,9 +194,11 @@ protected: virtual std::error_code writeSections(const StringMap &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 &ProfileMap); + writeOneSection(SecType Type, uint32_t LayoutIdx, + const StringMap &ProfileMap); // Helper function to write name table. virtual std::error_code writeNameTable() override; @@ -209,7 +212,7 @@ protected: std::error_code writeProfileSymbolListSection(); // Specifiy the order of sections in section header table. Note - // the order of sections in the profile may be different that the + // the order of sections in SecHdrTable may be different that the // order in SectionHdrLayout. sample Reader will follow the order // in SectionHdrLayout to read each section. SmallVector SectionHdrLayout; @@ -224,7 +227,6 @@ private: std::error_code writeSecHdrTable(); virtual std::error_code writeHeader(const StringMap &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 @@ private: // 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 different from the order in + // SectionHdrLayout which specifies the sequence in which sections will + // be read. std::vector 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 18f9ec7..ebae9ce 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(); if (std::error_code EC = Size.getError()) return EC; @@ -819,7 +824,7 @@ std::error_code SampleProfileReaderBinary::readNameTable() { auto Size = readNumber(); 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(); 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 47f9409..60cfe50 100644 --- a/llvm/lib/ProfileData/SampleProfWriter.cpp +++ b/llvm/lib/ProfileData/SampleProfWriter.cpp @@ -73,19 +73,16 @@ SampleProfileWriter::write(const StringMap &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 &ProfileMap) { + SecType Type, uint32_t LayoutIdx, + const StringMap &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 &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 IndexMap; - for (uint32_t i = 0; i < SecHdrTable.size(); i++) { - IndexMap.insert({static_cast(SecHdrTable[i].Type), i}); + assert(SecHdrTable.size() == SectionHdrLayout.size() && + "SecHdrTable entries doesn't match SectionHdrLayout"); + SmallVector 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 different 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(SectionHdrLayout[i].Type)]; - Writer.write(static_cast(SecHdrTable[idx].Type)); - Writer.write(static_cast(SecHdrTable[idx].Flags)); - Writer.write(static_cast(SecHdrTable[idx].Offset)); - Writer.write(static_cast(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 different + // 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(Entry.Type)); + Writer.write(static_cast(Entry.Flags)); + Writer.write(static_cast(Entry.Offset)); + Writer.write(static_cast(Entry.Size)); } // Reset OutputStream. -- 2.7.4