[llvm-profdata] ProfileReader cleanup - preparation for MD5 refactoring - 2

Cleanup profile reader classes to prepare for complex refactoring as propsed in D147740, continuing D148868
This is patch 2/n. This patch refactors CSNameTable and related things

The decision to move CSNameTable up to the base class is because a planned improvement (D147740) to use MD5 to lookup Functions/Context frames. In this case we want a unified data structure between contextless function or Context frames, so that it can be mapped by MD5 value. Since Context Frames can represent contextless functions, it is being used for MD5 lookup, therefore exposing it to the base class

Reviewed By: snehasish, wenlei

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

Author: huangjd

llvm/include/llvm/ProfileData/SampleProfReader.h

@@ -657,6 +657,13 @@ class SampleProfileReaderBinary : public SampleProfileReader {
   /// Read a string indirectly via the name table.
   ErrorOr<StringRef> readStringFromTable();
 
+  /// Read a context indirectly via the CSNameTable.
+  ErrorOr<SampleContextFrames> readContextFromTable();
+
+  /// Read a context indirectly via the CSNameTable if the profile has context,
+  /// otherwise same as readStringFromTable.
+  ErrorOr<SampleContext> readSampleContextFromTable();
+
   /// Points to the current location in the buffer.
   const uint8_t *Data = nullptr;
 
@@ -678,7 +685,9 @@ class SampleProfileReaderBinary : public SampleProfileReader {
   /// The starting address of NameTable containing fixed length MD5.
   const uint8_t *MD5NameMemStart = nullptr;
 
-  virtual ErrorOr<SampleContext> readSampleContextFromTable();
+  /// CSNameTable is used to save full context vectors. It is the backing buffer
+  /// for SampleContextFrames.
+  std::vector<SampleContextFrameVector> CSNameTable;
 
 private:
   std::error_code readSummaryEntry(std::vector<ProfileSummaryEntry> &Entries);
@@ -746,8 +755,6 @@ class SampleProfileReaderExtBinaryBase : public SampleProfileReaderBinary {
                                          const SecHdrTableEntry &Entry);
   // placeholder for subclasses to dispatch their own section readers.
   virtual std::error_code readCustomSection(const SecHdrTableEntry &Entry) = 0;
-  ErrorOr<SampleContext> readSampleContextFromTable() override;
-  ErrorOr<SampleContextFrames> readContextFromTable();
 
   std::unique_ptr<ProfileSymbolList> ProfSymList;
 
@@ -762,10 +769,6 @@ class SampleProfileReaderExtBinaryBase : public SampleProfileReaderBinary {
   /// The set containing the functions to use when compiling a module.
   DenseSet<StringRef> FuncsToUse;
 
-  /// CSNameTable is used to save full context vectors. This serves as an
-  /// underlying immutable buffer for all clients.
-  std::unique_ptr<const std::vector<SampleContextFrameVector>> CSNameTable;
-
   /// If SkipFlatProf is true, skip the sections with
   /// SecFlagFlat flag.
   bool SkipFlatProf = false;

llvm/lib/ProfileData/SampleProfReader.cpp

@@ -546,11 +546,27 @@ ErrorOr<StringRef> SampleProfileReaderBinary::readStringFromTable() {
   return SR;
 }
 
-ErrorOr<SampleContext> SampleProfileReaderBinary::readSampleContextFromTable() {
-  auto FName(readStringFromTable());
-  if (std::error_code EC = FName.getError())
+ErrorOr<SampleContextFrames> SampleProfileReaderBinary::readContextFromTable() {
+  auto ContextIdx = readNumber<size_t>();
+  if (std::error_code EC = ContextIdx.getError())
     return EC;
-  return SampleContext(*FName);
+  if (*ContextIdx >= CSNameTable.size())
+    return sampleprof_error::truncated_name_table;
+  return CSNameTable[*ContextIdx];
+}
+
+ErrorOr<SampleContext> SampleProfileReaderBinary::readSampleContextFromTable() {
+  if (ProfileIsCS) {
+    auto FContext(readContextFromTable());
+    if (std::error_code EC = FContext.getError())
+      return EC;
+    return SampleContext(*FContext);
+  } else {
+    auto FName(readStringFromTable());
+    if (std::error_code EC = FName.getError())
+      return EC;
+    return SampleContext(*FName);
+  }
 }
 
 std::error_code
@@ -671,31 +687,6 @@ std::error_code SampleProfileReaderBinary::readImpl() {
   return sampleprof_error::success;
 }
 
-ErrorOr<SampleContextFrames>
-SampleProfileReaderExtBinaryBase::readContextFromTable() {
-  auto ContextIdx = readNumber<size_t>();
-  if (std::error_code EC = ContextIdx.getError())
-    return EC;
-  if (*ContextIdx >= CSNameTable->size())
-    return sampleprof_error::truncated_name_table;
-  return (*CSNameTable)[*ContextIdx];
-}
-
-ErrorOr<SampleContext>
-SampleProfileReaderExtBinaryBase::readSampleContextFromTable() {
-  if (ProfileIsCS) {
-    auto FContext(readContextFromTable());
-    if (std::error_code EC = FContext.getError())
-      return EC;
-    return SampleContext(*FContext);
-  } else {
-    auto FName(readStringFromTable());
-    if (std::error_code EC = FName.getError())
-      return EC;
-    return SampleContext(*FName);
-  }
-}
-
 std::error_code SampleProfileReaderExtBinaryBase::readOneSection(
     const uint8_t *Start, uint64_t Size, const SecHdrTableEntry &Entry) {
   Data = Start;
@@ -1035,7 +1026,7 @@ std::error_code
 SampleProfileReaderExtBinaryBase::readNameTableSec(bool IsMD5,
                                                    bool FixedLengthMD5) {
   if (FixedLengthMD5) {
-    if (IsMD5)
+    if (!IsMD5)
       errs() << "If FixedLengthMD5 is true, UseMD5 has to be true";
     auto Size = readNumber<size_t>();
     if (std::error_code EC = Size.getError())
@@ -1089,11 +1080,10 @@ std::error_code SampleProfileReaderExtBinaryBase::readCSNameTableSec() {
   if (std::error_code EC = Size.getError())
     return EC;
 
-  std::vector<SampleContextFrameVector> *PNameVec =
-      new std::vector<SampleContextFrameVector>();
-  PNameVec->reserve(*Size);
+  CSNameTable.clear();
+  CSNameTable.reserve(*Size);
   for (size_t I = 0; I < *Size; ++I) {
-    PNameVec->emplace_back(SampleContextFrameVector());
+    CSNameTable.emplace_back(SampleContextFrameVector());
     auto ContextSize = readNumber<uint32_t>();
     if (std::error_code EC = ContextSize.getError())
       return EC;
@@ -1112,18 +1102,15 @@ std::error_code SampleProfileReaderExtBinaryBase::readCSNameTableSec() {
       if (std::error_code EC = Discriminator.getError())
         return EC;
 
-      PNameVec->back().emplace_back(
+      CSNameTable.back().emplace_back(
           FName.get(), LineLocation(LineOffset.get(), Discriminator.get()));
     }
   }
 
-  // From this point the underlying object of CSNameTable should be immutable.
-  CSNameTable.reset(PNameVec);
   return sampleprof_error::success;
 }
 
 std::error_code
-
 SampleProfileReaderExtBinaryBase::readFuncMetadata(bool ProfileHasAttribute,
                                                    FunctionSamples *FProfile) {
   if (Data < End) {