[PATCH] D28440: [ThinLTO] Import static functions from the same module as caller
Teresa Johnson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 12 14:24:22 PST 2017
This revision was automatically updated to reflect the committed changes.
Closed by commit rL291841: [ThinLTO] Import static functions from the same module as caller (authored by tejohnson).
Changed prior to commit:
https://reviews.llvm.org/D28440?vs=84177&id=84178#toc
Repository:
rL LLVM
https://reviews.llvm.org/D28440
Files:
llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
llvm/trunk/test/ThinLTO/X86/local_name_conflict.ll
Index: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
===================================================================
--- llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
+++ llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
@@ -124,7 +124,7 @@
static const GlobalValueSummary *
selectCallee(const ModuleSummaryIndex &Index,
const GlobalValueSummaryList &CalleeSummaryList,
- unsigned Threshold) {
+ unsigned Threshold, StringRef CallerModulePath) {
auto It = llvm::find_if(
CalleeSummaryList,
[&](const std::unique_ptr<GlobalValueSummary> &SummaryPtr) {
@@ -145,6 +145,21 @@
auto *Summary = cast<FunctionSummary>(GVSummary);
+ // If this is a local function, make sure we import the copy
+ // in the caller's module. The only time a local function can
+ // share an entry in the index is if there is a local with the same name
+ // in another module that had the same source file name (in a different
+ // directory), where each was compiled in their own directory so there
+ // was not distinguishing path.
+ // However, do the import from another module if there is only one
+ // entry in the list - in that case this must be a reference due
+ // to indirect call profile data, since a function pointer can point to
+ // a local in another module.
+ if (GlobalValue::isLocalLinkage(Summary->linkage()) &&
+ CalleeSummaryList.size() > 1 &&
+ Summary->modulePath() != CallerModulePath)
+ return false;
+
if (Summary->instCount() > Threshold)
return false;
@@ -163,11 +178,13 @@
/// null if there's no match.
static const GlobalValueSummary *selectCallee(GlobalValue::GUID GUID,
unsigned Threshold,
- const ModuleSummaryIndex &Index) {
+ const ModuleSummaryIndex &Index,
+ StringRef CallerModulePath) {
auto CalleeSummaryList = Index.findGlobalValueSummaryList(GUID);
if (CalleeSummaryList == Index.end())
return nullptr; // This function does not have a summary
- return selectCallee(Index, CalleeSummaryList->second, Threshold);
+ return selectCallee(Index, CalleeSummaryList->second, Threshold,
+ CallerModulePath);
}
using EdgeInfo = std::tuple<const FunctionSummary *, unsigned /* Threshold */,
@@ -202,7 +219,8 @@
const auto NewThreshold =
Threshold * GetBonusMultiplier(Edge.second.Hotness);
- auto *CalleeSummary = selectCallee(GUID, NewThreshold, Index);
+ auto *CalleeSummary =
+ selectCallee(GUID, NewThreshold, Index, Summary.modulePath());
if (!CalleeSummary) {
DEBUG(dbgs() << "ignored! No qualifying callee with summary found.\n");
continue;
Index: llvm/trunk/test/ThinLTO/X86/local_name_conflict.ll
===================================================================
--- llvm/trunk/test/ThinLTO/X86/local_name_conflict.ll
+++ llvm/trunk/test/ThinLTO/X86/local_name_conflict.ll
@@ -1,14 +1,25 @@
+; Test handling when two files with the same source file name contain
+; static functions with the same name (which will have the same GUID
+; in the combined index.
+
; Do setup work for all below tests: generate bitcode and combined index
; RUN: opt -module-summary -module-hash %s -o %t.bc
; RUN: opt -module-summary -module-hash %p/Inputs/local_name_conflict1.ll -o %t2.bc
; RUN: opt -module-summary -module-hash %p/Inputs/local_name_conflict2.ll -o %t3.bc
; RUN: llvm-lto -thinlto-action=thinlink -o %t4.bc %t.bc %t2.bc %t3.bc
-; Make sure foo is promoted and renamed without complaint in both
-; Inputs/local_name_conflict1.ll and Inputs/local_name_conflict2.ll
-; FIXME: Once the importer is fixed to import the correct copy of the
-; local, we should be able to verify that via an import action.
-; RUN: llvm-lto -thinlto-action=promote %t2.bc -thinlto-index=%t4.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=EXPORTSTATIC
+; This module will import b() which should cause the copy of foo from
+; that module (%t3.bc) to be imported. Check that the imported reference's
+; promoted name matches the imported copy.
+; RUN: llvm-lto -thinlto-action=import %t.bc -thinlto-index=%t4.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=IMPORT
+; IMPORT: call i32 @foo.llvm.[[HASH:[0-9A-F]+]]
+; IMPORT: define available_externally hidden i32 @foo.llvm.[[HASH]]()
+
+; The copy in %t2.bc should not be exported/promoted/renamed
+; RUN: llvm-lto -thinlto-action=promote %t2.bc -thinlto-index=%t4.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=NOEXPORTSTATIC
+; NOEXPORTSTATIC: define internal i32 @foo()
+
+; Make sure foo is promoted and renamed without complaint in %t3.bc.
; RUN: llvm-lto -thinlto-action=promote %t3.bc -thinlto-index=%t4.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=EXPORTSTATIC
; EXPORTSTATIC: define hidden i32 @foo.llvm.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D28440.84178.patch
Type: text/x-patch
Size: 5069 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170112/9799d93f/attachment.bin>
More information about the llvm-commits
mailing list