[InstrProf] Encode linkage names in IRPGO counter names

[ellis]

Summary

Prior to this diff, names in the __llvm_prf_names section had the format [<filepath>:]<function-name>, e.g., main.cpp:foo, bar. <filepath> is used to discriminate between possibly identical function names when linkage is local and <function-name> simply comes from F.getName(). This has two problems:

• : is commonly found in Objective-C functions so that names like main.mm:-[C foo::] and -[C bar::] are difficult to parse
• <function-name> might be different from the linkage name, so it cannot be used to pass a function order to the linker via -symbol-ordering-file or -order_file (see https://discourse.llvm.org/t/rfc-temporal-profiling-extension-for-irpgo/68068)

Instead, this diff changes the format to [<filepath>;]<linkage-name>, e.g., main.cpp;_foo, _bar. The hope is that ; won't realistically be found in either <filepath> or <linkage-name>.

To prevent invalidating all prior IRPGO profiles, we also lookup the prior name format when a record is not found (see InstrProfSymtab::create(), readMemprof(), and getInstrProfRecord()). It seems that Swift and Clang FE-PGO rely on the original getPGOFuncName(), so we cannot simply replace it.

[davidxl]

llvm/lib/ProfileData/InstrProf.cpp

269

and --> is

271

is it better to use '>' to indicate context:

foo.c > bar

or <foo.c> bar

295

Consider sharing code with the legacy version. They are almost identical.

[ellis]

llvm/lib/ProfileData/InstrProf.cpp

271

I don't think > would work well because objc_direct methods have <> symbols: -<C foo:>. I thought ; worked well because I don't know of any name mangling that uses it.

[davidxl]

llvm/lib/ProfileData/InstrProf.cpp

271

ok. Note the <> is used in the unknown case.

[ellis]

llvm/lib/ProfileData/InstrProf.cpp

271

Yep! <unknown>;_blah is a possible case, which is why we still need the ; in between the two. I can't imagine ; in a filepath, and I don't know of a case where ; is in a mangled name.

[ellis]

Rebase and address comments

[MaskRay]

Looks reasonable to me. Thanks for fixing this FIXME

// GlobalValue::getGlobalIdentifier() prefixes the filename if the symbol
// is local. This logic will break if there is a colon in the filename,
// but we cannot use rsplit() because ObjC symbols can have colons.

llvm/lib/ProfileData/InstrProf.cpp

313

https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions

358

360

361

contains and split duplicate the work to search ;. You may just call split and check whether the second element is empty.

387

while you are here

llvm/lib/ProfileData/InstrProfReader.cpp

1224

llvm/test/Transforms/PGOProfile/statics_counter_naming.ll

8

This line tests -static-func-strip-dirname-prefix=1 by asserting that the NOPATH line does not occur. This is a bit weak.
It's probably better to rm -rf %t && mkdir -p %t/dir && cd %t; cp %s dir/a.ll; opt ... dir/a.ll so that we can actually check -static-func-strip-dirname-prefix=1 , but such a change is not the scope of this patch.

llvm/unittests/ProfileData/InstrProfTest.cpp

526

// Use Mach-O mangling that non-private symbols get a `_` prefix. 
M->setDataLayout(DataLayout("..."))

537

Add a comment that they are ObjC symbols.

549

Personally I think this ASSERT_NE can be removed as *F expresses the intention.

595

// Use Mach-O mangling that non-private symbols get a `_` prefix. 
M->setDataLayout(DataLayout("..."))

599

Perhaps a non-internal linkage for more variance.

1339

[MaskRay]

llvm/lib/ProfileData/InstrProf.cpp

289

Mach-O linker

ELF linkers use different option names and don't have the mangling issue.

[ellis]

Address comments and rebase

llvm/lib/ProfileData/InstrProf.cpp

313

Unfortunately most of this file is wrapped in namespace llvm {} so this gives a warning

warning: extra qualification on member 'getIRPGOFuncName' [-Wextra-qualification]

[thakis]

Looks like this breaks tests on macOS: http://45.33.8.238/macm1/66536/step_7.txt

Please take a look and revert for now if it takes a while to fix.

[ellis]

In D156569#4566803, @thakis wrote:

Looks like this breaks tests on macOS: http://45.33.8.238/macm1/66536/step_7.txt

Please take a look and revert for now if it takes a while to fix.

Thanks. Looking into this.

[ellis]

In D156569#4566825, @ellis wrote:

In D156569#4566803, @thakis wrote:

Looks like this breaks tests on macOS: http://45.33.8.238/macm1/66536/step_7.txt

Please take a look and revert for now if it takes a while to fix.

Thanks. Looking into this.

This should be fixed by https://reviews.llvm.org/D157321

llvm/include/llvm/ProfileData/InstrProf.h

 /// used the key for profile lookup. The function's original
 /// name is \c RawFuncName and has linkage of type \c Linkage.
 /// The function is defined in module \c FileName.
 std::string getPGOFuncName(StringRef RawFuncName,
                            GlobalValue::LinkageTypes Linkage,
                            StringRef FileName,
                            uint64_t Version = INSTR_PROF_INDEX_VERSION);
 
+/// \return the modified name for function \c F suitable to be
+/// used as the key for IRPGO profile lookup. \c InLTO indicates if this is
+/// called from LTO optimization passes.
+std::string getIRPGOFuncName(const Function &F, bool InLTO = false);
+
+/// \return the filename and the function name parsed from the output of
+/// \c getIRPGOFuncName()
+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 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 name of the function returned
private:
   // This map is only populated and used by raw instr profile reader.
   AddrHashMap AddrToMD5Map;
   bool Sorted = false;
 
   static StringRef getExternalSymbol() {
     return "** External Symbol **";
   }
 
+  Error addFuncWithName(Function &F, StringRef PGOFuncName);
+
   // If the symtab is created by a series of calls to \c addFuncName, \c
   // finalizeSymtab needs to be called before looking up function names.
   // This is required because the underlying map is a vector (for space
   // efficiency) which needs to be sorted.
   inline void finalizeSymtab();
 
 public:
   InstrProfSymtab() = default;
public:
   /// True if Symbol is the value used to represent external symbols.
   static bool isExternalSymbol(const StringRef &Symbol) {
     return Symbol == InstrProfSymtab::getExternalSymbol();
   }
 
   /// Return function from the name's md5 hash. Return nullptr if not found.
   inline Function *getFunction(uint64_t FuncMD5Hash);
 
-  /// Return the function's original assembly name by stripping off
-  /// the prefix attached (to symbols with priviate linkage). For
-  /// global functions, it returns the same string as getFuncName.
-  inline StringRef getOrigFuncName(uint64_t FuncMD5Hash);
-
   /// Return the name section data.
   inline StringRef getNameData() const { return Data; }
 
   /// Dump the symbols in this table.
   void dumpNames(raw_ostream &OS) const;
 };
 
 Error InstrProfSymtab::create(StringRef D, uint64_t BaseAddr) {
Function* InstrProfSymtab::getFunction(uint64_t FuncMD5Hash) {
   auto Result = llvm::lower_bound(MD5FuncMap, FuncMD5Hash,
                                   [](const std::pair<uint64_t, Function *> &LHS,
                                      uint64_t RHS) { return LHS.first < RHS; });
   if (Result != MD5FuncMap.end() && Result->first == FuncMD5Hash)
     return Result->second;
   return nullptr;
 }
 
-// See also getPGOFuncName implementation. These two need to be
-// matched.
-StringRef InstrProfSymtab::getOrigFuncName(uint64_t FuncMD5Hash) {
-  StringRef PGOName = getFuncName(FuncMD5Hash);
-  size_t S = PGOName.find_first_of(':');
-  if (S == StringRef::npos)
-    return PGOName;
-  return PGOName.drop_front(S + 1);
-}
-
 // To store the sums of profile count values, or the percentage of
 // the sums of the total count values.
 struct CountSumOrPercent {
   uint64_t NumEntries;
   double CountSum;
   double ValueCounts[IPVK_Last - IPVK_First + 1];
   CountSumOrPercent() : NumEntries(0), CountSum(0.0f), ValueCounts() {}
   void reset() {

llvm/include/llvm/ProfileData/InstrProfReader.h

public:
   Error readHeader() override;
   /// Read a single record.
   Error readNextRecord(NamedInstrProfRecord &Record) override;
 
   /// Return the NamedInstrProfRecord associated with FuncName and FuncHash.
   /// When return a hash_mismatch error and MismatchedFuncSum is not nullptr,
   /// the sum of all counters in the mismatched function will be set to
   /// MismatchedFuncSum. If there are multiple instances of mismatched
-  /// functions, MismatchedFuncSum returns the maximum.
+  /// functions, MismatchedFuncSum returns the maximum. If \c FuncName is not
+  /// found, try to lookup \c DeprecatedFuncName to handle profiles built by
+  /// older compilers.
   Expected<InstrProfRecord>
   getInstrProfRecord(StringRef FuncName, uint64_t FuncHash,
+                     StringRef DeprecatedFuncName = "",
                      uint64_t *MismatchedFuncSum = nullptr);
 
   /// Return the memprof record for the function identified by
   /// llvm::md5(Name).
   Expected<memprof::MemProfRecord> getMemProfRecord(uint64_t FuncNameHash);
 
   /// Fill Counts with the profile data for the given function name.
   Error getFunctionCounts(StringRef FuncName, uint64_t FuncHash,

llvm/lib/ProfileData/InstrProf.cpp

 #include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
+#include "llvm/IR/Mangler.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Type.h"
 #include "llvm/ProfileData/InstrProfReader.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Compression.h"
if (llvm::sys::path::is_separator(CI)) {
       --Count;
     }
     if (Count == 0)
       break;
   }
   return PathNameStr.substr(LastPos);
 }
 
+static StringRef getStrippedSourceFileName(const Function &F) {
+  StringRef FileName(F.getParent()->getSourceFileName());
+  uint32_t StripLevel = StaticFuncFullModulePrefix ? 0 : (uint32_t)-1;
+  if (StripLevel < StaticFuncStripDirNamePrefix)
+    StripLevel = StaticFuncStripDirNamePrefix;
+  if (StripLevel)
+    FileName = stripDirPrefix(FileName, StripLevel);
+  return FileName;
+}
+
+// The PGO name has the format [<filepath>;]<linkage-name> where <filepath>; is
+// provided if linkage is local and <linkage-name> is the mangled function
+// name. The filepath is used to discriminate possibly identical function names.
+// ; is used because it is unlikely to be found in either <filepath> or
+// <linkage-name>.
+//
+// Older compilers used getPGOFuncName() which has the format
+// [<filepath>:]<function-name>. <filepath> is used to discriminate between
+// possibly identical function names when linkage is local and <function-name>
+// simply comes from F.getName(). This caused trouble for Objective-C functions
+// which commonly have :'s in their names. Also, since <function-name> is not
+// 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) {
+  SmallString<64> Name;
+  if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
+    Name.append(FileName.empty() ? "<unknown>" : FileName);
+    Name.append(";");
+  }
+  Mangler().getNameWithPrefix(Name, &F, /*CannotUsePrivateLabel=*/true);
+  return Name.str().str();
+}
+
+static std::optional<std::string> lookupPGOFuncName(const Function &F) {
+  if (MDNode *MD = getPGOFuncNameMetadata(F)) {
+    StringRef S = cast<MDString>(MD->getOperand(0))->getString();
+    return S.str();
+  }
+  return {};
+}
+
+// See getPGOFuncName()
+std::string getIRPGOFuncName(const Function &F, bool InLTO) {
+  if (!InLTO) {
+    auto FileName = getStrippedSourceFileName(F);
+    return getIRPGOFuncName(F, F.getLinkage(), FileName);
+  }
+
+  // In LTO mode (when InLTO is true), first check if there is a meta data.
+  if (auto IRPGOFuncName = lookupPGOFuncName(F))
+    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 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.
 std::string getPGOFuncName(const Function &F, bool InLTO, uint64_t Version) {
   if (!InLTO) {
-    StringRef FileName(F.getParent()->getSourceFileName());
-    uint32_t StripLevel = StaticFuncFullModulePrefix ? 0 : (uint32_t)-1;
-    if (StripLevel < StaticFuncStripDirNamePrefix)
-      StripLevel = StaticFuncStripDirNamePrefix;
-    if (StripLevel)
-      FileName = stripDirPrefix(FileName, StripLevel);
+    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 (MDNode *MD = getPGOFuncNameMetadata(F)) {
-    StringRef S = cast<MDString>(MD->getOperand(0))->getString();
-    return S.str();
-  }
+  if (auto PGOFuncName = lookupPGOFuncName(F))
+    return *PGOFuncName;
 
   // 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 getPGOFuncName(F.getName(), GlobalValue::ExternalLinkage, "");
 }
 
+// See getIRPGOFuncName() for a discription of the format.
+std::pair<StringRef, StringRef>
+getParsedIRPGOFuncName(StringRef IRPGOFuncName) {
+  auto [FileName, FuncName] = IRPGOFuncName.split(';');
+  if (FuncName.empty())
+    return std::make_pair(StringRef(), IRPGOFuncName);
+  return std::make_pair(FileName, FuncName);
+}
+
 StringRef getFuncNameWithoutPrefix(StringRef PGOFuncName, StringRef FileName) {
   if (FileName.empty())
     return PGOFuncName;
   // Drop the file name including ':'. See also getPGOFuncName.
   if (PGOFuncName.startswith(FileName))
     PGOFuncName = PGOFuncName.drop_front(FileName.size() + 1);
   return PGOFuncName;
 }
 
 // \p FuncName is the string used as profile lookup key for the function. A
 // symbol is created to hold the name. Return the legalized symbol name.
 std::string getPGOFuncNameVarName(StringRef FuncName,
                                   GlobalValue::LinkageTypes Linkage) {
   std::string VarName = std::string(getInstrProfNameVarPrefix());
   VarName += FuncName;
 
   if (!GlobalValue::isLocalLinkage(Linkage))
     return VarName;
 
   // Now fix up illegal chars in local VarName that may upset the assembler.
-  const char *InvalidChars = "-:<>/\"'";
+  const char InvalidChars[] = "-:;<>/\"'";
   size_t found = VarName.find_first_of(InvalidChars);
   while (found != std::string::npos) {
     VarName[found] = '_';
     found = VarName.find_first_of(InvalidChars, found + 1);
   }
   return VarName;
 }
 

 }
 
 Error InstrProfSymtab::create(Module &M, bool InLTO) {
   for (Function &F : M) {
     // Function may not have a name: like using asm("") to overwrite the name.
     // Ignore in this case.
     if (!F.hasName())
       continue;
-    const std::string &PGOFuncName = getPGOFuncName(F, InLTO);
-    if (Error E = addFuncName(PGOFuncName))
-      return E;
-    MD5FuncMap.emplace_back(Function::getGUID(PGOFuncName), &F);
-    // In ThinLTO, local function may have been promoted to global and have
-    // suffix ".llvm." added to the function name. We need to add the
-    // stripped function name to the symbol table so that we can find a match
-    // from profile.
-    //
-    // We may have other suffixes similar as ".llvm." which are needed to
-    // be stripped before the matching, but ".__uniq." suffix which is used
-    // to differentiate internal linkage functions in different modules
-    // should be kept. Now this is the only suffix with the pattern ".xxx"
-    // which is kept before matching.
-    const std::string UniqSuffix = ".__uniq.";
-    auto pos = PGOFuncName.find(UniqSuffix);
-    // Search '.' after ".__uniq." if ".__uniq." exists, otherwise
-    // search '.' from the beginning.
-    if (pos != std::string::npos)
-      pos += UniqSuffix.length();
-    else
-      pos = 0;
-    pos = PGOFuncName.find('.', pos);
-    if (pos != std::string::npos && pos != 0) {
-      const std::string &OtherFuncName = PGOFuncName.substr(0, pos);
-      if (Error E = addFuncName(OtherFuncName))
-        return E;
-      MD5FuncMap.emplace_back(Function::getGUID(OtherFuncName), &F);
-    }
-  }
-  Sorted = false;
-  finalizeSymtab();
+    if (Error E = addFuncWithName(F, getIRPGOFuncName(F, InLTO)))
+      return E;
+    // Also use getPGOFuncName() so that we can find records from older profiles
+    if (Error E = addFuncWithName(F, getPGOFuncName(F, InLTO)))
+      return E;
+  }
+  Sorted = false;
+  finalizeSymtab();
+  return Error::success();
+}
+
+Error InstrProfSymtab::addFuncWithName(Function &F, StringRef PGOFuncName) {
+    StringRef OtherFuncName = PGOFuncName.substr(0, pos);
   return Error::success();
 }
 
 uint64_t InstrProfSymtab::getFunctionHashFromAddress(uint64_t Address) {
   finalizeSymtab();
   auto It = partition_point(AddrToMD5Map, [=](std::pair<uint64_t, uint64_t> A) {
     return A.first < Address;
   });

llvm/lib/ProfileData/InstrProfReader.cpp

if (Error E = Index->populateSymtab(*NewSymtab)) {
     consumeError(error(ErrCode, Msg));
   }
 
   Symtab = std::move(NewSymtab);
   return *Symtab;
 }
 
 Expected<InstrProfRecord> IndexedInstrProfReader::getInstrProfRecord(
-    StringRef FuncName, uint64_t FuncHash, uint64_t *MismatchedFuncSum) {
+    StringRef FuncName, uint64_t FuncHash, StringRef DeprecatedFuncName,
+    uint64_t *MismatchedFuncSum) {
   ArrayRef<NamedInstrProfRecord> Data;
   uint64_t FuncSum = 0;
-  Error Err = Remapper->getRecords(FuncName, Data);
-  if (Err)
-    return std::move(Err);
+  auto Err = Remapper->getRecords(FuncName, Data);
+  if (Err) {
+    // If we don't find FuncName, try DeprecatedFuncName to handle profiles
+    // built by older compilers.
+    auto Err2 =
+        handleErrors(std::move(Err), [&](const InstrProfError &IE) -> Error {
+          if (IE.get() != instrprof_error::unknown_function)
+            return make_error<InstrProfError>(IE);
+          if (auto Err = Remapper->getRecords(DeprecatedFuncName, Data))
+            return Err;
+          return Error::success();
+        });
+    if (Err2)
+      return std::move(Err2);
+  }
   // Found it. Look for counters with the right hash.
 
   // A flag to indicate if the records are from the same type
   // of profile (i.e cs vs nocs).
   bool CSBitMatch = false;
   auto getFuncSum = [](const std::vector<uint64_t> &Counts) {
     uint64_t ValueSum = 0;
     for (uint64_t CountValue : Counts) {

llvm/lib/Transforms/Instrumentation/MemProfiler.cpp

stackFrameIncludesInlinedCallStack(ArrayRef<Frame> ProfileCallStack,
   return InlCallStackIter == InlinedCallStack.end();
 }
 
 static void readMemprof(Module &M, Function &F,
                         IndexedInstrProfReader *MemProfReader,
                         const TargetLibraryInfo &TLI) {
   auto &Ctx = M.getContext();
 
+  auto FuncName = getIRPGOFuncName(F);
+  auto FuncGUID = Function::getGUID(FuncName);
+  std::optional<memprof::MemProfRecord> MemProfRec;
+  auto Err = MemProfReader->getMemProfRecord(FuncGUID).moveInto(MemProfRec);
+  if (Err) {
+    // If we don't find getIRPGOFuncName(), try getPGOFuncName() to handle
+    // profiles built by older compilers
+    Err = handleErrors(std::move(Err), [&](const InstrProfError &IE) -> Error {
+      if (IE.get() != instrprof_error::unknown_function)
+        return make_error<InstrProfError>(IE);
       auto FuncName = getPGOFuncName(F);
       auto FuncGUID = Function::getGUID(FuncName);
-  Expected<memprof::MemProfRecord> MemProfResult =
-      MemProfReader->getMemProfRecord(FuncGUID);
-  if (Error E = MemProfResult.takeError()) {
-    handleAllErrors(std::move(E), [&](const InstrProfError &IPE) {
+      if (auto Err =
+              MemProfReader->getMemProfRecord(FuncGUID).moveInto(MemProfRec))
+        return Err;
+      return Error::success();
+    });
+  }
+  if (Err) {
+    handleAllErrors(std::move(Err), [&](const InstrProfError &IPE) {
       auto Err = IPE.get();
       bool SkipWarning = false;
       LLVM_DEBUG(dbgs() << "Error in reading profile for Func " << FuncName
                         << ": ");
       if (Err == instrprof_error::unknown_function) {
         NumOfMemProfMissing++;
         SkipWarning = !PGOWarnMissing;
         LLVM_DEBUG(dbgs() << "unknown function");
static void readMemprof(Module &M, Function &F,
 
   // Build maps of the location hash to all profile data with that leaf location
   // (allocation info and the callsites).
   std::map<uint64_t, std::set<const AllocationInfo *>> LocHashToAllocInfo;
   // For the callsites we need to record the index of the associated frame in
   // the frame array (see comments below where the map entries are added).
   std::map<uint64_t, std::set<std::pair<const SmallVector<Frame> *, unsigned>>>
       LocHashToCallSites;
-  const auto MemProfRec = std::move(MemProfResult.get());
-  for (auto &AI : MemProfRec.AllocSites) {
+  for (auto &AI : MemProfRec->AllocSites) {
     // Associate the allocation info with the leaf frame. The later matching
     // code will match any inlined call sequences in the IR with a longer prefix
     // of call stack frames.
     uint64_t StackId = computeStackId(AI.CallStack[0]);
     LocHashToAllocInfo[StackId].insert(&AI);
   }
-  for (auto &CS : MemProfRec.CallSites) {
+  for (auto &CS : MemProfRec->CallSites) {
     // Need to record all frames from leaf up to and including this function,
     // as any of these may or may not have been inlined at this point.
     unsigned Idx = 0;
     for (auto &StackFrame : CS) {
       uint64_t StackId = computeStackId(StackFrame);
       LocHashToCallSites[StackId].insert(std::make_pair(&CS, Idx++));
       // Once we find this function, we can stop recording.
       if (StackFrame.Function == FuncGUID)

llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp

private:
   void computeCFGHash();
   void renameComdatFunction();
 
 public:
   const TargetLibraryInfo &TLI;
   std::vector<std::vector<VPCandidateInfo>> ValueSites;
   SelectInstVisitor SIVisitor;
   std::string FuncName;
+  std::string DeprecatedFuncName;
   GlobalVariable *FuncNameVar;
 
   // CFG hash value for this function.
   uint64_t FunctionHash = 0;
 
   // The Minimum Spanning Tree of function CFG.
   CFGMST<Edge, BBInfo> MST;
 
if (!IsCS) {
       NumOfPGOBB += MST.BBInfos.size();
       ValueSites[IPVK_IndirectCallTarget] = VPC.get(IPVK_IndirectCallTarget);
     } else {
       NumOfCSPGOSelectInsts += SIVisitor.getNumOfSelectInsts();
       NumOfCSPGOMemIntrinsics += ValueSites[IPVK_MemOPSize].size();
       NumOfCSPGOBB += MST.BBInfos.size();
     }
 
-    FuncName = getPGOFuncName(F);
+    FuncName = getIRPGOFuncName(F);
+    DeprecatedFuncName = getPGOFuncName(F);
     computeCFGHash();
     if (!ComdatMembers.empty())
       renameComdatFunction();
     LLVM_DEBUG(dumpInfo("after CFGMST"));
 
     for (auto &E : MST.AllEdges) {
       if (E->Removed)
         continue;

 // Read the profile from ProfileFileName and assign the value to the
 // instrumented BB and the edges. This function also updates ProgramMaxCount.
 // Return true if the profile are successfully read, and false on errors.
 bool PGOUseFunc::readCounters(IndexedInstrProfReader *PGOReader, bool &AllZeros,
                               InstrProfRecord::CountPseudoKind &PseudoKind) {
   auto &Ctx = M->getContext();
   uint64_t MismatchedFuncSum = 0;
   Expected<InstrProfRecord> Result = PGOReader->getInstrProfRecord(
-      FuncInfo.FuncName, FuncInfo.FunctionHash, &MismatchedFuncSum);
+      FuncInfo.FuncName, FuncInfo.FunctionHash, FuncInfo.DeprecatedFuncName,
+      &MismatchedFuncSum);
   if (Error E = Result.takeError()) {
     handleInstrProfError(std::move(E), MismatchedFuncSum);
     return false;
   }
   ProfileRecord = std::move(Result.get());
   PseudoKind = ProfileRecord.getCountPseudoKind();
   if (PseudoKind != InstrProfRecord::NotPseudo) {
     return true;
bool PGOUseFunc::readCounters(IndexedInstrProfReader *PGOReader, bool &AllZeros,
   }
   ProgramMaxCount = PGOReader->getMaximumFunctionCount(IsCS);
   return true;
 }
 
 void PGOUseFunc::populateCoverage(IndexedInstrProfReader *PGOReader) {
   uint64_t MismatchedFuncSum = 0;
   Expected<InstrProfRecord> Result = PGOReader->getInstrProfRecord(
-      FuncInfo.FuncName, FuncInfo.FunctionHash, &MismatchedFuncSum);
+      FuncInfo.FuncName, FuncInfo.FunctionHash, FuncInfo.DeprecatedFuncName,
+      &MismatchedFuncSum);
   if (auto Err = Result.takeError()) {
     handleInstrProfError(std::move(Err), MismatchedFuncSum);
     return;
   }
 
   std::vector<uint64_t> &CountsFromProfile = Result.get().Counts;
   DenseMap<const BasicBlock *, bool> Coverage;
   unsigned Index = 0;

llvm/test/Transforms/PGOProfile/comdat_internal.ll

 ; RUN: opt < %s -passes=pgo-instr-gen,instrprof -S | FileCheck %s
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 $foo = comdat any
 ; CHECK: $foo = comdat any
 
 ; CHECK: $__llvm_profile_raw_version = comdat any
 ; CHECK: $__profc__stdin__foo.[[#FOO_HASH:]] = comdat any
 
 @bar = global ptr @foo, align 8
 
 ; CHECK: @__llvm_profile_raw_version = hidden constant i64 {{[0-9]+}}, comdat
 ; CHECK-NOT: __profn__stdin__foo
 ; CHECK: @__profc__stdin__foo.[[#FOO_HASH]] = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
-; CHECK: @__profd__stdin__foo.[[#FOO_HASH]] = private global { i64, i64, i64, ptr, ptr, i32, [2 x i16] } { i64 -5640069336071256030, i64 [[#FOO_HASH]], i64 sub (i64 ptrtoint (ptr @__profc__stdin__foo.742261418966908927 to i64), i64 ptrtoint (ptr @__profd__stdin__foo.742261418966908927 to i64)), ptr null
+; CHECK: @__profd__stdin__foo.[[#FOO_HASH]] = private global { i64, i64, i64, ptr, ptr, i32, [2 x i16] } { i64 {{.*}}, i64 [[#FOO_HASH]], i64 sub (i64 ptrtoint (ptr @__profc__stdin__foo.742261418966908927 to i64), i64 ptrtoint (ptr @__profd__stdin__foo.742261418966908927 to i64)), ptr null
 ; CHECK-NOT: @foo
 ; CHECK-SAME: , ptr null, i32 1, [2 x i16] zeroinitializer }, section "__llvm_prf_data", comdat($__profc__stdin__foo.[[#FOO_HASH]]), align 8
 ; CHECK: @__llvm_prf_nm
 ; CHECK: @llvm.compiler.used
 
 define internal i32 @foo() comdat {
 entry:
   ret i32 1
 }

llvm/test/Transforms/PGOProfile/criticaledge.ll

 ; RUN: opt < %s -passes=pgo-instr-gen -pgo-instrument-entry=false -S | FileCheck %s --check-prefix=GEN
 ; RUN: llvm-profdata merge %S/Inputs/criticaledge.proftext -o %t.profdata
 ; RUN: opt < %s -passes=pgo-instr-use -pgo-instrument-entry=false -pgo-test-profile-file=%t.profdata -S | FileCheck %s --check-prefix=USE
 ;
 ; RUN: opt < %s -passes=pgo-instr-gen -pgo-instrument-entry=true -S | FileCheck %s --check-prefix=GEN
 ; RUN: llvm-profdata merge %S/Inputs/criticaledge_entry.proftext -o %t2.profdata
 ; RUN: opt < %s -passes=pgo-instr-use -pgo-instrument-entry=true -pgo-test-profile-file=%t2.profdata -S | FileCheck %s --check-prefix=USE
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; GEN: $__llvm_profile_raw_version = comdat any
 ; GEN: @__llvm_profile_raw_version = hidden constant i64 {{[0-9]+}}, comdat
 ; GEN: @__profn_test_criticalEdge = private constant [17 x i8] c"test_criticalEdge"
-; GEN: @__profn__stdin__bar = private constant [11 x i8] c"<stdin>:bar"
+; GEN: @__profn__stdin__bar = private constant [11 x i8] c"<stdin>;bar"
 
 define i32 @test_criticalEdge(i32 %i, i32 %j) {
 entry:
 ; CHECK: entry:
 ; NOTENTRY-NOT: call void @llvm.instrprof.increment
 ; ENTRY:   call void @llvm.instrprof.increment(ptr @__profn_test_criticalEdge, i64 {{[0-9]+}}, i32 8, i32 0)
   switch i32 %i, label %sw.default [
     i32 1, label %sw.bb

llvm/test/Transforms/PGOProfile/statics_counter_naming.ll

 ; RUN: opt %s -passes=pgo-instr-gen -static-func-full-module-prefix=false -S | FileCheck %s --check-prefix=NOPATH
 ; RUN: opt %s -passes=pgo-instr-gen -static-func-strip-dirname-prefix=1000 -S | FileCheck %s --check-prefix=NOPATH
 ; RUN: opt %s -passes=pgo-instr-gen -static-func-strip-dirname-prefix=1 -S | FileCheck %s --check-prefix=HASPATH
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-; NOPATH: @__profn_statics_counter_naming.ll_func = private constant [30 x i8] c"statics_counter_naming.ll:func"
-; HASPATH-NOT: @__profn_statics_counter_naming.ll_func = private constant [30 x i8] c"statics_counter_naming.ll:func"
+; NOPATH: @__profn_statics_counter_naming.ll_func = private constant [30 x i8] c"statics_counter_naming.ll;func"
+; HASPATH-NOT: @__profn_statics_counter_naming.ll_func = private constant [30 x i8] c"statics_counter_naming.ll;func"
 
 define internal i32 @func() {
 entry:
   ret i32 0
 }

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

static int order_main(int argc, const char *argv[]) {
   auto &Traces = Reader->getTemporalProfTraces();
   auto Nodes = TemporalProfTraceTy::createBPFunctionNodes(Traces);
   BalancedPartitioningConfig Config;
   BalancedPartitioning BP(Config);
   BP.run(Nodes);
 
   WithColor::note() << "# Ordered " << Nodes.size() << " functions\n";
   for (auto &N : Nodes) {
-    auto FuncName = Reader->getSymtab().getFuncName(N.Id);
-    if (FuncName.contains(':')) {
-      // GlobalValue::getGlobalIdentifier() prefixes the filename if the symbol
-      // is local. This logic will break if there is a colon in the filename,
-      // but we cannot use rsplit() because ObjC symbols can have colons.
-      auto [Filename, ParsedFuncName] = FuncName.split(':');
-      // Emit a comment describing where this symbol came from
+    auto [Filename, ParsedFuncName] =
+        getParsedIRPGOFuncName(Reader->getSymtab().getFuncName(N.Id));
+    if (!Filename.empty())
       OS << "# " << Filename << "\n";
-      FuncName = ParsedFuncName;
-    }
-    OS << FuncName << "\n";
+    OS << ParsedFuncName << "\n";
   }
   return 0;
 }
 
 typedef int (*llvm_profdata_subcommand)(int, const char *[]);
 
 static std::tuple<StringRef, llvm_profdata_subcommand>
     llvm_profdata_subcommands[] = {

llvm/unittests/ProfileData/InstrProfTest.cpp

 #include "llvm/IR/Module.h"
 #include "llvm/ProfileData/InstrProfReader.h"
 #include "llvm/ProfileData/InstrProfWriter.h"
 #include "llvm/ProfileData/MemProf.h"
 #include "llvm/ProfileData/MemProfData.inc"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/Error.h"
-#include "llvm/Testing/Support/SupportHelpers.h"
 #include "gtest/gtest.h"
 #include <cstdarg>
 
 using namespace llvm;
+using ::testing::EndsWith;
 using ::testing::IsSubsetOf;
 using ::testing::SizeIs;
 using ::testing::UnorderedElementsAre;
 
 [[nodiscard]] static ::testing::AssertionResult
 ErrorEquals(instrprof_error Expected, Error E) {
   instrprof_error Found;
   std::string FoundMsg;
TEST_F(InstrProfTest, test_memprof_merge) {
   };
 
   const memprof::MemProfRecord WantRecord(IndexedMR, IdToFrameCallback);
   ASSERT_FALSE(HasFrameMappingError)
       << "could not map frame id: " << LastUnmappedFrameId;
   EXPECT_THAT(WantRecord, EqualsRecord(Record));
 }
 
+TEST_F(InstrProfTest, test_irpgo_function_name) {
+  LLVMContext Ctx;
+  auto M = std::make_unique<Module>("MyModule.cpp", Ctx);
+  // Use Mach-O mangling so that non-private symbols get a `_` prefix.
+  M->setDataLayout(DataLayout("m:o"));
+  auto *FTy = FunctionType::get(Type::getVoidTy(Ctx), /*isVarArg=*/false);
+
+  std::vector<std::tuple<StringRef, Function::LinkageTypes, StringRef>> Data;
+  Data.emplace_back("ExternalFoo", Function::ExternalLinkage, "_ExternalFoo");
+  Data.emplace_back("InternalFoo", Function::InternalLinkage,
+                    "MyModule.cpp;_InternalFoo");
+  Data.emplace_back("PrivateFoo", Function::PrivateLinkage,
+                    "MyModule.cpp;l_PrivateFoo");
+  Data.emplace_back("WeakODRFoo", Function::WeakODRLinkage, "_WeakODRFoo");
+  // Test Objective-C symbols
+  Data.emplace_back("\01-[C dynamicFoo:]", Function::ExternalLinkage,
+                    "-[C dynamicFoo:]");
+  Data.emplace_back("-<C directFoo:>", Function::ExternalLinkage,
+                    "_-<C directFoo:>");
+  Data.emplace_back("\01-[C internalFoo:]", Function::InternalLinkage,
+                    "MyModule.cpp;-[C internalFoo:]");
+
+  for (auto &[Name, Linkage, ExpectedIRPGOFuncName] : Data)
+    Function::Create(FTy, Linkage, Name, M.get());
+
+  for (auto &[Name, Linkage, ExpectedIRPGOFuncName] : Data) {
+    auto *F = M->getFunction(Name);
+    auto IRPGOFuncName = getIRPGOFuncName(*F);
+    EXPECT_EQ(IRPGOFuncName, ExpectedIRPGOFuncName);
+
+    auto [Filename, ParsedIRPGOFuncName] =
+        getParsedIRPGOFuncName(IRPGOFuncName);
+    StringRef ExpectedParsedIRPGOFuncName = IRPGOFuncName;
+    if (ExpectedParsedIRPGOFuncName.consume_front("MyModule.cpp;")) {
+      EXPECT_EQ(Filename, "MyModule.cpp");
+    } else {
+      EXPECT_EQ(Filename, "");
+    }
+    EXPECT_EQ(ParsedIRPGOFuncName, ExpectedParsedIRPGOFuncName);
+  }
+}
+
+TEST_F(InstrProfTest, test_pgo_function_name) {
+  LLVMContext Ctx;
+  auto M = std::make_unique<Module>("MyModule.cpp", Ctx);
+  auto *FTy = FunctionType::get(Type::getVoidTy(Ctx), /*isVarArg=*/false);
+
+  std::vector<std::tuple<StringRef, Function::LinkageTypes, StringRef>> Data;
+  Data.emplace_back("ExternalFoo", Function::ExternalLinkage, "ExternalFoo");
+  Data.emplace_back("InternalFoo", Function::InternalLinkage,
+                    "MyModule.cpp:InternalFoo");
+  Data.emplace_back("PrivateFoo", Function::PrivateLinkage,
+                    "MyModule.cpp:PrivateFoo");
+  Data.emplace_back("WeakODRFoo", Function::WeakODRLinkage, "WeakODRFoo");
+  // Test Objective-C symbols
+  Data.emplace_back("\01-[C externalFoo:]", Function::ExternalLinkage,
+                    "-[C externalFoo:]");
+  Data.emplace_back("\01-[C internalFoo:]", Function::InternalLinkage,
+                    "MyModule.cpp:-[C internalFoo:]");
+
+  for (auto &[Name, Linkage, ExpectedPGOFuncName] : Data)
+    Function::Create(FTy, Linkage, Name, M.get());
+
+  for (auto &[Name, Linkage, ExpectedPGOFuncName] : Data) {
+    auto *F = M->getFunction(Name);
+    EXPECT_EQ(getPGOFuncName(*F), ExpectedPGOFuncName);
+  }
+}
+
+TEST_F(InstrProfTest, test_irpgo_read_deprecated_names) {
+  LLVMContext Ctx;
+  auto M = std::make_unique<Module>("MyModule.cpp", Ctx);
+  // Use Mach-O mangling so that non-private symbols get a `_` prefix.
+  M->setDataLayout(DataLayout("m:o"));
+  auto *FTy = FunctionType::get(Type::getVoidTy(Ctx), /*isVarArg=*/false);
+  auto *InternalFooF =
+      Function::Create(FTy, Function::InternalLinkage, "InternalFoo", M.get());
+  auto *ExternalFooF =
+      Function::Create(FTy, Function::ExternalLinkage, "ExternalFoo", M.get());
+
+  auto *InternalBarF =
+      Function::Create(FTy, Function::InternalLinkage, "InternalBar", M.get());
+  auto *ExternalBarF =
+      Function::Create(FTy, Function::ExternalLinkage, "ExternalBar", M.get());
+
+  Writer.addRecord({getIRPGOFuncName(*InternalFooF), 0x1234, {1}}, Err);
+  Writer.addRecord({getIRPGOFuncName(*ExternalFooF), 0x5678, {1}}, Err);
+  // Write a record with a deprecated name
+  Writer.addRecord({getPGOFuncName(*InternalBarF), 0x1111, {2}}, Err);
+  Writer.addRecord({getPGOFuncName(*ExternalBarF), 0x2222, {2}}, Err);
+
+  auto Profile = Writer.writeBuffer();
+  readProfile(std::move(Profile));
+
+  EXPECT_THAT_EXPECTED(
+      Reader->getInstrProfRecord(getIRPGOFuncName(*InternalFooF), 0x1234,
+                                 getPGOFuncName(*InternalFooF)),
+      Succeeded());
+  EXPECT_THAT_EXPECTED(
+      Reader->getInstrProfRecord(getIRPGOFuncName(*ExternalFooF), 0x5678,
+                                 getPGOFuncName(*ExternalFooF)),
+      Succeeded());
+  // Ensure we can still read this old record name
+  EXPECT_THAT_EXPECTED(
+      Reader->getInstrProfRecord(getIRPGOFuncName(*InternalBarF), 0x1111,
+                                 getPGOFuncName(*InternalBarF)),
+      Succeeded());
+  EXPECT_THAT_EXPECTED(
+      Reader->getInstrProfRecord(getIRPGOFuncName(*ExternalBarF), 0x2222,
+                                 getPGOFuncName(*ExternalBarF)),
+      Succeeded());
+}
+
 static const char callee1[] = "callee1";
 static const char callee2[] = "callee2";
 static const char callee3[] = "callee3";
 static const char callee4[] = "callee4";
 static const char callee5[] = "callee5";
 static const char callee6[] = "callee6";
 
 TEST_P(MaybeSparseInstrProfTest, get_icall_data_read_write) {
TEST_P(MaybeSparseInstrProfTest, instr_prof_symtab_module_test) {
   InstrProfSymtab ProfSymtab;
   EXPECT_THAT_ERROR(ProfSymtab.create(*M), Succeeded());
 
   StringRef Funcs[] = {"Gfoo", "Gblah", "Gbar", "Ifoo", "Iblah", "Ibar",
                        "Pfoo", "Pblah", "Pbar", "Wfoo", "Wblah", "Wbar"};
 
   for (unsigned I = 0; I < std::size(Funcs); I++) {
     Function *F = M->getFunction(Funcs[I]);
-    ASSERT_TRUE(F != nullptr);
+
+    std::string IRPGOName = getIRPGOFuncName(*F);
+    auto IRPGOFuncName =
+        ProfSymtab.getFuncName(IndexedInstrProf::ComputeHash(IRPGOName));
+    EXPECT_EQ(StringRef(IRPGOName), IRPGOFuncName);
+    EXPECT_EQ(StringRef(Funcs[I]),
+              getParsedIRPGOFuncName(IRPGOFuncName).second);
+    // Ensure we can still read this old record name.
     std::string PGOName = getPGOFuncName(*F);
-    uint64_t Key = IndexedInstrProf::ComputeHash(PGOName);
-    ASSERT_EQ(StringRef(PGOName),
-              ProfSymtab.getFuncName(Key));
-    ASSERT_EQ(StringRef(Funcs[I]), ProfSymtab.getOrigFuncName(Key));
+    auto PGOFuncName =
+        ProfSymtab.getFuncName(IndexedInstrProf::ComputeHash(PGOName));
+    EXPECT_EQ(StringRef(PGOName), PGOFuncName);
+    EXPECT_THAT(PGOFuncName.str(), EndsWith(Funcs[I].str()));
   }
 }
 
 // Testing symtab serialization and creator/deserialization interface
 // used by coverage map reader, and raw profile reader.
 TEST_P(MaybeSparseInstrProfTest, instr_prof_symtab_compression_test) {
   std::vector<std::string> FuncNames1;
   std::vector<std::string> FuncNames2;