From 17c19063288c545ea2bc085819e31dc6242e4b5e Mon Sep 17 00:00:00 2001 From: pchintalapudi <34727397+pchintalapudi@users.noreply.github.com> Date: Wed, 3 Aug 2022 14:29:00 -0700 Subject: [PATCH] Fix a few JITLink nits (#46216) --- doc/src/devdocs/locks.md | 13 ++++++--- src/codegen.cpp | 4 ++- src/jitlayers.cpp | 61 +++++++++++++++++++++++++--------------- 3 files changed, 51 insertions(+), 27 deletions(-) diff --git a/doc/src/devdocs/locks.md b/doc/src/devdocs/locks.md index 59dac6ad794983..6390277826f7ac 100644 --- a/doc/src/devdocs/locks.md +++ b/doc/src/devdocs/locks.md @@ -31,6 +31,7 @@ The following are definitely leaf locks (level 1), and must not try to acquire a > * ResourcePool::mutex > * RLST_mutex > * jl_locked_stream::mutex +> * debuginfo_asyncsafe > > > flisp itself is already threadsafe, this lock only protects the `jl_ast_context_list_t` pool > > likewise, the ResourcePool::mutexes just protect the associated resource pool @@ -39,6 +40,7 @@ The following is a leaf lock (level 2), and only acquires level 1 locks (safepoi > * typecache > * Module->lock +> * JLDebuginfoPlugin::PluginMutex The following is a level 3 lock, which can only acquire level 1 or level 2 locks internally: @@ -50,10 +52,13 @@ The following is a level 4 lock, which can only recurse to acquire level 1, 2, o No Julia code may be called while holding a lock above this point. -orc::ThreadSafeContext locks occupy a special spot in the locking diagram. They are used to protect -LLVM's global non-threadsafe state, but there may be an arbitrary number of them. For now, there is -only one global context, and thus acquiring it is a level 5 lock. However, acquiring such a lock -should only be done at the same time that the codegen lock is acquired. +orc::ThreadSafeContext (TSCtx) locks occupy a special spot in the locking hierarchy. They are used to +protect LLVM's global non-threadsafe state, but there may be an arbitrary number of them. By default, +all of these locks may be treated as level 5 locks for the purposes of comparing with the rest of the +hierarchy. Acquiring a TSCtx should only be done from the JIT's pool of TSCtx's, and all locks on +that TSCtx should be released prior to returning it to the pool. If multiple TSCtx locks must be +acquired at the same time (due to recursive compilation), then locks should be acquired in the order +that the TSCtxs were borrowed from the pool. The following are a level 6 lock, which can only recurse to acquire locks at lower levels: diff --git a/src/codegen.cpp b/src/codegen.cpp index eb48cccef17039..2f3974c3a51104 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -8603,7 +8603,7 @@ extern "C" void jl_init_llvm(void) defined(JL_USE_OPROFILE_JITEVENTS) || \ defined(JL_USE_PERF_JITEVENTS) #ifdef JL_USE_JITLINK -#error "JIT profiling support (JL_USE_*_JITEVENTS) not yet available on platforms that use JITLink" +#pragma message("JIT profiling support (JL_USE_*_JITEVENTS) not yet available on platforms that use JITLink") #else const char *jit_profiling = getenv("ENABLE_JITPROFILING"); @@ -8625,6 +8625,7 @@ extern "C" void jl_init_llvm(void) } #endif +#ifndef JL_USE_JITLINK #ifdef JL_USE_INTEL_JITEVENTS if (jl_using_intel_jitevents) jl_ExecutionEngine->RegisterJITEventListener(JITEventListener::createIntelJITEventListener()); @@ -8640,6 +8641,7 @@ extern "C" void jl_init_llvm(void) jl_ExecutionEngine->RegisterJITEventListener(JITEventListener::createPerfJITEventListener()); #endif #endif +#endif #endif cl::PrintOptionValues(); diff --git a/src/jitlayers.cpp b/src/jitlayers.cpp index 009b9692011648..6e5253a952fa32 100644 --- a/src/jitlayers.cpp +++ b/src/jitlayers.cpp @@ -540,6 +540,7 @@ struct JITObjectInfo { }; class JLDebuginfoPlugin : public ObjectLinkingLayer::Plugin { + std::mutex PluginMutex; std::map> PendingObjs; // Resources from distinct MaterializationResponsibilitys can get merged // after emission, so we can have multiple debug objects per resource key. @@ -560,33 +561,40 @@ class JLDebuginfoPlugin : public ObjectLinkingLayer::Plugin { auto NewObj = cantFail(object::ObjectFile::createObjectFile(NewBuffer->getMemBufferRef())); - assert(PendingObjs.count(&MR) == 0); - PendingObjs[&MR] = std::unique_ptr( - new JITObjectInfo{std::move(NewBuffer), std::move(NewObj), {}}); + { + std::lock_guard lock(PluginMutex); + assert(PendingObjs.count(&MR) == 0); + PendingObjs[&MR] = std::unique_ptr( + new JITObjectInfo{std::move(NewBuffer), std::move(NewObj), {}}); + } } Error notifyEmitted(MaterializationResponsibility &MR) override { - auto It = PendingObjs.find(&MR); - if (It == PendingObjs.end()) - return Error::success(); - - auto NewInfo = PendingObjs[&MR].get(); - auto getLoadAddress = [NewInfo](const StringRef &Name) -> uint64_t { - auto result = NewInfo->SectionLoadAddresses.find(Name); - if (result == NewInfo->SectionLoadAddresses.end()) { - LLVM_DEBUG({ - dbgs() << "JLDebuginfoPlugin: No load address found for section '" - << Name << "'\n"; - }); - return 0; - } - return result->second; - }; + { + std::lock_guard lock(PluginMutex); + auto It = PendingObjs.find(&MR); + if (It == PendingObjs.end()) + return Error::success(); + + auto NewInfo = PendingObjs[&MR].get(); + auto getLoadAddress = [NewInfo](const StringRef &Name) -> uint64_t { + auto result = NewInfo->SectionLoadAddresses.find(Name); + if (result == NewInfo->SectionLoadAddresses.end()) { + LLVM_DEBUG({ + dbgs() << "JLDebuginfoPlugin: No load address found for section '" + << Name << "'\n"; + }); + return 0; + } + return result->second; + }; - jl_register_jit_object(*NewInfo->Object, getLoadAddress, nullptr); + jl_register_jit_object(*NewInfo->Object, getLoadAddress, nullptr); + } cantFail(MR.withResourceKeyDo([&](ResourceKey K) { + std::lock_guard lock(PluginMutex); RegisteredObjs[K].push_back(std::move(PendingObjs[&MR])); PendingObjs.erase(&MR); })); @@ -596,12 +604,14 @@ class JLDebuginfoPlugin : public ObjectLinkingLayer::Plugin { Error notifyFailed(MaterializationResponsibility &MR) override { + std::lock_guard lock(PluginMutex); PendingObjs.erase(&MR); return Error::success(); } Error notifyRemovingResources(ResourceKey K) override { + std::lock_guard lock(PluginMutex); RegisteredObjs.erase(K); // TODO: If we ever unload code, need to notify debuginfo registry. return Error::success(); @@ -609,6 +619,7 @@ class JLDebuginfoPlugin : public ObjectLinkingLayer::Plugin { void notifyTransferringResources(ResourceKey DstKey, ResourceKey SrcKey) override { + std::lock_guard lock(PluginMutex); auto SrcIt = RegisteredObjs.find(SrcKey); if (SrcIt != RegisteredObjs.end()) { for (std::unique_ptr &Info : SrcIt->second) @@ -620,13 +631,16 @@ class JLDebuginfoPlugin : public ObjectLinkingLayer::Plugin { void modifyPassConfig(MaterializationResponsibility &MR, jitlink::LinkGraph &, jitlink::PassConfiguration &PassConfig) override { + std::lock_guard lock(PluginMutex); auto It = PendingObjs.find(&MR); if (It == PendingObjs.end()) return; JITObjectInfo &Info = *It->second; - PassConfig.PostAllocationPasses.push_back([&Info](jitlink::LinkGraph &G) -> Error { + PassConfig.PostAllocationPasses.push_back([&Info, this](jitlink::LinkGraph &G) -> Error { + std::lock_guard lock(PluginMutex); for (const jitlink::Section &Sec : G.sections()) { +#ifdef _OS_DARWIN_ // Canonical JITLink section names have the segment name included, e.g. // "__TEXT,__text" or "__DWARF,__debug_str". There are some special internal // sections without a comma separator, which we can just ignore. @@ -639,6 +653,9 @@ class JLDebuginfoPlugin : public ObjectLinkingLayer::Plugin { continue; } auto SecName = Sec.getName().substr(SepPos + 1); +#else + auto SecName = Sec.getName(); +#endif // https://github.com/llvm/llvm-project/commit/118e953b18ff07d00b8f822dfbf2991e41d6d791 #if JL_LLVM_VERSION >= 140000 Info.SectionLoadAddresses[SecName] = jitlink::SectionRange(Sec).getStart().getValue(); @@ -1051,7 +1068,7 @@ JuliaOJIT::JuliaOJIT() OptSelLayer(Pipelines) { #ifdef JL_USE_JITLINK -# if defined(_OS_DARWIN_) && defined(LLVM_SHLIB) +# if defined(LLVM_SHLIB) // When dynamically linking against LLVM, use our custom EH frame registration code // also used with RTDyld to inform both our and the libc copy of libunwind. auto ehRegistrar = std::make_unique();