Revert "Re-revert r343129."

This reverts commit 4e2557d.

llvm-svn: 343161

Author: lhames

llvm/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 <functional>
 #include <memory>
@@ -40,7 +41,7 @@ class ThreadSafeContext {
 public:
 
   // RAII based lock for ThreadSafeContext.
-  class Lock {
+  class LLVM_NODISCARD Lock {
   private:
     using UnderlyingLock = std::lock_guard<std::recursive_mutex>;
   public:
@@ -85,16 +86,26 @@ class ThreadSafeModule {
   /// 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<Module> and a
   /// unique_ptr<LLVMContext>. This creates a new ThreadSafeContext from the
   /// given context.
-  ThreadSafeModule(std::unique_ptr<Module> M,
-                   std::unique_ptr<LLVMContext> Ctx)
-    : M(std::move(M)), TSCtx(std::move(Ctx)) {}
+  ThreadSafeModule(std::unique_ptr<Module> M, std::unique_ptr<LLVMContext> Ctx)
+      : TSCtx(std::move(Ctx)), M(std::move(M)) {}
 
-  ThreadSafeModule(std::unique_ptr<Module> M,
-                   ThreadSafeContext TSCtx)
-    : M(std::move(M)), TSCtx(std::move(TSCtx)) {}
+  ThreadSafeModule(std::unique_ptr<Module> M, ThreadSafeContext TSCtx)
+      : TSCtx(std::move(TSCtx)), M(std::move(M)) {}
 
   Module* getModule() { return M.get(); }
 
@@ -109,8 +120,8 @@ class ThreadSafeModule {
   }
 
 private:
-  std::unique_ptr<Module> M;
   ThreadSafeContext TSCtx;
+  std::unique_ptr<Module> M;
 };
 
 using GVPredicate = std::function<bool(const GlobalValue&)>;

llvm/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})

llvm/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 <atomic>
+#include <future>
+#include <thread>
+
+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<LLVMContext>());
+  ThreadSafeModule TSM(llvm::make_unique<Module>("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<LLVMContext>());
+
+  ThreadSafeModule TSM1(llvm::make_unique<Module>("M1", *TSCtx.getContext()),
+                        TSCtx);
+  ThreadSafeModule TSM2(llvm::make_unique<Module>("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<LLVMContext>());
+
+  {
+    // Create and destroy a module.
+    ThreadSafeModule TSM1(llvm::make_unique<Module>("M1", *TSCtx.getContext()),
+                          TSCtx);
+  }
+
+  // Verify that the context is still available for re-use.
+  ThreadSafeModule TSM2(llvm::make_unique<Module>("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<LLVMContext>());
+  ThreadSafeModule TSM(llvm::make_unique<Module>("M", *TSCtx.getContext()),
+                       std::move(TSCtx));
+  TSM = ThreadSafeModule();
+}
+
+TEST(ThreadSafeModuleTest, BasicContextLockAPI) {
+  // Test that basic lock API calls work.
+  ThreadSafeContext TSCtx(llvm::make_unique<LLVMContext>());
+  ThreadSafeModule TSM(llvm::make_unique<Module>("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<LLVMContext>());
+  auto L = TSCtx.getLock();
+  auto &Ctx = *TSCtx.getContext();
+  auto M = llvm::make_unique<Module>("M", Ctx);
+  TSCtx = ThreadSafeContext();
+}
+
+} // end anonymous namespace