This is an archive of the discontinued LLVM Phabricator instance.

[Orc] Fix global variable destructor function support when --jit-kind=orc-lazy
ClosedPublic

Authored by StephenFan on Oct 2 2021, 2:40 AM.

Details

Summary

The bug was reported here https://bugs.llvm.org/show_bug.cgi?id=52030

This patch follows the idea that @lhames commented in the above webpage.

Event Timeline

StephenFan created this revision.Oct 2 2021, 2:40 AM
StephenFan requested review of this revision.Oct 2 2021, 2:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 2 2021, 2:40 AM
StephenFan updated this revision to Diff 376683.Oct 2 2021, 2:41 AM

clang-format

StephenFan retitled this revision from [Orc] Fix destructor function support when --jit-kind=orc-lazy to [Orc] Fix global variable destructor function support when --jit-kind=orc-lazy.Oct 2 2021, 6:54 AM
StephenFan updated this revision to Diff 376703.Oct 2 2021, 6:58 AM

Rename file.

StephenFan updated this revision to Diff 376704.Oct 2 2021, 7:00 AM

Remove blank line

lhames accepted this revision.Oct 2 2021, 10:48 AM

LGTM. Thanks Stephen!

llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
479

It would be good to use a DeInitFunctionPrefix here (along the same lines as InitFunctionPrefix).

Once the deinit functions are named with a known prefix we can look them up in GenericLLVMIRPlatformSupport::notifyAdding and add them to the DeInitFunctions map automatically. This will make destructors work from cached objects. This could be done in this patch, or in a follow-up.

This revision is now accepted and ready to land.Oct 2 2021, 10:48 AM

Solves my original problem that lead to https://bugs.llvm.org/show_bug.cgi?id=52030. Thank you for working on this!

Add deInitFuctionPrefix

Thanks for your review! @lhames
Thanks for confirming! @awarzynski

StephenFan marked an inline comment as done.Oct 6 2021, 12:48 AM
StephenFan added inline comments.
llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
479

Once the deinit functions are named with a known prefix we can look them up in GenericLLVMIRPlatformSupport::notifyAdding and add them to the DeInitFunctions map automatically. This will make destructors work from cached objects. This could be done in this patch, or in a follow-up.

I have a question that what do cached objects mean in ORC ? After looking through the source code, I found that when define the symbol in JITDylib, the notifyAdding will be called, and when emit the symbol (MaterializationUnit), the layer will call the transform function to transform the module, in which the GlobalCtorDtorScraper::operator() will be called, is my understand right ?

lhames added inline comments.Oct 10 2021, 8:49 PM
llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
479

Hi Stephen,

I have a question that what do cached objects mean in ORC ?

By "cached objects", we mean stored object files that were created by an ObjectCache instance from LLVM IR that was added to the JIT.

In MCJIT we used to discover constructors at the IR layer only by looking for llvm.global_ctors instances. In GenericLLVMIRPlatformSupport we rewrite llvm.global_ctors instances into functions with reserved names that we can look for at the object level. We could do the same thing with llvm.global_dtors.

This approach is a stop-gap until we have generic JITLink support on all platforms. Once we have JITLink everywhere we can remove GenericLLVMIRPlatformSupport entirely.

StephenFan added inline comments.Oct 16 2021, 7:55 AM
llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
479

Thanks for your explanation! @lhames