[llvm] [NFC][InstrProf]Factor out getCanonicalName to compute the canonical name given a pgo name. (PR #81547)
Mingming Liu via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 13 09:44:31 PST 2024
https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/81547
>From dc936bc756300d24060b84cbf02d50a66f3e3a6c Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Mon, 12 Feb 2024 14:47:05 -0800
Subject: [PATCH 1/4] [NFC][InstrProf]Factor out getCanonicalName out of
InstrProf::AddFuncWithName
---
llvm/include/llvm/ProfileData/InstrProf.h | 13 +++++++
llvm/lib/ProfileData/InstrProf.cpp | 43 +++++++++++++----------
2 files changed, 38 insertions(+), 18 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index aa08e949b5eaf2..b5d04b592e4f2f 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -449,6 +449,19 @@ class InstrProfSymtab {
return "** External Symbol **";
}
+ // Returns the canonial name of the given PGOName. In a canonical name, all
+ // suffixes that begins with "." except ".__uniq." are stripped.
+ // FIXME: Unify this with `FunctionSamples::getCanonicalFnName`. And both
+ // should probably preserve `.specialized.<NSpecs>` suffix now that
+ // function-specialization is enabled by default.
+ static StringRef getCanonicalName(StringRef PGOName);
+
+ // Add the function into the symbol table, by creating the following
+ // map entries:
+ // name-set = {PGOFuncName} + {getCanonicalName(PGOFuncName)} if the canonical
+ // name is different from pgo name
+ // - In MD5NameMap: <MD5Hash(name), name> for name in name-set
+ // - In MD5FuncMap: <MD5Hash(name), &F> for name in name-set
Error addFuncWithName(Function &F, StringRef PGOFuncName);
// 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 d26004e2385bca..ae4462bc1f57ba 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -517,34 +517,41 @@ Error InstrProfSymtab::create(StringRef NameStrings) {
std::bind(&InstrProfSymtab::addFuncName, this, std::placeholders::_1));
}
-Error InstrProfSymtab::addFuncWithName(Function &F, StringRef PGOFuncName) {
- if (Error E = addFuncName(PGOFuncName))
- return E;
- MD5FuncMap.emplace_back(Function::getGUID(PGOFuncName), &F);
+StringRef InstrProfSymtab::getCanonicalName(StringRef PGOName) {
// 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.
+ // ".__uniq." suffix is used to differentiate internal linkage functions in
+ // different modules and should be kept. This is the only suffix with the
+ // pattern ".xxx" which is kept before matching, other suffixes similar as
+ // ".llvm." will be stripped.
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)
+ size_t pos = PGOName.find(UniqSuffix);
+ if (pos != StringRef::npos)
pos += UniqSuffix.length();
else
pos = 0;
- pos = PGOFuncName.find('.', pos);
- if (pos != std::string::npos && pos != 0) {
- StringRef OtherFuncName = PGOFuncName.substr(0, pos);
- if (Error E = addFuncName(OtherFuncName))
+
+ // Search '.' after ".__uniq." if ".__uniq." exists, otherwise search '.' from
+ // the beginning.
+ pos = PGOName.find('.', pos);
+ if (pos != StringRef::npos && pos != 0)
+ return PGOName.substr(0, pos);
+
+ return PGOName;
+}
+
+Error InstrProfSymtab::addFuncWithName(Function &F, StringRef PGOFuncName) {
+ StringSet<> Names;
+ Names.insert(PGOFuncName);
+ Names.insert(InstrProfSymtab::getCanonicalName(PGOFuncName));
+ for (const auto &NameEntry : Names) {
+ StringRef Name = NameEntry.getKey();
+ if (Error E = addFuncName(Name))
return E;
- MD5FuncMap.emplace_back(Function::getGUID(OtherFuncName), &F);
+ MD5FuncMap.emplace_back(Function::getGUID(Name), &F);
}
return Error::success();
}
>From 37d090ab5a4053f1a0f80585c380692dc91d771e Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Mon, 12 Feb 2024 18:57:09 -0800
Subject: [PATCH 2/4] delete the comment on function-specialization as
specialized functions doesn't exist in the IR at profile annotation time
---
llvm/include/llvm/ProfileData/InstrProf.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index b5d04b592e4f2f..a928ba6961f367 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -451,9 +451,7 @@ class InstrProfSymtab {
// Returns the canonial name of the given PGOName. In a canonical name, all
// suffixes that begins with "." except ".__uniq." are stripped.
- // FIXME: Unify this with `FunctionSamples::getCanonicalFnName`. And both
- // should probably preserve `.specialized.<NSpecs>` suffix now that
- // function-specialization is enabled by default.
+ // FIXME: Unify this with `FunctionSamples::getCanonicalFnName`.
static StringRef getCanonicalName(StringRef PGOName);
// Add the function into the symbol table, by creating the following
>From 795aa98c178bb534850a3489e8ad81c2460f526a Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Mon, 12 Feb 2024 20:44:06 -0800
Subject: [PATCH 3/4] Use SmallVector<StringRef,2> rather than StringSet<>
---
llvm/lib/ProfileData/InstrProf.cpp | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index ae4462bc1f57ba..ff99c275eae35a 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -544,11 +544,12 @@ StringRef InstrProfSymtab::getCanonicalName(StringRef PGOName) {
}
Error InstrProfSymtab::addFuncWithName(Function &F, StringRef PGOFuncName) {
- StringSet<> Names;
- Names.insert(PGOFuncName);
- Names.insert(InstrProfSymtab::getCanonicalName(PGOFuncName));
- for (const auto &NameEntry : Names) {
- StringRef Name = NameEntry.getKey();
+ SmallVector<StringRef, 2> Names;
+ Names.push_back(PGOFuncName);
+ StringRef CanonicalFuncName = getCanonicalName(PGOFuncName);
+ if (CanonicalFuncName != PGOFuncName)
+ Names.push_back(CanonicalFuncName);
+ for (StringRef Name : Names) {
if (Error E = addFuncName(Name))
return E;
MD5FuncMap.emplace_back(Function::getGUID(Name), &F);
>From f59fa0b3505b98ffdc3d299a1bc5fee12a5b3dfc Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Tue, 13 Feb 2024 09:44:04 -0800
Subject: [PATCH 4/4] resolve review feedback
---
llvm/lib/ProfileData/InstrProf.cpp | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index ff99c275eae35a..6d5dd317c536f5 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -544,17 +544,20 @@ StringRef InstrProfSymtab::getCanonicalName(StringRef PGOName) {
}
Error InstrProfSymtab::addFuncWithName(Function &F, StringRef PGOFuncName) {
- SmallVector<StringRef, 2> Names;
- Names.push_back(PGOFuncName);
- StringRef CanonicalFuncName = getCanonicalName(PGOFuncName);
- if (CanonicalFuncName != PGOFuncName)
- Names.push_back(CanonicalFuncName);
- for (StringRef Name : Names) {
+ auto mapName = [&](StringRef Name) -> Error {
if (Error E = addFuncName(Name))
return E;
MD5FuncMap.emplace_back(Function::getGUID(Name), &F);
- }
- return Error::success();
+ return Error::success();
+ };
+ if (Error E = mapName(PGOFuncName))
+ return E;
+
+ StringRef CanonicalFuncName = getCanonicalName(PGOFuncName);
+ if (CanonicalFuncName == PGOFuncName)
+ return Error::success();
+
+ return mapName(CanonicalFuncName);
}
uint64_t InstrProfSymtab::getFunctionHashFromAddress(uint64_t Address) {
More information about the llvm-commits
mailing list