[SampleFDO] Port MD5 name table support to extbinary format.

[wmi]

Summary

Compbinary format uses MD5 to represent strings in name table. That gives smaller profile. The patch adds the support in extbinary format. It is off by default but user can choose to enable it.

Note the feature of using MD5 in name table can bring very small chance of name conflict leading to profile mismatch. Besides, profile using the feature won't have the profile remapping support.

[davidxl]

llvm/include/llvm/ProfileData/SampleProfReader.h

637

What is the use of MD5Names? It seems only used in useMD5() method.

Also why does it need to be unique_ptr?

[wmi]

llvm/include/llvm/ProfileData/SampleProfReader.h

637

Profile only contains md5 numbers (type of uint64_t). NameTable contains StringRef to the names (All formats of profile use the same NameTable). When reading profile using md5, to populate the NameTable structure, I need to convert uint64_t number to std::string and then convert it to StringRef, so I need a buffer to save those std::string because StringRef doesn't own any string data. To be a buffer of NameTable, that is the use of MD5Names.

I make it a unique_ptr because for formats not using md5, MD5Names is useless, so I think an empty unique_ptr may have less cost than an empty std::vector in that case.

[davidxl]

llvm/include/llvm/ProfileData/SampleProfReader.h

637

The type of NameTable is vector<string> not vector<StringRef> right?

664

NameTable Decl here

llvm/lib/ProfileData/SampleProfReader.cpp

733

The string is pushed into MD5Names, and immediately copied to NameTable. Can MD5Names be skipped here?

[wmi]

llvm/include/llvm/ProfileData/SampleProfReader.h

637

There is another one defined as vector<StringRef> in the base class SampleProfileReaderBinary.

664

Yes, Compbinary format uses std::vector<std::string> type of NameTable. When we use std::vector<std::string> for NameTable, we can avoid the buffer, but we are going to add more override functions.

Currently I used the buffer in the patch, but it is confusing that we used two different ways in Compbinary format and in ExtBinary format for md5 support. So maybe using std::vector<std::string> type for NameTable is better. I will change it.

llvm/lib/ProfileData/SampleProfReader.cpp

733

Ok, I will change it.

[wmi]

llvm/include/llvm/ProfileData/SampleProfReader.h

664

I just tried then I realized a problem I forgot to mention: When ExtBinary format doesn't use md5, it wants NameTable to be std::vector<StringRef> type. When ExtBinary format uses md5, it wants NameTable to be std::vector<std::string>. I can create a new nametable called MD5NameTable, but it makes things more complicate.

We also don't want to use std::vector<std::string> type NameTable for both cases. That is because for ExtBinary format without using md5 the string has already existed in file buffer. If NameTable can use StringRef, it can refer to the string in file buffer directly without having to copy the string to NameTable. That will save some cost.

So looks like keeping the MD5Names buffer is a simpler way to me.

[davidxl]

llvm/include/llvm/ProfileData/SampleProfReader.h

637

Make the comment clearer that MD5Names serves as string buffer that is referenced by NameTable (vector of StringRef).

Is the lifetime of this buffer guarenteed to be longer than NameTable?

llvm/lib/ProfileData/SampleProfReader.cpp

733

Add a comment here NameTable is a vector of StringRef, and the second push_back is not a copy but a refernece. Otherwise the code looks confusing.

[wenlei]

Great to MD5 support added to ext-bin, thanks @wmi. After this change it seems compact-bin is completely superseded by ext-bin, do you plan to deprecate/remove it?

llvm/include/llvm/ProfileData/SampleProf.h

157

I thought SecFlags is a set of general purpose flags that can be applied to all section, at least SecFlagCompress falls under that category. For Name vs MD5Name, to some extended it's like section with different content. So would it make sense to have SecMD5NameTable in addition to SecNameTable for SecType, instead of relying on SecFlags? All reader/writer code can still be shared between the two.

llvm/tools/llvm-profdata/llvm-profdata.cpp

474

The incompatibility between -use-md5 and format is detected and handled at runtime. Can we do the same for setUseMD5? Specifically, make setUseMD5 part of SampleProfileWriter, and error/assert in the base version of it - that would avoid the casting and simplifies a bit in couple places. (Could probably do the same for compression).

[wmi]

In D76255#1931410, @wenlei wrote:

Great to MD5 support added to ext-bin, thanks @wmi. After this change it seems compact-bin is completely superseded by ext-bin, do you plan to deprecate/remove it?

Sorry for the late reply.

I plan to deprecate it, but only after people migrate to the new format. The problem is I don't know who is using the old format. I think I can leave it for longer time before deprecate it.

llvm/include/llvm/ProfileData/SampleProf.h

157

SecFlags was not designed to be general purpose only. I want to make it more flexible so that every section can define their own flags. I want to set aside some bits for general purpose only flags and some bits for section specific flags. For section specific flag, every section can reuse the same bit for different purpose. We may need a verification in place to prevent misuse. If it sounds good to you, I will add some comments describing it and put the verification in place.

llvm/include/llvm/ProfileData/SampleProfReader.h

637

Yes, they are both in SampleProfileReader object so they have the same lifetime.

llvm/tools/llvm-profdata/llvm-profdata.cpp

474

I was trying to find some balance here and not to put too much special things in the base class. For SampleProfileReader, if we don't add the interfaces in it, we will have many places that we need the casting, so it has a bunch of special interfaces there. For SampleProfileWriter, we only have a few places needing the special interfaces so adding the casting is still ok. I can change the code and only add casting once here.

