<div dir="ltr">Hi JIT clients,<div><br></div><div>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.</div><div><br></div><div><div>Quickly summarizing the issues that I'm trying to address with this commit:</div><div><br></div><div>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.</div></div><div><br></div><div>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.</div><div><br></div><div>To fix these issues I made the following high-level changes:</div><div><br></div><div>(a) Introduced two new classes to split the responsibilities of RTDyldMemoryManager: RuntimeDyld::MemoryManager and RuntimeDyld::SymbolResolver.</div><div><br></div><div>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.</div><div><br></div><div>Nether class provides the notifyObjectLoaded method: This functionality is not related to memory management or symbol resolution.</div><div><br></div><div>(b) For backwards compatibility, RTDyldMemoryManager remains and inherits from the new classes.</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>(c) The RuntimeDyld::SymbolInfo class is woven through MCJIT and OrcMCJITReplacement.</div><div><br></div><div>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:</div><div><br></div><div>(1) In your CustomRTDyldMemoryManager class, override findSymbol and findSymboInLogicalDylib, rather than getSymbolAddress and getSymbolAddressInLogicalDylib.  If your existing implementation was:</div><div><br></div><div><font face="monospace, monospace">uint64_t getSymbolAddress(const std::string &Name) {</font></div><div><font face="monospace, monospace">  return Foo;</font></div><div><font face="monospace, monospace">}</font></div><div><br></div><div>Your new implementation will be:</div><div><br></div><div><font face="monospace, monospace">RuntimeDyld::SymbolInfo findSymbol(const std::string &Name) {</font></div><div><font face="monospace, monospace">  return RuntimeDyld::SymbolInfo(Foo, JITSymbolFlags::Exported);</font></div><div><font face="monospace, monospace">}</font></div><div><br></div><div>(2) Split your CustomRTDyldMemoryManager class into a CustomMemoryManager class (inheriting from MCJITMemoryManager) and a CustomSymbolResolver (inheriting from RuntimeDyld::SymbolResolver). </div><div><br></div><div>(3) Call EngineBuilder::setMemoryManager and EngineBuilder::setSymbolResolver rather than EngineBuilder::setMCJITMemoryManager.</div><div><br></div><div>That should be it.</div><div><br></div><div>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.</div><div><br></div><div>Cheers,</div><div>Lang.</div><div><br></div></div>