Refactor PGO function naming and MD5 hashing support out of ProfileData

[tejohnson]

Summary

Move the function renaming logic into the Function class, and the
MD5Hash routine into the MD5 header.

This will enable these routines to be shared with ThinLTO, which
will be changed to store the MD5 hash instead of full function name
in the combined index for significant size reductions. And using the same
function naming for locals in the function index facilitates future
integration with indirect call value profiles.

[davidxl]

include/llvm/Support/MD5.h

70

Remove the 'static' keyword here (obviously it was there before but not cleaned up) -- to avoid leaving multiple copies of the function when inlining does not happen.

[tejohnson]

• Address David's comment.

[davidxl]

This one LGTM. I am also expecting another interface like 'getFunctionGuid' to wrap around the MD5Hash method..

[tejohnson]

In D17006#347035, @davidxl wrote:

This one LGTM. I am also expecting another interface like 'getFunctionGuid' to wrap around the MD5Hash method..

Do you mean to basically do MD5Hash(getGlobalIdentifier(...))? I could add that with the follow-on patch to use this in the ThinLTO function map. Although for now in the draft patch I am testing they are invoked in slightly different places (the MD5Hash is hidden within the FunctionInfoIndex methods which take a string, and getGlobalIdentifier is invoked as needed above that. And for PGO, it didn't look like MD5Hash and getPGOFuncName were invoked simultaneously.

include/llvm/IR/Function.h

/// @}
   void setSubprogram(DISubprogram *SP);
 
   /// \brief Get the attached subprogram.
   ///
   /// Calls \a getMetadata() with \a LLVMContext::MD_dbg and casts the result
   /// to \a DISubprogram.
   DISubprogram *getSubprogram() const;
 
+  /// Return the modified name for a function suitable to be
+  /// used as the key for a global lookup (e.g. profile or ThinLTO).
+  /// The function's original name is \c FuncName and has linkage of type
+  /// \c Linkage. The function is defined in module \c FileName.
+  static std::string getGlobalIdentifier(StringRef FuncName,
+                                         GlobalValue::LinkageTypes Linkage,
+                                         StringRef FileName);
+
 private:
   void allocHungoffUselist();
   template<int Idx> void setHungoffOperand(Constant *C);
 
   // Shadow Value::setValueSubclassData with a private forwarding method so that
   // subclasses cannot accidentally use it.
   void setValueSubclassData(unsigned short D) {
     Value::setValueSubclassData(D);

include/llvm/ProfileData/InstrProf.h

 namespace IndexedInstrProf {
 
 enum class HashT : uint32_t {
   MD5,
 
   Last = MD5
 };
 
-static inline uint64_t MD5Hash(StringRef Str) {
-  MD5 Hash;
-  Hash.update(Str);
-  llvm::MD5::MD5Result Result;
-  Hash.final(Result);
-  // Return the least significant 8 bytes. Our MD5 implementation returns the
-  // result in little endian, so we may need to swap bytes.
-  using namespace llvm::support;
-  return endian::read<uint64_t, little, unaligned>(Result);
-}
-
 inline uint64_t ComputeHash(HashT Type, StringRef K) {
   switch (Type) {
   case HashT::MD5:
-    return IndexedInstrProf::MD5Hash(K);
+    return MD5Hash(K);
   }
   llvm_unreachable("Unhandled hash type");
 }
 
 const uint64_t Magic = 0x8169666f72706cff; // "\xfflprofi\x81"
 
 enum ProfVersion {
   // Version 1 is the first version. In this version, the value of

include/llvm/Support/MD5.h

  */
 
 #ifndef LLVM_SUPPORT_MD5_H
 #define LLVM_SUPPORT_MD5_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/DataTypes.h"
+#include "llvm/Support/Endian.h"
 
 namespace llvm {
 
 class MD5 {
   // Any 32-bit or wider unsigned integer data type will do.
   typedef uint32_t MD5_u32plus;
 
   MD5_u32plus a, b, c, d;
public:
   /// \brief Translates the bytes in \p Res to a hex string that is
   /// deposited into \p Str. The result will be of length 32.
   static void stringifyResult(MD5Result &Result, SmallString<32> &Str);
 
 private:
   const uint8_t *body(ArrayRef<uint8_t> Data);
 };
 
+/// Helper to compute and return a 64-bit MD5 Hash of a given string.
+inline uint64_t MD5Hash(StringRef Str) {
+  MD5 Hash;
+  Hash.update(Str);
+  llvm::MD5::MD5Result Result;
+  Hash.final(Result);
+  // Return the least significant 8 bytes. Our MD5 implementation returns the
+  // result in little endian, so we may need to swap bytes.
+  using namespace llvm::support;
+  return endian::read<uint64_t, little, unaligned>(Result);
+}
+
 }
 
 #endif

lib/IR/Function.cpp

Optional<uint64_t> Function::getEntryCount() const {
   if (MD && MD->getOperand(0))
     if (MDString *MDS = dyn_cast<MDString>(MD->getOperand(0)))
       if (MDS->getString().equals("function_entry_count")) {
         ConstantInt *CI = mdconst::extract<ConstantInt>(MD->getOperand(1));
         return CI->getValue().getZExtValue();
       }
   return None;
 }
+
+std::string Function::getGlobalIdentifier(StringRef FuncName,
+                                          GlobalValue::LinkageTypes Linkage,
+                                          StringRef FileName) {
+
+  // Function names may be prefixed with a binary '1' to indicate
+  // that the backend should not modify the symbols due to any platform
+  // naming convention. Do not include that '1' in the PGO profile name.
+  if (FuncName[0] == '\1')
+    FuncName = FuncName.substr(1);
+
+  std::string NewFuncName = FuncName;
+  if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
+    // For local symbols, prepend the main file name to distinguish them.
+    // Do not include the full path in the file name since there's no guarantee
+    // that it will stay the same, e.g., if the files are checked out from
+    // version control in different locations.
+    if (FileName.empty())
+      NewFuncName = NewFuncName.insert(0, "<unknown>:");
+    else
+      NewFuncName = NewFuncName.insert(0, FileName.str() + ":");
+  }
+  return NewFuncName;
+}

lib/ProfileData/InstrProf.cpp

 }
 
 namespace llvm {
 
 std::string getPGOFuncName(StringRef RawFuncName,
                            GlobalValue::LinkageTypes Linkage,
                            StringRef FileName,
                            uint64_t Version LLVM_ATTRIBUTE_UNUSED) {
-
-  // Function names may be prefixed with a binary '1' to indicate
-  // that the backend should not modify the symbols due to any platform
-  // naming convention. Do not include that '1' in the PGO profile name.
-  if (RawFuncName[0] == '\1')
-    RawFuncName = RawFuncName.substr(1);
-
-  std::string FuncName = RawFuncName;
-  if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
-    // For local symbols, prepend the main file name to distinguish them.
-    // Do not include the full path in the file name since there's no guarantee
-    // that it will stay the same, e.g., if the files are checked out from
-    // version control in different locations.
-    if (FileName.empty())
-      FuncName = FuncName.insert(0, "<unknown>:");
-    else
-      FuncName = FuncName.insert(0, FileName.str() + ":");
-  }
-  return FuncName;
+  return Function::getGlobalIdentifier(RawFuncName, Linkage, FileName);
 }
 
 std::string getPGOFuncName(const Function &F, uint64_t Version) {
   return getPGOFuncName(F.getName(), F.getLinkage(), F.getParent()->getName(),
                         Version);
 }
 
 StringRef getFuncNameWithoutPrefix(StringRef PGOFuncName, StringRef FileName) {