From: Dehao Chen Date: Mon, 17 Apr 2017 22:23:05 +0000 (+0000) Subject: Build SymbolMap in SampleProfileLoader to help matchin function names with suffix. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=62b2c39c08511b4863f306e85a378276f0056c7d;p=llvm Build SymbolMap in SampleProfileLoader to help matchin function names with suffix. Summary: If there is suffix added in the function name (e.g. module hash added by thinLTO), we will not be able to find a match in profile as the suffix does not exist in profile. This patch build a map from function name to Function *. The map includes the entry for the stripped function name so that inlineHotFunctions can find the corresponding function to promote/inline. Reviewers: davidxl, dnovillo, tejohnson Reviewed By: davidxl Subscribers: mehdi_amini, llvm-commits Differential Revision: https://reviews.llvm.org/D31952 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@300507 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Transforms/IPO/SampleProfile.cpp b/lib/Transforms/IPO/SampleProfile.cpp index 3371de6e3d1..599774e3e86 100644 --- a/lib/Transforms/IPO/SampleProfile.cpp +++ b/lib/Transforms/IPO/SampleProfile.cpp @@ -43,6 +43,7 @@ #include "llvm/IR/MDBuilder.h" #include "llvm/IR/Metadata.h" #include "llvm/IR/Module.h" +#include "llvm/IR/ValueSymbolTable.h" #include "llvm/Pass.h" #include "llvm/ProfileData/InstrProf.h" #include "llvm/ProfileData/SampleProfReader.h" @@ -208,6 +209,12 @@ protected: /// the same number of times. EquivalenceClassMap EquivalenceClass; + /// Map from function name to Function *. Used to find the function from + /// the function name. If the function name contains suffix, additional + /// entry is added to map from the stripped name to the function if there + /// is one-to-one mapping. + StringMap SymbolMap; + /// \brief Dominance, post-dominance and loop information. std::unique_ptr DT; std::unique_ptr> PDT; @@ -689,7 +696,10 @@ bool SampleProfileLoader::inlineHotFunctions( for (const auto *FS : findIndirectCallFunctionSamples(*I)) { auto CalleeFunctionName = FS->getName(); const char *Reason = "Callee function not available"; - CalledFunction = F.getParent()->getFunction(CalleeFunctionName); + auto R = SymbolMap.find(CalleeFunctionName); + if (R == SymbolMap.end()) + continue; + CalledFunction = R->getValue(); if (CalledFunction && isLegalToPromote(I, CalledFunction, &Reason)) { // The indirect target was promoted and inlined in the profile, as a // result, we do not have profile info for the branch probability. @@ -1414,6 +1424,26 @@ bool SampleProfileLoader::runOnModule(Module &M) { for (const auto &I : Reader->getProfiles()) TotalCollectedSamples += I.second.getTotalSamples(); + // Populate the symbol map. + for (const auto &N_F : M.getValueSymbolTable()) { + std::string OrigName = N_F.getKey(); + Function *F = dyn_cast(N_F.getValue()); + if (F == nullptr) + continue; + SymbolMap[OrigName] = F; + auto pos = OrigName.find('.'); + if (pos != std::string::npos) { + std::string NewName = OrigName.substr(0, pos); + auto r = SymbolMap.insert(std::make_pair(NewName, F)); + // Failiing to insert means there is already an entry in SymbolMap, + // thus there are multiple functions that are mapped to the same + // stripped name. In this case of name conflicting, set the value + // to nullptr to avoid confusion. + if (!r.second) + r.first->second = nullptr; + } + } + bool retval = false; for (auto &F : M) if (!F.isDeclaration()) { diff --git a/test/Transforms/SampleProfile/Inputs/indirect-call.prof b/test/Transforms/SampleProfile/Inputs/indirect-call.prof index 428d4cedef5..aaf9ec15d02 100644 --- a/test/Transforms/SampleProfile/Inputs/indirect-call.prof +++ b/test/Transforms/SampleProfile/Inputs/indirect-call.prof @@ -11,3 +11,9 @@ test_noinline:3000:0 test_direct:3000:0 5: foo_direct:3000 1: 3000 +test_inline_strip:3000:0 + 5: foo_inline_strip:3000 + 1: 3000 +test_inline_strip_confilict:3000:0 + 5: foo_inline_strip_conflict:3000 + 1: 3000 diff --git a/test/Transforms/SampleProfile/indirect-call.ll b/test/Transforms/SampleProfile/indirect-call.ll index 5a4913d6358..4647dd42129 100644 --- a/test/Transforms/SampleProfile/indirect-call.ll +++ b/test/Transforms/SampleProfile/indirect-call.ll @@ -29,6 +29,34 @@ define void @test_inline(i64* (i32*)*, i32* %x) !dbg !3 { ret void } +; CHECK-LABEL: @test_inline_strip +; If the indirect call is promoted and inlined in profile, and the callee name +; is stripped we should promote and inline it. +define void @test_inline_strip(i64* (i32*)*, i32* %x) !dbg !3 { + %2 = alloca i64* (i32*)* + store i64* (i32*)* %0, i64* (i32*)** %2 + %3 = load i64* (i32*)*, i64* (i32*)** %2 +; CHECK: icmp {{.*}} @foo_inline_strip.suffix +; CHECK: if.true.direct_targ: +; CHECK-NOT: call +; CHECK: if.false.orig_indirect: +; CHECK: call + call i64* %3(i32* %x), !dbg !5 + ret void +} + +; CHECK-LABEL: @test_inline_strip_conflict +; If the indirect call is promoted and inlined in profile, and the callee name +; is stripped, but have more than 1 potential match, we should not promote. +define void @test_inline_strip_conflict(i64* (i32*)*, i32* %x) !dbg !3 { + %2 = alloca i64* (i32*)* + store i64* (i32*)* %0, i64* (i32*)** %2 + %3 = load i64* (i32*)*, i64* (i32*)** %2 +; CHECK-NOT: if.true.direct_targ: + call i64* %3(i32* %x), !dbg !5 + ret void +} + ; CHECK-LABEL: @test_noinline ; If the indirect call target is not available, we should not promote it. define void @test_noinline(void ()*) !dbg !3 { @@ -47,6 +75,22 @@ define i32* @foo_inline1(i32* %x) !dbg !3 { ret i32* %x } +define i32* @foo_inline_strip.suffix(i32* %x) !dbg !3 { + ret i32* %x +} + +define i32* @foo_inline_strip_conflict.suffix1(i32* %x) !dbg !3 { + ret i32* %x +} + +define i32* @foo_inline_strip_conflict.suffix2(i32* %x) !dbg !3 { + ret i32* %x +} + +define i32* @foo_inline_strip_conflict.suffix3(i32* %x) !dbg !3 { + ret i32* %x +} + define i32* @foo_inline2(i32* %x) !dbg !3 { ret i32* %x }