[llvm] r239561 - [Orc] Make partition identification in the CompileOnDemand layer lazy.
Lang Hames
lhames at gmail.com
Fri Jun 12 16:18:39 PDT 2015
Hi Dave,
> std::shared_ptr<T> x = func();
>
> The explicit ctor call in the code at the moment has that problem with it
being unclear if it's taking ownership (using the raw pointer ctor) or just
transferring ownership (using the copy/move ctor).
Good point. Fixed in r239645.
- Lang.
On Fri, Jun 12, 2015 at 3:45 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Fri, Jun 12, 2015 at 3:28 PM, Lang Hames <lhames at gmail.com> wrote:
>
>> Hi Dave,
>>
>> Thanks for the review. Some suggestions applied in r239642, comments
>> inline below.
>>
>> - Lang.
>>
>> On Thu, Jun 11, 2015 at 3:18 PM, David Blaikie <dblaikie at gmail.com>
>> wrote:
>>
>>>
>>>
>>> On Thu, Jun 11, 2015 at 2:45 PM, Lang Hames <lhames at gmail.com> wrote:
>>>
>>>> Author: lhames
>>>> Date: Thu Jun 11 16:45:19 2015
>>>> New Revision: 239561
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=239561&view=rev
>>>> Log:
>>>> [Orc] Make partition identification in the CompileOnDemand layer lazy.
>>>>
>>>> This also breaks out the logical dylib symbol resolution logic.
>>>>
>>>>
>>>> Added:
>>>> llvm/trunk/include/llvm/ExecutionEngine/Orc/LogicalDylib.h
>>>> Modified:
>>>> llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h
>>>> llvm/trunk/tools/lli/OrcLazyJIT.h
>>>>
>>>> Modified:
>>>> llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h?rev=239561&r1=239560&r2=239561&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h
>>>> (original)
>>>> +++ llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h
>>>> Thu Jun 11 16:45:19 2015
>>>> @@ -15,9 +15,9 @@
>>>> #ifndef LLVM_EXECUTIONENGINE_ORC_COMPILEONDEMANDLAYER_H
>>>> #define LLVM_EXECUTIONENGINE_ORC_COMPILEONDEMANDLAYER_H
>>>>
>>>> -//#include "CloneSubModule.h"
>>>> #include "IndirectionUtils.h"
>>>> #include "LambdaResolver.h"
>>>> +#include "LogicalDylib.h"
>>>> #include "llvm/ADT/STLExtras.h"
>>>> #include "llvm/ExecutionEngine/SectionMemoryManager.h"
>>>> #include "llvm/Transforms/Utils/Cloning.h"
>>>> @@ -36,7 +36,9 @@ namespace orc {
>>>> /// added to the layer below. When a stub is called it triggers the
>>>> extraction
>>>> /// of the function body from the original module. The extracted body
>>>> is then
>>>> /// compiled and executed.
>>>> -template <typename BaseLayerT, typename CompileCallbackMgrT>
>>>> +template <typename BaseLayerT, typename CompileCallbackMgrT,
>>>> + typename PartitioningFtor =
>>>> + std::function<std::set<Function*>(Function&)>>
>>>>
>>>
>>> I can't seem to find where/why this is a template parameter - it looks
>>> like it's a private implementation detail that users of this class couldn't
>>> change even if they wanted to? (the only place it's used is for the
>>> Partitioner member of the private nested struct, and the only thing that's
>>> initialized with is a lambda of the same type in the addModuleSet function)
>>>
>>> Are there other uses/pieces of this class I should be looking at to get
>>> the whole picture?
>>>
>>
>> I plan to expose the partitioning stuff soon, but am holding off while I
>> consider exactly how I want the user to supply it. Passing it along with
>> the module set to the addModuleSet method seems like the best option, but I
>> want to keep that method's signature as uniform as possible across layers
>> to maximize opportunities for composition.
>>
>
> Might be easier to review if the templating is added with the usage -
> otherwise it's a bit harder to figure out the design when it's split across
> commits like this. No biggie.
>
>
>>
>> class CompileOnDemandLayer {
>>>> private:
>>>>
>>>> @@ -58,311 +60,23 @@ private:
>>>> };
>>>>
>>>> typedef typename BaseLayerT::ModuleSetHandleT
>>>> BaseLayerModuleSetHandleT;
>>>> - class UncompiledPartition;
>>>>
>>>> - // Logical module.
>>>> - //
>>>> - // This struct contains the handles for the global values and
>>>> stubs (which
>>>> - // cover the external symbols of the original module), plus the
>>>> handes for
>>>> - // each of the extracted partitions. These handleds are used for
>>>> lookup (only
>>>> - // the globals/stubs module is searched) and memory management. The
>>>> actual
>>>> - // searching and resource management are handled by the LogicalDylib
>>>> that owns
>>>> - // the LogicalModule.
>>>> - struct LogicalModule {
>>>> - LogicalModule() {}
>>>> -
>>>> - LogicalModule(LogicalModule &&Other)
>>>> - : SrcM(std::move(Other.SrcM)),
>>>> - GVsAndStubsHandle(std::move(Other.GVsAndStubsHandle)),
>>>> - ImplHandles(std::move(Other.ImplHandles)) {}
>>>> -
>>>> - std::unique_ptr<Module> SrcM;
>>>> - BaseLayerModuleSetHandleT GVsAndStubsHandle;
>>>> - std::vector<BaseLayerModuleSetHandleT> ImplHandles;
>>>> + struct LogicalModuleResources {
>>>> + std::shared_ptr<Module> SourceModule;
>>>> };
>>>>
>>>> - // Logical dylib.
>>>> - //
>>>> - // This class handles symbol resolution and resource management
>>>> for a set of
>>>> - // modules that were added together as a logical dylib.
>>>> - //
>>>> - // A logical dylib contains one-or-more LogicalModules plus a set
>>>> of
>>>> - // UncompiledPartitions. LogicalModules support symbol resolution
>>>> and resource
>>>> - // management for for code that has already been emitted.
>>>> UncompiledPartitions
>>>> - // represent code that has not yet been compiled.
>>>> - class LogicalDylib {
>>>> - private:
>>>> - friend class UncompiledPartition;
>>>> - typedef std::list<LogicalModule> LogicalModuleList;
>>>> - public:
>>>> -
>>>> - typedef unsigned UncompiledPartitionID;
>>>> - typedef typename LogicalModuleList::iterator LMHandle;
>>>> -
>>>> - // Construct a logical dylib.
>>>> - LogicalDylib(CompileOnDemandLayer &CODLayer) : CODLayer(CODLayer)
>>>> { }
>>>> -
>>>> - // Delete this logical dylib, release logical module resources.
>>>> - virtual ~LogicalDylib() {
>>>> - releaseLogicalModuleResources();
>>>> - }
>>>> -
>>>> - // Get a reference to the containing layer.
>>>> - CompileOnDemandLayer& getCODLayer() { return CODLayer; }
>>>> -
>>>> - // Get a reference to the base layer.
>>>> - BaseLayerT& getBaseLayer() { return CODLayer.BaseLayer; }
>>>> -
>>>> - // Start a new context for a single logical module.
>>>> - LMHandle createLogicalModule() {
>>>> - LogicalModules.push_back(LogicalModule());
>>>> - return std::prev(LogicalModules.end());
>>>> - }
>>>> -
>>>> - // Set the global-values-and-stubs module handle for this logical
>>>> module.
>>>> - void setGVsAndStubsHandle(LMHandle LMH, BaseLayerModuleSetHandleT
>>>> H) {
>>>> - LMH->GVsAndStubsHandle = H;
>>>> - }
>>>> -
>>>> - // Return the global-values-and-stubs module handle for this
>>>> logical module.
>>>> - BaseLayerModuleSetHandleT getGVsAndStubsHandle(LMHandle LMH) {
>>>> - return LMH->GVsAndStubsHandle;
>>>> - }
>>>> -
>>>> - // Add a handle to a module containing lazy function bodies to
>>>> the given
>>>> - // logical module.
>>>> - void addToLogicalModule(LMHandle LMH, BaseLayerModuleSetHandleT H)
>>>> {
>>>> - LMH->ImplHandles.push_back(H);
>>>> - }
>>>> -
>>>> - // Create an UncompiledPartition attached to this LogicalDylib.
>>>> - UncompiledPartition& createUncompiledPartition(LMHandle LMH,
>>>> -
>>>> std::shared_ptr<Module> SrcM);
>>>> -
>>>> - // Take ownership of the given UncompiledPartition from the
>>>> logical dylib.
>>>> - std::unique_ptr<UncompiledPartition>
>>>> - takeUPOwnership(UncompiledPartitionID ID);
>>>> -
>>>> - // Look up a symbol in this context.
>>>> - JITSymbol findSymbolInternally(LMHandle LMH, const std::string
>>>> &Name) {
>>>> - if (auto Symbol =
>>>> getBaseLayer().findSymbolIn(LMH->GVsAndStubsHandle,
>>>> - Name, false))
>>>> - return Symbol;
>>>> -
>>>> - for (auto I = LogicalModules.begin(), E = LogicalModules.end();
>>>> I != E;
>>>> - ++I)
>>>> - if (I != LMH)
>>>> - if (auto Symbol =
>>>> getBaseLayer().findSymbolIn(I->GVsAndStubsHandle,
>>>> - Name, false))
>>>> - return Symbol;
>>>> -
>>>> - return nullptr;
>>>> - }
>>>> -
>>>> - JITSymbol findSymbol(const std::string &Name, bool
>>>> ExportedSymbolsOnly) {
>>>> - for (auto &LM : LogicalModules)
>>>> - if (auto Symbol =
>>>> getBaseLayer().findSymbolIn(LM.GVsAndStubsHandle,
>>>> - Name,
>>>> -
>>>> ExportedSymbolsOnly))
>>>> - return Symbol;
>>>> - return nullptr;
>>>> - }
>>>> -
>>>> - // Find an external symbol (via the user supplied SymbolResolver).
>>>> - virtual RuntimeDyld::SymbolInfo
>>>> - findSymbolExternally(const std::string &Name) const = 0;
>>>> -
>>>> - private:
>>>> -
>>>> - void releaseLogicalModuleResources() {
>>>> - for (auto I = LogicalModules.begin(), E = LogicalModules.end();
>>>> I != E;
>>>> - ++I) {
>>>> - getBaseLayer().removeModuleSet(I->GVsAndStubsHandle);
>>>> - for (auto H : I->ImplHandles)
>>>> - getBaseLayer().removeModuleSet(H);
>>>> - }
>>>> - }
>>>> -
>>>> - CompileOnDemandLayer &CODLayer;
>>>> - LogicalModuleList LogicalModules;
>>>> - std::vector<std::unique_ptr<UncompiledPartition>>
>>>> UncompiledPartitions;
>>>> - };
>>>> -
>>>> - template <typename ResolverPtrT>
>>>> - class LogicalDylibImpl : public LogicalDylib {
>>>> - public:
>>>> - LogicalDylibImpl(CompileOnDemandLayer &CODLayer, ResolverPtrT
>>>> Resolver)
>>>> - : LogicalDylib(CODLayer), Resolver(std::move(Resolver)) {}
>>>> -
>>>> - RuntimeDyld::SymbolInfo
>>>> - findSymbolExternally(const std::string &Name) const override {
>>>> - return Resolver->findSymbol(Name);
>>>> - }
>>>> -
>>>> - private:
>>>> - ResolverPtrT Resolver;
>>>> + struct LogicalDylibResources {
>>>> + typedef std::function<RuntimeDyld::SymbolInfo(const std::string&)>
>>>> + SymbolResolverFtor;
>>>> + SymbolResolverFtor ExternalSymbolResolver;
>>>> + PartitioningFtor Partitioner;
>>>> };
>>>>
>>>> - template <typename ResolverPtrT>
>>>> - static std::unique_ptr<LogicalDylib>
>>>> - createLogicalDylib(CompileOnDemandLayer &CODLayer,
>>>> - ResolverPtrT Resolver) {
>>>> - typedef LogicalDylibImpl<ResolverPtrT> Impl;
>>>> - return llvm::make_unique<Impl>(CODLayer, std::move(Resolver));
>>>> - }
>>>> + typedef LogicalDylib<BaseLayerT, LogicalModuleResources,
>>>> + LogicalDylibResources> CODLogicalDylib;
>>>>
>>>> - // Uncompiled partition.
>>>> - //
>>>> - // Represents one as-yet uncompiled portion of a module.
>>>> - class UncompiledPartition {
>>>> - public:
>>>> -
>>>> - struct PartitionEntry {
>>>> - PartitionEntry(Function *F, TargetAddress CallbackID)
>>>> - : F(F), CallbackID(CallbackID) {}
>>>> - Function *F;
>>>> - TargetAddress CallbackID;
>>>> - };
>>>> -
>>>> - typedef std::vector<PartitionEntry> PartitionEntryList;
>>>> -
>>>> - // Creates an uncompiled partition with the list of functions that
>>>> make up
>>>> - // this partition.
>>>> - UncompiledPartition(LogicalDylib &LD, typename
>>>> LogicalDylib::LMHandle LMH,
>>>> - std::shared_ptr<Module> SrcM)
>>>> - : LD(LD), LMH(LMH), SrcM(std::move(SrcM)), ID(~0U) {}
>>>> -
>>>> - ~UncompiledPartition() {
>>>> - // FIXME: When we want to support threaded lazy compilation
>>>> we'll need to
>>>> - // lock the callback manager here.
>>>> - auto &CCMgr = LD.getCODLayer().CompileCallbackMgr;
>>>> - for (auto PEntry : PartitionEntries)
>>>> - CCMgr.releaseCompileCallback(PEntry.CallbackID);
>>>> - }
>>>> -
>>>> - // Set the ID for this partition.
>>>> - void setID(typename LogicalDylib::UncompiledPartitionID ID) {
>>>> - this->ID = ID;
>>>> - }
>>>> -
>>>> - // Set the function set and callbacks for this partition.
>>>> - void setPartitionEntries(PartitionEntryList PartitionEntries) {
>>>> - this->PartitionEntries = std::move(PartitionEntries);
>>>> - }
>>>> -
>>>> - // Handle a compile callback for the function at index FnIdx.
>>>> - TargetAddress compile(unsigned FnIdx) {
>>>> - // Take ownership of self. This will ensure we delete the
>>>> partition and
>>>> - // free all its resources once we're done compiling.
>>>> - std::unique_ptr<UncompiledPartition> This =
>>>> LD.takeUPOwnership(ID);
>>>> -
>>>> - // Release all other compile callbacks for this partition.
>>>> - // We skip the callback for this function because that's the one
>>>> that
>>>> - // called us, and the callback manager will already have removed
>>>> it.
>>>> - auto &CCMgr = LD.getCODLayer().CompileCallbackMgr;
>>>> - for (unsigned I = 0; I < PartitionEntries.size(); ++I)
>>>> - if (I != FnIdx)
>>>> - CCMgr.releaseCompileCallback(PartitionEntries[I].CallbackID);
>>>> -
>>>> - // Grab the name of the function being called here.
>>>> - Function *F = PartitionEntries[FnIdx].F;
>>>> - std::string CalledFnName = Mangle(F->getName(),
>>>> SrcM->getDataLayout());
>>>> -
>>>> - // Extract the function and add it to the base layer.
>>>> - auto PartitionImplH = emitPartition();
>>>> - LD.addToLogicalModule(LMH, PartitionImplH);
>>>> -
>>>> - // Update body pointers.
>>>> - // FIXME: When we start supporting remote lazy jitting this will
>>>> need to
>>>> - // be replaced with a user-supplied callback for updating
>>>> the
>>>> - // remote pointers.
>>>> - TargetAddress CalledAddr = 0;
>>>> - for (unsigned I = 0; I < PartitionEntries.size(); ++I) {
>>>> - auto F = PartitionEntries[I].F;
>>>> - std::string FName(F->getName());
>>>> - auto FnBodySym =
>>>> - LD.getBaseLayer().findSymbolIn(PartitionImplH,
>>>> - Mangle(FName,
>>>> SrcM->getDataLayout()),
>>>> - false);
>>>> - auto FnPtrSym =
>>>> - LD.getBaseLayer().findSymbolIn(LD.getGVsAndStubsHandle(LMH),
>>>> - Mangle(FName + "$orc_addr",
>>>> - SrcM->getDataLayout()),
>>>> - false);
>>>> - assert(FnBodySym && "Couldn't find function body.");
>>>> - assert(FnPtrSym && "Couldn't find function body pointer.");
>>>> -
>>>> - auto FnBodyAddr = FnBodySym.getAddress();
>>>> - void *FnPtrAddr = reinterpret_cast<void*>(
>>>> -
>>>> static_cast<uintptr_t>(FnPtrSym.getAddress()));
>>>> -
>>>> - // If this is the function we're calling record the address so
>>>> we can
>>>> - // return it from this function.
>>>> - if (I == FnIdx)
>>>> - CalledAddr = FnBodyAddr;
>>>> -
>>>> - memcpy(FnPtrAddr, &FnBodyAddr, sizeof(uintptr_t));
>>>> - }
>>>> -
>>>> - // Finally, clear the partition structure so we don't try to
>>>> - // double-release the callbacks in the UncompiledPartition
>>>> destructor.
>>>> - PartitionEntries.clear();
>>>> -
>>>> - return CalledAddr;
>>>> - }
>>>> -
>>>> - private:
>>>> -
>>>> - BaseLayerModuleSetHandleT emitPartition() {
>>>> - // Create the module.
>>>> - std::string NewName(SrcM->getName());
>>>> - for (auto &PEntry : PartitionEntries) {
>>>> - NewName += ".";
>>>> - NewName += PEntry.F->getName();
>>>> - }
>>>> - auto PM = llvm::make_unique<Module>(NewName, SrcM->getContext());
>>>> - PM->setDataLayout(SrcM->getDataLayout());
>>>> - ValueToValueMapTy VMap;
>>>> - GlobalDeclMaterializer GDM(*PM);
>>>> -
>>>> - // Create decls in the new module.
>>>> - for (auto &PEntry : PartitionEntries)
>>>> - cloneFunctionDecl(*PM, *PEntry.F, &VMap);
>>>> -
>>>> - // Move the function bodies.
>>>> - for (auto &PEntry : PartitionEntries)
>>>> - moveFunctionBody(*PEntry.F, VMap);
>>>> -
>>>> - // Create memory manager and symbol resolver.
>>>> - auto MemMgr = llvm::make_unique<SectionMemoryManager>();
>>>> - auto Resolver = createLambdaResolver(
>>>> - [this](const std::string &Name) {
>>>> - if (auto Symbol = LD.findSymbolInternally(LMH, Name))
>>>> - return RuntimeDyld::SymbolInfo(Symbol.getAddress(),
>>>> - Symbol.getFlags());
>>>> - return LD.findSymbolExternally(Name);
>>>> - },
>>>> - [this](const std::string &Name) {
>>>> - if (auto Symbol = LD.findSymbolInternally(LMH, Name))
>>>> - return RuntimeDyld::SymbolInfo(Symbol.getAddress(),
>>>> - Symbol.getFlags());
>>>> - return RuntimeDyld::SymbolInfo(nullptr);
>>>> - });
>>>> - std::vector<std::unique_ptr<Module>> PartMSet;
>>>> - PartMSet.push_back(std::move(PM));
>>>> - return LD.getBaseLayer().addModuleSet(std::move(PartMSet),
>>>> - std::move(MemMgr),
>>>> - std::move(Resolver));
>>>> - }
>>>> -
>>>> - LogicalDylib &LD;
>>>> - typename LogicalDylib::LMHandle LMH;
>>>> - std::shared_ptr<Module> SrcM;
>>>> - typename LogicalDylib::UncompiledPartitionID ID;
>>>> - PartitionEntryList PartitionEntries;
>>>> - };
>>>> -
>>>> - typedef std::list<std::unique_ptr<LogicalDylib>> LogicalDylibList;
>>>> + typedef typename CODLogicalDylib::LogicalModuleHandle
>>>> LogicalModuleHandle;
>>>> + typedef std::list<CODLogicalDylib> LogicalDylibList;
>>>>
>>>> public:
>>>> /// @brief Handle to a set of loaded modules.
>>>> @@ -382,20 +96,25 @@ public:
>>>> assert(MemMgr == nullptr &&
>>>> "User supplied memory managers not supported with COD
>>>> yet.");
>>>>
>>>> - LogicalDylibs.push_back(createLogicalDylib(*this,
>>>> std::move(Resolver)));
>>>> + LogicalDylibs.push_back(CODLogicalDylib(BaseLayer));
>>>> + auto &LDLResources = LogicalDylibs.back().getDylibResources();
>>>> +
>>>> + LDLResources.ExternalSymbolResolver =
>>>> + [Resolver](const std::string &Name) {
>>>> + return Resolver->findSymbol(Name);
>>>> + };
>>>> +
>>>> + LDLResources.Partitioner =
>>>> + [](Function &F) {
>>>> + std::set<Function*> Partition;
>>>> + Partition.insert(&F);
>>>> + return Partition;
>>>> + };
>>>>
>>>
>>> Maybe run this through clang-format (might be worth just
>>> clang-formatting this whole patch, see if it looks better that way)? It
>>> looks like this:
>>>
>>> LDLResources.Partitioner = [](Function &F) {
>>> std::set<Function *> Partition;
>>> Partition.insert(&F);
>>> return Partition;
>>> };
>>>
>>> Which is possibly more idiomatic.
>>>
>>
>> Yep. I'll probably run clang-format over all the Orc headers soon - I
>> suspect there are a few bits of non-idiomatic formatting in there.
>>
>>
>>>> // Process each of the modules in this module set.
>>>> - for (auto &M : Ms) {
>>>> - std::vector<std::vector<Function*>> Partitioning;
>>>> - for (auto &F : *M) {
>>>> - if (F.isDeclaration())
>>>> - continue;
>>>> - Partitioning.emplace_back(1, &F);
>>>> - }
>>>> - addLogicalModule(*LogicalDylibs.back(),
>>>> - std::shared_ptr<Module>(std::move(M)),
>>>> - std::move(Partitioning));
>>>> - }
>>>> + for (auto &M : Ms)
>>>> + addLogicalModule(LogicalDylibs.back(),
>>>> + std::shared_ptr<Module>(std::move(M)));
>>>>
>>>> return std::prev(LogicalDylibs.end());
>>>> }
>>>> @@ -420,13 +139,12 @@ public:
>>>> /// below this one.
>>>> JITSymbol findSymbolIn(ModuleSetHandleT H, const std::string &Name,
>>>> bool ExportedSymbolsOnly) {
>>>> - return (*H)->findSymbol(Name, ExportedSymbolsOnly);
>>>> + return H->findSymbol(Name, ExportedSymbolsOnly);
>>>> }
>>>>
>>>> private:
>>>>
>>>> - void addLogicalModule(LogicalDylib &LD, std::shared_ptr<Module> SrcM,
>>>> - std::vector<std::vector<Function*>>
>>>> Partitions) {
>>>> + void addLogicalModule(CODLogicalDylib &LD, std::shared_ptr<Module>
>>>> SrcM) {
>>>>
>>>> // Bump the linkage and rename any anonymous/privote members in
>>>> SrcM to
>>>> // ensure that everything will resolve properly after we partition
>>>> SrcM.
>>>> @@ -434,6 +152,7 @@ private:
>>>>
>>>> // Create a logical module handle for SrcM within the logical
>>>> dylib.
>>>> auto LMH = LD.createLogicalModule();
>>>> + LD.getLogicalModuleResources(LMH).SourceModule = SrcM;
>>>>
>>>> // Create the GVs-and-stubs module.
>>>> auto GVsAndStubsM = llvm::make_unique<Module>(
>>>> @@ -442,31 +161,31 @@ private:
>>>> GVsAndStubsM->setDataLayout(SrcM->getDataLayout());
>>>> ValueToValueMapTy VMap;
>>>>
>>>> - // Process partitions and create stubs.
>>>> + // Process module and create stubs.
>>>> // We create the stubs before copying the global variables as we
>>>> know the
>>>> // stubs won't refer to any globals (they only refer to their
>>>> implementation
>>>> // pointer) so there's no ordering/value-mapping issues.
>>>> - for (auto& Partition : Partitions) {
>>>> - auto &UP = LD.createUncompiledPartition(LMH, SrcM);
>>>> - typename UncompiledPartition::PartitionEntryList
>>>> PartitionEntries;
>>>> - for (auto &F : Partition) {
>>>> - assert(!F->isDeclaration() &&
>>>> - "Partition should only contain definitions");
>>>> - unsigned FnIdx = PartitionEntries.size();
>>>> - auto CCI =
>>>> CompileCallbackMgr.getCompileCallback(SrcM->getContext());
>>>> - PartitionEntries.push_back(
>>>> - typename UncompiledPartition::PartitionEntry(F,
>>>> CCI.getAddress()));
>>>> - Function *StubF = cloneFunctionDecl(*GVsAndStubsM, *F, &VMap);
>>>> - GlobalVariable *FnBodyPtr =
>>>> - createImplPointer(*StubF->getType(), *StubF->getParent(),
>>>> - StubF->getName() + "$orc_addr",
>>>> -
>>>> createIRTypedAddress(*StubF->getFunctionType(),
>>>> - CCI.getAddress()));
>>>> - makeStub(*StubF, *FnBodyPtr);
>>>> - CCI.setCompileAction([&UP, FnIdx]() { return
>>>> UP.compile(FnIdx); });
>>>> - }
>>>> + for (auto &F : *SrcM) {
>>>>
>>>> - UP.setPartitionEntries(std::move(PartitionEntries));
>>>> + // Skip declarations.
>>>> + if (F.isDeclaration())
>>>> + continue;
>>>> +
>>>> + // For each definition: create a callback, a stub, and a
>>>> function body
>>>> + // pointer. Initialize the function body pointer to point at the
>>>> callback,
>>>> + // and set the callback to compile the function body.
>>>> + auto CCInfo =
>>>> CompileCallbackMgr.getCompileCallback(SrcM->getContext());
>>>> + Function *StubF = cloneFunctionDecl(*GVsAndStubsM, F, &VMap);
>>>> + GlobalVariable *FnBodyPtr =
>>>> + createImplPointer(*StubF->getType(), *StubF->getParent(),
>>>> + StubF->getName() + "$orc_addr",
>>>> +
>>>> createIRTypedAddress(*StubF->getFunctionType(),
>>>> + CCInfo.getAddress()));
>>>> + makeStub(*StubF, *FnBodyPtr);
>>>> + CCInfo.setCompileAction(
>>>> + [this, &LD, LMH, &F]() {
>>>> + return extractAndCompile(LD, LMH, F);
>>>> + });
>>>> }
>>>>
>>>> // Now clone the global variable declarations.
>>>> @@ -483,12 +202,9 @@ private:
>>>> // Build a resolver for the stubs module and add it to the base
>>>> layer.
>>>> auto GVsAndStubsResolver = createLambdaResolver(
>>>> [&LD](const std::string &Name) {
>>>> - if (auto Symbol = LD.findSymbol(Name, false))
>>>> - return RuntimeDyld::SymbolInfo(Symbol.getAddress(),
>>>> - Symbol.getFlags());
>>>> - return LD.findSymbolExternally(Name);
>>>> + return LD.getDylibResources().ExternalSymbolResolver(Name);
>>>> },
>>>> - [&LD](const std::string &Name) {
>>>> + [](const std::string &Name) {
>>>> return RuntimeDyld::SymbolInfo(nullptr);
>>>> });
>>>>
>>>> @@ -498,7 +214,7 @@ private:
>>>> BaseLayer.addModuleSet(std::move(GVsAndStubsMSet),
>>>> llvm::make_unique<SectionMemoryManager>(),
>>>> std::move(GVsAndStubsResolver));
>>>> - LD.setGVsAndStubsHandle(LMH, GVsAndStubsH);
>>>> + LD.addToLogicalModule(LMH, GVsAndStubsH);
>>>> }
>>>>
>>>> static std::string Mangle(StringRef Name, const DataLayout &DL) {
>>>> @@ -511,35 +227,101 @@ private:
>>>> return MangledName;
>>>> }
>>>>
>>>> + TargetAddress extractAndCompile(CODLogicalDylib &LD,
>>>> + LogicalModuleHandle LMH,
>>>> + Function &F) {
>>>> + Module &SrcM = *LD.getLogicalModuleResources(LMH).SourceModule;
>>>> +
>>>> + // If F is a declaration we must already have compiled it.
>>>> + if (F.isDeclaration())
>>>> + return 0;
>>>> +
>>>> + // Grab the name of the function being called here.
>>>> + std::string CalledFnName = Mangle(F.getName(),
>>>> SrcM.getDataLayout());
>>>> +
>>>> + const auto &Partition = LD.getDylibResources().Partitioner(F);
>>>>
>>>
>>> Taking this by const (auto) ref is a bit confusing - what's the intent
>>> there? To support non-value-returning partitioning functors/functions? (&
>>> like the feedback above, I'm still a bit confused by what kind of
>>> instantiations of this class can exist/what types can be specified for the
>>> PartitioningFtor type)
>>>
>>
>> That's a think-o: the partition should be taken by value. Fixed in
>> r239642.
>>
>>
>>> + auto PartitionH = emitPartition(LD, LMH, Partition);
>>>>
>>>
>>> In fact, emitPartition here requires that Partition be a " const
>>> std::set<Function*> &" - so perhaps it'd be easier if Partition were just
>>> declared as a "std::set<Function*>" directly (no reference lifetime
>>> extensions of temporaries, etc)?
>>>
>>
>> What heresy is this? You love 'auto'. ;)
>> I think the real problem is that I'd pinned down the return type of the
>> Partitioner. That has also been fixed in r239642.
>>
>>
>>> +
>>>> + TargetAddress CalledAddr = 0;
>>>> + for (auto *SubF : Partition) {
>>>> + std::string FName(SubF->getName());
>>>>
>>>
>>> Prefer copy-init over direct-init?
>>>
>>
>> Yep. This is a hangover from the previous code. Fixed in r239642.
>>
>>
>>> + auto FnBodySym =
>>>> + BaseLayer.findSymbolIn(PartitionH, Mangle(FName,
>>>> SrcM.getDataLayout()),
>>>> + false);
>>>> + auto FnPtrSym =
>>>> + BaseLayer.findSymbolIn(*LD.moduleHandlesBegin(LMH),
>>>> + Mangle(FName + "$orc_addr",
>>>> + SrcM.getDataLayout()),
>>>> + false);
>>>> + assert(FnBodySym && "Couldn't find function body.");
>>>> + assert(FnPtrSym && "Couldn't find function body pointer.");
>>>> +
>>>> + auto FnBodyAddr = FnBodySym.getAddress();
>>>>
>>>
>>> Not sure if these autos improve readability (I guess they're all
>>> JITSymbols? (FnBodySym, FnPtrSym, FnBodyAddr))
>>>
>>
>> The first two are JITSymbols. The latter is a TargetAddress. I think the
>> type is implied by the method names and variable suffixes (Sym/findSymbol,
>> Addr/getAddress), but that's a matter of personal taste. I've actually
>> spelled out the TargetAddress type in r239642. JITSymbol has been left to
>> deduction.
>>
>> + void *FnPtrAddr = reinterpret_cast<void*>(
>>>> + static_cast<uintptr_t>(FnPtrSym.getAddress()));
>>>> +
>>>> + // If this is the function we're calling record the address so
>>>> we can
>>>> + // return it from this function.
>>>> + if (SubF == &F)
>>>> + CalledAddr = FnBodyAddr;
>>>> +
>>>> + memcpy(FnPtrAddr, &FnBodyAddr, sizeof(uintptr_t));
>>>> + }
>>>> +
>>>> + return CalledAddr;
>>>> + }
>>>> +
>>>> + BaseLayerModuleSetHandleT emitPartition(CODLogicalDylib &LD,
>>>> + LogicalModuleHandle LMH,
>>>> + const std::set<Function*>
>>>> &Partition) {
>>>> + Module &SrcM = *LD.getLogicalModuleResources(LMH).SourceModule;
>>>> +
>>>> + // Create the module.
>>>> + std::string NewName(SrcM.getName());
>>>>
>>>
>>> Copy-init over direct-init?
>>>
>>
>> Yep. Fixed in r239642.
>>
>>
>>> + for (auto *F : Partition) {
>>>> + NewName += ".";
>>>> + NewName += F->getName();
>>>> + }
>>>> +
>>>> + auto M = llvm::make_unique<Module>(NewName, SrcM.getContext());
>>>> + M->setDataLayout(SrcM.getDataLayout());
>>>> + ValueToValueMapTy VMap;
>>>> + GlobalDeclMaterializer GDM(*M);
>>>> +
>>>> + // Create decls in the new module.
>>>> + for (auto *F : Partition)
>>>> + cloneFunctionDecl(*M, *F, &VMap);
>>>> +
>>>> + // Move the function bodies.
>>>> + for (auto *F : Partition)
>>>> + moveFunctionBody(*F, VMap);
>>>> +
>>>> + // Create memory manager and symbol resolver.
>>>> + auto MemMgr = llvm::make_unique<SectionMemoryManager>();
>>>> + auto Resolver = createLambdaResolver(
>>>> + [this, &LD, LMH](const std::string &Name) {
>>>> + if (auto Symbol = LD.findSymbolInternally(LMH, Name))
>>>> + return RuntimeDyld::SymbolInfo(Symbol.getAddress(),
>>>> + Symbol.getFlags());
>>>> + return LD.getDylibResources().ExternalSymbolResolver(Name);
>>>> + },
>>>> + [this, &LD, LMH](const std::string &Name) {
>>>> + if (auto Symbol = LD.findSymbolInternally(LMH, Name))
>>>> + return RuntimeDyld::SymbolInfo(Symbol.getAddress(),
>>>> + Symbol.getFlags());
>>>> + return RuntimeDyld::SymbolInfo(nullptr);
>>>> + });
>>>> + std::vector<std::unique_ptr<Module>> PartMSet;
>>>> + PartMSet.push_back(std::move(M));
>>>> + return BaseLayer.addModuleSet(std::move(PartMSet),
>>>> std::move(MemMgr),
>>>> + std::move(Resolver));
>>>
>>> + }
>>>> +
>>>> BaseLayerT &BaseLayer;
>>>> CompileCallbackMgrT &CompileCallbackMgr;
>>>> LogicalDylibList LogicalDylibs;
>>>> };
>>>>
>>>> -template <typename BaseLayerT, typename CompileCallbackMgrT>
>>>> -typename CompileOnDemandLayer<BaseLayerT, CompileCallbackMgrT>::
>>>> - UncompiledPartition&
>>>> -CompileOnDemandLayer<BaseLayerT, CompileCallbackMgrT>::LogicalDylib::
>>>> - createUncompiledPartition(LMHandle LMH, std::shared_ptr<Module>
>>>> SrcM) {
>>>> - UncompiledPartitions.push_back(
>>>> - llvm::make_unique<UncompiledPartition>(*this, LMH,
>>>> std::move(SrcM)));
>>>> - UncompiledPartitions.back()->setID(UncompiledPartitions.size() - 1);
>>>> - return *UncompiledPartitions.back();
>>>> -}
>>>> -
>>>> -template <typename BaseLayerT, typename CompileCallbackMgrT>
>>>> -std::unique_ptr<typename CompileOnDemandLayer<BaseLayerT,
>>>> CompileCallbackMgrT>::
>>>> - UncompiledPartition>
>>>> -CompileOnDemandLayer<BaseLayerT, CompileCallbackMgrT>::LogicalDylib::
>>>> - takeUPOwnership(UncompiledPartitionID ID) {
>>>> -
>>>> - std::swap(UncompiledPartitions[ID], UncompiledPartitions.back());
>>>> - UncompiledPartitions[ID]->setID(ID);
>>>> - auto UP = std::move(UncompiledPartitions.back());
>>>> - UncompiledPartitions.pop_back();
>>>> - return UP;
>>>> -}
>>>> -
>>>> } // End namespace orc.
>>>> } // End namespace llvm.
>>>>
>>>>
>>>> Added: llvm/trunk/include/llvm/ExecutionEngine/Orc/LogicalDylib.h
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/LogicalDylib.h?rev=239561&view=auto
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/include/llvm/ExecutionEngine/Orc/LogicalDylib.h (added)
>>>> +++ llvm/trunk/include/llvm/ExecutionEngine/Orc/LogicalDylib.h Thu Jun
>>>> 11 16:45:19 2015
>>>> @@ -0,0 +1,118 @@
>>>> +//===--- LogicalDylib.h - Simulates dylib-style symbol lookup ---*-
>>>> C++ -*-===//
>>>> +//
>>>> +// The LLVM Compiler Infrastructure
>>>> +//
>>>> +// This file is distributed under the University of Illinois Open
>>>> Source
>>>> +// License. See LICENSE.TXT for details.
>>>> +//
>>>>
>>>> +//===----------------------------------------------------------------------===//
>>>> +//
>>>> +// Simulates symbol resolution inside a dylib.
>>>> +//
>>>>
>>>> +//===----------------------------------------------------------------------===//
>>>> +
>>>> +#ifndef LLVM_EXECUTIONENGINE_ORC_LOGICALDYLIB_H
>>>> +#define LLVM_EXECUTIONENGINE_ORC_LOGICALDYLIB_H
>>>> +
>>>> +#include "llvm/ADT/iterator.h"
>>>> +#include "llvm/ADT/Optional.h"
>>>> +
>>>> +namespace llvm {
>>>> +namespace orc {
>>>> +
>>>> +template <typename BaseLayerT,
>>>> + typename LogicalModuleResources,
>>>> + typename LogicalDylibResources>
>>>> +class LogicalDylib {
>>>> +public:
>>>> + typedef typename BaseLayerT::ModuleSetHandleT
>>>> BaseLayerModuleSetHandleT;
>>>> +private:
>>>> +
>>>> + typedef std::vector<BaseLayerModuleSetHandleT> BaseLayerHandleList;
>>>> +
>>>> + struct LogicalModule {
>>>> + LogicalModuleResources Resources;
>>>> + BaseLayerHandleList BaseLayerHandles;
>>>> + };
>>>> + typedef std::vector<LogicalModule> LogicalModuleList;
>>>> +
>>>> +public:
>>>> +
>>>> + typedef typename BaseLayerHandleList::iterator
>>>> BaseLayerHandleIterator;
>>>> + typedef typename LogicalModuleList::iterator LogicalModuleHandle;
>>>> +
>>>> + LogicalDylib(BaseLayerT &BaseLayer) : BaseLayer(BaseLayer) {}
>>>> +
>>>> + ~LogicalDylib() {
>>>> + for (auto &LM : LogicalModules)
>>>> + for (auto BLH : LM.BaseLayerHandles)
>>>> + BaseLayer.removeModuleSet(BLH);
>>>> + }
>>>> +
>>>> + LogicalModuleHandle createLogicalModule() {
>>>> + LogicalModules.push_back(LogicalModule());
>>>> + return std::prev(LogicalModules.end());
>>>> + }
>>>> +
>>>> + void addToLogicalModule(LogicalModuleHandle LMH,
>>>> + BaseLayerModuleSetHandleT BaseLayerHandle) {
>>>> + LMH->BaseLayerHandles.push_back(BaseLayerHandle);
>>>> + }
>>>> +
>>>> + LogicalModuleResources&
>>>> getLogicalModuleResources(LogicalModuleHandle LMH) {
>>>> + return LMH->Resources;
>>>> + }
>>>> +
>>>> + BaseLayerHandleIterator moduleHandlesBegin(LogicalModuleHandle LMH) {
>>>> + return LMH->BaseLayerHandles.begin();
>>>> + }
>>>> +
>>>> + BaseLayerHandleIterator moduleHandlesEnd(LogicalModuleHandle LMH) {
>>>> + return LMH->BaseLayerHandles.end();
>>>> + }
>>>> +
>>>> + JITSymbol findSymbolInLogicalModule(LogicalModuleHandle LMH,
>>>> + const std::string &Name) {
>>>> + for (auto BLH : LMH->BaseLayerHandles)
>>>> + if (auto Symbol = BaseLayer.findSymbolIn(BLH, Name, false))
>>>> + return Symbol;
>>>> + return nullptr;
>>>> + }
>>>> +
>>>> + JITSymbol findSymbolInternally(LogicalModuleHandle LMH,
>>>> + const std::string &Name) {
>>>> + if (auto Symbol = findSymbolInLogicalModule(LMH, Name))
>>>> + return Symbol;
>>>> +
>>>> + for (auto LMI = LogicalModules.begin(), LME = LogicalModules.end();
>>>> + LMI != LME; ++LMI) {
>>>> + if (LMI != LMH)
>>>> + if (auto Symbol = findSymbolInLogicalModule(LMI, Name))
>>>> + return Symbol;
>>>> + }
>>>> +
>>>> + return nullptr;
>>>> + }
>>>> +
>>>> + JITSymbol findSymbol(const std::string &Name, bool
>>>> ExportedSymbolsOnly) {
>>>> + for (auto &LM : LogicalModules)
>>>> + for (auto BLH : LM.BaseLayerHandles)
>>>> + if (auto Symbol =
>>>> + BaseLayer.findSymbolIn(BLH, Name, ExportedSymbolsOnly))
>>>> + return Symbol;
>>>> + return nullptr;
>>>> + }
>>>> +
>>>> + LogicalDylibResources& getDylibResources() { return DylibResources; }
>>>> +
>>>> +protected:
>>>> + BaseLayerT BaseLayer;
>>>> + LogicalModuleList LogicalModules;
>>>> + LogicalDylibResources DylibResources;
>>>> +
>>>> +};
>>>> +
>>>> +} // End namespace orc.
>>>> +} // End namespace llvm.
>>>> +
>>>> +#endif // LLVM_EXECUTIONENGINE_ORC_LOGICALDYLIB_H
>>>>
>>>> Modified: llvm/trunk/tools/lli/OrcLazyJIT.h
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/lli/OrcLazyJIT.h?rev=239561&r1=239560&r2=239561&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/tools/lli/OrcLazyJIT.h (original)
>>>> +++ llvm/trunk/tools/lli/OrcLazyJIT.h Thu Jun 11 16:45:19 2015
>>>> @@ -89,22 +89,26 @@ public:
>>>> // 2) Check for C++ runtime overrides.
>>>> // 3) Search the host process (LLI)'s symbol table.
>>>> auto Resolver =
>>>> - orc::createLambdaResolver(
>>>> - [this](const std::string &Name) {
>>>> + std::shared_ptr<RuntimeDyld::SymbolResolver>(
>>>>
>>>
>>> Prefer copy init over direct init (otherwise it looks like
>>> createLambdaResolver might be returning a raw pointer you're taking
>>> ownership of here - which raises a bit more care than is otherwise due) ^
>>>
>>
>> This is copy-init, isn't it?
>>
>
> Oh, had a bit of trouble parsing the diff, I guess.
>
> So this code is:
>
> auto x = std::shared_ptr<T>(func());
>
> when it could/should be:
>
> auto x = func();
>
> or
>
> std::shared_ptr<T> x = func();
>
> The explicit ctor call in the code at the moment has that problem with it
> being unclear if it's taking ownership (using the raw pointer ctor) or just
> transferring ownership (using the copy/move ctor).
>
>
>>
>>
>>> + orc::createLambdaResolver(
>>>> + [this](const std::string &Name) {
>>>> + if (auto Sym = CODLayer.findSymbol(Name, true))
>>>> + return RuntimeDyld::SymbolInfo(Sym.getAddress(),
>>>> + Sym.getFlags());
>>>> +
>>>> + if (auto Sym = CXXRuntimeOverrides.searchOverrides(Name))
>>>> + return Sym;
>>>>
>>>> - if (auto Sym = CODLayer.findSymbol(Name, true))
>>>> - return RuntimeDyld::SymbolInfo(Sym.getAddress(),
>>>> Sym.getFlags());
>>>> + if (auto Addr =
>>>> + RTDyldMemoryManager::getSymbolAddressInProcess(Name))
>>>> + return RuntimeDyld::SymbolInfo(Addr,
>>>> JITSymbolFlags::Exported);
>>>>
>>>> - if (auto Sym = CXXRuntimeOverrides.searchOverrides(Name))
>>>> - return Sym;
>>>> -
>>>> - if (auto Addr =
>>>> RTDyldMemoryManager::getSymbolAddressInProcess(Name))
>>>> - return RuntimeDyld::SymbolInfo(Addr,
>>>> JITSymbolFlags::Exported);
>>>> -
>>>> - return RuntimeDyld::SymbolInfo(nullptr);
>>>> - },
>>>> - [](const std::string &Name) { return
>>>> RuntimeDyld::SymbolInfo(nullptr); }
>>>> - );
>>>> + return RuntimeDyld::SymbolInfo(nullptr);
>>>> + },
>>>> + [](const std::string &Name) {
>>>> + return RuntimeDyld::SymbolInfo(nullptr);
>>>> + }
>>>> + ));
>>>>
>>>> // Add the module to the JIT.
>>>> std::vector<std::unique_ptr<Module>> S;
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150612/b902a97e/attachment.html>
More information about the llvm-commits
mailing list