I am just not quite sure which way is better. If you feel adding the interfaces in SampleProfileWriter is better, I will be happy to do it.

[wenlei]

llvm/include/llvm/ProfileData/SampleProf.h

157

Thanks for clarification - that sounds good to me, and yes some comments and verification would be useful.

llvm/tools/llvm-profdata/llvm-profdata.cpp

474

Conceptually, MD5 name doesn't have to be tied to extbinary, e.g. we could potentially have MD5 as name even for text profile. So maybe it's indeed general enough that warrant it being in base class, it's just we only support that for extbinary today. That said, I don't have a strong opinion either..

[davidxl]

llvm/include/llvm/ProfileData/SampleProf.h

157

Perhaps reserve a range of common flags?

[wmi]

llvm/include/llvm/ProfileData/SampleProf.h

157

I reserved the lower 32bits in SecHdrTableEntry::Flags for common flags and left the higher 32bits for section specific flags. Now each section can define 32 custom flags maximumly. I added the verification to make sure no custom flag will be misused on unrelevant section.

llvm/lib/ProfileData/SampleProfReader.cpp

733

Done.

llvm/tools/llvm-profdata/llvm-profdata.cpp

474

I added the interfaces in SampleProfileWriter.

[wmi]

Address David and Wenlei's comments.

[wenlei]

LGTM, thanks!

[davidxl]

lgtm

[gchatelet]

FYI rG159343cd1965c69233597b36ab27adce8bd34a14

llvm/include/llvm/ProfileData/SampleProf.h

 
 static inline uint64_t SPMagic(SampleProfileFormat Format = SPF_Binary) {
   return uint64_t('S') << (64 - 8) | uint64_t('P') << (64 - 16) |
          uint64_t('R') << (64 - 24) | uint64_t('O') << (64 - 32) |
          uint64_t('F') << (64 - 40) | uint64_t('4') << (64 - 48) |
          uint64_t('2') << (64 - 56) | uint64_t(Format);
 }
 
