[PATCH] Change base mutex implementations to use STL-provided mutexes

[Alp Toker]

Hi Zachary,

In LLVM platform-specific code goes in the implementation files (.inc) 
and never in the portable headers. Certainly for a high-level class like 
Mutex I wouldn't expect to see platform-specific conditionals exposed.

A couple of exceptions have slipped in like Support/FileSystem.h and 
Support/Process.h with ifdef WIN32 but those are meant to get fixed at 
some point.

On a more practical level it's not a good idea to include windows.h 
globally throughout the public headers. These headers need to get 
included in portable software that embeds clang for tooling etc. and 
windows.h is known to be problematic.

For thread primitives, the previous implementation looks broadly 
correct. You've explained the mechanics of your changes in detail but 
can you explain the motivation here? We need to find a better way to 
achieve that without introducing all these OS-specific low-level headers 
globally into embedding applications.

Alp.



On 05/06/2014 20:06, Zachary Turner wrote:
> Hi rnk, aaron.ballman, chandlerc,
>
> This is a re-submission of an earlier broken patch.
>
> Change llvm::sys::Mutex implementation to use std::mutex and std::recursive_mutex.
>
> This patch deletes the platform specific mutex implementations, and delegates to the underlying STL implementation. Because STL mutex and recursive_mutex are implemented as separate classes, it is no longer possible to specify as a runtime param whether a recursive mutex is desired, and instead the decision must be made at compile time.
>
> As part of this refactor, the ScopedLock class is deleted in favor of MutexGuard, which provides equivalent functionality.
>
> To make reviewing easier, you can refer to the following table of equivalences in the format (old_type -> new_type)
>
> SmartMutex<true>([true]) -> RecursiveDebugMutex
>
> SmartMutex<false>([true]) -> RecursiveMutex
> sys::Mutex -> SmartMutex<false>() -> RecursiveMutex
>
> SmartMutex<true>(false) -> NonRecursiveDebugMutex
> SmartMutex<false>(false) -> NonRecursiveMutex
>
> One caveat of note is that on Windows std::*mutex cannot be used during an atexit handler.  To get around this, a new type AtexitSafeMutex is defined, and on Windows this is typedefed to a critical section.
>
> http://reviews.llvm.org/D4033
>
> Files:
>    include/llvm/ExecutionEngine/ExecutionEngine.h
>    include/llvm/IR/ValueMap.h
>    include/llvm/Support/Mutex.h
>    include/llvm/Support/MutexGuard.h
>    lib/CodeGen/PseudoSourceValue.cpp
>    lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>    lib/ExecutionEngine/ExecutionEngine.cpp
>    lib/ExecutionEngine/Interpreter/ExternalFunctions.cpp
>    lib/ExecutionEngine/JIT/JIT.cpp
>    lib/ExecutionEngine/JIT/JITEmitter.cpp
>    lib/ExecutionEngine/OProfileJIT/OProfileWrapper.cpp
>    lib/ExecutionEngine/RuntimeDyld/GDBRegistrar.cpp
>    lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h
>    lib/IR/LeakDetector.cpp
>    lib/IR/LegacyPassManager.cpp
>    lib/Support/CMakeLists.txt
>    lib/Support/CrashRecoveryContext.cpp
>    lib/Support/DynamicLibrary.cpp
>    lib/Support/Mutex.cpp
>    lib/Support/PluginLoader.cpp
>    lib/Support/Statistic.cpp
>    lib/Support/Threading.cpp
>    lib/Support/Timer.cpp
>    lib/Support/Unix/Mutex.inc
>    lib/Support/Unix/Process.inc
>    lib/Support/Unix/Signals.inc
>    lib/Support/Windows/DynamicLibrary.inc
>    lib/Support/Windows/Mutex.inc
>    lib/Target/NVPTX/NVPTXUtilities.cpp
>    unittests/IR/ValueMapTest.cpp
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-- 
http://www.nuanti.com
the browser experts