[llvm] [NFC]Rename collectPGOFuncNameStrings to collectGlobalObjectNameStrings (PR #70287)

Mingming Liu via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 26 12:40:58 PDT 2023


https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/70287

>From 04ead27f0dd8e0e444187abe8a88163ebd70df73 Mon Sep 17 00:00:00 2001
From: Mingming Liu <mingmingl at google.com>
Date: Wed, 25 Oct 2023 21:05:14 -0700
Subject: [PATCH 1/5] [NFC]Rename collectPGOFuncNameStrings to
 collectGlobalVariableNameStrings

- 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.
---
 llvm/include/llvm/ProfileData/InstrProf.h    | 8 ++++----
 llvm/lib/ProfileData/InstrProf.cpp           | 2 +-
 llvm/unittests/ProfileData/InstrProfTest.cpp | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index f9096b46157200b..7832b4fbf35cc71 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -220,8 +220,8 @@ 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
+/// Given a vector of strings (names of global variables) \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.
@@ -229,8 +229,8 @@ StringRef getFuncNameWithoutPrefix(StringRef PGOFuncName,
 ///  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 collectGlobalVariableNameStrings(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.
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index e82cb5c535f1f5a..963d4703395b35b 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -545,7 +545,7 @@ Error collectPGOFuncNameStrings(ArrayRef<GlobalVariable *> NameVars,
   for (auto *NameVar : NameVars) {
     NameStrs.push_back(std::string(getPGOFuncNameVarInitializer(NameVar)));
   }
-  return collectPGOFuncNameStrings(
+  return collectGlobalVariableNameStrings(
       NameStrs, compression::zlib::isAvailable() && doCompression, Result);
 }
 
diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp
index 76f88c058424d03..378795bd724b588 100644
--- a/llvm/unittests/ProfileData/InstrProfTest.cpp
+++ b/llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -1368,7 +1368,7 @@ TEST_P(MaybeSparseInstrProfTest, instr_prof_symtab_compression_test) {
   for (bool DoCompression : {false, true}) {
     // Compressing:
     std::string FuncNameStrings1;
-    EXPECT_THAT_ERROR(collectPGOFuncNameStrings(
+    EXPECT_THAT_ERROR(collectGlobalVariableNameStrings(
                           FuncNames1,
                           (DoCompression && compression::zlib::isAvailable()),
                           FuncNameStrings1),
@@ -1376,7 +1376,7 @@ TEST_P(MaybeSparseInstrProfTest, instr_prof_symtab_compression_test) {
 
     // Compressing:
     std::string FuncNameStrings2;
-    EXPECT_THAT_ERROR(collectPGOFuncNameStrings(
+    EXPECT_THAT_ERROR(collectGlobalVariableNameStrings(
                           FuncNames2,
                           (DoCompression && compression::zlib::isAvailable()),
                           FuncNameStrings2),

>From 001790783c558f597e8d4abcce46d9e68658f906 Mon Sep 17 00:00:00 2001
From: Mingming Liu <mingmingl at google.com>
Date: Wed, 25 Oct 2023 21:14:49 -0700
Subject: [PATCH 2/5] Address comments and update another callsite (in
 InstrProfCorrelator.cpp) that calls this function.

---
 llvm/include/llvm/ProfileData/InstrProf.h    | 4 ++--
 llvm/lib/ProfileData/InstrProf.cpp           | 6 +++---
 llvm/lib/ProfileData/InstrProfCorrelator.cpp | 2 +-
 llvm/unittests/ProfileData/InstrProfTest.cpp | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index 7832b4fbf35cc71..a92e82b446fca76 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -229,8 +229,8 @@ StringRef getFuncNameWithoutPrefix(StringRef PGOFuncName,
 ///  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 collectGlobalVariableNameStrings(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.
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index 963d4703395b35b..db9b111d37c2fc3 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -494,8 +494,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 +545,7 @@ Error collectPGOFuncNameStrings(ArrayRef<GlobalVariable *> NameVars,
   for (auto *NameVar : NameVars) {
     NameStrs.push_back(std::string(getPGOFuncNameVarInitializer(NameVar)));
   }
-  return collectGlobalVariableNameStrings(
+  return collectGlobalObjectNameStrings(
       NameStrs, compression::zlib::isAvailable() && doCompression, Result);
 }
 
diff --git a/llvm/lib/ProfileData/InstrProfCorrelator.cpp b/llvm/lib/ProfileData/InstrProfCorrelator.cpp
index 71787c9bd8577b4..f298fcab1220cfd 100644
--- 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
index 378795bd724b588..4e1786597b39674 100644
--- a/llvm/unittests/ProfileData/InstrProfTest.cpp
+++ b/llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -1368,7 +1368,7 @@ TEST_P(MaybeSparseInstrProfTest, instr_prof_symtab_compression_test) {
   for (bool DoCompression : {false, true}) {
     // Compressing:
     std::string FuncNameStrings1;
-    EXPECT_THAT_ERROR(collectGlobalVariableNameStrings(
+    EXPECT_THAT_ERROR(collectGlobalObjectNameStrings(
                           FuncNames1,
                           (DoCompression && compression::zlib::isAvailable()),
                           FuncNameStrings1),
@@ -1376,7 +1376,7 @@ TEST_P(MaybeSparseInstrProfTest, instr_prof_symtab_compression_test) {
 
     // Compressing:
     std::string FuncNameStrings2;
-    EXPECT_THAT_ERROR(collectGlobalVariableNameStrings(
+    EXPECT_THAT_ERROR(collectGlobalObjectNameStrings(
                           FuncNames2,
                           (DoCompression && compression::zlib::isAvailable()),
                           FuncNameStrings2),

>From 569b70a87f0f7831042240451d7f2db3cf5bde37 Mon Sep 17 00:00:00 2001
From: Mingming Liu <mingmingl at google.com>
Date: Wed, 25 Oct 2023 21:54:44 -0700
Subject: [PATCH 3/5] update function comment

---
 llvm/include/llvm/ProfileData/InstrProf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index a92e82b446fca76..61f9bae1a83f775 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -220,7 +220,7 @@ StringRef getPGOFuncNameVarInitializer(GlobalVariable *NameVar);
 StringRef getFuncNameWithoutPrefix(StringRef PGOFuncName,
                                    StringRef FileName = "<unknown>");
 
-/// Given a vector of strings (names of global variables) \c NameStrs,
+/// Given a vector of strings (names of global objects, like func PGO names, virtual tables, etc) \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

>From be264c173e293ee06866492ef41edcde33d8ff79 Mon Sep 17 00:00:00 2001
From: Mingming Liu <mingmingl at google.com>
Date: Wed, 25 Oct 2023 22:22:29 -0700
Subject: [PATCH 4/5] run clang-format

---
 llvm/include/llvm/ProfileData/InstrProf.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index 61f9bae1a83f775..39f33b8363332d2 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -220,12 +220,12 @@ StringRef getPGOFuncNameVarInitializer(GlobalVariable *NameVar);
 StringRef getFuncNameWithoutPrefix(StringRef PGOFuncName,
                                    StringRef FileName = "<unknown>");
 
-/// Given a vector of strings (names of global objects, like func PGO names, virtual tables, etc) \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 func PGO names,
+/// virtual tables, etc) \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.

>From 9a96d611d01c219c2b345139941935df770d83dc Mon Sep 17 00:00:00 2001
From: Mingming Liu <mingmingl at google.com>
Date: Thu, 26 Oct 2023 12:07:08 -0700
Subject: [PATCH 5/5] 1. In InstrProf.cpp, change two static functions
 (function name or argument type) to use them for general global objects
 (functions, vtable objects) 2. In InstrProf.h, add a function comment around
 the element in NameVars.

---
 llvm/include/llvm/ProfileData/InstrProf.h |  3 +++
 llvm/lib/ProfileData/InstrProf.cpp        | 17 +++++++++--------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index 39f33b8363332d2..b751ac8bd6e95dc 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -234,6 +234,9 @@ Error collectGlobalObjectNameStrings(ArrayRef<std::string> NameStrs,
 
 /// 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
index db9b111d37c2fc3..b46e20938725cd6 100644
--- 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,15 +289,16 @@ 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();
 }
 
@@ -313,7 +314,7 @@ static std::optional<std::string> lookupPGOFuncName(const Function &F) {
 std::string getIRPGOFuncName(const Function &F, bool InLTO) {
   if (!InLTO) {
     auto FileName = getStrippedSourceFileName(F);
-    return getIRPGOFuncName(F, F.getLinkage(), FileName);
+    return getIRPGONameForGlobalObject(F, F.getLinkage(), FileName);
   }
 
   // In LTO mode (when InLTO is true), first check if there is a meta data.
@@ -323,7 +324,7 @@ std::string getIRPGOFuncName(const Function &F, bool InLTO) {
   // 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(F, GlobalValue::ExternalLinkage, "");
 }
 
 // Return the PGOFuncName. This function has some special handling when called



More information about the llvm-commits mailing list