[ORC] Add absoluteSymbolsLinkGraph to expose absolute symbols to platform #77008
Conversation
| case Triple::aarch64: | ||
| case llvm::Triple::riscv64: | ||
| case Triple::x86_64: | ||
| PointerSize = 8; |
There was a problem hiding this comment.
Not thrilled with this, but didn't see an existing method for calculating pointer size for the execution session/target.
There was a problem hiding this comment.
Yeah -- I don't think this is a blocker here.
I think the triple was added to the LinkGraph after the pointer-size / endianness pair, but now that we have the triple the pointer-size and endianness seem redundant -- is there any situation where we can't derive them from the triple? Maybe we can just get rid of them in a follow-up patch.
There was a problem hiding this comment.
tryToGenerate just has to add the definitions to the target MU, so what if you replaced
using AbsoluteSymbolsMUFactory =
unique_function<std::unique_ptr<MaterializationUnit>(SymbolMap)>;with
using AddAbsoluteSymbolsFn =
unique_function<Error(JITDylib&, SymbolMap)>;then you could replace absoluteSymbolsObjectLayer with a more generic absoluteSymbolsLinkGraph function that just returns the graph, and your adder could be:
[&ObjLinkingLayer](JITDylib &JD, SymbolMap Symbols) {
// Build graph.
return ObjLinkingLayer.add(JD, std::move(G));
}That'd avoid exposing ObjectLinkingLayer::createLinkGraphMaterializationUnit.
| case Triple::aarch64: | ||
| case llvm::Triple::riscv64: | ||
| case Triple::x86_64: | ||
| PointerSize = 8; |
There was a problem hiding this comment.
Yeah -- I don't think this is a blocker here.
I think the triple was added to the LinkGraph after the pointer-size / endianness pair, but now that we have the triple the pointer-size and endianness seem redundant -- is there any situation where we can't derive them from the triple? Maybe we can just get rid of them in a follow-up patch.
benlangmuir
force-pushed
the
absolute-symbols-object-layer
branch
from
eebaa0c
to
90d9a7d
Compare
|
Updated per review; changing LinkGraph itself to use the triple to compute pointer width is left as a potential follow up |
benlangmuir
changed the title
| Ctx->getMemoryManager().allocate( | ||
| Ctx->getJITLinkDylib(), *G, | ||
| [S = std::move(Self)](AllocResult AR) mutable { | ||
| // FIXME: Once MSVC implements c++17 order of evaluation rules for calls | ||
| // this can be simplified to | ||
| // S->linkPhase2(std::move(S), std::move(AR)); | ||
| auto *TmpSelf = S.get(); | ||
| TmpSelf->linkPhase2(std::move(S), std::move(AR)); | ||
| }); | ||
| auto LinkPhase2 = [S = std::move(Self)](AllocResult AR) mutable { | ||
| // FIXME: Once MSVC implements c++17 order of evaluation rules for calls | ||
| // this can be simplified to | ||
| // S->linkPhase2(std::move(S), std::move(AR)); | ||
| auto *TmpSelf = S.get(); | ||
| TmpSelf->linkPhase2(std::move(S), std::move(AR)); | ||
| }; | ||
|
|
||
| if (G->allocActions().empty() && llvm::all_of(G->sections(), [](Section &S) { | ||
| return S.getMemLifetime() == orc::MemLifetime::NoAlloc; | ||
| })) { | ||
| LinkPhase2(nullptr); | ||
| } else { | ||
| Ctx->getMemoryManager().allocate(Ctx->getJITLinkDylib(), *G, | ||
| std::move(LinkPhase2)); | ||
| } |
There was a problem hiding this comment.
Alternatively this could also be:
// Skip straight to phase 2 if the graph is empty and has no associated actions.
if (G->allocActions().empty() && llvm::all_of(G->sections(), [](Section &S) {
return S.getMemLifetime() == orc::MemLifetime::NoAlloc;
})) {
linkPhase2(std::move(Self), nullptr);
return;
}
Ctx->getMemoryManager().allocate(Ctx->getJITLinkDylib(), *G,
[S = std::move(Self)](AllocResult AR) mutable {
// FIXME: Once MSVC implements c++17 order of evaluation rules for calls
// this can be simplified to
// S->linkPhase2(std::move(S), std::move(AR));
auto *TmpSelf = S.get();
TmpSelf->linkPhase2(std::move(S), std::move(AR));
});Similarly for the call to phase 4.
That reads more clearly to me, but only marginally.
…form Adds a function to create a LinkGraph of absolute symbols, and a callback in dynamic library search generators to enable using it to expose its symbols to the platform/orc runtime. This allows e.g. using __orc_rt_run_program to run a precompiled function that was found via dlsym. Ideally we would use this in llvm-jitlink's own search generator, but it will require more work to align with the Process/Platform JITDylib split, so not handled here. As part of this change we need to handle LinkGraphs that only have absolute symbols.
benlangmuir
force-pushed
the
absolute-symbols-object-layer
branch
from
90d9a7d
to
2133aa7
Compare
…form (llvm#77008) Adds a function to create a LinkGraph of absolute symbols, and a callback in dynamic library search generators to enable using it to expose its symbols to the platform/orc runtime. This allows e.g. using __orc_rt_run_program to run a precompiled function that was found via dlsym. Ideally we would use this in llvm-jitlink's own search generator, but it will require more work to align with the Process/Platform JITDylib split, so not handled here. As part of this change we need to handle LinkGraphs that only have absolute symbols.
Adds a function to create a LinkGraph of absolute symbols, and a callback in dynamic library search generators to enable using it to expose its symbols to the platform/orc runtime. This allows e.g. using __orc_rt_run_program to run a precompiled function that was found via dlsym. Ideally we would use this in llvm-jitlink's own search generator, but it will require more work to align with the Process/Platform JITDylib split, so not handled here.
As part of this change we need to handle LinkGraphs that only have absolute symbols.