[llvm] r254486 - Change ModuleLinker to take a set of GlobalValues to import instead of a single one
Sean Silva via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 2 15:02:12 PST 2015
On Tue, Dec 1, 2015 at 8:34 PM, Mehdi Amini via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: mehdi_amini
> Date: Tue Dec 1 22:34:28 2015
> New Revision: 254486
>
> URL: http://llvm.org/viewvc/llvm-project?rev=254486&view=rev
> Log:
> Change ModuleLinker to take a set of GlobalValues to import instead of a
> single one
>
> For efficiency reason, when importing multiple functions for the same
> Module,
> we can avoid reparsing it every time.
>
> Differential Revision: http://reviews.llvm.org/D15102
>
> From: Mehdi Amini <mehdi.amini at apple.com>
>
> Modified:
> llvm/trunk/include/llvm/Linker/Linker.h
> llvm/trunk/lib/Linker/LinkModules.cpp
> llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
> llvm/trunk/tools/llvm-link/llvm-link.cpp
>
> Modified: llvm/trunk/include/llvm/Linker/Linker.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Linker/Linker.h?rev=254486&r1=254485&r2=254486&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Linker/Linker.h (original)
> +++ llvm/trunk/include/llvm/Linker/Linker.h Tue Dec 1 22:34:28 2015
> @@ -82,7 +82,7 @@ public:
> /// Returns true on error.
> bool linkInModule(Module &Src, unsigned Flags = Flags::None,
> const FunctionInfoIndex *Index = nullptr,
> - Function *FuncToImport = nullptr);
> + DenseSet<const GlobalValue *> *FuncToImport =
> nullptr);
>
> static bool linkModules(Module &Dest, Module &Src,
> DiagnosticHandlerFunction DiagnosticHandler,
>
> Modified: llvm/trunk/lib/Linker/LinkModules.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=254486&r1=254485&r2=254486&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Linker/LinkModules.cpp (original)
> +++ llvm/trunk/lib/Linker/LinkModules.cpp Tue Dec 1 22:34:28 2015
> @@ -406,7 +406,7 @@ class ModuleLinker {
>
> /// Function to import from source module, all other functions are
> /// imported as declarations instead of definitions.
> - Function *ImportFunction;
> + DenseSet<const GlobalValue *> *ImportFunction;
>
> /// Set to true if the given FunctionInfoIndex contains any functions
> /// from this source module, in which case we must conservatively assume
> @@ -425,7 +425,7 @@ public:
> ModuleLinker(Module &DstM, Linker::IdentifiedStructTypeSet &Set, Module
> &SrcM,
> DiagnosticHandlerFunction DiagnosticHandler, unsigned
> Flags,
> const FunctionInfoIndex *Index = nullptr,
> - Function *FuncToImport = nullptr)
> + DenseSet<const GlobalValue *> *FuncToImport = nullptr)
> : DstM(DstM), SrcM(SrcM), TypeMap(Set), ValMaterializer(this),
> DiagnosticHandler(DiagnosticHandler), Flags(Flags),
> ImportIndex(Index),
> ImportFunction(FuncToImport) {
> @@ -632,7 +632,7 @@ bool ModuleLinker::doImportAsDefinition(
> return true;
> // Only import the function requested for importing.
> auto *SF = dyn_cast<Function>(SGV);
> - if (SF && SF == ImportFunction)
> + if (SF && ImportFunction->count(SF))
> return true;
> // Otherwise no.
> return false;
> @@ -1058,7 +1058,7 @@ bool ModuleLinker::shouldLinkFromSource(
> if (isa<Function>(&Src)) {
> // For functions, LinkFromSrc iff this is the function requested
> // for importing. For variables, decide below normally.
> - LinkFromSrc = (&Src == ImportFunction);
> + LinkFromSrc = ImportFunction->count(&Src);
> return false;
> }
>
> @@ -2033,7 +2033,7 @@ Linker::Linker(Module &M)
>
> bool Linker::linkInModule(Module &Src, unsigned Flags,
> const FunctionInfoIndex *Index,
> - Function *FuncToImport) {
> + DenseSet<const GlobalValue *> *FuncToImport) {
> ModuleLinker TheLinker(Composite, IdentifiedStructTypes, Src,
> DiagnosticHandler, Flags, Index, FuncToImport);
> bool RetCode = TheLinker.run();
>
> Modified: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp?rev=254486&r1=254485&r2=254486&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp (original)
> +++ llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp Tue Dec 1 22:34:28
> 2015
> @@ -182,7 +182,10 @@ bool FunctionImporter::importFunctions(M
> }
>
> // Link in the specified function.
> - if (L.linkInModule(Module, Linker::Flags::None, &Index, F))
> + DenseSet<const GlobalValue *> FunctionsToImport;
> + FunctionsToImport.insert(F);
> + if (L.linkInModule(Module, Linker::Flags::None, &Index,
> + &FunctionsToImport))
> report_fatal_error("Function Import: link error");
>
> // Process the newly imported function and add callees to the
> worklist.
> @@ -194,6 +197,7 @@ bool FunctionImporter::importFunctions(M
>
> Changed = true;
> }
> +
> return Changed;
> }
>
>
> Modified: llvm/trunk/tools/llvm-link/llvm-link.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-link/llvm-link.cpp?rev=254486&r1=254485&r2=254486&view=diff
>
> ==============================================================================
> --- llvm/trunk/tools/llvm-link/llvm-link.cpp (original)
> +++ llvm/trunk/tools/llvm-link/llvm-link.cpp Tue Dec 1 22:34:28 2015
> @@ -198,7 +198,10 @@ static bool importFunctions(const char *
> }
>
> // Link in the specified function.
> - if (L.linkInModule(*M, Linker::Flags::None, Index.get(), F))
> + DenseSet<const GlobalValue *> FunctionToImport;
> + FunctionToImport.insert(F);
> + if (L.linkInModule(*M, Linker::Flags::None, Index.get(),
> + &FunctionToImport))
>
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.
It is also inconsistent with FunctionImport.cpp where you do use the name
"FunctionsToImport".
-- Sean Silva
> return false;
> }
> return true;
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151202/45fc75a6/attachment.html>
More information about the llvm-commits
mailing list