<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Dec 1, 2015 at 8:34 PM, Mehdi Amini via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: mehdi_amini<br>
Date: Tue Dec 1 22:34:28 2015<br>
New Revision: 254486<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=254486&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=254486&view=rev</a><br>
Log:<br>
Change ModuleLinker to take a set of GlobalValues to import instead of a single one<br>
<br>
For efficiency reason, when importing multiple functions for the same Module,<br>
we can avoid reparsing it every time.<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D15102" rel="noreferrer" target="_blank">http://reviews.llvm.org/D15102</a><br>
<br>
From: Mehdi Amini <<a href="mailto:mehdi.amini@apple.com">mehdi.amini@apple.com</a>><br>
<br>
Modified:<br>
llvm/trunk/include/llvm/Linker/Linker.h<br>
llvm/trunk/lib/Linker/LinkModules.cpp<br>
llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp<br>
llvm/trunk/tools/llvm-link/llvm-link.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/Linker/Linker.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Linker/Linker.h?rev=254486&r1=254485&r2=254486&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Linker/Linker.h?rev=254486&r1=254485&r2=254486&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Linker/Linker.h (original)<br>
+++ llvm/trunk/include/llvm/Linker/Linker.h Tue Dec 1 22:34:28 2015<br>
@@ -82,7 +82,7 @@ public:<br>
/// Returns true on error.<br>
bool linkInModule(Module &Src, unsigned Flags = Flags::None,<br>
const FunctionInfoIndex *Index = nullptr,<br>
- Function *FuncToImport = nullptr);<br>
+ DenseSet<const GlobalValue *> *FuncToImport = nullptr);<br>
<br>
static bool linkModules(Module &Dest, Module &Src,<br>
DiagnosticHandlerFunction DiagnosticHandler,<br>
<br>
Modified: llvm/trunk/lib/Linker/LinkModules.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=254486&r1=254485&r2=254486&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=254486&r1=254485&r2=254486&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Linker/LinkModules.cpp (original)<br>
+++ llvm/trunk/lib/Linker/LinkModules.cpp Tue Dec 1 22:34:28 2015<br>
@@ -406,7 +406,7 @@ class ModuleLinker {<br>
<br>
/// Function to import from source module, all other functions are<br>
/// imported as declarations instead of definitions.<br>
- Function *ImportFunction;<br>
+ DenseSet<const GlobalValue *> *ImportFunction;<br>
<br>
/// Set to true if the given FunctionInfoIndex contains any functions<br>
/// from this source module, in which case we must conservatively assume<br>
@@ -425,7 +425,7 @@ public:<br>
ModuleLinker(Module &DstM, Linker::IdentifiedStructTypeSet &Set, Module &SrcM,<br>
DiagnosticHandlerFunction DiagnosticHandler, unsigned Flags,<br>
const FunctionInfoIndex *Index = nullptr,<br>
- Function *FuncToImport = nullptr)<br>
+ DenseSet<const GlobalValue *> *FuncToImport = nullptr)<br>
: DstM(DstM), SrcM(SrcM), TypeMap(Set), ValMaterializer(this),<br>
DiagnosticHandler(DiagnosticHandler), Flags(Flags), ImportIndex(Index),<br>
ImportFunction(FuncToImport) {<br>
@@ -632,7 +632,7 @@ bool ModuleLinker::doImportAsDefinition(<br>
return true;<br>
// Only import the function requested for importing.<br>
auto *SF = dyn_cast<Function>(SGV);<br>
- if (SF && SF == ImportFunction)<br>
+ if (SF && ImportFunction->count(SF))<br>
return true;<br>
// Otherwise no.<br>
return false;<br>
@@ -1058,7 +1058,7 @@ bool ModuleLinker::shouldLinkFromSource(<br>
if (isa<Function>(&Src)) {<br>
// For functions, LinkFromSrc iff this is the function requested<br>
// for importing. For variables, decide below normally.<br>
- LinkFromSrc = (&Src == ImportFunction);<br>
+ LinkFromSrc = ImportFunction->count(&Src);<br>
return false;<br>
}<br>
<br>
@@ -2033,7 +2033,7 @@ Linker::Linker(Module &M)<br>
<br>
bool Linker::linkInModule(Module &Src, unsigned Flags,<br>
const FunctionInfoIndex *Index,<br>
- Function *FuncToImport) {<br>
+ DenseSet<const GlobalValue *> *FuncToImport) {<br>
ModuleLinker TheLinker(Composite, IdentifiedStructTypes, Src,<br>
DiagnosticHandler, Flags, Index, FuncToImport);<br>
bool RetCode = TheLinker.run();<br>
<br>
Modified: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp?rev=254486&r1=254485&r2=254486&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp?rev=254486&r1=254485&r2=254486&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp Tue Dec 1 22:34:28 2015<br>
@@ -182,7 +182,10 @@ bool FunctionImporter::importFunctions(M<br>
}<br>
<br>
// Link in the specified function.<br>
- if (L.linkInModule(Module, Linker::Flags::None, &Index, F))<br>
+ DenseSet<const GlobalValue *> FunctionsToImport;<br>
+ FunctionsToImport.insert(F);<br>
+ if (L.linkInModule(Module, Linker::Flags::None, &Index,<br>
+ &FunctionsToImport))<br>
report_fatal_error("Function Import: link error");<br>
<br>
// Process the newly imported function and add callees to the worklist.<br>
@@ -194,6 +197,7 @@ bool FunctionImporter::importFunctions(M<br>
<br>
Changed = true;<br>
}<br>
+<br>
return Changed;<br>
}<br>
<br>
<br>
Modified: llvm/trunk/tools/llvm-link/llvm-link.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-link/llvm-link.cpp?rev=254486&r1=254485&r2=254486&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-link/llvm-link.cpp?rev=254486&r1=254485&r2=254486&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/tools/llvm-link/llvm-link.cpp (original)<br>
+++ llvm/trunk/tools/llvm-link/llvm-link.cpp Tue Dec 1 22:34:28 2015<br>
@@ -198,7 +198,10 @@ static bool importFunctions(const char *<br>
}<br>
<br>
// Link in the specified function.<br>
- if (L.linkInModule(*M, Linker::Flags::None, Index.get(), F))<br>
+ DenseSet<const GlobalValue *> FunctionToImport;<br>
+ FunctionToImport.insert(F);<br>
+ if (L.linkInModule(*M, Linker::Flags::None, Index.get(),<br>
+ &FunctionToImport))<br></blockquote><div><br></div><div>It's sort of weird to use "FunctionToImport" to describe a set. I would expect "FunctionsToImport", even if we know it will only contain one function.</div><div>It is also inconsistent with FunctionImport.cpp where you do use the name "FunctionsToImport".</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
return false;<br>
}<br>
return true;<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>