From 30786279385a92c09b686372d4215a2e6ede9afe Mon Sep 17 00:00:00 2001 From: Lang Hames Date: Thu, 27 Sep 2018 02:09:36 +0000 Subject: [PATCH] Revert "Re-revert r343129." This reverts commit 4e2557dbc76704beb8c4cf1191cb786e719db5d3. llvm-svn: 343161 --- .../ExecutionEngine/Orc/ThreadSafeModule.h | 27 ++++-- unittests/ExecutionEngine/Orc/CMakeLists.txt | 1 + .../Orc/ThreadSafeModuleTest.cpp | 94 +++++++++++++++++++ 3 files changed, 114 insertions(+), 8 deletions(-) create mode 100644 unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp diff --git a/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h b/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h index d979c8dda1a..c5490a0da99 100644 --- a/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h +++ b/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h @@ -16,6 +16,7 @@ #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Module.h" +#include "llvm/Support/Compiler.h" #include #include @@ -40,7 +41,7 @@ private: public: // RAII based lock for ThreadSafeContext. - class Lock { + class LLVM_NODISCARD Lock { private: using UnderlyingLock = std::lock_guard; public: @@ -85,16 +86,26 @@ public: /// null context. ThreadSafeModule() = default; + ThreadSafeModule(ThreadSafeModule &&Other) = default; + + ThreadSafeModule& operator=(ThreadSafeModule &&Other) { + // We have to explicitly define this move operator to copy the fields in + // reverse order (i.e. module first) to ensure the dependencies are + // protected: The old module that is being overwritten must be destroyed + // *before* the context that it depends on. + M = std::move(Other.M); + TSCtx = std::move(Other.TSCtx); + return *this; + } + /// Construct a ThreadSafeModule from a unique_ptr and a /// unique_ptr. This creates a new ThreadSafeContext from the /// given context. - ThreadSafeModule(std::unique_ptr M, - std::unique_ptr Ctx) - : M(std::move(M)), TSCtx(std::move(Ctx)) {} + ThreadSafeModule(std::unique_ptr M, std::unique_ptr Ctx) + : TSCtx(std::move(Ctx)), M(std::move(M)) {} - ThreadSafeModule(std::unique_ptr M, - ThreadSafeContext TSCtx) - : M(std::move(M)), TSCtx(std::move(TSCtx)) {} + ThreadSafeModule(std::unique_ptr M, ThreadSafeContext TSCtx) + : TSCtx(std::move(TSCtx)), M(std::move(M)) {} Module* getModule() { return M.get(); } @@ -109,8 +120,8 @@ public: } private: - std::unique_ptr M; ThreadSafeContext TSCtx; + std::unique_ptr M; }; using GVPredicate = std::function; diff --git a/unittests/ExecutionEngine/Orc/CMakeLists.txt b/unittests/ExecutionEngine/Orc/CMakeLists.txt index c18c9361cb0..3a47bfa20bf 100644 --- a/unittests/ExecutionEngine/Orc/CMakeLists.txt +++ b/unittests/ExecutionEngine/Orc/CMakeLists.txt @@ -26,6 +26,7 @@ add_llvm_unittest(OrcJITTests RTDyldObjectLinkingLayerTest.cpp RTDyldObjectLinkingLayer2Test.cpp SymbolStringPoolTest.cpp + ThreadSafeModuleTest.cpp ) target_link_libraries(OrcJITTests PRIVATE ${ORC_JIT_TEST_LIBS}) diff --git a/unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp b/unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp new file mode 100644 index 00000000000..363c0976718 --- /dev/null +++ b/unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp @@ -0,0 +1,94 @@ +//===--- ThreadSafeModuleTest.cpp - Test basic use of ThreadSafeModule ----===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "llvm/ExecutionEngine/Orc/ThreadSafeModule.h" +#include "gtest/gtest.h" + +#include +#include +#include + +using namespace llvm; +using namespace llvm::orc; + +namespace { + +TEST(ThreadSafeModuleTest, ContextWhollyOwnedByOneModule) { + // Test that ownership of a context can be transferred to a single + // ThreadSafeModule. + ThreadSafeContext TSCtx(llvm::make_unique()); + ThreadSafeModule TSM(llvm::make_unique("M", *TSCtx.getContext()), + std::move(TSCtx)); +} + +TEST(ThreadSafeModuleTest, ContextOwnershipSharedByTwoModules) { + // Test that ownership of a context can be shared between more than one + // ThreadSafeModule. + ThreadSafeContext TSCtx(llvm::make_unique()); + + ThreadSafeModule TSM1(llvm::make_unique("M1", *TSCtx.getContext()), + TSCtx); + ThreadSafeModule TSM2(llvm::make_unique("M2", *TSCtx.getContext()), + std::move(TSCtx)); +} + +TEST(ThreadSafeModuleTest, ContextOwnershipSharedWithClient) { + // Test that ownership of a context can be shared with a client-held + // ThreadSafeContext so that it can be re-used for new modules. + ThreadSafeContext TSCtx(llvm::make_unique()); + + { + // Create and destroy a module. + ThreadSafeModule TSM1(llvm::make_unique("M1", *TSCtx.getContext()), + TSCtx); + } + + // Verify that the context is still available for re-use. + ThreadSafeModule TSM2(llvm::make_unique("M2", *TSCtx.getContext()), + std::move(TSCtx)); +} + +TEST(ThreadSafeModuleTest, ThreadSafeModuleMoveAssignment) { + // Move assignment needs to move the module before the context (opposite + // to the field order) to ensure that overwriting with an empty + // ThreadSafeModule does not destroy the context early. + ThreadSafeContext TSCtx(llvm::make_unique()); + ThreadSafeModule TSM(llvm::make_unique("M", *TSCtx.getContext()), + std::move(TSCtx)); + TSM = ThreadSafeModule(); +} + +TEST(ThreadSafeModuleTest, BasicContextLockAPI) { + // Test that basic lock API calls work. + ThreadSafeContext TSCtx(llvm::make_unique()); + ThreadSafeModule TSM(llvm::make_unique("M", *TSCtx.getContext()), + TSCtx); + + { auto L = TSCtx.getLock(); } + + { auto L = TSM.getContextLock(); } +} + +TEST(ThreadSafeModuleTest, ContextLockPreservesContext) { + // Test that the existence of a context lock preserves the attached + // context. + // The trick to verify this is a bit of a hack: We attach a Module + // (without the ThreadSafeModule wrapper) to the context, then verify + // that this Module destructs safely (which it will not if its context + // has been destroyed) even though all references to the context have + // been thrown away (apart from the lock). + + ThreadSafeContext TSCtx(llvm::make_unique()); + auto L = TSCtx.getLock(); + auto &Ctx = *TSCtx.getContext(); + auto M = llvm::make_unique("M", Ctx); + TSCtx = ThreadSafeContext(); +} + +} // end anonymous namespace