<div dir="ltr">No worries. :)<div><br></div><div>Sorry about the cryptic commit message too. Looks like I forgot to update the git default one before applying. :/</div><div><br></div><div>-- Lang.</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Sep 27, 2018 at 1:15 AM Vitaly Buka <<a href="mailto:vitalybuka@google.com">vitalybuka@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">My mistake, it's caused by a different patch.</div><br><div class="gmail_quote"><div dir="ltr">On Thu, Sep 27, 2018 at 12:28 AM Vitaly Buka <<a href="mailto:vitalybuka@google.com" target="_blank">vitalybuka@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div dir="ltr">It still fails <a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/29351/steps/build%20release%20tsan%20with%20clang/logs/stdio" target="_blank">http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/29351/steps/build%20release%20tsan%20with%20clang/logs/stdio</a></div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Sep 26, 2018 at 7:11 PM Lang Hames via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: lhames<br>
Date: Wed Sep 26 19:09:36 2018<br>
New Revision: 343161<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=343161&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=343161&view=rev</a><br>
Log:<br>
Revert "Re-revert r343129."<br>
<br>
This reverts commit 4e2557dbc76704beb8c4cf1191cb786e719db5d3.<br>
<br>
Added:<br>
llvm/trunk/unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp<br>
Modified:<br>
llvm/trunk/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h<br>
llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt<br>
<br>
Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h?rev=343161&r1=343160&r2=343161&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h?rev=343161&r1=343160&r2=343161&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h (original)<br>
+++ llvm/trunk/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h Wed Sep 26 19:09:36 2018<br>
@@ -16,6 +16,7 @@<br>
<br>
#include "llvm/IR/LLVMContext.h"<br>
#include "llvm/IR/Module.h"<br>
+#include "llvm/Support/Compiler.h"<br>
<br>
#include <functional><br>
#include <memory><br>
@@ -40,7 +41,7 @@ private:<br>
public:<br>
<br>
// RAII based lock for ThreadSafeContext.<br>
- class Lock {<br>
+ class LLVM_NODISCARD Lock {<br>
private:<br>
using UnderlyingLock = std::lock_guard<std::recursive_mutex>;<br>
public:<br>
@@ -85,16 +86,26 @@ public:<br>
/// null context.<br>
ThreadSafeModule() = default;<br>
<br>
+ ThreadSafeModule(ThreadSafeModule &&Other) = default;<br>
+<br>
+ ThreadSafeModule& operator=(ThreadSafeModule &&Other) {<br>
+ // We have to explicitly define this move operator to copy the fields in<br>
+ // reverse order (i.e. module first) to ensure the dependencies are<br>
+ // protected: The old module that is being overwritten must be destroyed<br>
+ // *before* the context that it depends on.<br>
+ M = std::move(Other.M);<br>
+ TSCtx = std::move(Other.TSCtx);<br>
+ return *this;<br>
+ }<br>
+<br>
/// Construct a ThreadSafeModule from a unique_ptr<Module> and a<br>
/// unique_ptr<LLVMContext>. This creates a new ThreadSafeContext from the<br>
/// given context.<br>
- ThreadSafeModule(std::unique_ptr<Module> M,<br>
- std::unique_ptr<LLVMContext> Ctx)<br>
- : M(std::move(M)), TSCtx(std::move(Ctx)) {}<br>
-<br>
- ThreadSafeModule(std::unique_ptr<Module> M,<br>
- ThreadSafeContext TSCtx)<br>
- : M(std::move(M)), TSCtx(std::move(TSCtx)) {}<br>
+ ThreadSafeModule(std::unique_ptr<Module> M, std::unique_ptr<LLVMContext> Ctx)<br>
+ : TSCtx(std::move(Ctx)), M(std::move(M)) {}<br>
+<br>
+ ThreadSafeModule(std::unique_ptr<Module> M, ThreadSafeContext TSCtx)<br>
+ : TSCtx(std::move(TSCtx)), M(std::move(M)) {}<br>
<br>
Module* getModule() { return M.get(); }<br>
<br>
@@ -109,8 +120,8 @@ public:<br>
}<br>
<br>
private:<br>
- std::unique_ptr<Module> M;<br>
ThreadSafeContext TSCtx;<br>
+ std::unique_ptr<Module> M;<br>
};<br>
<br>
using GVPredicate = std::function<bool(const GlobalValue&)>;<br>
<br>
Modified: llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt?rev=343161&r1=343160&r2=343161&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt?rev=343161&r1=343160&r2=343161&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt (original)<br>
+++ llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt Wed Sep 26 19:09:36 2018<br>
@@ -26,6 +26,7 @@ add_llvm_unittest(OrcJITTests<br>
RTDyldObjectLinkingLayerTest.cpp<br>
RTDyldObjectLinkingLayer2Test.cpp<br>
SymbolStringPoolTest.cpp<br>
+ ThreadSafeModuleTest.cpp<br>
)<br>
<br>
target_link_libraries(OrcJITTests PRIVATE ${ORC_JIT_TEST_LIBS})<br>
<br>
Added: llvm/trunk/unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp?rev=343161&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp?rev=343161&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp (added)<br>
+++ llvm/trunk/unittests/ExecutionEngine/Orc/ThreadSafeModuleTest.cpp Wed Sep 26 19:09:36 2018<br>
@@ -0,0 +1,94 @@<br>
+//===--- ThreadSafeModuleTest.cpp - Test basic use of ThreadSafeModule ----===//<br>
+//<br>
+// The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#include "llvm/ExecutionEngine/Orc/ThreadSafeModule.h"<br>
+#include "gtest/gtest.h"<br>
+<br>
+#include <atomic><br>
+#include <future><br>
+#include <thread><br>
+<br>
+using namespace llvm;<br>
+using namespace llvm::orc;<br>
+<br>
+namespace {<br>
+<br>
+TEST(ThreadSafeModuleTest, ContextWhollyOwnedByOneModule) {<br>
+ // Test that ownership of a context can be transferred to a single<br>
+ // ThreadSafeModule.<br>
+ ThreadSafeContext TSCtx(llvm::make_unique<LLVMContext>());<br>
+ ThreadSafeModule TSM(llvm::make_unique<Module>("M", *TSCtx.getContext()),<br>
+ std::move(TSCtx));<br>
+}<br>
+<br>
+TEST(ThreadSafeModuleTest, ContextOwnershipSharedByTwoModules) {<br>
+ // Test that ownership of a context can be shared between more than one<br>
+ // ThreadSafeModule.<br>
+ ThreadSafeContext TSCtx(llvm::make_unique<LLVMContext>());<br>
+<br>
+ ThreadSafeModule TSM1(llvm::make_unique<Module>("M1", *TSCtx.getContext()),<br>
+ TSCtx);<br>
+ ThreadSafeModule TSM2(llvm::make_unique<Module>("M2", *TSCtx.getContext()),<br>
+ std::move(TSCtx));<br>
+}<br>
+<br>
+TEST(ThreadSafeModuleTest, ContextOwnershipSharedWithClient) {<br>
+ // Test that ownership of a context can be shared with a client-held<br>
+ // ThreadSafeContext so that it can be re-used for new modules.<br>
+ ThreadSafeContext TSCtx(llvm::make_unique<LLVMContext>());<br>
+<br>
+ {<br>
+ // Create and destroy a module.<br>
+ ThreadSafeModule TSM1(llvm::make_unique<Module>("M1", *TSCtx.getContext()),<br>
+ TSCtx);<br>
+ }<br>
+<br>
+ // Verify that the context is still available for re-use.<br>
+ ThreadSafeModule TSM2(llvm::make_unique<Module>("M2", *TSCtx.getContext()),<br>
+ std::move(TSCtx));<br>
+}<br>
+<br>
+TEST(ThreadSafeModuleTest, ThreadSafeModuleMoveAssignment) {<br>
+ // Move assignment needs to move the module before the context (opposite<br>
+ // to the field order) to ensure that overwriting with an empty<br>
+ // ThreadSafeModule does not destroy the context early.<br>
+ ThreadSafeContext TSCtx(llvm::make_unique<LLVMContext>());<br>
+ ThreadSafeModule TSM(llvm::make_unique<Module>("M", *TSCtx.getContext()),<br>
+ std::move(TSCtx));<br>
+ TSM = ThreadSafeModule();<br>
+}<br>
+<br>
+TEST(ThreadSafeModuleTest, BasicContextLockAPI) {<br>
+ // Test that basic lock API calls work.<br>
+ ThreadSafeContext TSCtx(llvm::make_unique<LLVMContext>());<br>
+ ThreadSafeModule TSM(llvm::make_unique<Module>("M", *TSCtx.getContext()),<br>
+ TSCtx);<br>
+<br>
+ { auto L = TSCtx.getLock(); }<br>
+<br>
+ { auto L = TSM.getContextLock(); }<br>
+}<br>
+<br>
+TEST(ThreadSafeModuleTest, ContextLockPreservesContext) {<br>
+ // Test that the existence of a context lock preserves the attached<br>
+ // context.<br>
+ // The trick to verify this is a bit of a hack: We attach a Module<br>
+ // (without the ThreadSafeModule wrapper) to the context, then verify<br>
+ // that this Module destructs safely (which it will not if its context<br>
+ // has been destroyed) even though all references to the context have<br>
+ // been thrown away (apart from the lock).<br>
+<br>
+ ThreadSafeContext TSCtx(llvm::make_unique<LLVMContext>());<br>
+ auto L = TSCtx.getLock();<br>
+ auto &Ctx = *TSCtx.getContext();<br>
+ auto M = llvm::make_unique<Module>("M", Ctx);<br>
+ TSCtx = ThreadSafeContext();<br>
+}<br>
+<br>
+} // end anonymous namespace<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>
</blockquote></div>
</blockquote></div>