Revert "Reland "[PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. ""

compiler-rt/test/profile/CMakeLists.txt

@@ -6,7 +6,7 @@ set(PROFILE_TESTSUITES)
 set(PROFILE_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS} compiler-rt-headers)
 list(APPEND PROFILE_TEST_DEPS profile)
 if(NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND PROFILE_TEST_DEPS llvm-cov llvm-dis llvm-lto llvm-profdata opt)
+  list(APPEND PROFILE_TEST_DEPS llvm-profdata llvm-cov)
   if(NOT APPLE AND COMPILER_RT_HAS_LLD AND "lld" IN_LIST LLVM_ENABLE_PROJECTS)
     list(APPEND PROFILE_TEST_DEPS lld)
   endif()

llvm/include/llvm/IR/GlobalValue.h

@@ -41,10 +41,6 @@ namespace Intrinsic {
 typedef unsigned ID;
 } // end namespace Intrinsic
 
-// Choose ';' as the delimiter. ':' was used once but it doesn't work well for
-// Objective-C functions which commonly have :'s in their names.
-inline constexpr char kGlobalIdentifierDelimiter = ';';
-
 class GlobalValue : public Constant {
 public:
   /// An enumeration for the kinds of linkage for global values.

llvm/include/llvm/ProfileData/InstrProf.h

@@ -171,8 +171,6 @@ inline StringRef getInstrProfCounterBiasVarName() {
 /// Return the marker used to separate PGO names during serialization.
 inline StringRef getInstrProfNameSeparator() { return "\01"; }
 
-/// Please use getIRPGOFuncName for LLVM IR instrumentation. This function is
-/// for front-end (Clang, etc) instrumentation.
 /// Return the modified name for function \c F suitable to be
 /// used the key for profile lookup. Variable \c InLTO indicates if this
 /// is called in LTO optimization passes.
@@ -198,22 +196,20 @@ std::string getIRPGOFuncName(const Function &F, bool InLTO = false);
 std::pair<StringRef, StringRef> getParsedIRPGOFuncName(StringRef IRPGOFuncName);
 
 /// Return the name of the global variable used to store a function
-/// name in PGO instrumentation. \c FuncName is the IRPGO function name
-/// (returned by \c getIRPGOFuncName) for LLVM IR instrumentation and PGO
-/// function name (returned by \c getPGOFuncName) for front-end instrumentation.
+/// name in PGO instrumentation. \c FuncName is the name of the function
+/// returned by the \c getPGOFuncName call.
 std::string getPGOFuncNameVarName(StringRef FuncName,
                                   GlobalValue::LinkageTypes Linkage);
 
 /// Create and return the global variable for function name used in PGO
-/// instrumentation. \c FuncName is the IRPGO function name (returned by
-/// \c getIRPGOFuncName) for LLVM IR instrumentation and PGO function name
-/// (returned by \c getPGOFuncName) for front-end instrumentation.
+/// instrumentation. \c FuncName is the name of the function returned
+/// by \c getPGOFuncName call.
 GlobalVariable *createPGOFuncNameVar(Function &F, StringRef PGOFuncName);
 
 /// Create and return the global variable for function name used in PGO
-/// instrumentation. \c FuncName is the IRPGO function name (returned by
-/// \c getIRPGOFuncName) for LLVM IR instrumentation and PGO function name
-/// (returned by \c getPGOFuncName) for front-end instrumentation.
+/// instrumentation.  /// \c FuncName is the name of the function
+/// returned by \c getPGOFuncName call, \c M is the owning module,
+/// and \c Linkage is the linkage of the instrumented function.
 GlobalVariable *createPGOFuncNameVar(Module &M,
                                      GlobalValue::LinkageTypes Linkage,
                                      StringRef PGOFuncName);
@@ -421,11 +417,11 @@ uint64_t ComputeHash(StringRef K);
 
 } // end namespace IndexedInstrProf
 
-/// A symbol table used for function [IR]PGO name look-up with keys
+/// A symbol table used for function PGO name look-up with keys
 /// (such as pointers, md5hash values) to the function. A function's
-/// [IR]PGO name or name's md5hash are used in retrieving the profile
-/// data of the function. See \c getIRPGOFuncName() and \c getPGOFuncName
-/// methods for details how [IR]PGO name is formed.
+/// PGO name or name's md5hash are used in retrieving the profile
+/// data of the function. See \c getPGOFuncName() method for details
+/// on how PGO name is formed.
 class InstrProfSymtab {
 public:
   using AddrHashMap = std::vector<std::pair<uint64_t, uint64_t>>;

llvm/lib/IR/Globals.cpp

@@ -144,27 +144,25 @@ void GlobalObject::copyAttributesFrom(const GlobalObject *Src) {
 std::string GlobalValue::getGlobalIdentifier(StringRef Name,
                                              GlobalValue::LinkageTypes Linkage,
                                              StringRef FileName) {
+
   // Value 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 (Name[0] == '\1')
     Name = Name.substr(1);
 
-  std::string GlobalName;
+  std::string NewName = std::string(Name);
   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())
-      GlobalName += "<unknown>";
+      NewName = NewName.insert(0, "<unknown>:");
     else
-      GlobalName += FileName;
-
-    GlobalName += kGlobalIdentifierDelimiter;
+      NewName = NewName.insert(0, FileName.str() + ":");
   }
-  GlobalName += Name;
-  return GlobalName;
+  return NewName;
 }
 
 std::string GlobalValue::getGlobalIdentifier() const {

llvm/lib/ProfileData/InstrProf.cpp

@@ -246,27 +246,11 @@ std::string InstrProfError::message() const {
 
 char InstrProfError::ID = 0;
 
-std::string getPGOFuncName(StringRef Name, GlobalValue::LinkageTypes Linkage,
+std::string getPGOFuncName(StringRef RawFuncName,
+                           GlobalValue::LinkageTypes Linkage,
                            StringRef FileName,
                            uint64_t Version LLVM_ATTRIBUTE_UNUSED) {
-  // Value 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 (Name[0] == '\1')
-    Name = Name.substr(1);
-
-  std::string NewName = std::string(Name);
-  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())
-      NewName = NewName.insert(0, "<unknown>:");
-    else
-      NewName = NewName.insert(0, FileName.str() + ":");
-  }
-  return NewName;
+  return GlobalValue::getGlobalIdentifier(RawFuncName, Linkage, FileName);
 }
 
 // Strip NumPrefix level of directory name from PathNameStr. If the number of
@@ -316,10 +300,12 @@ getIRPGONameForGlobalObject(const GlobalObject &GO,
                             GlobalValue::LinkageTypes Linkage,
                             StringRef FileName) {
   SmallString<64> Name;
-  // FIXME: Mangler's handling is kept outside of `getGlobalIdentifier` for now.
-  // For more details please check issue #74565.
+  if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
+    Name.append(FileName.empty() ? "<unknown>" : FileName);
+    Name.append(";");
+  }
   Mangler().getNameWithPrefix(Name, &GO, /*CannotUsePrivateLabel=*/true);
-  return GlobalValue::getGlobalIdentifier(Name, Linkage, FileName);
+  return Name.str().str();
 }
 
 static std::optional<std::string> lookupPGONameFromMetadata(MDNode *MD) {
@@ -366,9 +352,6 @@ std::string getIRPGOFuncName(const Function &F, bool InLTO) {
   return getIRPGOObjectName(F, InLTO, getPGOFuncNameMetadata(F));
 }
 
-// Please use getIRPGOFuncName for LLVM IR instrumentation. This function is
-// for front-end (Clang, etc) instrumentation.
-// The implementation is kept for profile matching from older profiles.
 // 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
@@ -401,8 +384,7 @@ getParsedIRPGOFuncName(StringRef IRPGOFuncName) {
 StringRef getFuncNameWithoutPrefix(StringRef PGOFuncName, StringRef FileName) {
   if (FileName.empty())
     return PGOFuncName;
-  // Drop the file name including ':' or ';'. See getIRPGONameForGlobalObject as
-  // well.
+  // Drop the file name including ':'. See also getPGOFuncName.
   if (PGOFuncName.starts_with(FileName))
     PGOFuncName = PGOFuncName.drop_front(FileName.size() + 1);
   return PGOFuncName;

llvm/lib/ProfileData/InstrProfReader.cpp

@@ -1008,13 +1008,12 @@ class llvm::InstrProfReaderItaniumRemapper
 
   /// Extract the original function name from a PGO function name.
   static StringRef extractName(StringRef Name) {
-    // We can have multiple pieces separated by kGlobalIdentifierDelimiter (
-    // semicolon now and colon in older profiles); there can be pieces both
-    // before and after the mangled name. Find the first part that starts with
-    // '_Z'; we'll assume that's the mangled name we want.
+    // We can have multiple :-separated pieces; there can be pieces both
+    // before and after the mangled name. Find the first part that starts
+    // with '_Z'; we'll assume that's the mangled name we want.
     std::pair<StringRef, StringRef> Parts = {StringRef(), Name};
     while (true) {
-      Parts = Parts.second.split(kGlobalIdentifierDelimiter);
+      Parts = Parts.second.split(':');
       if (Parts.first.starts_with("_Z"))
         return Parts.first;
       if (Parts.second.empty())

llvm/test/Bitcode/thinlto-function-summary-originalnames.ll

@@ -6,13 +6,13 @@
 ; COMBINED:       <GLOBALVAL_SUMMARY_BLOCK
 ; COMBINED-NEXT:    <VERSION
 ; COMBINED-NEXT:    <FLAGS
-; COMBINED-NEXT:    <VALUE_GUID {{.*}} op1=686735765308251824/>
-; COMBINED-NEXT:    <VALUE_GUID {{.*}} op1=4507502870619175775/>
-; COMBINED-NEXT:    <VALUE_GUID {{.*}} op1=-8118561185538785069/>
+; COMBINED-NEXT:    <VALUE_GUID {{.*}} op1=4947176790635855146/>
+; COMBINED-NEXT:    <VALUE_GUID {{.*}} op1=-6591587165810580810/>
+; COMBINED-NEXT:    <VALUE_GUID {{.*}} op1=-4377693495213223786/>
 ; COMBINED-DAG:    <COMBINED_PROFILE{{ }}
-; COMBINED-DAG:    <COMBINED_ORIGINAL_NAME op0=-2012135647395072713/>
-; COMBINED-DAG:    <COMBINED_GLOBALVAR_INIT_REFS
 ; COMBINED-DAG:    <COMBINED_ORIGINAL_NAME op0=6699318081062747564/>
+; COMBINED-DAG:    <COMBINED_GLOBALVAR_INIT_REFS
+; COMBINED-DAG:    <COMBINED_ORIGINAL_NAME op0=-2012135647395072713/>
 ; COMBINED-DAG:    <COMBINED_ALIAS
 ; COMBINED-DAG:    <COMBINED_ORIGINAL_NAME op0=-4170563161550796836/>
 ; COMBINED-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>