[LLVMdev] [PATCH][MCJIT][Orc] RTDyldMemoryManager refactor.

[Lang Hames]

Hi JIT clients,

Just a heads up - r233509 refactored the RTDyldMemoryManager to address two
long-standing issues: (1) Coupling between memory management and symbol
resolution, and (2) lack of flags on symbols addresses. The fixes should be
fully backwards compatible, but they also open up opportunities to tidy
existing code and support new use-cases. Below, I argue that we should
consider deprecating RTDyldMemoryManager in favor of two new classes
introduced in this patch. I hope that RTDyldMemoryManager can be removed in
some future release.

Quickly summarizing the issues that I'm trying to address with this commit:

Regarding (1), MCJIT and Orc both manipulate symbol resolution in
interesting ways, but RTDyldMemoryManager made this awkward by forcing you
to address memory management whenever you wanted to alter symbol
resolution. In MCJIT's LinkingMemoryManager and Orc's
LookasideRTDyldMemoryManager, both derived from RTDyldMemoryManager, this
resulted in a lot of pass-through memory-management calls to an underlying
RTDyldMemoryManager instance. I expect many clients who override symbol
lookup in their custom memory managers would have run in to similar issues.

Regarding (2), returning only symbol addresses made it impossible for
RuntimeDyld to reason about weak symbols, or other properties of symbols
that a linker my reasonably be interested in. This patch provides the
plumbing necessary to get symbol flags to where they're needed, which is
the first step in fixing some of the weak symbol issues that people have
run in to.

To fix these issues I made the following high-level changes:

(a) Introduced two new classes to split the responsibilities of
RTDyldMemoryManager: RuntimeDyld::MemoryManager and
RuntimeDyld::SymbolResolver.

The SymbolResolver class introduces a new symbol resolution method:
findSymbol, which returns a RuntimeDyld::SymbolInfo (address + flags)
rather than just a uint64_t. This takes care of the symbol-flags issue.

Nether class provides the notifyObjectLoaded method: This functionality is
not related to memory management or symbol resolution.

(b) For backwards compatibility, RTDyldMemoryManager remains and inherits
from the new classes.

On the memory management side, RTDyldMemoryManager actually inherits from a
new MCJITMemoryManager class, rather than RuntimeDyld::MemoryManager
directly. MCJITMemoryManager is just a subclass of
RuntimeDyld::MemoryManager with the notifyObjectLoaded method added back in
for backwards compatibility.

On the symbol resolution side, RTDyldMemoryManager provides default
implementations of findSymbol and findSymbolInLogicalDylib that call
through to getSymbolAddress/getSymbolAddressInDylib and return a non-weak,
exported symbol with the resulting address.

(c) The RuntimeDyld::SymbolInfo class is woven through MCJIT and
OrcMCJITReplacement.

As noted, these changes should be backwards compatible, but I think we
should consider deprecating RTDyldMemoryManager in the near future, and
removing it once all clients have moved over to the new classes.
Transitioning away from RTDyldMemoryManager is easy. Just follow this
recipe:

(1) In your CustomRTDyldMemoryManager class, override findSymbol and
findSymboInLogicalDylib, rather than getSymbolAddress and
getSymbolAddressInLogicalDylib.  If your existing implementation was:

uint64_t getSymbolAddress(const std::string &Name) {
  return Foo;
}

Your new implementation will be:

RuntimeDyld::SymbolInfo findSymbol(const std::string &Name) {
  return RuntimeDyld::SymbolInfo(Foo, JITSymbolFlags::Exported);
}

(2) Split your CustomRTDyldMemoryManager class into a CustomMemoryManager
class (inheriting from MCJITMemoryManager) and a CustomSymbolResolver
(inheriting from RuntimeDyld::SymbolResolver).

(3) Call EngineBuilder::setMemoryManager and
EngineBuilder::setSymbolResolver rather than
EngineBuilder::setMCJITMemoryManager.

That should be it.

Assuming the transition is as easy as I'm hoping, I see no reason to keep
RTDyldMemoryManager around long term. I'm going to leave this discussion
open for a week or so. After that, if there are no objections, I'll start
noting in documentation that RTDyldMemoryManager is deprecated, and we can
discuss a timeline for removal. If there are objections and/or problems
that I haven't foreseen then we can hold off on deprecation while we
discuss fixes/alternatives.

Cheers,
Lang.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150329/742318c6/attachment.html>