From 83aaf358fdd08647ae21f08bb4c5aaec0b47c584 Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Thu, 12 Jan 2017 22:04:45 +0000 Subject: [PATCH] [ThinLTO] Import static functions from the same module as caller Summary: We can sometimes end up with multiple copies of a local function that have the same GUID in the index. This happens when there are local functions with the same name that are in different source files with the same name (but in different directories), and they were compiled in their own directory so had the same path at compile time. In this case make sure we import the copy in the caller's module. While it isn't a correctness problem (the renamed reference which is based on the module IR hash will be unique since the module must have had an externally visible function that was imported), importing the wrong copy will result in lost performance opportunity since it won't be referenced and inlined. Reviewers: mehdi_amini Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D28440 llvm-svn: 291841 --- llvm/lib/Transforms/IPO/FunctionImport.cpp | 26 +++++++++++++++++--- llvm/test/ThinLTO/X86/local_name_conflict.ll | 21 ++++++++++++---- 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp index 6b32f6c31f72..4ce50b4e6428 100644 --- a/llvm/lib/Transforms/IPO/FunctionImport.cpp +++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp @@ -124,7 +124,7 @@ namespace { 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 &SummaryPtr) { @@ -145,6 +145,21 @@ selectCallee(const ModuleSummaryIndex &Index, auto *Summary = cast(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 @@ selectCallee(const ModuleSummaryIndex &Index, /// 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