[llvm] ef9d7db - [IndirectCallPromotion] Recommit "Don't strip ".__uniq." suffix when it strips
Wei Mi via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 12 13:48:26 PST 2021
Author: Wei Mi
Date: 2021-03-12T13:48:14-08:00
New Revision: ef9d7db72362b2df1237a87d5dc7dad6b0105df6
URL: https://github.com/llvm/llvm-project/commit/ef9d7db72362b2df1237a87d5dc7dad6b0105df6
DIFF: https://github.com/llvm/llvm-project/commit/ef9d7db72362b2df1237a87d5dc7dad6b0105df6.diff
LOG: [IndirectCallPromotion] Recommit "Don't strip ".__uniq." suffix when it strips
".llvm." suffix".
The recommit fixed a bug that symbols with "." at the beginning is not
properly handled in the last commit.
Original commit message:
Currently IndirectCallPromotion simply strip everything after the first "."
in LTO mode, in order to match the symbol name and the name with ".llvm."
suffix in the value profile. However, if -funique-internal-linkage-names
and thinlto are both enabled, the name may have both ".__uniq." suffix and
".llvm." suffix, and the current mechanism will strip them both, which is
unexpected. The patch fixes the problem.
Differential Revision: https://reviews.llvm.org/D98389
Added:
llvm/test/Transforms/PGOProfile/indirect_call_promotion_unique.ll
Modified:
llvm/lib/ProfileData/InstrProf.cpp
llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp
Removed:
################################################################################
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index f1167bb45579..d2900c2a99f9 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -353,16 +353,29 @@ Error InstrProfSymtab::create(Module &M, bool InLTO) {
return E;
MD5FuncMap.emplace_back(Function::getGUID(PGOFuncName), &F);
// In ThinLTO, local function may have been promoted to global and have
- // suffix 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.
- if (InLTO) {
- auto pos = PGOFuncName.find('.');
- if (pos != std::string::npos) {
- const std::string &OtherFuncName = PGOFuncName.substr(0, pos);
- if (Error E = addFuncName(OtherFuncName))
- return E;
- MD5FuncMap.emplace_back(Function::getGUID(OtherFuncName), &F);
- }
+ // 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
diff erentiate internal linkage functions in
diff erent 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;
diff --git a/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp b/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp
index 5b9557a9b328..b630cb46c486 100644
--- a/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp
+++ b/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp
@@ -393,8 +393,7 @@ static bool promoteIndirectCalls(Module &M, ProfileSummaryInfo *PSI,
InstrProfSymtab Symtab;
if (Error E = Symtab.create(M, InLTO)) {
std::string SymtabFailure = toString(std::move(E));
- LLVM_DEBUG(dbgs() << "Failed to create symtab: " << SymtabFailure << "\n");
- (void)SymtabFailure;
+ M.getContext().emitError("Failed to create symtab: " + SymtabFailure);
return false;
}
bool Changed = false;
diff --git a/llvm/test/Transforms/PGOProfile/indirect_call_promotion_unique.ll b/llvm/test/Transforms/PGOProfile/indirect_call_promotion_unique.ll
new file mode 100644
index 000000000000..b41afc0d04db
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/indirect_call_promotion_unique.ll
@@ -0,0 +1,101 @@
+; RUN: opt < %s -passes=pgo-icall-prom -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"
+
+ at foo = common global i32 ()* null, align 8
+
+; The names on the IR and in the profile are both "func1".
+define i32 @func1() {
+entry:
+ ret i32 1
+}
+
+define i32 @bar1() {
+entry:
+ %tmp1 = load i32 ()*, i32 ()** @foo, align 8
+; CHECK: icmp eq i32 ()* %tmp1, @func1
+ %call = call i32 %tmp1(), !prof !1
+ ret i32 %call
+}
+
+; The name on the IR has ".llvm." suffix: "func2.llvm.10895321227755557127".
+; The name in the profile has no ".llvm." suffix: "func2".
+define i32 @func2.llvm.10895321227755557127() {
+entry:
+ ret i32 1
+}
+
+define i32 @bar2() {
+entry:
+ %tmp2 = load i32 ()*, i32 ()** @foo, align 8
+; CHECK: icmp eq i32 ()* %tmp2, @func2.llvm.10895321227755557127
+ %call = call i32 %tmp2(), !prof !2
+ ret i32 %call
+}
+
+; The names on the IR and in the profile are both
+; "func3.__uniq.258901567653530696343884446915951489119".
+define i32 @func3.__uniq.258901567653530696343884446915951489119() {
+entry:
+ ret i32 2
+}
+
+define i32 @bar3() {
+entry:
+ %tmp3 = load i32 ()*, i32 ()** @foo, align 8
+; CHECK: icmp eq i32 ()* %tmp3, @func3.__uniq.258901567653530696343884446915951489119
+ %call = call i32 %tmp3(), !prof !3
+ ret i32 %call
+}
+
+; The name on the IR has ".__uniq." and ".llvm." suffix:
+; "func4.__uniq.140291095734751150107370763113257199296.llvm.10650195578168450516".
+; The name in the profile has ".__uniq." but no ".llvm." suffix:
+; "func4.__uniq.140291095734751150107370763113257199296".
+define i32 @func4.__uniq.140291095734751150107370763113257199296.llvm.10650195578168450516() {
+entry:
+ ret i32 3
+}
+
+define i32 @bar4() {
+entry:
+ %tmp4 = load i32 ()*, i32 ()** @foo, align 8
+; CHECK: icmp eq i32 ()* %tmp4, @func4.__uniq.140291095734751150107370763113257199296.llvm.10650195578168450516
+ %call = call i32 %tmp4(), !prof !4
+ ret i32 %call
+}
+
+; The name on the IR has ".__uniq.", ".part." and ".llvm." suffix:
+; "func4.__uniq.127882361580787111523790444488985774976.part.818292359123831.llvm.10650195578168450516".
+; The name in the profile has ".__uniq." but no ".llvm." and no ".part." suffix:
+; "func4.__uniq.127882361580787111523790444488985774976".
+define i32 @func5.__uniq.127882361580787111523790444488985774976.part.818292359123831.llvm.10650195578168450516() {
+entry:
+ ret i32 3
+}
+
+define i32 @bar5() {
+entry:
+ %tmp5 = load i32 ()*, i32 ()** @foo, align 8
+; CHECK: icmp eq i32 ()* %tmp5, @func5.__uniq.127882361580787111523790444488985774976.part.818292359123831.llvm.10650195578168450516
+ %call = call i32 %tmp5(), !prof !5
+ ret i32 %call
+}
+
+; Check symbol with '.' in the front is handled properly.
+define i32 @..func6() {
+entry:
+ ret i32 3
+}
+
+; GUID of "func1" is -2545542355363006406.
+; GUID of "func2" is -4377547752858689819.
+; GUID of "func3.__uniq.258901567653530696343884446915951489119" is 8271224222042874235.
+; GUID of "func4.__uniq.140291095734751150107370763113257199296" is 1491826207425861106.
+; GUID of "func5.__uniq.127882361580787111523790444488985774976" is -4238550483433487304.
+!1 = !{!"VP", i32 0, i64 1600, i64 -2545542355363006406, i64 1600}
+!2 = !{!"VP", i32 0, i64 1600, i64 -4377547752858689819, i64 1600}
+!3 = !{!"VP", i32 0, i64 1600, i64 8271224222042874235, i64 1600}
+!4 = !{!"VP", i32 0, i64 1600, i64 1491826207425861106, i64 1600}
+!5 = !{!"VP", i32 0, i64 1600, i64 -4238550483433487304, i64 1600}
More information about the llvm-commits
mailing list