https://github.com/orodley updated https://github.com/llvm/llvm-project/pull/214134
>From 82785cc3e0110514e54b8f5d8c5ca0909c7514f1 Mon Sep 17 00:00:00 2001 From: Owen Rodley <[email protected]> Date: Mon, 3 Aug 2026 13:20:42 +1000 Subject: [PATCH 1/8] [InstrProf] Remove PGO name reading and writing Simplify symtab GUID lookups by not emitting, reading, or writing legacy !PGOFuncName and !PGOName metadata in Clang and LLVM IR. Since we now have stable GUIDs, we no longer need to maintain the bespoke PGOName metadata to achieve the same purpose. This still includes a fallback to manual GUID calculation, as otherwise we break lots of tests. This will be removed and tests fixed separately. --- clang/lib/CodeGen/CodeGenPGO.cpp | 2 - llvm/include/llvm/ProfileData/InstrProf.h | 36 ++---- llvm/lib/ProfileData/InstrProf.cpp | 117 ++++-------------- .../Instrumentation/InstrProfiling.cpp | 2 +- .../Instrumentation/PGOInstrumentation.cpp | 15 +-- .../Transforms/Scalar/JumpTableToSwitch.cpp | 2 +- llvm/test/Instrumentation/cgprofile.ll | 13 +- llvm/test/Transforms/PGOProfile/icp_invoke.ll | 8 +- .../Transforms/PGOProfile/icp_invoke_nouse.ll | 4 +- .../thinlto_indirect_call_promotion.ll | 6 +- llvm/unittests/ProfileData/InstrProfTest.cpp | 18 +-- 11 files changed, 58 insertions(+), 165 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 59faa3aef2460..1b16be8106f56 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -50,8 +50,6 @@ void CodeGenPGO::setFuncName(StringRef Name, void CodeGenPGO::setFuncName(llvm::Function *Fn) { setFuncName(Fn->getName(), Fn->getLinkage()); - // Create PGOFuncName meta data. - llvm::createPGOFuncNameMetadata(*Fn, FuncName); } /// The version of the PGO hash algorithm. diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h index bff17fcf64781..01f4790077654 100644 --- a/llvm/include/llvm/ProfileData/InstrProf.h +++ b/llvm/include/llvm/ProfileData/InstrProf.h @@ -228,7 +228,7 @@ inline StringRef getInstrProfNameSeparator() { return "\01"; } /// instrumentation LLVM_ABI bool isGPUProfTarget(const Module &M); -/// Please use getIRPGOFuncName for LLVM IR instrumentation. This function is +/// Please use getIRPGOObjectName 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 @@ -245,33 +245,33 @@ LLVM_ABI 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 +/// \return the modified name for global object \c GO suitable to be /// used as the key for IRPGO profile lookup. \c InLTO indicates if this is /// called from LTO optimization passes. -LLVM_ABI std::string getIRPGOFuncName(const Function &F, bool InLTO = false); +LLVM_ABI std::string getIRPGOObjectName(const GlobalObject &GO, bool InLTO = false); /// \return the filename and the function name parsed from the output of -/// \c getIRPGOFuncName() +/// \c getIRPGOObjectName() LLVM_ABI std::pair<StringRef, StringRef> getParsedIRPGOName(StringRef IRPGOName); /// 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 +/// (returned by \c getIRPGOObjectName) for LLVM IR instrumentation and PGO /// function name (returned by \c getPGOFuncName) for front-end instrumentation. LLVM_ABI 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 +/// \c getIRPGOObjectName) for LLVM IR instrumentation and PGO function name /// (returned by \c getPGOFuncName) for front-end instrumentation. LLVM_ABI 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 +/// \c getIRPGOObjectName) for LLVM IR instrumentation and PGO function name /// (returned by \c getPGOFuncName) for front-end instrumentation. LLVM_ABI GlobalVariable *createPGOFuncNameVar(Module &M, GlobalValue::LinkageTypes Linkage, @@ -350,26 +350,6 @@ getValueProfDataFromInst(const Instruction &Inst, InstrProfValueKind ValueKind, uint32_t MaxNumValueData, uint64_t &TotalC, bool GetNoICPValue = false); -inline StringRef getPGOFuncNameMetadataName() { return "PGOFuncName"; } - -inline StringRef getPGONameMetadataName() { return "PGOName"; } - -/// Return the PGOFuncName meta data associated with a function. -LLVM_ABI MDNode *getPGOFuncNameMetadata(const Function &F); - -LLVM_ABI std::string getPGOName(const GlobalVariable &V, bool InLTO = false); - -/// Create the PGOFuncName meta data if PGOFuncName is different from -/// function's raw name. This should only apply to internal linkage functions -/// declared by users only. -/// TODO: Update all callers to 'createPGONameMetadata' and deprecate this -/// function. -LLVM_ABI void createPGOFuncNameMetadata(Function &F, StringRef PGOFuncName); - -/// Create the PGOName metadata if a global object's PGO name is different from -/// its mangled name. This should apply to local-linkage global objects only. -LLVM_ABI void createPGONameMetadata(GlobalObject &GO, StringRef PGOName); - /// Check if we can use Comdat for profile variables. This will eliminate /// the duplicated profile variables for Comdat functions. LLVM_ABI bool needsComdatForCounter(const GlobalObject &GV, const Module &M); @@ -514,7 +494,7 @@ uint64_t ComputeHash(StringRef K); /// A symbol table used for function [IR]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 +/// data of the function. See \c getIRPGOObjectName() and \c getPGOFuncName /// methods for details how [IR]PGO name is formed. class InstrProfSymtab { public: diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp index ca5b17aa3c248..3976dbdee1b7e 100644 --- a/llvm/lib/ProfileData/InstrProf.cpp +++ b/llvm/lib/ProfileData/InstrProf.cpp @@ -375,55 +375,24 @@ getIRPGONameForGlobalObject(const GlobalObject &GO, return GlobalValue::getGlobalIdentifier(GO.getName(), Linkage, FileName); } -static std::optional<std::string> lookupPGONameFromMetadata(MDNode *MD) { - if (MD != nullptr) { - StringRef S = cast<MDString>(MD->getOperand(0))->getString(); - return S.str(); - } - return {}; -} - // 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) { +// when called in LTO optimization. In LTO mode (when InLTO is true), +// LTO's internalization privatizes many global linkage symbols, so we assume +// non-internal linkage without a source prefix. +std::string getIRPGOObjectName(const GlobalObject &GO, bool InLTO) { if (!InLTO) { 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 = 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 getIRPGONameForGlobalObject(GO, GlobalValue::ExternalLinkage, ""); } -// 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)); -} - -// Please use getIRPGOFuncName for LLVM IR instrumentation. This function is +// Please use getIRPGOObjectName 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 +// This is similar to `getIRPGOObjectName` except that this function calls +// 'getPGOFuncName' to get a name and `getIRPGOObjectName` calls // 'getIRPGONameForGlobalObject'. See the difference between two callees in the // comments of `getIRPGONameForGlobalObject`. std::string getPGOFuncName(const Function &F, bool InLTO, uint64_t Version) { @@ -432,23 +401,9 @@ std::string getPGOFuncName(const Function &F, bool InLTO, uint64_t Version) { 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 = lookupPGONameFromMetadata(getPGOFuncNameMetadata(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, ""); } -std::string getPGOName(const GlobalVariable &V, bool InLTO) { - // PGONameMetadata should be set by compiler at profile use time - // and read by symtab creation to look up symbols corresponding to - // a MD5 hash. - return getIRPGOObjectName(V, InLTO, V.getMetadata(getPGONameMetadataName())); -} - // See getIRPGOObjectName() for a discription of the format. std::pair<StringRef, StringRef> getParsedIRPGOName(StringRef IRPGOName) { auto [FileName, MangledName] = IRPGOName.split(GlobalIdentifierDelimiter); @@ -532,7 +487,7 @@ Error InstrProfSymtab::create(Module &M, bool InLTO, bool AddCanonical) { // Ignore in this case. if (!F.hasName()) continue; - auto IRPGOFuncName = getIRPGOFuncName(F, InLTO); + auto IRPGOFuncName = getIRPGOObjectName(F, InLTO); if (Error E = addFuncWithName(F, IRPGOFuncName, AddCanonical)) return E; // Also use getPGOFuncName() so that we can find records from older profiles @@ -545,7 +500,7 @@ Error InstrProfSymtab::create(Module &M, bool InLTO, bool AddCanonical) { for (GlobalVariable &G : M.globals()) { if (!G.hasName() || !G.hasMetadata(LLVMContext::MD_type)) continue; - if (Error E = addVTableWithName(G, getPGOName(G, InLTO))) + if (Error E = addVTableWithName(G, getIRPGOObjectName(G, InLTO))) return E; } @@ -561,8 +516,9 @@ Error InstrProfSymtab::addVTableWithName(GlobalVariable &VTable, return E; bool Inserted = true; - std::tie(std::ignore, Inserted) = MD5VTableMap.try_emplace( - GlobalValue::getGUIDAssumingExternalLinkage(Name), &VTable); + uint64_t GUID = VTable.getGUIDIfAssigned().value_or( + GlobalValue::getGUIDAssumingExternalLinkage(Name)); + std::tie(std::ignore, Inserted) = MD5VTableMap.try_emplace(GUID, &VTable); if (!Inserted) LLVM_DEBUG(dbgs() << "GUID conflict within one module"); return Error::success(); @@ -667,21 +623,22 @@ StringRef InstrProfSymtab::getCanonicalName(StringRef PGOName) { Error InstrProfSymtab::addFuncWithName(Function &F, StringRef PGOFuncName, bool AddCanonical) { - auto NameToGUIDMap = [&](StringRef Name) -> Error { - if (Error E = addFuncName(Name)) - return E; - MD5FuncMap.emplace_back(Function::getGUIDAssumingExternalLinkage(Name), &F); - return Error::success(); - }; - if (Error E = NameToGUIDMap(PGOFuncName)) + if (Error E = addFuncName(PGOFuncName)) return E; + uint64_t GUID = F.getGUIDIfAssigned().value_or( + Function::getGUIDAssumingExternalLinkage(PGOFuncName)); + MD5FuncMap.emplace_back(GUID, &F); if (!AddCanonical) return Error::success(); StringRef CanonicalFuncName = getCanonicalName(PGOFuncName); - if (!CanonicalFuncName.empty() && CanonicalFuncName != PGOFuncName) - return NameToGUIDMap(CanonicalFuncName); + if (!CanonicalFuncName.empty() && CanonicalFuncName != PGOFuncName) { + if (Error E = addFuncName(CanonicalFuncName)) + return E; + MD5FuncMap.emplace_back( + Function::getGUIDAssumingExternalLinkage(CanonicalFuncName), &F); + } return Error::success(); } @@ -772,7 +729,7 @@ Error collectVTableStrings(ArrayRef<GlobalVariable *> VTables, std::string &Result, bool DoCompression) { std::vector<std::string> VTableNameStrs; for (auto *VTable : VTables) - VTableNameStrs.push_back(getPGOName(*VTable)); + VTableNameStrs.push_back(getIRPGOObjectName(*VTable)); return collectGlobalObjectNameStrings( VTableNameStrs, compression::zlib::isAvailable() && DoCompression, Result); @@ -1546,34 +1503,6 @@ getValueProfDataFromInst(const Instruction &Inst, InstrProfValueKind ValueKind, return ValueData; } -MDNode *getPGOFuncNameMetadata(const Function &F) { - return F.getMetadata(getPGOFuncNameMetadataName()); -} - -static void createPGONameMetadata(GlobalObject &GO, StringRef MetadataName, - StringRef PGOName) { - // Only for internal linkage functions or global variables. The name is not - // the same as PGO name for these global objects. - if (GO.getName() == PGOName) - return; - - // Don't create duplicated metadata. - if (GO.getMetadata(MetadataName)) - return; - - LLVMContext &C = GO.getContext(); - MDNode *N = MDNode::get(C, MDString::get(C, PGOName)); - GO.setMetadata(MetadataName, N); -} - -void createPGOFuncNameMetadata(Function &F, StringRef PGOFuncName) { - return createPGONameMetadata(F, getPGOFuncNameMetadataName(), PGOFuncName); -} - -void createPGONameMetadata(GlobalObject &GO, StringRef PGOName) { - return createPGONameMetadata(GO, getPGONameMetadataName(), PGOName); -} - bool needsComdatForCounter(const GlobalObject &GO, const Module &M) { if (GO.hasComdat()) return true; diff --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp index 4a40f826f9ca1..a7d8cd50b115d 100644 --- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp +++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp @@ -1767,7 +1767,7 @@ void InstrLowerer::getOrCreateVTableProfData(GlobalVariable *GV) { // Used by INSTR_PROF_VTABLE_DATA MACRO Constant *VTableAddr = getVTableAddrForProfData(GV); - const std::string PGOVTableName = getPGOName(*GV); + const std::string PGOVTableName = getIRPGOObjectName(*GV); // Record the length of the vtable. This is needed since vtable pointers // loaded from C++ objects might be from the middle of a vtable definition. uint32_t VTableSizeVal = GV->getGlobalSize(M.getDataLayout()); diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp index 06ce7525a0fd8..9f055cb214dc9 100644 --- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp +++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp @@ -675,7 +675,7 @@ template <class Edge, class BBInfo> class FuncPGOInstrumentation { NumOfCSPGOBB += MST.bbInfoSize(); } - FuncName = getIRPGOFuncName(F); + FuncName = getIRPGOObjectName(F); DeprecatedFuncName = getPGOFuncName(F); computeCFGHash(); if (!ComdatMembers.empty()) @@ -1893,9 +1893,6 @@ void PGOUseFunc::annotateValueSites() { if (DisableValueProfiling) return; - // Create the PGOFuncName meta data. - createPGOFuncNameMetadata(F, FuncInfo.FuncName); - for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind) annotateValueSites(Kind); } @@ -2248,16 +2245,6 @@ static bool annotateAllFunctions( return false; } - if (EnableVTableProfileUse) { - for (GlobalVariable &G : M.globals()) { - if (!G.hasName() || !G.hasMetadata(LLVMContext::MD_type)) - continue; - - // Create the PGOFuncName meta data. - createPGONameMetadata(G, getPGOName(G, false /* InLTO*/)); - } - } - // Add the profile summary (read from the header of the indexed summary) here // so that we can use it below when reading counters (which checks if the // function should be marked with a cold or inlinehint attribute). diff --git a/llvm/lib/Transforms/Scalar/JumpTableToSwitch.cpp b/llvm/lib/Transforms/Scalar/JumpTableToSwitch.cpp index a9247e5107b5b..bc9092280f726 100644 --- a/llvm/lib/Transforms/Scalar/JumpTableToSwitch.cpp +++ b/llvm/lib/Transforms/Scalar/JumpTableToSwitch.cpp @@ -213,7 +213,7 @@ PreservedAnalyses JumpTableToSwitchPass::run(Function &F, return *MaybeGUID; return Function::getGUIDAssumingExternalLinkage( - getIRPGOFuncName(Fct, InLTO)); + getIRPGOObjectName(Fct, InLTO)); }; for (BasicBlock &BB : make_early_inc_range(F)) { diff --git a/llvm/test/Instrumentation/cgprofile.ll b/llvm/test/Instrumentation/cgprofile.ll index 72d10f6754c48..bf767a2301f32 100644 --- a/llvm/test/Instrumentation/cgprofile.ll +++ b/llvm/test/Instrumentation/cgprofile.ll @@ -1,5 +1,5 @@ -; RUN: opt < %s -passes='cg-profile<in-lto-post-link>' -S | FileCheck %s --check-prefixes=CHECK,LTO -; RUN: opt < %s -passes='cg-profile' -S | FileCheck %s --check-prefixes=CHECK,NOLTO --implicit-check-not="!{ptr @freq, ptr @func3.llvm.12345" +; RUN: opt < %s -passes='cg-profile<in-lto-post-link>' -S | FileCheck %s --check-prefixes=CHECK +; RUN: opt < %s -passes='cg-profile' -S | FileCheck %s --check-prefixes=CHECK declare void @b() @@ -8,7 +8,7 @@ define void @a() !prof !1 { ret void } -define void @func3.llvm.12345() !PGOFuncName !4 { +define void @func3.llvm.12345() !guid !4 { ret void } @@ -35,16 +35,15 @@ B: !1 = !{!"function_entry_count", i64 32} !2 = !{!"branch_weights", i32 5, i32 10} !3 = !{!"VP", i32 0, i64 1600, i64 7651369219802541373, i64 1030, i64 -4377547752858689819, i64 410, i64 5415368997850289431, i64 150, i64 -2545542355363006406, i64 10, i64 3667884930908592509, i64 1, i64 15435711456043681792, i64 0} -!4 = !{!"cgprofile.ll;func3"} +!4 = !{i64 5415368997850289431} ; CHECK: !llvm.module.flags = !{![[cgprof:[0-9]+]]} ; CHECK: ![[cgprof]] = !{i32 5, !"CG Profile", ![[prof:[0-9]+]]} -; LTO: ![[prof]] = distinct !{![[e0:[0-9]+]], ![[e1:[0-9]+]], ![[e2:[0-9]+]], ![[e3:[0-9]+]], ![[e4:[0-9]+]], ![[e5:[0-9]+]], ![[e6:[0-9]+]]} -; NOLTO: ![[prof]] = distinct !{![[e0:[0-9]+]], ![[e1:[0-9]+]], ![[e2:[0-9]+]], ![[e4:[0-9]+]], ![[e5:[0-9]+]], ![[e6:[0-9]+]]} +; CHECK: ![[prof]] = distinct !{![[e0:[0-9]+]], ![[e1:[0-9]+]], ![[e2:[0-9]+]], ![[e3:[0-9]+]], ![[e4:[0-9]+]], ![[e5:[0-9]+]], ![[e6:[0-9]+]]} ; CHECK: ![[e0]] = !{ptr @a, ptr @b, i64 32} ; CHECK: ![[e1]] = !{ptr @freq, ptr @func4, i64 1030} ; CHECK: ![[e2]] = !{ptr @freq, ptr @func2, i64 410} -; LTO: ![[e3]] = !{ptr @freq, ptr @func3.llvm.12345, i64 150} +; CHECK: ![[e3]] = !{ptr @freq, ptr @func3.llvm.12345, i64 150} ; CHECK: ![[e4]] = !{ptr @freq, ptr @func1, i64 10} ; CHECK: ![[e5]] = !{ptr @freq, ptr @a, i64 11} ; CHECK: ![[e6]] = !{ptr @freq, ptr @b, i64 21} diff --git a/llvm/test/Transforms/PGOProfile/icp_invoke.ll b/llvm/test/Transforms/PGOProfile/icp_invoke.ll index 04fc012c8458a..b7e0a608a29a6 100644 --- a/llvm/test/Transforms/PGOProfile/icp_invoke.ll +++ b/llvm/test/Transforms/PGOProfile/icp_invoke.ll @@ -6,12 +6,12 @@ target triple = "x86_64-unknown-linux-gnu" @foo2 = global ptr null, align 8 @_ZTIi = external constant ptr -define internal void @_ZL4bar1v() !PGOFuncName !0 { +define internal void @_ZL4bar1v() !guid !0 { entry: ret void } -define internal i32 @_ZL4bar2v() !PGOFuncName !1 { +define internal i32 @_ZL4bar2v() !guid !1 { entry: ret i32 100 } @@ -96,8 +96,8 @@ declare ptr @__cxa_begin_catch(ptr) declare void @__cxa_end_catch() -!0 = !{!"invoke.ll:_ZL4bar1v"} -!1 = !{!"invoke.ll:_ZL4bar2v"} +!0 = !{i64 -2732222848796217051} +!1 = !{i64 -6116256810522035449} !2 = !{!"VP", i32 0, i64 1, i64 -2732222848796217051, i64 1} !3 = !{!"VP", i32 0, i64 1, i64 -6116256810522035449, i64 1} ; ICP-NOT: !3 = !{!"VP", i32 0, i64 1, i64 -2732222848796217051, i64 1} diff --git a/llvm/test/Transforms/PGOProfile/icp_invoke_nouse.ll b/llvm/test/Transforms/PGOProfile/icp_invoke_nouse.ll index 9eb7b962ce736..575a162eb288e 100644 --- a/llvm/test/Transforms/PGOProfile/icp_invoke_nouse.ll +++ b/llvm/test/Transforms/PGOProfile/icp_invoke_nouse.ll @@ -5,7 +5,7 @@ target triple = "x86_64-unknown-linux-gnu" @_ZTISt9exception = external constant ptr @pfptr = global ptr null, align 8 -define internal i32 @_ZL4bar1v() !PGOFuncName !0 { +define internal i32 @_ZL4bar1v() !guid !0 { entry: ret i32 100 } @@ -64,5 +64,5 @@ declare ptr @__cxa_begin_catch(ptr) local_unnamed_addr declare void @__cxa_end_catch() local_unnamed_addr -!0 = !{!"invoke.ll:_ZL4bar1v"} +!0 = !{i64 -2732222848796217051} !1 = !{!"VP", i32 0, i64 10000, i64 -2732222848796217051, i64 10000} diff --git a/llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll b/llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll index 27fea917576ff..68bb2a029eaf0 100644 --- a/llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll +++ b/llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll @@ -5,11 +5,11 @@ ; RUN: opt -passes=pgo-instr-use -pgo-test-profile-file=%t/a.profdata -module-summary %t/lib.ll -o %t/lib.bc ; RUN: llvm-lto -thinlto -o %t/summary %t/main.bc %t/lib.bc -; Test that callee with local linkage has `PGOFuncName` metadata while callee with external doesn't have it. +; Test that internal callee has !guid metadata. ; RUN: llvm-dis %t/lib.bc -o - | FileCheck %s --check-prefix=PGONAME ; PGONAME-DAG: define void @_Z7callee1v() {{.*}} !prof ![[#]] -; PGONAME-DAG: define internal void @_ZL7callee0v() {{.*}} !prof ![[#]] !guid ![[#]] !PGOFuncName ![[#MD:]] -; PGONAME: ![[#MD]] = !{!"lib.cc;_ZL7callee0v"} +; PGONAME-DAG: define internal void @_ZL7callee0v() {{.*}} !prof ![[#]] !guid ![[#]] +; PGONAME-NOT: !PGOFuncName ; Tests that both external and internal callees are correctly imported. ; RUN: opt -passes=function-import -summary-file %t/summary.thinlto.bc %t/main.bc -o %t/main.import.bc -print-imports 2>&1 | FileCheck %s --check-prefix=IMPORTS diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp index d6d39c2e4ea02..edaba632ebdc2 100644 --- a/llvm/unittests/ProfileData/InstrProfTest.cpp +++ b/llvm/unittests/ProfileData/InstrProfTest.cpp @@ -639,7 +639,7 @@ TEST_F(InstrProfTest, test_irpgo_function_name) { for (auto &[Name, Linkage, ExpectedIRPGOFuncName] : Data) { auto *F = M->getFunction(Name); - auto IRPGOFuncName = getIRPGOFuncName(*F); + auto IRPGOFuncName = getIRPGOObjectName(*F); EXPECT_EQ(IRPGOFuncName, ExpectedIRPGOFuncName); auto [Filename, ParsedIRPGOFuncName] = getParsedIRPGOName(IRPGOFuncName); @@ -690,8 +690,8 @@ TEST_F(InstrProfTest, test_irpgo_read_deprecated_names) { auto *ExternalBarF = Function::Create(FTy, Function::ExternalLinkage, "ExternalBar", M.get()); - Writer.addRecord({getIRPGOFuncName(*InternalFooF), 0x1234, {1}}, Err); - Writer.addRecord({getIRPGOFuncName(*ExternalFooF), 0x5678, {1}}, Err); + Writer.addRecord({getIRPGOObjectName(*InternalFooF), 0x1234, {1}}, Err); + Writer.addRecord({getIRPGOObjectName(*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); @@ -700,20 +700,20 @@ TEST_F(InstrProfTest, test_irpgo_read_deprecated_names) { readProfile(std::move(Profile)); EXPECT_THAT_EXPECTED( - Reader->getInstrProfRecord(getIRPGOFuncName(*InternalFooF), 0x1234, + Reader->getInstrProfRecord(getIRPGOObjectName(*InternalFooF), 0x1234, getPGOFuncName(*InternalFooF)), Succeeded()); EXPECT_THAT_EXPECTED( - Reader->getInstrProfRecord(getIRPGOFuncName(*ExternalFooF), 0x5678, + Reader->getInstrProfRecord(getIRPGOObjectName(*ExternalFooF), 0x5678, getPGOFuncName(*ExternalFooF)), Succeeded()); // Ensure we can still read this old record name EXPECT_THAT_EXPECTED( - Reader->getInstrProfRecord(getIRPGOFuncName(*InternalBarF), 0x1111, + Reader->getInstrProfRecord(getIRPGOObjectName(*InternalBarF), 0x1111, getPGOFuncName(*InternalBarF)), Succeeded()); EXPECT_THAT_EXPECTED( - Reader->getInstrProfRecord(getIRPGOFuncName(*ExternalBarF), 0x2222, + Reader->getInstrProfRecord(getIRPGOObjectName(*ExternalBarF), 0x2222, getPGOFuncName(*ExternalBarF)), Succeeded()); } @@ -1758,7 +1758,7 @@ TEST(SymtabTest, instr_prof_symtab_module_test) { for (unsigned I = 0; I < std::size(Funcs); I++) { Function *F = M->getFunction(Funcs[I]); - std::string IRPGOName = getIRPGOFuncName(*F); + std::string IRPGOName = getIRPGOObjectName(*F); auto IRPGOFuncName = ProfSymtab.getFuncOrVarName(IndexedInstrProf::ComputeHash(IRPGOName)); EXPECT_EQ(IRPGOName, IRPGOFuncName); @@ -1777,7 +1777,7 @@ TEST(SymtabTest, instr_prof_symtab_module_test) { M->getGlobalVariable(VTableName, /* AllowInternal=*/true); // Test that ProfSymtab returns the expected name given a hash. - std::string IRPGOName = getPGOName(*GV); + std::string IRPGOName = getIRPGOObjectName(*GV); EXPECT_STREQ(IRPGOName.c_str(), PGOName); uint64_t GUID = IndexedInstrProf::ComputeHash(IRPGOName); EXPECT_EQ(IRPGOName, ProfSymtab.getFuncOrVarName(GUID)); >From c33d1036def60dee7a59a3ce65d64e679c676df2 Mon Sep 17 00:00:00 2001 From: Owen Rodley <[email protected]> Date: Wed, 5 Aug 2026 16:40:24 +1000 Subject: [PATCH 2/8] Fix formatting --- llvm/include/llvm/ProfileData/InstrProf.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h index 01f4790077654..70fba625bf09a 100644 --- a/llvm/include/llvm/ProfileData/InstrProf.h +++ b/llvm/include/llvm/ProfileData/InstrProf.h @@ -248,7 +248,8 @@ getPGOFuncName(StringRef RawFuncName, GlobalValue::LinkageTypes Linkage, /// \return the modified name for global object \c GO suitable to be /// used as the key for IRPGO profile lookup. \c InLTO indicates if this is /// called from LTO optimization passes. -LLVM_ABI std::string getIRPGOObjectName(const GlobalObject &GO, bool InLTO = false); +LLVM_ABI std::string getIRPGOObjectName(const GlobalObject &GO, + bool InLTO = false); /// \return the filename and the function name parsed from the output of /// \c getIRPGOObjectName() >From a959075de871717e0fe14c6aecdc747e75c330a8 Mon Sep 17 00:00:00 2001 From: Owen Rodley <[email protected]> Date: Mon, 14 Sep 2026 11:24:39 +1000 Subject: [PATCH 3/8] [InstrProf] Fix up comments after PGO name metadata removal Update references to the renamed getIRPGOObjectName, and document how InstrProfSymtab::addFuncWithName now keys MD5FuncMap off the GUID assigned to a function, falling back to hashing its PGO name. --- llvm/lib/Transforms/Instrumentation/MemProfUse.cpp | 4 ++-- .../Transforms/JumpTableToSwitch/profile-no-guid-metadata.ll | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp b/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp index 09ed88640f3ba..d0c3fdb2c026a 100644 --- a/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp +++ b/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp @@ -691,8 +691,8 @@ readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader, OptimizationRemarkEmitter &ORE, uint64_t MaxColdSize, DenseSet<uint64_t> &SeenStacks, DenseSet<uint64_t> &SeenFrames) { auto &Ctx = M.getContext(); - // Previously we used getIRPGOFuncName() here. If F is local linkage, - // getIRPGOFuncName() returns FuncName with prefix 'FileName;'. But + // Previously we used getIRPGOObjectName() here. If F is local linkage, + // getIRPGOObjectName() returns FuncName with prefix 'FileName;'. But // llvm-profdata uses FuncName in dwarf to create GUID which doesn't // contain FileName's prefix. It caused local linkage function can't // find MemProfRecord. So we use getName() now. diff --git a/llvm/test/Transforms/JumpTableToSwitch/profile-no-guid-metadata.ll b/llvm/test/Transforms/JumpTableToSwitch/profile-no-guid-metadata.ll index 3c902e36224dc..930c4702571ef 100644 --- a/llvm/test/Transforms/JumpTableToSwitch/profile-no-guid-metadata.ll +++ b/llvm/test/Transforms/JumpTableToSwitch/profile-no-guid-metadata.ll @@ -10,7 +10,7 @@ @jt = constant [2 x ptr] [ptr @jt_target_0, ptr @jt_target_1] ;; Note: these functions intentionally do NOT have !guid metadata, -;; forcing the pass to compute GUIDs via getIRPGOFuncName. +;; forcing the pass to compute GUIDs via getIRPGOObjectName. define i32 @jt_target_0() { ; CHECK-LABEL: define i32 @jt_target_0() { ; CHECK-NEXT: ret i32 10 >From 95f04149a9e4287402a77de9b3aa0c4fb41f7924 Mon Sep 17 00:00:00 2001 From: Owen Rodley <[email protected]> Date: Mon, 14 Sep 2026 11:24:39 +1000 Subject: [PATCH 4/8] [InstrProf] Remove obsolete !PGOFuncName from tests Nothing reads !PGOFuncName any more, so remove it from these test inputs. DeadArgElim/func_metadata.ll keeps it as that test is checking that arbitrary function metadata is retained. --- llvm/test/Transforms/JumpThreading/thread-prob-1.ll | 3 +-- llvm/test/Transforms/PGOProfile/Inputs/cspgo_bar_sample.ll | 3 +-- .../test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_gen.ll | 3 +-- .../test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_use.ll | 3 +-- llvm/test/tools/gold/X86/Inputs/thinlto_cspgo_bar.ll | 3 +-- 5 files changed, 5 insertions(+), 10 deletions(-) diff --git a/llvm/test/Transforms/JumpThreading/thread-prob-1.ll b/llvm/test/Transforms/JumpThreading/thread-prob-1.ll index e59539123d6ea..a8a063f9f76a9 100644 --- a/llvm/test/Transforms/JumpThreading/thread-prob-1.ll +++ b/llvm/test/Transforms/JumpThreading/thread-prob-1.ll @@ -4,7 +4,7 @@ ; Make sure that we set the branch probability for the newly created ; basic block. -define void @foo(i1 %cond1, i1 %cond2) !prof !0 !PGOFuncName !1 { +define void @foo(i1 %cond1, i1 %cond2) !prof !0 { entry: br i1 %cond1, label %bb.f1, label %bb.f2, !prof !2 @@ -45,7 +45,6 @@ declare void @f3() declare void @f4() !0 = !{!"function_entry_count", i64 15985} -!1 = !{!"foo.cpp:foo"} !2 = !{!"branch_weights", i32 0, i32 36865} !3 = !{!"branch_weights", i32 35058, i32 1807} !4 = !{!"branch_weights", i32 1807, i32 35058} diff --git a/llvm/test/Transforms/PGOProfile/Inputs/cspgo_bar_sample.ll b/llvm/test/Transforms/PGOProfile/Inputs/cspgo_bar_sample.ll index 4f28c9e078379..19410ae2fc506 100644 --- a/llvm/test/Transforms/PGOProfile/Inputs/cspgo_bar_sample.ll +++ b/llvm/test/Transforms/PGOProfile/Inputs/cspgo_bar_sample.ll @@ -34,7 +34,7 @@ if.end: ret void } -define internal fastcc i32 @cond(i32 %i) #1 !prof !30 !PGOFuncName !36 { +define internal fastcc i32 @cond(i32 %i) #1 !prof !30 { entry: %rem = srem i32 %i, 2 ret i32 %rem @@ -79,4 +79,3 @@ attributes #1 = { inlinehint noinline } !33 = !{!"int", !34, i64 0} !34 = !{!"omnipotent char", !35, i64 0} !35 = !{!"Simple C/C++ TBAA"} -!36 = !{!"cspgo_bar.c:cond"} diff --git a/llvm/test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_gen.ll b/llvm/test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_gen.ll index 8ba468554d439..6993acb2a8fd3 100644 --- a/llvm/test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_gen.ll +++ b/llvm/test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_gen.ll @@ -26,7 +26,7 @@ if.end: ret void } -define internal fastcc i32 @cond(i32 %i) #1 !prof !29 !PGOFuncName !35 { +define internal fastcc i32 @cond(i32 %i) #1 !prof !29 { entry: %rem = srem i32 %i, 2 ret i32 %rem @@ -66,4 +66,3 @@ attributes #1 = { inlinehint noinline } !27 = !{i32 999999, i64 1, i32 6} !29 = !{!"function_entry_count", i64 200000} !30 = !{!"branch_weights", i32 100000, i32 100000} -!35 = !{!"cspgo_bar.c:cond"} diff --git a/llvm/test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_use.ll b/llvm/test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_use.ll index 0412deb8aee21..37534aecd0f27 100644 --- a/llvm/test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_use.ll +++ b/llvm/test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_use.ll @@ -31,7 +31,7 @@ if.end: declare void @clobber() -define internal fastcc i32 @cond(i32 %i) #1 !prof !29 !PGOFuncName !35 { +define internal fastcc i32 @cond(i32 %i) #1 !prof !29 { entry: %rem = srem i32 %i, 2 ret i32 %rem @@ -72,4 +72,3 @@ attributes #1 = { inlinehint noinline } !27 = !{i32 999999, i64 1, i32 6} !29 = !{!"function_entry_count", i64 200000} !30 = !{!"branch_weights", i32 100000, i32 100000} -!35 = !{!"cspgo_bar.c:cond"} diff --git a/llvm/test/tools/gold/X86/Inputs/thinlto_cspgo_bar.ll b/llvm/test/tools/gold/X86/Inputs/thinlto_cspgo_bar.ll index 08f3ace6674d7..07314af6e9c42 100644 --- a/llvm/test/tools/gold/X86/Inputs/thinlto_cspgo_bar.ll +++ b/llvm/test/tools/gold/X86/Inputs/thinlto_cspgo_bar.ll @@ -63,7 +63,7 @@ for.inc.3: ret void } -define internal fastcc i32 @cond(i32 %i) #1 !prof !29 !PGOFuncName !36 { +define internal fastcc i32 @cond(i32 %i) #1 !prof !29 { entry: %rem = srem i32 %i, 2 ret i32 %rem @@ -108,4 +108,3 @@ attributes #1 = { inlinehint noinline } !33 = !{!"omnipotent char", !34, i64 0} !34 = !{!"Simple C/C++ TBAA"} !35 = !{!"branch_weights", i32 400001, i32 399999} -!36 = !{!"csfdo_bar.c:cond"} >From 7f6f6c0043f76191806a8f3869f9e2721901f21e Mon Sep 17 00:00:00 2001 From: Owen Rodley <[email protected]> Date: Mon, 14 Sep 2026 11:24:39 +1000 Subject: [PATCH 5/8] [InstrProf] Keep name-derived symtab keys for GUID-assigned objects InstrProfSymtab maps hashes back to functions and vtables, so that profile records can be matched to IR. Each one is registered under several names: - its IRPGO name - its deprecated front-end PGO name, for backwards compatibility - optionally, a canonical name with suffixes like .llvm. stripped Each of these should be findable by the hash of that name. The previous change keyed all of these entries on the object's assigned GUID instead. So once a function or vtable has a GUID, lookups by any of the other hashes fail. For example, a lookup by the hash of a function's deprecated name, or of a vtable's canonical name, no longer finds it. Fix this by keying each name on its own hash again, and adding the assigned GUID as an extra key. The GUID key is what lets us find an object that LTO has renamed since the profile was collected, which is what !PGOFuncName and !PGOName metadata used to be for. Tests: - A unit test that renames a function the way ThinLTO does, and checks it's still found by its GUID. - A unit test that checks a function with a GUID is still found by its deprecated name. - A unit test that checks a renamed vtable with a GUID is found by its GUID, its current name, and its canonical name. - guid-roundtrip.test, which round-trips a profile collected from a real program through the text format. It covers an internal function's "file;name" key, and indirect call targets, which are stored as hashes and printed as names. --- llvm/include/llvm/ProfileData/InstrProf.h | 8 +- llvm/lib/ProfileData/InstrProf.cpp | 43 ++++++--- .../llvm-profdata/Inputs/guid-roundtrip.c | 28 ++++++ .../Inputs/guid-roundtrip.proftext | 44 +++++++++ .../tools/llvm-profdata/guid-roundtrip.test | 35 +++++++ llvm/unittests/ProfileData/InstrProfTest.cpp | 93 +++++++++++++++++++ 6 files changed, 236 insertions(+), 15 deletions(-) create mode 100644 llvm/test/tools/llvm-profdata/Inputs/guid-roundtrip.c create mode 100644 llvm/test/tools/llvm-profdata/Inputs/guid-roundtrip.proftext create mode 100644 llvm/test/tools/llvm-profdata/guid-roundtrip.test diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h index 70fba625bf09a..4134f9ddf2b28 100644 --- a/llvm/include/llvm/ProfileData/InstrProf.h +++ b/llvm/include/llvm/ProfileData/InstrProf.h @@ -548,7 +548,9 @@ class InstrProfSymtab { // map entries: // name-set = {PGOFuncName} union {getCanonicalName(PGOFuncName)} // - In MD5NameMap: <MD5Hash(name), name> for name in name-set - // - In MD5FuncMap: <MD5Hash(name), &F> for name in name-set + // - In MD5FuncMap: <MD5Hash(name), &F> for name in name-set, and + // <GUID, &F> if \c F has a GUID that differs from MD5Hash(PGOFuncName), + // e.g. because LTO renamed \c F. // The canonical name is only added if \c AddCanonical is true. Error addFuncWithName(Function &F, StringRef PGOFuncName, bool AddCanonical); @@ -556,7 +558,9 @@ class InstrProfSymtab { // map entries: // name-set = {PGOName} union {getCanonicalName(PGOName)} // - In MD5NameMap: <MD5Hash(name), name> for name in name-set - // - In MD5VTableMap: <MD5Hash(name), name> for name in name-set + // - In MD5VTableMap: <MD5Hash(name), &V> for name in name-set, and + // <GUID, &V> if \c V has been assigned a GUID that differs from + // MD5Hash(PGOVTableName). Error addVTableWithName(GlobalVariable &V, StringRef PGOVTableName); // If the symtab is created by a series of calls to \c addFuncName, \c diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp index 3976dbdee1b7e..2b7dc198735da 100644 --- a/llvm/lib/ProfileData/InstrProf.cpp +++ b/llvm/lib/ProfileData/InstrProf.cpp @@ -511,14 +511,15 @@ Error InstrProfSymtab::create(Module &M, bool InLTO, bool AddCanonical) { Error InstrProfSymtab::addVTableWithName(GlobalVariable &VTable, StringRef VTablePGOName) { + // Key each name by its own hash, so profiles that recorded that name can find + // the vtable. auto NameToGUIDMap = [&](StringRef Name) -> Error { if (Error E = addSymbolName(Name)) return E; bool Inserted = true; - uint64_t GUID = VTable.getGUIDIfAssigned().value_or( - GlobalValue::getGUIDAssumingExternalLinkage(Name)); - std::tie(std::ignore, Inserted) = MD5VTableMap.try_emplace(GUID, &VTable); + std::tie(std::ignore, Inserted) = MD5VTableMap.try_emplace( + GlobalValue::getGUIDAssumingExternalLinkage(Name), &VTable); if (!Inserted) LLVM_DEBUG(dbgs() << "GUID conflict within one module"); return Error::success(); @@ -526,6 +527,13 @@ Error InstrProfSymtab::addVTableWithName(GlobalVariable &VTable, if (Error E = NameToGUIDMap(VTablePGOName)) return E; + // Also key the vtable by its GUID, so it can still be found if LTO has + // renamed it. See addFuncWithName. + if (auto GUID = VTable.getGUIDIfAssigned(); + GUID && + *GUID != GlobalValue::getGUIDAssumingExternalLinkage(VTablePGOName)) + MD5VTableMap.try_emplace(*GUID, &VTable); + StringRef CanonicalName = getCanonicalName(VTablePGOName); if (!CanonicalName.empty() && CanonicalName != VTablePGOName) return NameToGUIDMap(CanonicalName); @@ -623,22 +631,31 @@ StringRef InstrProfSymtab::getCanonicalName(StringRef PGOName) { Error InstrProfSymtab::addFuncWithName(Function &F, StringRef PGOFuncName, bool AddCanonical) { - if (Error E = addFuncName(PGOFuncName)) + // Key each name by its own hash, so profiles that recorded that name can find + // the function. This is called once per name, e.g. a second time with the + // deprecated PGO name, for profiles from older compilers. + auto NameToGUIDMap = [&](StringRef Name) -> Error { + if (Error E = addFuncName(Name)) + return E; + MD5FuncMap.emplace_back(Function::getGUIDAssumingExternalLinkage(Name), &F); + return Error::success(); + }; + if (Error E = NameToGUIDMap(PGOFuncName)) return E; - uint64_t GUID = F.getGUIDIfAssigned().value_or( - Function::getGUIDAssumingExternalLinkage(PGOFuncName)); - MD5FuncMap.emplace_back(GUID, &F); + + // Also key the function by its GUID, if it has one. The GUID is the hash of + // the function's name when the GUID was assigned, so this still finds the + // function if LTO has renamed it since. This used to need !PGOFuncName. + if (auto GUID = F.getGUIDIfAssigned(); + GUID && *GUID != Function::getGUIDAssumingExternalLinkage(PGOFuncName)) + MD5FuncMap.emplace_back(*GUID, &F); if (!AddCanonical) return Error::success(); StringRef CanonicalFuncName = getCanonicalName(PGOFuncName); - if (!CanonicalFuncName.empty() && CanonicalFuncName != PGOFuncName) { - if (Error E = addFuncName(CanonicalFuncName)) - return E; - MD5FuncMap.emplace_back( - Function::getGUIDAssumingExternalLinkage(CanonicalFuncName), &F); - } + if (!CanonicalFuncName.empty() && CanonicalFuncName != PGOFuncName) + return NameToGUIDMap(CanonicalFuncName); return Error::success(); } diff --git a/llvm/test/tools/llvm-profdata/Inputs/guid-roundtrip.c b/llvm/test/tools/llvm-profdata/Inputs/guid-roundtrip.c new file mode 100644 index 0000000000000..fe2f50321e6b8 --- /dev/null +++ b/llvm/test/tools/llvm-profdata/Inputs/guid-roundtrip.c @@ -0,0 +1,28 @@ +// Program used to generate guid-roundtrip.proftext. See guid-roundtrip.test +// for how to regenerate it. + +#include <stdio.h> + +static int local_callee(int x) { return x + 1; } + +int external_callee(int x) { return x + 2; } + +// Volatile so the indirect calls aren't optimized away, and get +// value-profiled. +typedef int (*callee_t)(int); +volatile callee_t indirect_callee; + +int main(void) { + int sum = 0; + + indirect_callee = local_callee; + for (int i = 0; i < 100; i++) + sum += indirect_callee(i); + + indirect_callee = external_callee; + for (int i = 0; i < 10; i++) + sum += indirect_callee(i); + + printf("%d\n", sum); + return 0; +} diff --git a/llvm/test/tools/llvm-profdata/Inputs/guid-roundtrip.proftext b/llvm/test/tools/llvm-profdata/Inputs/guid-roundtrip.proftext new file mode 100644 index 0000000000000..acc4226035fe4 --- /dev/null +++ b/llvm/test/tools/llvm-profdata/Inputs/guid-roundtrip.proftext @@ -0,0 +1,44 @@ +# IR level Instrumentation Flag +:ir +external_callee +# Func Hash: +742261418966908927 +# Num Counters: +1 +# Counter Values: +10 +# Num Bitmap Bytes: +$0 + +guid-roundtrip.c;local_callee +# Func Hash: +742261418966908927 +# Num Counters: +1 +# Counter Values: +100 +# Num Bitmap Bytes: +$0 + +main +# Func Hash: +783881052960888712 +# Num Counters: +3 +# Counter Values: +100 +10 +1 +# Num Bitmap Bytes: +$0 +# Num Value Kinds: +1 +# ValueKind = IPVK_IndirectCallTarget: +0 +# NumValueSites: +2 +1 +guid-roundtrip.c;local_callee:100 +1 +external_callee:10 + diff --git a/llvm/test/tools/llvm-profdata/guid-roundtrip.test b/llvm/test/tools/llvm-profdata/guid-roundtrip.test new file mode 100644 index 0000000000000..0c84e1e6de7cd --- /dev/null +++ b/llvm/test/tools/llvm-profdata/guid-roundtrip.test @@ -0,0 +1,35 @@ +# Round-trip a profile from a real program through the text format. +# +# Unlike roundtrip.test, this profile has an internal function, keyed as +# "<file>;<name>", and indirect call targets, which are stored as hashes and +# printed as names. Both depend on the name <-> GUID mapping. +# +# Inputs/guid-roundtrip.proftext was generated from Inputs/guid-roundtrip.c: +# clang -O2 -fprofile-generate=. -o guid-roundtrip Inputs/guid-roundtrip.c +# LLVM_PROFILE_FILE=guid.profraw ./guid-roundtrip +# llvm-profdata merge --text -o Inputs/guid-roundtrip.proftext guid.profraw + +# text -> indexed -> text +RUN: llvm-profdata merge -o %t.0.profdata %S/Inputs/guid-roundtrip.proftext +RUN: llvm-profdata merge --text -o %t.0.proftext %t.0.profdata +RUN: diff -b %t.0.proftext %S/Inputs/guid-roundtrip.proftext + +# Do it again from the output, to check it's stable. +RUN: llvm-profdata merge -o %t.1.profdata %t.0.proftext +RUN: llvm-profdata merge --text -o %t.1.proftext %t.1.profdata +RUN: diff -b %t.1.proftext %S/Inputs/guid-roundtrip.proftext + +RUN: FileCheck %s --input-file=%t.1.proftext + +# The internal function is keyed by its IRPGO name, "<file>;<name>". The hash +# of that name is the function's GUID. +CHECK: external_callee +CHECK: guid-roundtrip.c;local_callee +CHECK: main + +# Indirect call targets are printed as names, not hashes, so both must be in +# the symbol table. +CHECK: # ValueKind = IPVK_IndirectCallTarget: +CHECK: guid-roundtrip.c;local_callee:100 +CHECK-NEXT: 1 +CHECK-NEXT: external_callee:10 diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp index edaba632ebdc2..9578cf1497fbf 100644 --- a/llvm/unittests/ProfileData/InstrProfTest.cpp +++ b/llvm/unittests/ProfileData/InstrProfTest.cpp @@ -718,6 +718,99 @@ TEST_F(InstrProfTest, test_irpgo_read_deprecated_names) { Succeeded()); } +// Check that a function renamed by LTO after it was profiled can still be +// found by its GUID. This used to need !PGOFuncName metadata. +TEST_F(InstrProfTest, test_symtab_lookup_renamed_function_by_guid) { + LLVMContext Ctx; + auto M = std::make_unique<Module>("MyModule.cpp", Ctx); + auto *FTy = FunctionType::get(Type::getVoidTy(Ctx), /*isVarArg=*/false); + auto *F = + Function::Create(FTy, Function::InternalLinkage, "InternalFoo", M.get()); + + // Assign the GUID while the function still has its original name. + const std::string ProfiledName = getIRPGOObjectName(*F); + EXPECT_EQ(ProfiledName, "MyModule.cpp;InternalFoo"); + const uint64_t GUID = Function::getGUIDAssumingExternalLinkage(ProfiledName); + F->setMetadata(LLVMContext::MD_guid, + MDNode::get(Ctx, {ConstantAsMetadata::get(ConstantInt::get( + Type::getInt64Ty(Ctx), GUID))})); + + // Promote and rename the function the way ThinLTO would. + F->setName("InternalFoo.llvm.9999"); + F->setLinkage(Function::ExternalLinkage); + ASSERT_NE(getIRPGOObjectName(*F, /*InLTO=*/true), ProfiledName); + + // The original name is gone from the IR, but the GUID still finds the + // function. + InstrProfSymtab Symtab; + EXPECT_THAT_ERROR(Symtab.create(*M, /*InLTO=*/true), Succeeded()); + EXPECT_EQ(Symtab.getFunction(GUID), F); +} + +// Check that a function with a GUID can still be found by the hash of its +// deprecated PGO name, which is what profiles from older compilers use. +TEST_F(InstrProfTest, test_symtab_lookup_deprecated_name_with_assigned_guid) { + LLVMContext Ctx; + auto M = std::make_unique<Module>("MyModule.cpp", Ctx); + auto *FTy = FunctionType::get(Type::getVoidTy(Ctx), /*isVarArg=*/false); + auto *F = + Function::Create(FTy, Function::InternalLinkage, "InternalFoo", M.get()); + // Needs a body: declarations always have a GUID, even without !guid. + IRBuilder<> Builder(BasicBlock::Create(Ctx, "entry", F)); + Builder.CreateRetVoid(); + + const uint64_t GUID = + Function::getGUIDAssumingExternalLinkage(getIRPGOObjectName(*F)); + const uint64_t DeprecatedNameHash = + Function::getGUIDAssumingExternalLinkage(getPGOFuncName(*F)); + + InstrProfSymtab WithoutGUID; + EXPECT_THAT_ERROR(WithoutGUID.create(*M), Succeeded()); + EXPECT_EQ(WithoutGUID.getFunction(GUID), F) << "IRPGO name lookup, no !guid"; + EXPECT_EQ(WithoutGUID.getFunction(DeprecatedNameHash), F) + << "deprecated name lookup, no !guid"; + + F->setMetadata(LLVMContext::MD_guid, + MDNode::get(Ctx, {ConstantAsMetadata::get(ConstantInt::get( + Type::getInt64Ty(Ctx), GUID))})); + + InstrProfSymtab WithGUID; + EXPECT_THAT_ERROR(WithGUID.create(*M), Succeeded()); + EXPECT_EQ(WithGUID.getFunction(GUID), F) << "IRPGO name lookup, with !guid"; + EXPECT_EQ(WithGUID.getFunction(DeprecatedNameHash), F) + << "deprecated name lookup, with !guid"; +} + +// Check that a vtable renamed by LTO can be found by its GUID, and by the +// hashes of its current and canonical names. +TEST_F(InstrProfTest, test_symtab_lookup_vtable_with_assigned_guid) { + LLVMContext Ctx; + auto M = std::make_unique<Module>("MyModule.cpp", Ctx); + auto *Int32Ty = Type::getInt32Ty(Ctx); + auto *GV = new GlobalVariable( + *M, Int32Ty, /*isConstant=*/true, GlobalValue::ExternalLinkage, + ConstantInt::get(Int32Ty, 0), "_ZTV3Foo.llvm.7"); + // Only vtables with type metadata are added to the symtab. + GV->addTypeMetadata(16, MDString::get(Ctx, "_ZTS3Foo")); + + // The GUID from before LTO promoted and renamed the vtable. + const uint64_t GUID = + GlobalValue::getGUIDAssumingExternalLinkage("MyModule.cpp;_ZTV3Foo"); + GV->setMetadata(LLVMContext::MD_guid, + MDNode::get(Ctx, {ConstantAsMetadata::get(ConstantInt::get( + Type::getInt64Ty(Ctx), GUID))})); + + InstrProfSymtab Symtab; + EXPECT_THAT_ERROR(Symtab.create(*M), Succeeded()); + EXPECT_EQ(Symtab.getGlobalVariable(GUID), GV); + EXPECT_EQ(Symtab.getGlobalVariable( + GlobalValue::getGUIDAssumingExternalLinkage("_ZTV3Foo.llvm.7")), + GV); + EXPECT_EQ(Symtab.getGlobalVariable( + GlobalValue::getGUIDAssumingExternalLinkage("_ZTV3Foo")), + GV); +} + // callee1 to callee6 are from vtable1 to vtable6 respectively. static const char callee1[] = "callee1"; static const char callee2[] = "callee2"; >From 62656cc5fae7a26680aea70edcfbfe8840a5594f Mon Sep 17 00:00:00 2001 From: Owen Rodley <[email protected]> Date: Mon, 28 Sep 2026 11:13:32 +1000 Subject: [PATCH 6/8] [MemProf] Remove stale PGOFuncName references from comment --- .../Transforms/IPO/MemProfContextDisambiguation.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp index 421686fc64bac..d3849fd596070 100644 --- a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp +++ b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp @@ -5712,12 +5712,12 @@ bool MemProfContextDisambiguation::initializeIndirectCallPromotionInfo( // when they both have the same root name with "." suffixes stripped. // If we pick the wrong one then this could lead to incorrect ICP and calling // a memprof clone that we don't actually create (resulting in linker unsats). - // What this means is that the GUID of the function (or its PGOFuncName - // metadata) *must* match that in the VP metadata to allow promotion. - // In practice this should not be a limitation, since local functions should - // have PGOFuncName metadata and global function names shouldn't need any - // special handling (they should not get the ".llvm.*" suffix that the - // canonicalization handling is attempting to strip). + // What this means is that the GUID of the function *must* match that in the + // VP metadata to allow promotion. In practice this should not be a + // limitation, since local functions have a GUID from before they were + // promoted and global function names shouldn't need any special handling + // (they should not get the ".llvm.*" suffix that the canonicalization + // handling is attempting to strip). if (Error E = Symtab->create(M, /*InLTO=*/true, /*AddCanonical=*/false)) { std::string SymtabFailure = toString(std::move(E)); M.getContext().emitError("Failed to create symtab: " + SymtabFailure); >From 6868ec24ffc228bf7a15145f50e7c90cc2f05f5b Mon Sep 17 00:00:00 2001 From: Owen Rodley <[email protected]> Date: Mon, 28 Sep 2026 13:33:52 +1000 Subject: [PATCH 7/8] Fix formatting --- llvm/unittests/ProfileData/InstrProfTest.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp index 9578cf1497fbf..0c09467169abe 100644 --- a/llvm/unittests/ProfileData/InstrProfTest.cpp +++ b/llvm/unittests/ProfileData/InstrProfTest.cpp @@ -733,7 +733,7 @@ TEST_F(InstrProfTest, test_symtab_lookup_renamed_function_by_guid) { const uint64_t GUID = Function::getGUIDAssumingExternalLinkage(ProfiledName); F->setMetadata(LLVMContext::MD_guid, MDNode::get(Ctx, {ConstantAsMetadata::get(ConstantInt::get( - Type::getInt64Ty(Ctx), GUID))})); + Type::getInt64Ty(Ctx), GUID))})); // Promote and rename the function the way ThinLTO would. F->setName("InternalFoo.llvm.9999"); @@ -772,7 +772,7 @@ TEST_F(InstrProfTest, test_symtab_lookup_deprecated_name_with_assigned_guid) { F->setMetadata(LLVMContext::MD_guid, MDNode::get(Ctx, {ConstantAsMetadata::get(ConstantInt::get( - Type::getInt64Ty(Ctx), GUID))})); + Type::getInt64Ty(Ctx), GUID))})); InstrProfSymtab WithGUID; EXPECT_THAT_ERROR(WithGUID.create(*M), Succeeded()); >From cf184221e89cb8daea3683aa3a272f179ee7786a Mon Sep 17 00:00:00 2001 From: Owen Rodley <[email protected]> Date: Mon, 28 Sep 2026 14:26:41 +1000 Subject: [PATCH 8/8] [InstrProf] Check profile and GUID in compiler-rt ThinLTO ICP test The PGOName check lines in this test weren't used by any RUN line, and still expected !PGOFuncName metadata. Add a RUN line for them, and check for !guid instead. --- .../instrprof-thinlto-indirect-call-promotion.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler-rt/test/profile/instrprof-thinlto-indirect-call-promotion.cpp b/compiler-rt/test/profile/instrprof-thinlto-indirect-call-promotion.cpp index 4509c0149bdbd..a74ce48738b4b 100644 --- a/compiler-rt/test/profile/instrprof-thinlto-indirect-call-promotion.cpp +++ b/compiler-rt/test/profile/instrprof-thinlto-indirect-call-promotion.cpp @@ -44,6 +44,9 @@ // Use profile on main and get bitcode. // RUN: %clang -fprofile-use=main.profdata -flto=thin -O2 -c main.cpp -o main.bc +// Test that the profile was loaded, and the internal callee has a GUID. +// RUN: llvm-dis lib.bc -o - | FileCheck %s --check-prefix=PGOName + // Run llvm-lto to get summary file. // RUN: llvm-lto -thinlto -o summary main.bc lib.bc @@ -60,9 +63,9 @@ // IMPORTS-DAG: main.cpp: Import {{.*}}callee0{{.*}}llvm.[[#]] // IMPORTS-DAG: main.cpp: Import {{.*}}global_func{{.*}} -// PGOName-DAG: define {{.*}}callee1{{.*}} !prof ![[#]] { -// PGOName-DAG: define internal {{.*}}callee0{{.*}} !prof ![[#]] !PGOFuncName ![[#MD:]] { -// PGOName-DAG: ![[#MD]] = !{!"{{.*}}lib.cpp;{{.*}}callee0{{.*}}"} +// PGOName-DAG: define {{.*}}callee1{{.*}} !prof ![[#]] +// PGOName-DAG: define internal {{.*}}callee0{{.*}} !prof ![[#]] !guid ![[#]] +// PGOName-NOT: !PGOFuncName // ICP-REMARK: Promote indirect call to {{.*}}callee0{{.*}}llvm.[[#]] with count 1 out of 1 // ICP-REMARK: Promote indirect call to {{.*}}callee1{{.*}} with count 1 out of 1 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
