[llvm] r343129 - [ORC] Change the field order of ThreadSafeModule to ensure the Module is

[Lang Hames via llvm-commits]

Author: lhames
Date: Wed Sep 26 11:50:01 2018
New Revision: 343129

URL: http://llvm.org/viewvc/llvm-project?rev=343129&view=rev
Log:
[ORC] Change the field order of ThreadSafeModule to ensure the Module is
destroyed before its ThreadSharedContext.

Destroying the context first is an error if this ThreadSafeModule is the only
owner of its underlying context.

Add a unit test for ThreadSafeModule/ThreadSafeContext to catch this and other
basic usage issues.

Added:
    llvm/trunk/unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp
Modified:
    llvm/trunk/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h
    llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt

Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h?rev=343129&r1=343128&r2=343129&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h Wed Sep 26 11:50:01 2018
@@ -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 @@ private:
 public:
 
   // RAII based lock for ThreadSafeContext.
-  class Lock {
+  class LLVM_NODISCARD Lock {
   private:
     using UnderlyingLock = std::lock_guard<std::recursive_mutex>;
   public:
@@ -88,13 +89,11 @@ public:
   /// 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,
-                   ThreadSafeContext TSCtx)
-    : M(std::move(M)), TSCtx(std::move(TSCtx)) {}
+  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)
+      : TSCtx(std::move(TSCtx)), M(std::move(M)) {}
 
   Module* getModule() { return M.get(); }
 
@@ -109,8 +108,8 @@ public:
   }
 
 private:
-  std::unique_ptr<Module> M;
   ThreadSafeContext TSCtx;
+  std::unique_ptr<Module> M;
 };
 
 using GVPredicate = std::function<bool(const GlobalValue&)>;

Modified: llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt?rev=343129&r1=343128&r2=343129&view=diff
==============================================================================
--- llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt (original)
+++ llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt Wed Sep 26 11:50:01 2018
@@ -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})

Added: llvm/trunk/unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp?rev=343129&view=auto
==============================================================================
--- llvm/trunk/unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp (added)
+++ llvm/trunk/unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp Wed Sep 26 11:50:01 2018
@@ -0,0 +1,84 @@
+//===--- 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, 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