-// Get the proper representation of a string in the input Format.
-static inline StringRef getRepInFormat(StringRef Name,
-                                       SampleProfileFormat Format,
+/// Get the proper representation of a string according to whether the
+/// current Format uses MD5 to represent the string.
+static inline StringRef getRepInFormat(StringRef Name, bool UseMD5,
                                        std::string &GUIDBuf) {
   if (Name.empty())
     return Name;
   GUIDBuf = std::to_string(Function::getGUID(Name));
-  return (Format == SPF_Compact_Binary) ? StringRef(GUIDBuf) : Name;
+  return UseMD5 ? StringRef(GUIDBuf) : Name;
 }
 
 static inline uint64_t SPVersion() { return 103; }
 
 // Section Type used by SampleProfileExtBinaryBaseReader and
 // SampleProfileExtBinaryBaseWriter. Never change the existing
 // value of enum. Only append new ones.
 enum SecType {

 // and SampleProfileExtBinaryBaseWriter.
 struct SecHdrTableEntry {
   SecType Type;
   uint64_t Flags;
   uint64_t Offset;
   uint64_t Size;
 };
 
-enum SecFlags { SecFlagInValid = 0, SecFlagCompress = (1 << 0) };
+enum SecFlags { SecFlagInValid = 0, SecFlagCompress = (1 << 0), SecFlagMD5Name = (1 << 1) };
 
 static inline void addSecFlags(SecHdrTableEntry &Entry, uint64_t Flags) {
   Entry.Flags |= Flags;
 }
 
 static inline void removeSecFlags(SecHdrTableEntry &Entry, uint64_t Flags) {
   Entry.Flags &= ~Flags;
 }
 
-static inline bool hasSecFlag(SecHdrTableEntry &Entry, SecFlags Flag) {
+static inline bool hasSecFlag(const SecHdrTableEntry &Entry, SecFlags Flag) {
   return Entry.Flags & Flag;
 }
 
 /// Represents the relative location of an instruction.
 ///
 /// Instruction locations are specified by the line offset from the
 /// beginning of the function (marked by the line where the function
 /// header is) and the discriminator value within that line.
public:
 
   /// Returns a pointer to FunctionSamples at the given callsite location \p Loc
   /// with callee \p CalleeName. If no callsite can be found, relax the
   /// restriction to return the FunctionSamples at callsite location \p Loc
   /// with the maximum total sample count.
   const FunctionSamples *findFunctionSamplesAt(const LineLocation &Loc,
                                                StringRef CalleeName) const {
     std::string CalleeGUID;
-    CalleeName = getRepInFormat(CalleeName, Format, CalleeGUID);
+    CalleeName = getRepInFormat(CalleeName, UseMD5, CalleeGUID);
 
     auto iter = CallsiteSamples.find(Loc);
     if (iter == CallsiteSamples.end())
       return nullptr;
     auto FS = iter->second.find(CalleeName);
     if (FS != iter->second.end())
       return &FS->second;
     // If we cannot find exact match of the callee name, return the FS with
if (Attr == "" || Attr == "all") {
       return F.getName();
     } else {
       assert(false && "internal error: unknown suffix elision policy");
     }
     return F.getName();
   }
 
   /// Translate \p Name into its original name in Module.
-  /// When the Format is not SPF_Compact_Binary, \p Name needs no translation.
-  /// When the Format is SPF_Compact_Binary, \p Name in current FunctionSamples
+  /// When profile doesn't use MD5, \p Name needs no translation.
+  /// When profile uses MD5, \p Name in current FunctionSamples
   /// is actually GUID of the original function name. getNameInModule will
   /// translate \p Name in current FunctionSamples into its original name.
   /// If the original name doesn't exist in \p M, return empty StringRef.
   StringRef getNameInModule(StringRef Name, const Module *M) const {
-    if (Format != SPF_Compact_Binary)
+    if (!UseMD5)
       return Name;
 
     assert(GUIDToFuncNameMap && "GUIDToFuncNameMap needs to be popluated first");
     auto iter = GUIDToFuncNameMap->find(std::stoull(Name.data()));
     if (iter == GUIDToFuncNameMap->end())
       return StringRef();
     return iter->second;
   }
public:
   /// We traverse the inline stack of that instruction, and match it with the
   /// tree nodes in the profile.
   ///
   /// \returns the FunctionSamples pointer to the inlined instance.
   const FunctionSamples *findFunctionSamples(const DILocation *DIL) const;
 
   static SampleProfileFormat Format;
 
+  /// Whether the profile uses MD5 to represent string.
+  static bool UseMD5;
+
   /// GUIDToFuncNameMap saves the mapping from GUID to the symbol name, for
   /// all the function symbols defined or declared in current module.
   DenseMap<uint64_t, StringRef> *GUIDToFuncNameMap = nullptr;
 
   // Assume the input \p Name is a name coming from FunctionSamples itself.
-  // If the format is SPF_Compact_Binary, the name is already a GUID and we
+  // If UseMD5 is true, the name is already a GUID and we
   // don't want to return the GUID of GUID.
   static uint64_t getGUID(StringRef Name) {
-    return (Format == SPF_Compact_Binary) ? std::stoull(Name.data())
-                                          : Function::getGUID(Name);
+    return UseMD5 ? std::stoull(Name.data()) : Function::getGUID(Name);
   }
 
 private:
   /// Mangled name of the function.
   StringRef Name;
 
   /// Total number of samples collected inside this function.
   ///

llvm/include/llvm/ProfileData/SampleProfReader.h

public:
   virtual std::error_code readHeader() = 0;
 
   /// The interface to read sample profiles from the associated file.
   std::error_code read() {
     if (std::error_code EC = readImpl())
       return EC;
     if (Remapper)
       Remapper->applyRemapping(Ctx);
+    FunctionSamples::UseMD5 = useMD5();
     return sampleprof_error::success;
   }
 
   /// The implementaion to read sample profiles from the associated file.
   virtual std::error_code readImpl() = 0;
 
   /// Print the profile for \p FName on stream \p OS.
   void dumpFunctionProfile(StringRef FName, raw_ostream &OS = dbgs());
FunctionSamples *getSamplesFor(const Function &F) {
     return getSamplesFor(CanonName);
   }
 
   /// Return the samples collected for function \p F, create empty
   /// FunctionSamples if it doesn't exist.
   FunctionSamples *getOrCreateSamplesFor(const Function &F) {
     std::string FGUID;
     StringRef CanonName = FunctionSamples::getCanonicalFnName(F);
-    CanonName = getRepInFormat(CanonName, getFormat(), FGUID);
+    CanonName = getRepInFormat(CanonName, useMD5(), FGUID);
     return &Profiles[CanonName];
   }
 
   /// Return the samples collected for function \p F.
   virtual FunctionSamples *getSamplesFor(StringRef Fname) {
     if (Remapper) {
       if (auto FS = Remapper->getSamplesFor(Fname))
         return FS;
     }
     std::string FGUID;
-    Fname = getRepInFormat(Fname, getFormat(), FGUID);
+    Fname = getRepInFormat(Fname, useMD5(), FGUID);
     auto It = Profiles.find(Fname);
     if (It != Profiles.end())
       return &It->second;
     return nullptr;
   }
 
   /// Return all the profiles.
   StringMap<FunctionSamples> &getProfiles() { return Profiles; }
virtual std::unique_ptr<ProfileSymbolList> getProfileSymbolList() {
     return nullptr;
   };
 
   /// It includes all the names that have samples either in outline instance
   /// or inline instance.
   virtual std::vector<StringRef> *getNameTable() { return nullptr; }
   virtual bool dumpSectionInfo(raw_ostream &OS = dbgs()) { return false; };
 
+  /// Return whether names in the profile are all MD5 numbers.
+  virtual bool useMD5() { return false; }
+
 protected:
   /// Map every function to its associated profile.
   ///
   /// The profile of every function executed at runtime is collected
   /// in the structure FunctionSamples. This maps function objects
   /// to their corresponding profiles.
   StringMap<FunctionSamples> Profiles;
 

 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,
-                                         SecType Type) = 0;
+                                         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,
-                                         SecType Type) 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);
 
   /// The table mapping from function name to the offset of its FunctionSample
   /// towards file start.
   DenseMap<StringRef, uint64_t> FuncOffsetTable;
   /// The set containing the functions to use when compiling a module.
   DenseSet<StringRef> FuncsToUse;
   /// Use all functions from the input profile.
   bool UseAllFuncs = true;
 
+  /// If MD5 is used in NameTable section, the section saves uint64_t data.
+  /// When reading NameTable, MD5Names is the place to save the strings
+  /// converted from uint64_t data.
+  std::unique_ptr<std::vector<std::string>> MD5Names;
+
 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);
 
   virtual std::unique_ptr<ProfileSymbolList> getProfileSymbolList() override {
     return std::move(ProfSymList);
   };
 
   /// Collect functions with definitions in Module \p M.
   void collectFuncsFrom(const Module &M) override;
