[NFC] In InstrProf, generalize helper functions to take 'GlobalObject'. They currently take 'Functions' as function parameters or have 'Func' in the name. (llvm#70287)

mingmingl-llvm · web-flow · commit a1e9777b760f · 2023-10-26T14:48:36.000-07:00
- For instance, `collectPGOFuncNameStrings` is reused a lot in llvm#66825 to get the compressed vtable names; and in some added callsites it's just confusing to see 'func' since context clearly shows it's not. This function currently just takes a list of strings as input so name it to `collectGlobalObjectNameStrings` - Do the rename in a standalone patch since the method is used in non-llvm codebase. It's easier to rollback this NFC in case rename in that codebase takes longer.
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -220,20 +220,23 @@ StringRef getPGOFuncNameVarInitializer(GlobalVariable *NameVar);
 StringRef getFuncNameWithoutPrefix(StringRef PGOFuncName,
                                    StringRef FileName = "<unknown>");
 
-/// Given a vector of strings (function PGO names) \c NameStrs, the
-/// method generates a combined string \c Result that is ready to be
-/// serialized.  The \c Result string is comprised of three fields:
-/// The first field is the length of the uncompressed strings, and the
-/// the second field is the length of the zlib-compressed string.
-/// Both fields are encoded in ULEB128.  If \c doCompress is false, the
+/// Given a vector of strings (names of global objects like functions or,
+/// virtual tables) \c NameStrs, the method generates a combined string \c
+/// Result that is ready to be serialized.  The \c Result string is comprised of
+/// three fields: The first field is the length of the uncompressed strings, and
+/// the the second field is the length of the zlib-compressed string. Both
+/// fields are encoded in ULEB128.  If \c doCompress is false, the
 ///  third field is the uncompressed strings; otherwise it is the
 /// compressed string. When the string compression is off, the
 /// second field will have value zero.
-Error collectPGOFuncNameStrings(ArrayRef<std::string> NameStrs,
-                                bool doCompression, std::string &Result);
+Error collectGlobalObjectNameStrings(ArrayRef<std::string> NameStrs,
+                                     bool doCompression, std::string &Result);
 
 /// Produce \c Result string with the same format described above. The input
 /// is vector of PGO function name variables that are referenced.
+/// The global variable element in 'NameVars' is a string containing the pgo
+/// name of a function. See `createPGOFuncNameVar` that creates these global
+/// variables.
 Error collectPGOFuncNameStrings(ArrayRef<GlobalVariable *> NameVars,
                                 std::string &Result, bool doCompression = true);
 
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
@@ -265,8 +265,8 @@ static StringRef stripDirPrefix(StringRef PathNameStr, uint32_t NumPrefix) {
   return PathNameStr.substr(LastPos);
 }
 
-static StringRef getStrippedSourceFileName(const Function &F) {
-  StringRef FileName(F.getParent()->getSourceFileName());
+static StringRef getStrippedSourceFileName(const GlobalObject &GO) {
+  StringRef FileName(GO.getParent()->getSourceFileName());
   uint32_t StripLevel = StaticFuncFullModulePrefix ? 0 : (uint32_t)-1;
   if (StripLevel < StaticFuncStripDirNamePrefix)
     StripLevel = StaticFuncStripDirNamePrefix;
@@ -289,64 +289,75 @@ static StringRef getStrippedSourceFileName(const Function &F) {
 // mangled, they cannot be passed to Mach-O linkers via -order_file. We still
 // need to compute this name to lookup functions from profiles built by older
 // compilers.
-static std::string getIRPGOFuncName(const Function &F,
-                                    GlobalValue::LinkageTypes Linkage,
-                                    StringRef FileName) {
+static std::string
+getIRPGONameForGlobalObject(const GlobalObject &GO,
+                            GlobalValue::LinkageTypes Linkage,
+                            StringRef FileName) {
   SmallString<64> Name;
   if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
     Name.append(FileName.empty() ? "<unknown>" : FileName);
     Name.append(";");
   }
-  Mangler().getNameWithPrefix(Name, &F, /*CannotUsePrivateLabel=*/true);
+  Mangler().getNameWithPrefix(Name, &GO, /*CannotUsePrivateLabel=*/true);
   return Name.str().str();
 }
 
-static std::optional<std::string> lookupPGOFuncName(const Function &F) {
-  if (MDNode *MD = getPGOFuncNameMetadata(F)) {
+static std::optional<std::string> lookupPGONameFromMetadata(MDNode *MD) {
+  if (MD != nullptr) {
     StringRef S = cast<MDString>(MD->getOperand(0))->getString();
     return S.str();
   }
   return {};
 }
 
-// See getPGOFuncName()
-std::string getIRPGOFuncName(const Function &F, bool InLTO) {
+// Returns the PGO object name. This function has some special handling
+// when called in LTO optimization. The following only applies when calling in
+// LTO passes (when \c InLTO is true): LTO's internalization privatizes many
+// global linkage symbols. This happens after value profile annotation, but
+// those internal linkage functions should not have a source prefix.
+// Additionally, for ThinLTO mode, exported internal functions are promoted
+// and renamed. We need to ensure that the original internal PGO name is
+// used when computing the GUID that is compared against the profiled GUIDs.
+// To differentiate compiler generated internal symbols from original ones,
+// PGOFuncName meta data are created and attached to the original internal
+// symbols in the value profile annotation step
+// (PGOUseFunc::annotateIndirectCallSites). If a symbol does not have the meta
+// data, its original linkage must be non-internal.
+static std::string getIRPGOObjectName(const GlobalObject &GO, bool InLTO,
+                                      MDNode *PGONameMetadata) {
   if (!InLTO) {
-    auto FileName = getStrippedSourceFileName(F);
-    return getIRPGOFuncName(F, F.getLinkage(), FileName);
+    auto FileName = getStrippedSourceFileName(GO);
+    return getIRPGONameForGlobalObject(GO, GO.getLinkage(), FileName);
   }
 
   // In LTO mode (when InLTO is true), first check if there is a meta data.
-  if (auto IRPGOFuncName = lookupPGOFuncName(F))
+  if (auto IRPGOFuncName = lookupPGONameFromMetadata(PGONameMetadata))
     return *IRPGOFuncName;
 
   // If there is no meta data, the function must be a global before the value
   // profile annotation pass. Its current linkage may be internal if it is
   // internalized in LTO mode.
-  return getIRPGOFuncName(F, GlobalValue::ExternalLinkage, "");
+  return getIRPGONameForGlobalObject(GO, GlobalValue::ExternalLinkage, "");
 }
 
-// Return the PGOFuncName. This function has some special handling when called
-// in LTO optimization. The following only applies when calling in LTO passes
-// (when \c InLTO is true): LTO's internalization privatizes many global linkage
-// symbols. This happens after value profile annotation, but those internal
-// linkage functions should not have a source prefix.
-// Additionally, for ThinLTO mode, exported internal functions are promoted
-// and renamed. We need to ensure that the original internal PGO name is
-// used when computing the GUID that is compared against the profiled GUIDs.
-// To differentiate compiler generated internal symbols from original ones,
-// PGOFuncName meta data are created and attached to the original internal
-// symbols in the value profile annotation step
-// (PGOUseFunc::annotateIndirectCallSites). If a symbol does not have the meta
-// data, its original linkage must be non-internal.
+// Returns the IRPGO function name and does special handling when called
+// in LTO optimization. See the comments of `getIRPGOObjectName` for details.
+std::string getIRPGOFuncName(const Function &F, bool InLTO) {
+  return getIRPGOObjectName(F, InLTO, getPGOFuncNameMetadata(F));
+}
+
+// This is similar to `getIRPGOFuncName` except that this function calls
+// 'getPGOFuncName' to get a name and `getIRPGOFuncName` calls
+// 'getIRPGONameForGlobalObject'. See the difference between two callees in the
+// comments of `getIRPGONameForGlobalObject`.
 std::string getPGOFuncName(const Function &F, bool InLTO, uint64_t Version) {
   if (!InLTO) {
     auto FileName = getStrippedSourceFileName(F);
     return getPGOFuncName(F.getName(), F.getLinkage(), FileName, Version);
   }
 
   // In LTO mode (when InLTO is true), first check if there is a meta data.
-  if (auto PGOFuncName = lookupPGOFuncName(F))
+  if (auto PGOFuncName = lookupPGONameFromMetadata(getPGOFuncNameMetadata(F)))
     return *PGOFuncName;
 
   // If there is no meta data, the function must be a global before the value
@@ -494,8 +505,8 @@ void InstrProfSymtab::dumpNames(raw_ostream &OS) const {
     OS << S << '\n';
 }
 
-Error collectPGOFuncNameStrings(ArrayRef<std::string> NameStrs,
-                                bool doCompression, std::string &Result) {
+Error collectGlobalObjectNameStrings(ArrayRef<std::string> NameStrs,
+                                     bool doCompression, std::string &Result) {
   assert(!NameStrs.empty() && "No name data to emit");
 
   uint8_t Header[20], *P = Header;
@@ -545,7 +556,7 @@ Error collectPGOFuncNameStrings(ArrayRef<GlobalVariable *> NameVars,
   for (auto *NameVar : NameVars) {
     NameStrs.push_back(std::string(getPGOFuncNameVarInitializer(NameVar)));
   }
-  return collectPGOFuncNameStrings(
+  return collectGlobalObjectNameStrings(
       NameStrs, compression::zlib::isAvailable() && doCompression, Result);
 }
 
diff --git a/llvm/lib/ProfileData/InstrProfCorrelator.cpp b/llvm/lib/ProfileData/InstrProfCorrelator.cpp
@@ -152,7 +152,7 @@ Error InstrProfCorrelatorImpl<IntPtrT>::correlateProfileData(int MaxWarnings) {
         instrprof_error::unable_to_correlate_profile,
         "could not find any profile metadata in debug info");
   auto Result =
-      collectPGOFuncNameStrings(NamesVec, /*doCompression=*/false, Names);
+      collectGlobalObjectNameStrings(NamesVec, /*doCompression=*/false, Names);
   CounterOffsets.clear();
   NamesVec.clear();
   return Result;
diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -1368,15 +1368,15 @@ TEST_P(MaybeSparseInstrProfTest, instr_prof_symtab_compression_test) {
   for (bool DoCompression : {false, true}) {
     // Compressing:
     std::string FuncNameStrings1;
-    EXPECT_THAT_ERROR(collectPGOFuncNameStrings(
+    EXPECT_THAT_ERROR(collectGlobalObjectNameStrings(
                           FuncNames1,
                           (DoCompression && compression::zlib::isAvailable()),
                           FuncNameStrings1),
                       Succeeded());
 
     // Compressing:
     std::string FuncNameStrings2;
-    EXPECT_THAT_ERROR(collectPGOFuncNameStrings(
+    EXPECT_THAT_ERROR(collectGlobalObjectNameStrings(
                           FuncNames2,
                           (DoCompression && compression::zlib::isAvailable()),
                           FuncNameStrings2),
