[clang] [llvm] [nfc]Generalize PGOFuncName helper methods for general global objects (PR #73570)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 27 13:19:31 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-clang-codegen
Author: Mingming Liu (minglotus-6)
<details>
<summary>Changes</summary>
- Rename `PGOFuncName` metadata to `PGOName`. Updated tests accordingly.
- Generalize existing helper functions (rename, change parameter type) so they could be used for GlobalObject.
This is a split of https://github.com/llvm/llvm-project/pull/66825 to use PGOName for vtables. In particular, the old format is used. The difference of old and new format only matters in one scenario -- the instrPGO (not SamplePGO) for local functions when it comes to the import of (profiled) indirect functions or vtables.
1. ThinTLO computes the list of imported variables and functions using `GlobalValue::GUID`, which is computed using [global identifier](https://github.com/llvm/llvm-project/blob/39faf13dde5502cdba7aff1b967c51cd0a93bb71/llvm/include/llvm/IR/GlobalValue.h#L591).
2. When merging raw profiles into indexed profiles, the profiled address is [mapped](https://github.com/llvm/llvm-project/blob/39faf13dde5502cdba7aff1b967c51cd0a93bb71/llvm/lib/ProfileData/InstrProf.cpp#L882) to the MD5 hash. For local linkage functions or global variables. The MD5 is annotated in value profiles.
3. MD5 hash in the annotation and ThinLTO should be computed from the same name. For globals, the name is the same already. For local ones, old format uses `filename:name` but new format uses `filename;name`.
- This applies to local functions for ICP as well. I have a patch to record `hash-of-new-format` and `hash-of-old-format` in the raw profiles, so the remap could remap (and annotate) old hash. Still needs some work to make it work.
---
Full diff: https://github.com/llvm/llvm-project/pull/73570.diff
12 Files Affected:
- (modified) clang/lib/CodeGen/CodeGenPGO.cpp (+3-3)
- (modified) llvm/include/llvm/ProfileData/InstrProf.h (+9-12)
- (modified) llvm/lib/ProfileData/InstrProf.cpp (+47-39)
- (modified) llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp (+2-2)
- (modified) llvm/test/Transforms/DeadArgElim/func_metadata.ll (+6-6)
- (modified) llvm/test/Transforms/JumpThreading/thread-prob-1.ll (+1-1)
- (modified) llvm/test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_gen.ll (+1-1)
- (modified) llvm/test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_use.ll (+1-1)
- (modified) llvm/test/Transforms/PGOProfile/Inputs/thinlto_indirect_call_promotion.ll (+1-1)
- (modified) llvm/test/Transforms/PGOProfile/icp_invoke.ll (+2-2)
- (modified) llvm/test/Transforms/PGOProfile/icp_invoke_nouse.ll (+1-1)
- (modified) llvm/test/tools/gold/X86/Inputs/thinlto_cspgo_bar.ll (+1-1)
``````````diff
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 81bf8ea696b1647..18f5ef85a6ada55 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -34,7 +34,7 @@ using namespace CodeGen;
void CodeGenPGO::setFuncName(StringRef Name,
llvm::GlobalValue::LinkageTypes Linkage) {
llvm::IndexedInstrProfReader *PGOReader = CGM.getPGOReader();
- FuncName = llvm::getPGOFuncName(
+ FuncName = llvm::getPGOObjectName(
Name, Linkage, CGM.getCodeGenOpts().MainFileName,
PGOReader ? PGOReader->getVersion() : llvm::IndexedInstrProf::Version);
@@ -45,8 +45,8 @@ void CodeGenPGO::setFuncName(StringRef Name,
void CodeGenPGO::setFuncName(llvm::Function *Fn) {
setFuncName(Fn->getName(), Fn->getLinkage());
- // Create PGOFuncName meta data.
- llvm::createPGOFuncNameMetadata(*Fn, FuncName);
+ // Create PGOName meta data.
+ llvm::createPGONameMetadata(*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 3bc677d5b6d8670..b9c159a3fa41436 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -181,10 +181,10 @@ std::string getPGOFuncName(const Function &F, bool InLTO = false,
/// 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);
+std::string getPGOObjectName(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
@@ -279,15 +279,12 @@ bool getValueProfDataFromInst(const Instruction &Inst,
uint32_t &ActualNumValueData, uint64_t &TotalC,
bool GetNoICPValue = false);
-inline StringRef getPGOFuncNameMetadataName() { return "PGOFuncName"; }
-
-/// Return the PGOFuncName meta data associated with a function.
-MDNode *getPGOFuncNameMetadata(const Function &F);
+inline StringRef getPGONameMetadataName() { return "PGOName"; }
-/// 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.
-void createPGOFuncNameMetadata(Function &F, StringRef PGOFuncName);
+/// Create the PGOName meta data if PGOName is different from the object's raw
+/// name. This should only apply to internal linkage objects declared by users
+/// only (i.e., not internalized by LTO).
+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.
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index 236b083a1e2155b..1017d16ba23230a 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -246,11 +246,11 @@ std::string InstrProfError::message() const {
char InstrProfError::ID = 0;
-std::string getPGOFuncName(StringRef RawFuncName,
- GlobalValue::LinkageTypes Linkage,
- StringRef FileName,
- uint64_t Version LLVM_ATTRIBUTE_UNUSED) {
- return GlobalValue::getGlobalIdentifier(RawFuncName, Linkage, FileName);
+std::string getPGOObjectName(StringRef RawName,
+ GlobalValue::LinkageTypes Linkage,
+ StringRef FileName,
+ uint64_t Version LLVM_ATTRIBUTE_UNUSED) {
+ return GlobalValue::getGlobalIdentifier(RawName, Linkage, FileName);
}
// Strip NumPrefix level of directory name from PathNameStr. If the number of
@@ -287,11 +287,11 @@ static StringRef getStrippedSourceFileName(const GlobalObject &GO) {
// ; 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
+// Older compilers used getPGOObjectName() which has the format
+// [<filepath>:]<object-name>. <filepath> is used to discriminate between
+// possibly identical object names when linkage is local and <object-name>
+// simply comes from GO.getName(). This caused trouble for Objective-C functions
+// which commonly have :'s in their names. Also, since <object-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.
@@ -325,9 +325,9 @@ static std::optional<std::string> lookupPGONameFromMetadata(MDNode *MD) {
// 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
+// PGOName 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
+// (PGOUseFunc::annotateValueSites). 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) {
@@ -337,39 +337,51 @@ static std::string getIRPGOObjectName(const GlobalObject &GO, bool InLTO,
}
// In LTO mode (when InLTO is true), first check if there is a meta data.
- if (auto IRPGOFuncName = lookupPGONameFromMetadata(PGONameMetadata))
- return *IRPGOFuncName;
+ if (auto IRPGOName = lookupPGONameFromMetadata(PGONameMetadata))
+ return *IRPGOName;
- // 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
+ // If there is no meta data, the global object 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, "");
}
+/// Return the PGOName meta data associated with a global object.
+static MDNode *getPGONameMetadata(const GlobalObject &GO) {
+ return GO.getMetadata(getPGONameMetadataName());
+}
+
// 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));
+ return getIRPGOObjectName(F, InLTO, getPGONameMetadata(F));
}
-// 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
+// 'getPGOObjectName' 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) {
+static std::string getPGOObjectName(const GlobalObject &GO, bool InLTO,
+ uint64_t Version) {
if (!InLTO) {
- auto FileName = getStrippedSourceFileName(F);
- return getPGOFuncName(F.getName(), F.getLinkage(), FileName, Version);
+ auto FileName = getStrippedSourceFileName(GO);
+ return getPGOObjectName(GO.getName(), GO.getLinkage(), FileName, Version);
}
// In LTO mode (when InLTO is true), first check if there is a meta data.
- if (auto PGOFuncName = lookupPGONameFromMetadata(getPGOFuncNameMetadata(F)))
+ if (auto PGOFuncName = lookupPGONameFromMetadata(getPGONameMetadata(GO)))
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, "");
+ // If there is no meta data, the function or global variable must be a global
+ // before the value profile annotation pass. Its current linkage may be
+ // internal if it is internalized in LTO mode.
+ return getPGOObjectName(GO.getName(), GlobalValue::ExternalLinkage, "");
+}
+
+// Returns the PGO function name and does special handling when called in LTO
+// optimization. See the comments of `getPGOObjectName` for details.
+std::string getPGOFuncName(const Function &F, bool InLTO, uint64_t Version) {
+ return getPGOObjectName(F, InLTO, Version);
}
// See getIRPGOFuncName() for a discription of the format.
@@ -1255,20 +1267,16 @@ bool getValueProfDataFromInst(const Instruction &Inst,
return true;
}
-MDNode *getPGOFuncNameMetadata(const Function &F) {
- return F.getMetadata(getPGOFuncNameMetadataName());
-}
-
-void createPGOFuncNameMetadata(Function &F, StringRef PGOFuncName) {
- // Only for internal linkage functions.
- if (PGOFuncName == F.getName())
- return;
+void createPGONameMetadata(GlobalObject &GO, StringRef PGOName) {
+ // Only for internal linkage functions or global variables.
+ if (PGOName == GO.getName())
+ return;
// Don't create duplicated meta-data.
- if (getPGOFuncNameMetadata(F))
+ if (getPGONameMetadata(GO))
return;
- LLVMContext &C = F.getContext();
- MDNode *N = MDNode::get(C, MDString::get(C, PGOFuncName));
- F.setMetadata(getPGOFuncNameMetadataName(), N);
+ LLVMContext &C = GO.getContext();
+ MDNode *N = MDNode::get(C, MDString::get(C, PGOName));
+ GO.setMetadata(getPGONameMetadataName(), N);
}
bool needsComdatForCounter(const Function &F, const Module &M) {
diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
index 4a5a0b25bebbaf1..734051520b51435 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -1700,8 +1700,8 @@ void PGOUseFunc::annotateValueSites() {
if (DisableValueProfiling)
return;
- // Create the PGOFuncName meta data.
- createPGOFuncNameMetadata(F, FuncInfo.FuncName);
+ // Create the PGOName meta data.
+ createPGONameMetadata(F, FuncInfo.FuncName);
for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind)
annotateValueSites(Kind);
diff --git a/llvm/test/Transforms/DeadArgElim/func_metadata.ll b/llvm/test/Transforms/DeadArgElim/func_metadata.ll
index 4922798b3aaa35e..bf3962c1b82226c 100644
--- a/llvm/test/Transforms/DeadArgElim/func_metadata.ll
+++ b/llvm/test/Transforms/DeadArgElim/func_metadata.ll
@@ -7,8 +7,8 @@ target triple = "x86_64-unknown-linux-gnu"
@s = common dso_local local_unnamed_addr global i32 0, align 4
-define internal i32 @va_func(i32 %num, ...) !prof !28 !PGOFuncName !29{
-; CHECK: define internal void @va_func(i32 %num) !prof ![[ENTRYCOUNT:[0-9]+]] !PGOFuncName ![[PGOFUNCNAME1:[0-9]+]] {
+define internal i32 @va_func(i32 %num, ...) !prof !28 !PGOName !29{
+; CHECK: define internal void @va_func(i32 %num) !prof ![[ENTRYCOUNT:[0-9]+]] !PGOName ![[PGOName1:[0-9]+]] {
entry:
%0 = load i32, ptr @s, align 4, !tbaa !31
%add = add nsw i32 %0, %num
@@ -16,8 +16,8 @@ entry:
ret i32 0
}
-define internal fastcc i32 @foo() unnamed_addr !prof !28 !PGOFuncName !30 {
-; CHECK: define internal fastcc void @foo() unnamed_addr !prof ![[ENTRYCOUNT:[0-9]+]] !PGOFuncName ![[PGOFUNCNAME2:[0-9]+]] {
+define internal fastcc i32 @foo() unnamed_addr !prof !28 !PGOName !30 {
+; CHECK: define internal fastcc void @foo() unnamed_addr !prof ![[ENTRYCOUNT:[0-9]+]] !PGOName ![[PGOName2:[0-9]+]] {
entry:
%0 = load i32, ptr @s, align 4, !tbaa !31
%add = add nsw i32 %0, 8
@@ -58,9 +58,9 @@ entry:
!28 = !{!"function_entry_count", i64 1}
; CHECK: ![[ENTRYCOUNT]] = !{!"function_entry_count", i64 1}
!29 = !{!"foo.c:va_func"}
-; CHECK: ![[PGOFUNCNAME1]] = !{!"foo.c:va_func"}
+; CHECK: ![[PGOName1]] = !{!"foo.c:va_func"}
!30 = !{!"foo.c:foo"}
-; CHECK: ![[PGOFUNCNAME2]] = !{!"foo.c:foo"}
+; CHECK: ![[PGOName2]] = !{!"foo.c:foo"}
!31 = !{!32, !32, i64 0}
!32 = !{!"int", !33, i64 0}
!33 = !{!"omnipotent char", !34, i64 0}
diff --git a/llvm/test/Transforms/JumpThreading/thread-prob-1.ll b/llvm/test/Transforms/JumpThreading/thread-prob-1.ll
index e59539123d6ea38..aab578b68a70523 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 !PGOName !1 {
entry:
br i1 %cond1, label %bb.f1, label %bb.f2, !prof !2
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 8ba468554d4396e..5d8aeec60d0197a 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 !PGOName !35 {
entry:
%rem = srem i32 %i, 2
ret i32 %rem
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 0412deb8aee21af..defa8774b3ed2e2 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 !PGOName !35 {
entry:
%rem = srem i32 %i, 2
ret i32 %rem
diff --git a/llvm/test/Transforms/PGOProfile/Inputs/thinlto_indirect_call_promotion.ll b/llvm/test/Transforms/PGOProfile/Inputs/thinlto_indirect_call_promotion.ll
index 7412120bb52cf50..1b2ed4c3e5b86a7 100644
--- a/llvm/test/Transforms/PGOProfile/Inputs/thinlto_indirect_call_promotion.ll
+++ b/llvm/test/Transforms/PGOProfile/Inputs/thinlto_indirect_call_promotion.ll
@@ -8,7 +8,7 @@ entry:
ret void
}
-define internal void @c() !PGOFuncName !1 {
+define internal void @c() !PGOName !1 {
entry:
ret void
}
diff --git a/llvm/test/Transforms/PGOProfile/icp_invoke.ll b/llvm/test/Transforms/PGOProfile/icp_invoke.ll
index 04fc012c8458a9d..c3c9ebb3f6a2502 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() !PGOName !0 {
entry:
ret void
}
-define internal i32 @_ZL4bar2v() !PGOFuncName !1 {
+define internal i32 @_ZL4bar2v() !PGOName !1 {
entry:
ret i32 100
}
diff --git a/llvm/test/Transforms/PGOProfile/icp_invoke_nouse.ll b/llvm/test/Transforms/PGOProfile/icp_invoke_nouse.ll
index 9eb7b962ce7362f..cb5ff4468cafd12 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() !PGOName !0 {
entry:
ret i32 100
}
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 08f3ace6674d76e..c54c098ed332869 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 !PGOName !36 {
entry:
%rem = srem i32 %i, 2
ret i32 %rem
``````````
</details>
https://github.com/llvm/llvm-project/pull/73570
More information about the llvm-commits
mailing list