+
+  /// Return whether names in the profile are all MD5 numbers.
+  virtual bool useMD5() {
+    assert(!NameTable.empty() && "NameTable should have been initialized");
+    return MD5Names && !MD5Names->empty();
+  }
 };
 
 class SampleProfileReaderCompactBinary : public SampleProfileReaderBinary {
 private:
   /// Function name table.
   std::vector<std::string> NameTable;
   /// The table mapping from function name to the offset of its FunctionSample
   /// towards file start.
   DenseMap<StringRef, uint64_t> FuncOffsetTable;
   /// The set containing the functions to use when compiling a module.
   DenseSet<StringRef> FuncsToUse;
   /// Use all functions from the input profile.
   bool UseAllFuncs = true;
   virtual std::error_code verifySPMagic(uint64_t Magic) override;
public:
   /// \brief Return true if \p Buffer is in the format supported by this class.
   static bool hasFormat(const MemoryBuffer &Buffer);
 
   /// Read samples only for functions to use.
   std::error_code readImpl() override;
 
   /// Collect functions to be used when compiling Module \p M.
   void collectFuncsFrom(const Module &M) override;
+
+  /// Return whether names in the profile are all MD5 numbers.
+  virtual bool useMD5() { return true; }
 };
 
 using InlineCallStack = SmallVector<FunctionSamples *, 10>;
 
 // Supported histogram types in GCC.  Currently, we only need support for
 // call target histograms.
 enum HistType {
   HIST_TYPE_INTERVAL,

llvm/include/llvm/ProfileData/SampleProfWriter.h

public:
   write(const StringMap<FunctionSamples> &ProfileMap) override;
 
   void setToCompressAllSections();
   void setToCompressSection(SecType Type);
 
 protected:
   uint64_t markSectionStart(SecType Type);
   std::error_code addNewSection(SecType Sec, uint64_t SectionStart);
+  void addSectionFlags(SecType Type, SecFlags Flags);
   virtual void initSectionHdrLayout() = 0;
   virtual std::error_code
   writeSections(const StringMap<FunctionSamples> &ProfileMap) = 0;
 
   // Specifiy the order of sections in section header table. Note
   // the order of sections in the profile may be different that the
   // order in SectionHdrLayout. sample Reader will follow the order
   // in SectionHdrLayout to read each section.
   SmallVector<SecHdrTableEntry, 8> SectionHdrLayout;
 
 private:
   void allocSecHdrTable();
   std::error_code writeSecHdrTable();
   virtual std::error_code
   writeHeader(const StringMap<FunctionSamples> &ProfileMap) override;
-  void addSectionFlags(SecType Type, SecFlags Flags);
   SecHdrTableEntry &getEntryInLayout(SecType Type);
   std::error_code compressAndOutput();
 
   // We will swap the raw_ostream held by LocalBufStream and that
   // held by OutputStream if we try to add a section which needs
   // compression. After the swap, all the data written to output
   // will be temporarily buffered into the underlying raw_string_ostream
   // originally held by LocalBufStream. After the data writing for the
SampleProfileWriterExtBinary(std::unique_ptr<raw_ostream> &OS)
     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.
+  void setUseMD5() {
+    UseMD5 = true;
+    addSectionFlags(SecNameTable, SecFlagMD5Name);
+  }
+
 private:
   virtual void initSectionHdrLayout() override {
     // Note that SecFuncOffsetTable section is written after SecLBRProfile
     // in the profile, but is put before SecLBRProfile in SectionHdrLayout.
     //
     // This is because sample reader follows the order of SectionHdrLayout to
     // read each section, to read function profiles on demand sample reader
     // need to get the offset of each function profile first.
     //
     // SecFuncOffsetTable section is written after SecLBRProfile in the
     // profile because FuncOffsetTable needs to be populated while section
     // SecLBRProfile is written.
     SectionHdrLayout = {{SecProfSummary, 0, 0, 0},
                         {SecNameTable, 0, 0, 0},
                         {SecFuncOffsetTable, 0, 0, 0},
                         {SecLBRProfile, 0, 0, 0},
                         {SecProfileSymbolList, 0, 0, 0}};
   };
   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;
-  std::error_code writeFuncOffsetTable();
+  // Whether to use MD5 to represent string.
+  bool UseMD5 = false;
 };
 
 // CompactBinary is a compact format of binary profile which both reduces
 // the profile size and the load time needed when compiling. It has two
 // major difference with Binary format.
 // 1. It represents all the strings in name table using md5 hash.
 // 2. It saves a function offset table which maps function name index to
 // the offset of its function profile to the start of the binary profile,

