[ORC JIT] Exposing IndirectStubsManager from CompileOnDemandLayer.h

I needed to be able to update stub pointers for hot functions that get recompiled in a lazy JIT that uses CompileOnDemandLayer. In order to do this I added a method that allows pointers to be updated but does not expose any of the other internals of the COD layer.

Does anyone have a cleaner way to do this? Has something to facilitate this already been added? Would it be possible to merge this in?

diff --git a/CompileOnDemandLayer.h b/CompileOnDemandLayer.h
index bd192b8…4aa3362 100644
— a/CompileOnDemandLayer.h
+++ b/CompileOnDemandLayer.h
@@ -429,6 +429,28 @@ private:
return CalledAddr;
}

+public:

  • TargetAddress updatePointer(std::string FuncName, TargetAddress FnBodyAddr) {
  • //Find out which logical dylib contains our symbol
  • auto LDI = LogicalDylibs.begin();
  • for (auto LDE = LogicalDylibs.end(); LDI != LDE; ++LDI) {
  • if (auto LMResources = LDI->getLogicalModuleResourcesForSymbol(FuncName, false)) {
  • Module &SrcM = LMResources->SourceModule->getResource();
  • std::string CalledFnName = mangle(FuncName, SrcM.getDataLayout());
  • if (auto EC = LMResources->StubsMgr->updatePointer(CalledFnName, FnBodyAddr)) {
  • return 0;
  • }
  • else
  • return FnBodyAddr;
  • }
  • }
  • return 0;
  • }

+Lang Hames, Master Regent of the Three <No, Two sir> JITs

Hi Sean,

This is great, but it couples LogicalDylib too tightly to CompileOnDemandLayer. Does this alternative implementation of getLogicalModuleResourcesForSymbol work for you (unfortunately I don’t have a local test case for this yet):

LogicalModuleResources*
getLogicalModuleResourcesForSymbol(const std::string &Name,
bool ExportedSymbolsOnly) {
for (auto LMI = LogicalModules.begin(), LME = LogicalModules.end();
LMI != LME; ++LMI)
if (auto Sym = LMI->Resources.findSymbol(Name, ExportedSymbolsOnly))
return &LMI->Resources;
return nullptr;
}

If so I’d be happy to commit this on your behalf.

It might be nice to plumb support for this to the Orc C-bindings too: that would enable testing of this via the C-Bindings unit tests.

Thanks very much for working on this!

Cheers,
Lang.

It does work. I just tested it on my JIT. Thanks!

As for the part that couples them too tightly, would you recommend I just keep my own specialized version of CompileOnDemandLayer.h that includes this functionality, or do you have any ideas for a cleaner way to do this? I’ve noticed a couple of people asking for support for updating stub pointers for functions that are optimized at runtime, and I’d be willing to work on adding that support.

Hi Sean,

As for the part that couples them too tightly, would you recommend I just keep my own specialized version of CompileOnDemandLayer.h that includes this functionality, or do you have any ideas for a cleaner way to do this?

My apologies - I wasn’t very clear in my description of the issue. The only sense in which your original patch was tightly coupled was that it had getLogicalModuleResourcesForSymbol accessing LogicalModuleResources::SourceModule, which isn’t a required part of the ‘LogicalModuleResources’ interface at the moment. The alternative implementation of getLogicalModuleResourcesForSymbol doesn’t have that coupling, and since it works for you I think the issue is resolved. I’ve committed your patch with my suggested modification in r277257.

Thanks very much for contributing this - recompilation support is a popular request as you noted, and it’d be great to have a good interface for it.

There is still a lot more that can be done to support re-optimization: C bindings, test cases, on-stack counts for functions that might be replaced (so that we can release their memory once they’re no longer running), and more. If you continue working on this please keep submitting patches and I’ll be happy to review them. The standard process is to create a review on reviews.llvm.org (set me as the reviewer if it’s JIT related), or post a diff to the llvm-commits mailing list (and CC me).

Thanks again Sean!

Cheers,
Lang.

No problem. Thanks for committing it! I’m not sure how to create test cases and C bindings yet, but I will figure it out submit for review.