From 4e2557dbc76704beb8c4cf1191cb786e719db5d3 Mon Sep 17 00:00:00 2001 From: Lang Hames Date: Wed, 26 Sep 2018 23:32:53 +0000 Subject: [PATCH] Re-revert r343129. Apparently the fixes in r343149 did not cover all the issues. Re-reverting while I investigate. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@343151 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../llvm/ExecutionEngine/Orc/ThreadSafeModule.h | 27 ++----- unittests/ExecutionEngine/Orc/CMakeLists.txt | 1 - .../ExecutionEngine/Orc/ThreadSafeModuleTest.cpp | 94 ---------------------- 3 files changed, 8 insertions(+), 114 deletions(-) delete mode 100644 unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp diff --git a/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h b/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h index c5490a0da99..d979c8dda1a 100644 --- a/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h +++ b/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h @@ -16,7 +16,6 @@ #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Module.h" -#include "llvm/Support/Compiler.h" #include #include @@ -41,7 +40,7 @@ private: public: // RAII based lock for ThreadSafeContext. - class LLVM_NODISCARD Lock { + class Lock { private: using UnderlyingLock = std::lock_guard; public: @@ -86,26 +85,16 @@ 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) - : TSCtx(std::move(Ctx)), M(std::move(M)) {} + ThreadSafeModule(std::unique_ptr M, + std::unique_ptr Ctx) + : M(std::move(M)), TSCtx(std::move(Ctx)) {} - ThreadSafeModule(std::unique_ptr M, ThreadSafeContext TSCtx) - : TSCtx(std::move(TSCtx)), M(std::move(M)) {} + ThreadSafeModule(std::unique_ptr M, + ThreadSafeContext TSCtx) + : M(std::move(M)), TSCtx(std::move(TSCtx)) {} Module* getModule() { return M.get(); } @@ -120,8 +109,8 @@ public: } private: - ThreadSafeContext TSCtx; std::unique_ptr M; + ThreadSafeContext TSCtx; }; using GVPredicate = std::function; diff --git a/unittests/ExecutionEngine/Orc/CMakeLists.txt b/unittests/ExecutionEngine/Orc/CMakeLists.txt index 3a47bfa20bf..c18c9361cb0 100644 --- a/unittests/ExecutionEngine/Orc/CMakeLists.txt +++ b/unittests/ExecutionEngine/Orc/CMakeLists.txt @@ -26,7 +26,6 @@ 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 deleted file mode 100644 index 363c0976718..00000000000 --- a/unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp +++ /dev/null @@ -1,94 +0,0 @@ -//===--- 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 -- 2.11.0