llvm/lib/ProfileData/SampleProf.cpp

 #include <system_error>
 
 using namespace llvm;
 using namespace sampleprof;
 
 namespace llvm {
 namespace sampleprof {
 SampleProfileFormat FunctionSamples::Format;
+bool FunctionSamples::UseMD5;
 } // namespace sampleprof
 } // namespace llvm
 
 namespace {
 
 // FIXME: This class is only here to support the transition to llvm::Error. It
 // will be removed once this transition is complete. Clients should prefer to
 // deal with the Error value directly, rather than converting to error_code.

llvm/lib/ProfileData/SampleProfReader.cpp

std::error_code SampleProfileReaderBinary::readImpl() {
   while (!at_eof()) {
     if (std::error_code EC = readFuncProfile(Data))
       return EC;
   }
 
   return sampleprof_error::success;
 }
 
-std::error_code
-SampleProfileReaderExtBinary::readOneSection(const uint8_t *Start,
-                                             uint64_t Size, SecType Type) {
+std::error_code SampleProfileReaderExtBinary::readOneSection(
+    const uint8_t *Start, uint64_t Size, const SecHdrTableEntry &Entry) {
   Data = Start;
   End = Start + Size;
-  switch (Type) {
+  switch (Entry.Type) {
   case SecProfSummary:
     if (std::error_code EC = readSummary())
       return EC;
     break;
   case SecNameTable:
-    if (std::error_code EC = readNameTable())
+    if (std::error_code EC =
+            readNameTableSec(hasSecFlag(Entry, SecFlagMD5Name)))
       return EC;
     break;
   case SecLBRProfile:
     if (std::error_code EC = readFuncProfiles())
       return EC;
     break;
   case SecProfileSymbolList:
     if (std::error_code EC = readProfileSymbolList())
std::error_code SampleProfileReaderExtBinary::readFuncProfiles() {
   }
 
   if (Remapper) {
     for (auto Name : FuncsToUse) {
       Remapper->insert(Name);
     }
   }
 
+  if (useMD5()) {
+    for (auto Name : FuncsToUse) {
+      auto GUID = std::to_string(MD5Hash(Name));
+      auto iter = FuncOffsetTable.find(StringRef(GUID));
+      if (iter == FuncOffsetTable.end())
+        continue;
+      const uint8_t *FuncProfileAddr = Start + iter->second;
+      assert(FuncProfileAddr < End && "out of LBRProfile section");
+      if (std::error_code EC = readFuncProfile(FuncProfileAddr))
+        return EC;
+    }
+  } else {
     for (auto NameOffset : FuncOffsetTable) {
       auto FuncName = NameOffset.first;
       if (!FuncsToUse.count(FuncName) &&
           (!Remapper || !Remapper->exist(FuncName)))
         continue;
       const uint8_t *FuncProfileAddr = Start + NameOffset.second;
       assert(FuncProfileAddr < End && "out of LBRProfile section");
       if (std::error_code EC = readFuncProfile(FuncProfileAddr))
         return EC;
     }
+  }
 
   Data = End;
   return sampleprof_error::success;
 }
 
 std::error_code SampleProfileReaderExtBinary::readProfileSymbolList() {
   if (!ProfSymList)
     ProfSymList = std::make_unique<ProfileSymbolList>();
if (isCompressed) {
       uint64_t DecompressBufSize;
       if (std::error_code EC = decompressSection(
               SecStart, SecSize, DecompressBuf, DecompressBufSize))
         return EC;
       SecStart = DecompressBuf;
       SecSize = DecompressBufSize;
     }
 
-    if (std::error_code EC = readOneSection(SecStart, SecSize, Entry.Type))
+    if (std::error_code EC = readOneSection(SecStart, SecSize, Entry))
       return EC;
     if (Data != SecStart + SecSize)
       return sampleprof_error::malformed;
 
     // Change the pointee of 'Data' from DecompressBuf to original Buffer.
     if (isCompressed) {
       Data = BufStart + Entry.Offset;
       End = BufStart + Buffer->getBufferSize();
for (uint32_t I = 0; I < *Size; ++I) {
     if (std::error_code EC = Name.getError())
       return EC;
     NameTable.push_back(*Name);
   }
 
   return sampleprof_error::success;
 }
 
+std::error_code SampleProfileReaderExtBinary::readMD5NameTable() {
+  auto Size = readNumber<uint64_t>();
+  if (std::error_code EC = Size.getError())
+    return EC;
+  NameTable.reserve(*Size);
+  MD5Names = std::make_unique<std::vector<std::string>>();
+  MD5Names->reserve(*Size);
+  for (uint32_t I = 0; I < *Size; ++I) {
+    auto FID = readNumber<uint64_t>();
+    if (std::error_code EC = FID.getError())
+      return EC;
+    MD5Names->push_back(std::to_string(*FID));
+    NameTable.push_back(MD5Names->back());
+  }
+  return sampleprof_error::success;
+}
+
+std::error_code SampleProfileReaderExtBinary::readNameTableSec(bool IsMD5) {
+  if (IsMD5)
+    return readMD5NameTable();
+  return SampleProfileReaderBinary::readNameTable();
+}
+
 std::error_code SampleProfileReaderCompactBinary::readNameTable() {
   auto Size = readNumber<uint64_t>();
   if (std::error_code EC = Size.getError())
     return EC;
   NameTable.reserve(*Size);
   for (uint32_t I = 0; I < *Size; ++I) {
     auto FID = readNumber<uint64_t>();
     if (std::error_code EC = FID.getError())

 }
 
 bool SampleProfileReaderGCC::hasFormat(const MemoryBuffer &Buffer) {
   StringRef Magic(reinterpret_cast<const char *>(Buffer.getBufferStart()));
   return Magic == "adcg*704";
 }
 
 void SampleProfileReaderItaniumRemapper::applyRemapping(LLVMContext &Ctx) {
-  // If the reader is in compact format, we can't remap it because
+  // If the reader uses MD5 to represent string, we can't remap it because
   // we don't know what the original function names were.
-  if (Reader.getFormat() == SPF_Compact_Binary) {
+  if (Reader.useMD5()) {
     Ctx.diagnose(DiagnosticInfoSampleProfile(
         Reader.getBuffer()->getBufferIdentifier(),
         "Profile data remapping cannot be applied to profile data "
         "in compact format (original mangled names are not available).",
         DS_Warning));
     return;
   }
 

llvm/lib/ProfileData/SampleProfWriter.cpp

std::error_code SampleProfileWriterExtBinary::writeFuncOffsetTable() {
   // Write out FuncOffsetTable.
   for (auto entry : FuncOffsetTable) {
     writeNameIdx(entry.first);
     encodeULEB128(entry.second, OS);
   }
   return sampleprof_error::success;
 }
 
+std::error_code SampleProfileWriterExtBinary::writeNameTable() {
+  if (!UseMD5)
+    return SampleProfileWriterBinary::writeNameTable();
+
+  auto &OS = *OutputStream;
+  std::set<StringRef> V;
+  stablizeNameTable(V);
+
+  // Write out the name table.
+  encodeULEB128(NameTable.size(), OS);
+  for (auto N : V) {
+    encodeULEB128(MD5Hash(N), OS);
+  }
+  return sampleprof_error::success;
+}
+
 std::error_code SampleProfileWriterExtBinary::writeSections(
     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;

llvm/lib/Transforms/IPO/SampleProfile.cpp

 };
 
 class GUIDToFuncNameMapper {
 public:
   GUIDToFuncNameMapper(Module &M, SampleProfileReader &Reader,
                         DenseMap<uint64_t, StringRef> &GUIDToFuncNameMap)
       : CurrentReader(Reader), CurrentModule(M),
       CurrentGUIDToFuncNameMap(GUIDToFuncNameMap) {
-    if (CurrentReader.getFormat() != SPF_Compact_Binary)
+    if (!CurrentReader.useMD5())
       return;
 
     for (const auto &F : CurrentModule) {
       StringRef OrigName = F.getName();
       CurrentGUIDToFuncNameMap.insert(
           {Function::getGUID(OrigName), OrigName});
 
       // Local to global var promotion used by optimization like thinlto
for (const auto &F : CurrentModule) {
             {Function::getGUID(CanonName), CanonName});
     }
 
     // Update GUIDToFuncNameMap for each function including inlinees.
     SetGUIDToFuncNameMapForAll(&CurrentGUIDToFuncNameMap);
   }
 
   ~GUIDToFuncNameMapper() {
-    if (CurrentReader.getFormat() != SPF_Compact_Binary)
+    if (!CurrentReader.useMD5())
       return;
 
     CurrentGUIDToFuncNameMap.clear();
 
     // Reset GUIDToFuncNameMap for of each function as they're no
     // longer valid at this point.
     SetGUIDToFuncNameMapForAll(nullptr);
   }

llvm/test/Transforms/SampleProfile/profile-format.ll

 ; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/inline.prof -S | FileCheck %s
 ; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/inline.prof -S | FileCheck %s
 ; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/inline.compactbinary.afdo -S | FileCheck %s
 ; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/inline.compactbinary.afdo -S | FileCheck %s
 ; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/inline.extbinary.afdo -S | FileCheck %s
 ; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/inline.extbinary.afdo -S | FileCheck %s
+; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/inline.md5extbinary.afdo -S | FileCheck %s
+; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/inline.md5extbinary.afdo -S | FileCheck %s
 
 ; Original C++ test case
 ;
 ; #include <stdio.h>
 ;
 ; int sum(int x, int y) {
 ;   return x + y;
 ; }

llvm/test/tools/llvm-profdata/roundtrip.test

 RUN: llvm-profdata merge --sample --extbinary -output=%t.3.profdata %S/Inputs/sample-profile.proftext
 RUN: llvm-profdata merge --sample --text -output=%t.3.proftext %t.3.profdata
 RUN: diff -b %t.3.proftext %S/Inputs/sample-profile.proftext
 # Round trip from text --> binary --> extbinary --> text
 RUN: llvm-profdata merge --sample --binary -output=%t.4.profdata %S/Inputs/sample-profile.proftext
 RUN: llvm-profdata merge --sample --extbinary -output=%t.5.profdata %t.4.profdata
 RUN: llvm-profdata merge --sample --text -output=%t.4.proftext %t.5.profdata
 RUN: diff -b %t.4.proftext %S/Inputs/sample-profile.proftext
+# Trip from text --> extbinary --> md5text
+# Trip from text --> compbinary --> md5text
+# Compare the two md5 texts
+RUN: llvm-profdata merge --sample --compbinary -output=%t.6.profdata %S/Inputs/sample-profile.proftext
+RUN: llvm-profdata merge --sample --text -output=%t.6.proftext %S/Inputs/sample-profile.proftext
+RUN: llvm-profdata merge --sample --extbinary -use-md5 -output=%t.7.profdata %S/Inputs/sample-profile.proftext
+RUN: llvm-profdata merge --sample --text -output=%t.7.proftext %S/Inputs/sample-profile.proftext
+RUN: diff -b %t.6.proftext %t.7.proftext

llvm/tools/llvm-profdata/llvm-profdata.cpp

static void populateProfileSymbolList(MemoryBuffer *Buffer,
   for (StringRef symbol : SymbolVec)
     PSL.add(symbol);
 }
 
 static void handleExtBinaryWriter(sampleprof::SampleProfileWriter &Writer,
                                   ProfileFormat OutputFormat,
                                   MemoryBuffer *Buffer,
                                   sampleprof::ProfileSymbolList &WriterList,
-                                  bool CompressAllSections) {
+                                  bool CompressAllSections, bool UseMD5) {
   populateProfileSymbolList(Buffer, WriterList);
   if (WriterList.size() > 0 && OutputFormat != PF_Ext_Binary)
     warn("Profile Symbol list is not empty but the output format is not "
          "ExtBinary format. The list will be lost in the output. ");
 
   Writer.setProfileSymbolList(&WriterList);
 
   if (CompressAllSections) {
     if (OutputFormat != PF_Ext_Binary) {
       warn("-compress-all-section is ignored. Specify -extbinary to enable it");
     } else {
       auto ExtBinaryWriter =
           static_cast<sampleprof::SampleProfileWriterExtBinary *>(&Writer);
       ExtBinaryWriter->setToCompressAllSections();
     }
   }
+  if (UseMD5) {
+    if (OutputFormat != PF_Ext_Binary) {
+      warn("-use-md5 is ignored. Specify -extbinary to enable it");
+    } else {
+      auto ExtBinaryWriter =
+          static_cast<sampleprof::SampleProfileWriterExtBinary *>(&Writer);
+      ExtBinaryWriter->setUseMD5();
+    }
+  }
 }
 
-static void mergeSampleProfile(const WeightedFileVector &Inputs,
-                               SymbolRemapper *Remapper,
-                               StringRef OutputFilename,
-                               ProfileFormat OutputFormat,
-                               StringRef ProfileSymbolListFile,
-                               bool CompressAllSections, FailureMode FailMode) {
+static void
+mergeSampleProfile(const WeightedFileVector &Inputs, SymbolRemapper *Remapper,
+                   StringRef OutputFilename, ProfileFormat OutputFormat,
+                   StringRef ProfileSymbolListFile, bool CompressAllSections,
+                   bool UseMD5, FailureMode FailMode) {
   using namespace sampleprof;
   StringMap<FunctionSamples> ProfileMap;
   SmallVector<std::unique_ptr<sampleprof::SampleProfileReader>, 5> Readers;
   LLVMContext Context;
   sampleprof::ProfileSymbolList WriterList;
   for (const auto &Input : Inputs) {
     auto ReaderOrErr = SampleProfileReader::create(Input.Filename, Context);
     if (std::error_code EC = ReaderOrErr.getError()) {
mergeSampleProfile(const WeightedFileVector &Inputs, SymbolRemapper *Remapper,
   if (std::error_code EC = WriterOrErr.getError())
     exitWithErrorCode(EC, OutputFilename);
 
   auto Writer = std::move(WriterOrErr.get());
   // WriterList will have StringRef refering to string in Buffer.
   // Make sure Buffer lives as long as WriterList.
   auto Buffer = getInputFileBuf(ProfileSymbolListFile);
   handleExtBinaryWriter(*Writer, OutputFormat, Buffer.get(), WriterList,
-                        CompressAllSections);
+                        CompressAllSections, UseMD5);
   Writer->write(ProfileMap);
 }
 
 static WeightedFile parseWeightedFile(const StringRef &WeightedFilename) {
   StringRef WeightStr, FileName;
   std::tie(WeightStr, FileName) = WeightedFilename.split(',');
 
   uint64_t Weight;
static int merge_main(int argc, const char *argv[]) {
   cl::opt<std::string> ProfileSymbolListFile(
       "prof-sym-list", cl::init(""),
       cl::desc("Path to file containing the list of function symbols "
                "used to populate profile symbol list"));
   cl::opt<bool> CompressAllSections(
       "compress-all-sections", cl::init(false), cl::Hidden,
       cl::desc("Compress all sections when writing the profile (only "
                "meaningful for -extbinary)"));
+  cl::opt<bool> UseMD5(
+      "use-md5", cl::init(false), cl::Hidden,
+      cl::desc("Choose to use MD5 to represent string in name table (only "
+               "meaningful for -extbinary)"));
 
   cl::ParseCommandLineOptions(argc, argv, "LLVM profile data merger\n");
 
   WeightedFileVector WeightedInputs;
   for (StringRef Filename : InputFilenames)
     addWeightedInput(WeightedInputs, {std::string(Filename), 1});
   for (StringRef WeightedFilename : WeightedInputFilenames)
     addWeightedInput(WeightedInputs, parseWeightedFile(WeightedFilename));
if (!RemappingFile.empty())
     Remapper = SymbolRemapper::create(RemappingFile);
 
   if (ProfileKind == instr)
     mergeInstrProfile(WeightedInputs, Remapper.get(), OutputFilename,
                       OutputFormat, OutputSparse, NumThreads, FailureMode);
   else
     mergeSampleProfile(WeightedInputs, Remapper.get(), OutputFilename,
                        OutputFormat, ProfileSymbolListFile, CompressAllSections,
-                       FailureMode);
+                       UseMD5, FailureMode);
 
   return 0;
 }
 
 /// Computer the overlap b/w profile BaseFilename and profile TestFilename.
 static void overlapInstrProfile(const std::string &BaseFilename,
                                 const std::string &TestFilename,
                                 const OverlapFuncFilters &FuncFilter,

llvm/unittests/ProfileData/SampleProfTest.cpp

*OS << R"(
       # Types 'int' and 'long' are equivalent
       type i l
       # Function names 'foo' and 'faux' are equivalent
       name 3foo 4faux
     )";
     OS->close();
   }
 
-  void testRoundTrip(SampleProfileFormat Format, bool Remap) {
+  void testRoundTrip(SampleProfileFormat Format, bool Remap, bool UseMD5) {
     SmallVector<char, 128> ProfilePath;
     ASSERT_TRUE(NoError(llvm::sys::fs::createTemporaryFile("profile", "", ProfilePath)));
     StringRef Profile(ProfilePath.data(), ProfilePath.size());
     createWriter(Format, Profile);
+    if (Format == SampleProfileFormat::SPF_Ext_Binary && UseMD5)
+      static_cast<SampleProfileWriterExtBinary *>(Writer.get())->setUseMD5();
 
     StringRef FooName("_Z3fooi");
     FunctionSamples FooSamples;
     FooSamples.setName(FooName);
     FooSamples.addTotalSamples(7711);
     FooSamples.addHeadSamples(610);
     FooSamples.addBodySamples(1, 0, 610);
     FooSamples.addBodySamples(2, 0, 600);
if (Format == SampleProfileFormat::SPF_Ext_Binary) {
       std::unique_ptr<ProfileSymbolList> ReaderList =
           Reader->getProfileSymbolList();
       ReaderList->contains("zoo");
       ReaderList->contains("moo");
     }
 
     FunctionSamples *ReadFooSamples = Reader->getSamplesFor(FooName);
     ASSERT_TRUE(ReadFooSamples != nullptr);
-    if (Format != SampleProfileFormat::SPF_Compact_Binary) {
+    if (!UseMD5) {
       ASSERT_EQ("_Z3fooi", ReadFooSamples->getName());
     }
     ASSERT_EQ(7711u, ReadFooSamples->getTotalSamples());
     ASSERT_EQ(610u, ReadFooSamples->getHeadSamples());
 
     FunctionSamples *ReadBarSamples = Reader->getSamplesFor(BarName);
     ASSERT_TRUE(ReadBarSamples != nullptr);
-    if (Format != SampleProfileFormat::SPF_Compact_Binary) {
+    if (!UseMD5) {
       ASSERT_EQ("_Z3bari", ReadBarSamples->getName());
     }
     ASSERT_EQ(20301u, ReadBarSamples->getTotalSamples());
     ASSERT_EQ(1437u, ReadBarSamples->getHeadSamples());
     ErrorOr<SampleRecord::CallTargetMap> CTMap =
         ReadBarSamples->findCallTargetMapAt(1, 0);
     ASSERT_FALSE(CTMap.getError());
 
void testRoundTrip(SampleProfileFormat Format, bool Remap, bool UseMD5) {
     }
 
     FunctionSamples *ReadBooSamples = Reader->getSamplesFor(BooName);
     ASSERT_TRUE(ReadBooSamples != nullptr);
     ASSERT_EQ(1232u, ReadBooSamples->getTotalSamples());
 
     std::string MconstructGUID;
     StringRef MconstructRep =
-        getRepInFormat(MconstructName, Format, MconstructGUID);
+        getRepInFormat(MconstructName, UseMD5, MconstructGUID);
     std::string StringviewGUID;
     StringRef StringviewRep =
-        getRepInFormat(StringviewName, Format, StringviewGUID);
+        getRepInFormat(StringviewName, UseMD5, StringviewGUID);
     ASSERT_EQ(1000u, CTMap.get()[MconstructRep]);
     ASSERT_EQ(437u, CTMap.get()[StringviewRep]);
 
     auto VerifySummary = [](ProfileSummary &Summary) mutable {
       ASSERT_EQ(ProfileSummary::PSK_Sample, Summary.getKind());
       ASSERT_EQ(137392u, Summary.getTotalCount());
       ASSERT_EQ(8u, Summary.getNumCounts());
       ASSERT_EQ(4u, Summary.getNumFunctions());
for (auto I = Expected.begin(); I != Expected.end(); ++I) {
       if (Samples != nullptr)
         Esamples = Samples->getTotalSamples();
       ASSERT_EQ(I->getValue(), Esamples);
     }
   }
 };
 
 TEST_F(SampleProfTest, roundtrip_text_profile) {
-  testRoundTrip(SampleProfileFormat::SPF_Text, false);
+  testRoundTrip(SampleProfileFormat::SPF_Text, false, false);
 }
 
 TEST_F(SampleProfTest, roundtrip_raw_binary_profile) {
-  testRoundTrip(SampleProfileFormat::SPF_Binary, false);
+  testRoundTrip(SampleProfileFormat::SPF_Binary, false, false);
 }
 
 TEST_F(SampleProfTest, roundtrip_compact_binary_profile) {
-  testRoundTrip(SampleProfileFormat::SPF_Compact_Binary, false);
+  testRoundTrip(SampleProfileFormat::SPF_Compact_Binary, false, true);
 }
 
 TEST_F(SampleProfTest, roundtrip_ext_binary_profile) {
-  testRoundTrip(SampleProfileFormat::SPF_Ext_Binary, false);
+  testRoundTrip(SampleProfileFormat::SPF_Ext_Binary, false, false);
+}
+
+TEST_F(SampleProfTest, roundtrip_md5_ext_binary_profile) {
+  testRoundTrip(SampleProfileFormat::SPF_Ext_Binary, false, true);
 }
 
 TEST_F(SampleProfTest, remap_text_profile) {
-  testRoundTrip(SampleProfileFormat::SPF_Text, true);
+  testRoundTrip(SampleProfileFormat::SPF_Text, true, false);
 }
 
 TEST_F(SampleProfTest, remap_raw_binary_profile) {
-  testRoundTrip(SampleProfileFormat::SPF_Binary, true);
+  testRoundTrip(SampleProfileFormat::SPF_Binary, true, false);
 }
 
 TEST_F(SampleProfTest, remap_ext_binary_profile) {
-  testRoundTrip(SampleProfileFormat::SPF_Ext_Binary, true);
+  testRoundTrip(SampleProfileFormat::SPF_Ext_Binary, true, false);
 }
 
 TEST_F(SampleProfTest, sample_overflow_saturation) {
   const uint64_t Max = std::numeric_limits<uint64_t>::max();
   sampleprof_error Result;
 
   FunctionSamples FooSamples;
   Result = FooSamples.addTotalSamples(1);