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

Aaron Ballman aaron.ballman at gmail.com
Fri Jun 6 08:52:36 PDT 2014


Feedback below:

> Index: include/llvm/ExecutionEngine/ExecutionEngine.h
> ===================================================================
> --- include/llvm/ExecutionEngine/ExecutionEngine.h
> +++ include/llvm/ExecutionEngine/ExecutionEngine.h
> @@ -23,6 +23,7 @@
>  #include "llvm/MC/MCCodeGenInfo.h"
>  #include "llvm/Support/ErrorHandling.h"
>  #include "llvm/Support/Mutex.h"
> +#include "llvm/Support/MutexGuard.h"
>  #include "llvm/Target/TargetMachine.h"
>  #include "llvm/Target/TargetOptions.h"
>  #include <map>
> @@ -59,7 +60,7 @@
>  public:
>    struct AddressMapConfig : public ValueMapConfig<const GlobalValue*> {
>      typedef ExecutionEngineState *ExtraData;
> -    static sys::Mutex *getMutex(ExecutionEngineState *EES);
> +    static sys::MutexBase *getMutex(ExecutionEngineState *EES);
>      static void onDelete(ExecutionEngineState *EES, const GlobalValue *Old);
>      static void onRAUW(ExecutionEngineState *, const GlobalValue *,
>                         const GlobalValue *);
> @@ -164,7 +165,7 @@
>    /// lock - This lock protects the ExecutionEngine, MCJIT, JIT, JITResolver and
>    /// JITEmitter classes.  It must be held while changing the internal state of
>    /// any of those classes.
> -  sys::Mutex lock;
> +  sys::RecursiveMutex lock;
>
>    //===--------------------------------------------------------------------===//
>    //  ExecutionEngine Startup
> Index: include/llvm/IR/ValueMap.h
> ===================================================================
> --- include/llvm/IR/ValueMap.h
> +++ include/llvm/IR/ValueMap.h
> @@ -67,7 +67,7 @@
>    /// and onDelete) and not inside other ValueMap methods.  NULL means that no
>    /// mutex is necessary.
>    template<typename ExtraDataT>
> -  static sys::Mutex *getMutex(const ExtraDataT &/*Data*/) { return nullptr; }
> +  static sys::MutexBase *getMutex(const ExtraDataT &/*Data*/) { return nullptr; }
>  };
>
>  /// See the file comment.
> @@ -212,7 +212,7 @@
>    void deleted() override {
>      // Make a copy that won't get changed even when *this is destroyed.
>      ValueMapCallbackVH Copy(*this);
> -    sys::Mutex *M = Config::getMutex(Copy.Map->Data);
> +    sys::MutexBase *M = Config::getMutex(Copy.Map->Data);
>      if (M)
>        M->acquire();
>      Config::onDelete(Copy.Map->Data, Copy.Unwrap());  // May destroy *this.
> @@ -225,7 +225,7 @@
>             "Invalid RAUW on key of ValueMap<>");
>      // Make a copy that won't get changed even when *this is destroyed.
>      ValueMapCallbackVH Copy(*this);
> -    sys::Mutex *M = Config::getMutex(Copy.Map->Data);
> +    sys::MutexBase *M = Config::getMutex(Copy.Map->Data);
>      if (M)
>        M->acquire();
>
> Index: include/llvm/Support/Mutex.h
> ===================================================================
> --- include/llvm/Support/Mutex.h
> +++ include/llvm/Support/Mutex.h
> @@ -17,138 +17,169 @@
>  #include "llvm/Support/Compiler.h"
>  #include "llvm/Support/Threading.h"
>  #include <cassert>
> +#include <mutex>
> +#if defined(WIN32)
> +#define NOMINMAX
> +#include <Windows.h>

Please include "llvm/Support/Windows/WindowsSupport.h" instead (which
I believe removes the need for NOMINMAX). Also, please guard with
defined(LLVM_ON_WIN32).

> +#endif
>
>  namespace llvm
>  {
>    namespace sys
>    {
> -    /// @brief Platform agnostic Mutex class.
> -    class MutexImpl
> +    /// @brief Platform agnostic mutex interface.
> +    class MutexBase
>      {
>      /// @name Constructors
>      /// @{
>      public:
> -
> -      /// Initializes the lock but doesn't acquire it. if \p recursive is set
> -      /// to false, the lock will not be recursive which makes it cheaper but
> -      /// also more likely to deadlock (same thread can't acquire more than
> -      /// once).
> -      /// @brief Default Constructor.
> -      explicit MutexImpl(bool recursive = true);
> +      /// Initializes the lock
> +      /// @brief Constructor
> +      MutexBase() {}
>
>        /// Releases and removes the lock
>        /// @brief Destructor
> -      ~MutexImpl();
> +      virtual ~MutexBase() {}
>
>      /// @}
>      /// @name Methods
>      /// @{
>      public:
>
>        /// Attempts to unconditionally acquire the lock. If the lock is held by
>        /// another thread, this method will wait until it can acquire the lock.
> -      /// @returns false if any kind of error occurs, true otherwise.
>        /// @brief Unconditionally acquire the lock.
> -      bool acquire();
> +      virtual void acquire() = 0;
>
>        /// Attempts to release the lock. If the lock is held by the current
>        /// thread, the lock is released allowing other threads to acquire the
>        /// lock.
> -      /// @returns false if any kind of error occurs, true otherwise.
>        /// @brief Unconditionally release the lock.
> -      bool release();
> +      virtual void release() = 0;
>
>        /// Attempts to acquire the lock without blocking. If the lock is not
>        /// available, this function returns false quickly (without blocking). If
>        /// the lock is available, it is acquired.
>        /// @returns false if any kind of error occurs or the lock is not
>        /// available, true otherwise.
>        /// @brief Try to acquire the lock.
> -      bool tryacquire();
> -
> -    //@}
> -    /// @name Platform Dependent Data
> -    /// @{
> -    private:
> -      void* data_; ///< We don't know what the data will be
> +      virtual bool tryacquire() = 0;
>
>      /// @}
>      /// @name Do Not Implement
>      /// @{
>      private:
> -      MutexImpl(const MutexImpl &) LLVM_DELETED_FUNCTION;
> -      void operator=(const MutexImpl &) LLVM_DELETED_FUNCTION;
> +      MutexBase(const MutexBase &) LLVM_DELETED_FUNCTION;
> +      void operator=(const MutexBase &) LLVM_DELETED_FUNCTION;
>      /// @}
>      };

I'm slightly concerned about the overhead from making these functions
virtual. Do you happen to have any indication as to how this affects
performance? What was wrong with the previous design where the base
did not hold concrete data, but instead a void *?

>
> -
> -    /// SmartMutex - A mutex with a compile time constant parameter that
> +    /// Mutex - A mutex with a compile time constant parameter that
>      /// indicates whether this mutex should become a no-op when we're not
>      /// running in multithreaded mode.
> -    template<bool mt_only>
> -    class SmartMutex : public MutexImpl {
> +    template<bool mt_only, bool recursive>
> +    class Mutex : public MutexBase {
> +    public:
> +      typedef typename std::conditional<recursive,
> +                                        std::recursive_mutex,
> +                                        std::mutex>::type mutex_type;
> +
> +    private:
> +      mutex_type mutex;
>        unsigned acquired;
> -      bool recursive;
> +
>      public:
> -      explicit SmartMutex(bool rec = true) :
> -        MutexImpl(rec), acquired(0), recursive(rec) { }
> +      Mutex() : acquired(0) {}
>
> -      bool acquire() {
> -        if (!mt_only || llvm_is_multithreaded()) {
> -          return MutexImpl::acquire();
> -        } else {
> +      mutex_type& native_mutex() { return mutex; }
> +
> +      virtual void acquire() {

Drop the virtual, add override.

> +        if (!mt_only || llvm_is_multithreaded())
> +          mutex.lock();
> +        else {
>            // Single-threaded debugging code.  This would be racy in
>            // multithreaded mode, but provides not sanity checks in single
>            // threaded mode.
>            assert((recursive || acquired == 0) && "Lock already acquired!!");
>            ++acquired;
> -          return true;
>          }
>        }
>
> -      bool release() {
> -        if (!mt_only || llvm_is_multithreaded()) {
> -          return MutexImpl::release();
> -        } else {
> +      virtual void release() {

Drop virtual, add override.

> +        if (!mt_only || llvm_is_multithreaded())
> +          mutex.unlock();
> +        else {
>            // Single-threaded debugging code.  This would be racy in
>            // multithreaded mode, but provides not sanity checks in single
>            // threaded mode.
>            assert(((recursive && acquired) || (acquired == 1)) &&
> -                 "Lock not acquired before release!");
> +                  "Lock not acquired before release!");
>            --acquired;
> -          return true;
>          }
>        }
>
> -      bool tryacquire() {
> +      virtual bool tryacquire() {

Drop virtual, add override.

>          if (!mt_only || llvm_is_multithreaded())
> -          return MutexImpl::tryacquire();
> -        else return true;
> +          return mutex.try_lock();
> +        else {
> +          bool can_lock = (recursive || acquired == 0);
> +          if (can_lock)
> +            ++acquired;
> +          return can_lock;
> +        }
>        }
>
> -      private:
> -        SmartMutex(const SmartMutex<mt_only> & original);
> -        void operator=(const SmartMutex<mt_only> &);
> +    private:
> +      Mutex(const Mutex<mt_only, recursive> & original);
> +      void operator=(const Mutex<mt_only, recursive> &);
>      };

Class is missing a virtual destructor.

>
> -    /// Mutex - A standard, always enforced mutex.
> -    typedef SmartMutex<false> Mutex;
> +#if defined(LLVM_ON_WIN32)
> +    // On MSVC, std::mutex and std::recursive_mutex cannot be used during an
> +    // atexit handler.  So a lightweight critical section implementation is
> +    // provided, and a typedef AtexitSafeMutex is provided when locking is
> +    // required during an atexit handler.
> +    class CriticalSection : public MutexBase {
> +    private:
> +      CRITICAL_SECTION m_cs;

Naming convention would be CS instead of m_cs;

> +    public:
> +      CriticalSection() {
> +        ::InitializeCriticalSection(&m_cs);
> +      }
> +      ~CriticalSection() {
> +        ::DeleteCriticalSection(&m_cs);
> +      }

This destructor needs to be virtual.

>
> -    template<bool mt_only>
> -    class SmartScopedLock  {
> -      SmartMutex<mt_only>& mtx;
> +      virtual void acquire() override {

Drop the virtual.

> +        ::EnterCriticalSection(&m_cs);
> +      }
>
> -    public:
> -      SmartScopedLock(SmartMutex<mt_only>& m) : mtx(m) {
> -        mtx.acquire();
> +      virtual void release() override {

Drop the virtual.

> +        ::LeaveCriticalSection(&m_cs);
>        }
>
> -      ~SmartScopedLock() {
> -        mtx.release();
> +      virtual bool tryacquire() override {

Drop the virtual.

> +        return ::TryEnterCriticalSection(&m_cs);
>        }
>      };
> +#endif
> +
> +    /// RecursiveMutex, NonRecursiveMutex - Standard, always enforced mutexes.
> +    typedef Mutex<false, true> RecursiveMutex;
> +    typedef Mutex<false, false> NonRecursiveMutex;
> +
> +    /// RecursiveDebugMutex, NonRecursiveDebugMutex - Mutexes which are not
> +    /// enforced and provide additional debug information when LLVM runs in
> +    /// single-threaded mode.
> +    typedef Mutex<true, true> RecursiveDebugMutex;
> +    typedef Mutex<true, false> NonRecursiveDebugMutex;
> +
> +#if defined(LLVM_ON_WIN32)
> +    typedef CriticalSection AtexitSafeMutex;
> +#else
> +    typedef NonRecursiveMutex AtexitSafeMutex;
> +#endif
>
> -    typedef SmartScopedLock<false> ScopedLock;
>    }
>  }
>
> Index: include/llvm/Support/MutexGuard.h
> ===================================================================
> --- include/llvm/Support/MutexGuard.h
> +++ include/llvm/Support/MutexGuard.h
> @@ -25,16 +25,17 @@
>    /// a host of nasty multi-threading problems in the face of exceptions, etc.
>    /// @brief Guard a section of code with a Mutex.
>    class MutexGuard {
> -    sys::Mutex &M;
> +    sys::MutexBase &M;
> +
>      MutexGuard(const MutexGuard &) LLVM_DELETED_FUNCTION;
>      void operator=(const MutexGuard &) LLVM_DELETED_FUNCTION;
>    public:
> -    MutexGuard(sys::Mutex &m) : M(m) { M.acquire(); }
> +    MutexGuard(sys::MutexBase &m) : M(m) { M.acquire(); }
>      ~MutexGuard() { M.release(); }
>      /// holds - Returns true if this locker instance holds the specified lock.
>      /// This is mostly used in assertions to validate that the correct mutex
>      /// is held.
> -    bool holds(const sys::Mutex& lock) const { return &M == &lock; }
> +    bool holds(const sys::MutexBase& lock) const { return &M == &lock; }

Should be MutexBase &Lock instead of MutexBase& lock.

>    };
>  }
>
> Index: lib/CodeGen/PseudoSourceValue.cpp
> ===================================================================
> --- lib/CodeGen/PseudoSourceValue.cpp
> +++ lib/CodeGen/PseudoSourceValue.cpp
> @@ -18,15 +18,16 @@
>  #include "llvm/Support/ErrorHandling.h"
>  #include "llvm/Support/ManagedStatic.h"
>  #include "llvm/Support/Mutex.h"
> +#include "llvm/Support/MutexGuard.h"
>  #include "llvm/Support/raw_ostream.h"
>  #include <map>
>  using namespace llvm;
>
>  namespace {
>  struct PSVGlobalsTy {
>    // PseudoSourceValues are immutable so don't need locking.
>    const PseudoSourceValue PSVs[4];
> -  sys::Mutex Lock;  // Guards FSValues, but not the values inside it.
> +  sys::RecursiveMutex Lock;  // Guards FSValues, but not the values inside it.
>    std::map<int, const PseudoSourceValue *> FSValues;
>
>    PSVGlobalsTy() : PSVs() {}
> @@ -68,7 +69,7 @@
>
>  const PseudoSourceValue *PseudoSourceValue::getFixedStack(int FI) {
>    PSVGlobalsTy &PG = *PSVGlobals;
> -  sys::ScopedLock locked(PG.Lock);
> +  llvm::MutexGuard locked(PG.Lock);
>    const PseudoSourceValue *&V = PG.FSValues[FI];
>    if (!V)
>      V = new FixedStackPseudoSourceValue(FI);
> Index: lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> ===================================================================
> --- lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> +++ lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> @@ -38,6 +38,7 @@
>  #include "llvm/Support/ManagedStatic.h"
>  #include "llvm/Support/MathExtras.h"
>  #include "llvm/Support/Mutex.h"
> +#include "llvm/Support/MutexGuard.h"
>  #include "llvm/Support/raw_ostream.h"
>  #include "llvm/Target/TargetInstrInfo.h"
>  #include "llvm/Target/TargetIntrinsicInfo.h"
> @@ -6119,13 +6120,13 @@
>
>  static ManagedStatic<std::set<EVT, EVT::compareRawBits> > EVTs;
>  static ManagedStatic<EVTArray> SimpleVTArray;
> -static ManagedStatic<sys::SmartMutex<true> > VTMutex;
> +static ManagedStatic<sys::RecursiveDebugMutex> VTMutex;
>
>  /// getValueTypeList - Return a pointer to the specified value type.
>  ///
>  const EVT *SDNode::getValueTypeList(EVT VT) {
>    if (VT.isExtended()) {
> -    sys::SmartScopedLock<true> Lock(*VTMutex);
> +    llvm::MutexGuard Lock(*VTMutex);
>      return &(*EVTs->insert(VT).first);
>    } else {
>      assert(VT.getSimpleVT() < MVT::LAST_VALUETYPE &&
> Index: lib/ExecutionEngine/ExecutionEngine.cpp
> ===================================================================
> --- lib/ExecutionEngine/ExecutionEngine.cpp
> +++ lib/ExecutionEngine/ExecutionEngine.cpp
> @@ -1347,7 +1347,7 @@
>    : EE(EE), GlobalAddressMap(this) {
>  }
>
> -sys::Mutex *
> +sys::MutexBase *
>  ExecutionEngineState::AddressMapConfig::getMutex(ExecutionEngineState *EES) {
>    return &EES->EE.lock;
>  }
> Index: lib/ExecutionEngine/Interpreter/ExternalFunctions.cpp
> ===================================================================
> --- lib/ExecutionEngine/Interpreter/ExternalFunctions.cpp
> +++ lib/ExecutionEngine/Interpreter/ExternalFunctions.cpp
> @@ -28,6 +28,7 @@
>  #include "llvm/Support/ErrorHandling.h"
>  #include "llvm/Support/ManagedStatic.h"
>  #include "llvm/Support/Mutex.h"
> +#include "llvm/Support/MutexGuard.h"
>  #include <cmath>
>  #include <csignal>
>  #include <cstdio>
> @@ -46,7 +47,7 @@
>
>  using namespace llvm;
>
> -static ManagedStatic<sys::Mutex> FunctionsLock;
> +static ManagedStatic<sys::RecursiveMutex> FunctionsLock;
>
>  typedef GenericValue (*ExFunc)(FunctionType *,
>                                 const std::vector<GenericValue> &);
> @@ -96,7 +97,7 @@
>      ExtName += getTypeID(FT->getContainedType(i));
>    ExtName += "_" + F->getName().str();
>
> -  sys::ScopedLock Writer(*FunctionsLock);
> +  llvm::MutexGuard Writer(*FunctionsLock);
>    ExFunc FnPtr = FuncNames[ExtName];
>    if (!FnPtr)
>      FnPtr = FuncNames["lle_X_" + F->getName().str()];
> @@ -496,7 +497,7 @@
>  }
>
>  void Interpreter::initializeExternalFunctions() {
> -  sys::ScopedLock Writer(*FunctionsLock);
> +  llvm::MutexGuard Writer(*FunctionsLock);
>    FuncNames["lle_X_atexit"]       = lle_X_atexit;
>    FuncNames["lle_X_exit"]         = lle_X_exit;
>    FuncNames["lle_X_abort"]        = lle_X_abort;
> Index: lib/ExecutionEngine/JIT/JIT.cpp
> ===================================================================
> --- lib/ExecutionEngine/JIT/JIT.cpp
> +++ lib/ExecutionEngine/JIT/JIT.cpp
> @@ -96,7 +96,7 @@
>  /// bugpoint or gdb users to search for a function by name without any context.
>  class JitPool {
>    SmallPtrSet<JIT*, 1> JITs;  // Optimize for process containing just 1 JIT.
> -  mutable sys::Mutex Lock;
> +  mutable sys::RecursiveMutex Lock;
>  public:
>    void Add(JIT *jit) {
>      MutexGuard guard(Lock);
> Index: lib/ExecutionEngine/JIT/JITEmitter.cpp
> ===================================================================
> --- lib/ExecutionEngine/JIT/JITEmitter.cpp
> +++ lib/ExecutionEngine/JIT/JITEmitter.cpp
> @@ -237,7 +237,7 @@
>      std::map<void*, JITResolver*> Map;
>
>      /// Guards Map from concurrent accesses.
> -    mutable sys::Mutex Lock;
> +    mutable sys::RecursiveMutex Lock;
>
>    public:
>      /// Registers a Stub to be resolved by Resolver.
> Index: lib/ExecutionEngine/OProfileJIT/OProfileWrapper.cpp
> ===================================================================
> --- lib/ExecutionEngine/OProfileJIT/OProfileWrapper.cpp
> +++ lib/ExecutionEngine/OProfileJIT/OProfileWrapper.cpp
> @@ -33,7 +33,7 @@
>  namespace {
>
>  // Global mutex to ensure a single thread initializes oprofile agent.
> -llvm::sys::Mutex OProfileInitializationMutex;
> +llvm::sys::RecursiveMutex OProfileInitializationMutex;
>
>  } // anonymous namespace
>
> Index: lib/ExecutionEngine/RuntimeDyld/GDBRegistrar.cpp
> ===================================================================
> --- lib/ExecutionEngine/RuntimeDyld/GDBRegistrar.cpp
> +++ lib/ExecutionEngine/RuntimeDyld/GDBRegistrar.cpp
> @@ -101,7 +101,7 @@
>
>  /// Lock used to serialize all jit registration events, since they
>  /// modify global variables.
> -llvm::sys::Mutex JITDebugLock;
> +llvm::sys::RecursiveMutex JITDebugLock;
>
>  /// Do the registration.
>  void NotifyDebugger(jit_code_entry* JITCodeEntry) {
> Index: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h
> ===================================================================
> --- lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h
> +++ lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h
> @@ -220,7 +220,7 @@
>    // processRelocations, and that's OK.  The handling of the relocation lists
>    // is written in such a way as to work correctly if new elements are added to
>    // the end of the list while the list is being processed.
> -  sys::Mutex lock;
> +  sys::RecursiveMutex lock;
>
>    virtual unsigned getMaxStubSize() = 0;
>    virtual unsigned getStubAlignment() = 0;
> Index: lib/IR/LeakDetector.cpp
> ===================================================================
> --- lib/IR/LeakDetector.cpp
> +++ lib/IR/LeakDetector.cpp
> @@ -18,19 +18,20 @@
>  #include "llvm/Support/Compiler.h"
>  #include "llvm/Support/ManagedStatic.h"
>  #include "llvm/Support/Mutex.h"
> +#include "llvm/Support/MutexGuard.h"
>  #include "llvm/Support/Threading.h"
>  using namespace llvm;
>
> -static ManagedStatic<sys::SmartMutex<true> > ObjectsLock;
> +static ManagedStatic<sys::RecursiveDebugMutex> ObjectsLock;
>  static ManagedStatic<LeakDetectorImpl<void> > Objects;
>
>  static void clearGarbage(LLVMContext &Context) {
>    Objects->clear();
>    Context.pImpl->LLVMObjects.clear();
>  }
>
>  void LeakDetector::addGarbageObjectImpl(void *Object) {
> -  sys::SmartScopedLock<true> Lock(*ObjectsLock);
> +  llvm::MutexGuard Lock(*ObjectsLock);
>    Objects->addGarbage(Object);
>  }
>
> @@ -40,7 +41,7 @@
>  }
>
>  void LeakDetector::removeGarbageObjectImpl(void *Object) {
> -  sys::SmartScopedLock<true> Lock(*ObjectsLock);
> +  llvm::MutexGuard Lock(*ObjectsLock);
>    Objects->removeGarbage(Object);
>  }
>
> @@ -52,7 +53,7 @@
>  void LeakDetector::checkForGarbageImpl(LLVMContext &Context,
>                                         const std::string &Message) {
>    LLVMContextImpl *pImpl = Context.pImpl;
> -  sys::SmartScopedLock<true> Lock(*ObjectsLock);
> +  llvm::MutexGuard Lock(*ObjectsLock);
>
>    Objects->setName("GENERIC");
>    pImpl->LLVMObjects.setName("LLVM");
> Index: lib/IR/LegacyPassManager.cpp
> ===================================================================
> --- lib/IR/LegacyPassManager.cpp
> +++ lib/IR/LegacyPassManager.cpp
> @@ -23,6 +23,7 @@
>  #include "llvm/Support/ErrorHandling.h"
>  #include "llvm/Support/ManagedStatic.h"
>  #include "llvm/Support/Mutex.h"
> +#include "llvm/Support/MutexGuard.h"
>  #include "llvm/Support/TimeValue.h"
>  #include "llvm/Support/Timer.h"
>  #include "llvm/Support/raw_ostream.h"
> @@ -459,7 +460,7 @@
>  /// -time-passes is enabled on the command line.
>  ///
>
> -static ManagedStatic<sys::SmartMutex<true> > TimingInfoMutex;
> +static ManagedStatic<sys::RecursiveDebugMutex> TimingInfoMutex;
>
>  class TimingInfo {
>    DenseMap<Pass*, Timer*> TimingData;
> @@ -488,7 +489,7 @@
>      if (P->getAsPMDataManager())
>        return nullptr;
>
> -    sys::SmartScopedLock<true> Lock(*TimingInfoMutex);
> +    llvm::MutexGuard Lock(*TimingInfoMutex);
>      Timer *&T = TimingData[P];
>      if (!T)
>        T = new Timer(P->getPassName(), TG);
> Index: lib/Support/CMakeLists.txt
> ===================================================================
> --- lib/Support/CMakeLists.txt
> +++ lib/Support/CMakeLists.txt
> @@ -76,7 +76,6 @@
>    Host.cpp
>    IncludeFile.cpp
>    Memory.cpp
> -  Mutex.cpp
>    Path.cpp
>    Process.cpp
>    Program.cpp
> @@ -94,7 +93,6 @@
>    ADDITIONAL_HEADERS
>    Unix/Host.inc
>    Unix/Memory.inc
> -  Unix/Mutex.inc
>    Unix/Path.inc
>    Unix/Process.inc
>    Unix/Program.inc
> @@ -107,7 +105,6 @@
>    Windows/DynamicLibrary.inc
>    Windows/Host.inc
>    Windows/Memory.inc
> -  Windows/Mutex.inc
>    Windows/Path.inc
>    Windows/Process.inc
>    Windows/Program.inc
> Index: lib/Support/CrashRecoveryContext.cpp
> ===================================================================
> --- lib/Support/CrashRecoveryContext.cpp
> +++ lib/Support/CrashRecoveryContext.cpp
> @@ -13,6 +13,7 @@
>  #include "llvm/Support/ErrorHandling.h"
>  #include "llvm/Support/ManagedStatic.h"
>  #include "llvm/Support/Mutex.h"
> +#include "llvm/Support/MutexGuard.h"
>  #include "llvm/Support/ThreadLocal.h"
>  #include <cstdio>
>  #include <setjmp.h>
> @@ -63,7 +64,7 @@
>
>  }
>
> -static ManagedStatic<sys::Mutex> gCrashRecoveryContextMutex;
> +static ManagedStatic<sys::RecursiveMutex> gCrashRecoveryContextMutex;
>  static bool gCrashRecoveryEnabled = false;
>
>  static ManagedStatic<sys::ThreadLocal<const CrashRecoveryContextCleanup> >
> @@ -183,7 +184,7 @@
>  static sys::ThreadLocal<const void> sCurrentExceptionHandle;
>
>  void CrashRecoveryContext::Enable() {
> -  sys::ScopedLock L(*gCrashRecoveryContextMutex);
> +  llvm::MutexGuard L(*gCrashRecoveryContextMutex);
>
>    if (gCrashRecoveryEnabled)
>      return;
> @@ -199,7 +200,7 @@
>  }
>
>  void CrashRecoveryContext::Disable() {
> -  sys::ScopedLock L(*gCrashRecoveryContextMutex);
> +  llvm::MutexGuard L(*gCrashRecoveryContextMutex);
>
>    if (!gCrashRecoveryEnabled)
>      return;
> @@ -268,7 +269,7 @@
>  }
>
>  void CrashRecoveryContext::Enable() {
> -  sys::ScopedLock L(*gCrashRecoveryContextMutex);
> +  llvm::MutexGuard L(*gCrashRecoveryContextMutex);
>
>    if (gCrashRecoveryEnabled)
>      return;
> @@ -287,7 +288,7 @@
>  }
>
>  void CrashRecoveryContext::Disable() {
> -  sys::ScopedLock L(*gCrashRecoveryContextMutex);
> +  llvm::MutexGuard L(*gCrashRecoveryContextMutex);
>
>    if (!gCrashRecoveryEnabled)
>      return;
> Index: lib/Support/DynamicLibrary.cpp
> ===================================================================
> --- lib/Support/DynamicLibrary.cpp
> +++ lib/Support/DynamicLibrary.cpp
> @@ -20,16 +20,17 @@
>  #include "llvm/Config/config.h"
>  #include "llvm/Support/ManagedStatic.h"
>  #include "llvm/Support/Mutex.h"
> +#include "llvm/Support/MutexGuard.h"
>  #include <cstdio>
>  #include <cstring>
>
>  // Collection of symbol name/value pairs to be searched prior to any libraries.
>  static llvm::ManagedStatic<llvm::StringMap<void *> > ExplicitSymbols;
> -static llvm::ManagedStatic<llvm::sys::SmartMutex<true> > SymbolsMutex;
> +static llvm::ManagedStatic<llvm::sys::RecursiveDebugMutex> SymbolsMutex;
>
>  void llvm::sys::DynamicLibrary::AddSymbol(StringRef symbolName,
>                                            void *symbolValue) {
> -  SmartScopedLock<true> lock(*SymbolsMutex);
> +  llvm::MutexGuard lock(*SymbolsMutex);

Lock instead of lock.

>    (*ExplicitSymbols)[symbolName] = symbolValue;
>  }
>
> @@ -55,7 +56,7 @@
>
>  DynamicLibrary DynamicLibrary::getPermanentLibrary(const char *filename,
>                                                     std::string *errMsg) {
> -  SmartScopedLock<true> lock(*SymbolsMutex);
> +  llvm::MutexGuard lock(*SymbolsMutex);

Lock instead of lock.

>
>    void *handle = dlopen(filename, RTLD_LAZY|RTLD_GLOBAL);
>    if (!handle) {
> @@ -109,7 +110,7 @@
>  }
>
>  void* DynamicLibrary::SearchForAddressOfSymbol(const char *symbolName) {
> -  SmartScopedLock<true> Lock(*SymbolsMutex);
> +  llvm::MutexGuard lock(*SymbolsMutex);

Lock instead of lock.

>
>    // First check symbols added via AddSymbol().
>    if (ExplicitSymbols.isConstructed()) {
> Index: lib/Support/Mutex.cpp
> ===================================================================
> --- lib/Support/Mutex.cpp
> +++ /dev/null
> @@ -1,122 +0,0 @@
> -//===- Mutex.cpp - Mutual Exclusion Lock ------------------------*- C++ -*-===//
> -//
> -//                     The LLVM Compiler Infrastructure
> -//
> -// This file is distributed under the University of Illinois Open Source
> -// License. See LICENSE.TXT for details.
> -//
> -//===----------------------------------------------------------------------===//
> -//
> -// This file implements the llvm::sys::Mutex class.
> -//
> -//===----------------------------------------------------------------------===//
> -
> -#include "llvm/Config/config.h"
> -#include "llvm/Support/Mutex.h"
> -
> -//===----------------------------------------------------------------------===//
> -//=== WARNING: Implementation here must contain only TRULY operating system
> -//===          independent code.
> -//===----------------------------------------------------------------------===//
> -
> -#if !defined(LLVM_ENABLE_THREADS) || LLVM_ENABLE_THREADS == 0
> -// Define all methods as no-ops if threading is explicitly disabled
> -namespace llvm {
> -using namespace sys;
> -MutexImpl::MutexImpl( bool recursive) { }
> -MutexImpl::~MutexImpl() { }
> -bool MutexImpl::acquire() { return true; }
> -bool MutexImpl::release() { return true; }
> -bool MutexImpl::tryacquire() { return true; }
> -}
> -#else
> -
> -#if defined(HAVE_PTHREAD_H) && defined(HAVE_PTHREAD_MUTEX_LOCK)
> -
> -#include <cassert>
> -#include <pthread.h>
> -#include <stdlib.h>
> -
> -namespace llvm {
> -using namespace sys;
> -
> -// Construct a Mutex using pthread calls
> -MutexImpl::MutexImpl( bool recursive)
> -  : data_(nullptr)
> -{
> -  // Declare the pthread_mutex data structures
> -  pthread_mutex_t* mutex =
> -    static_cast<pthread_mutex_t*>(malloc(sizeof(pthread_mutex_t)));
> -  pthread_mutexattr_t attr;
> -
> -  // Initialize the mutex attributes
> -  int errorcode = pthread_mutexattr_init(&attr);
> -  assert(errorcode == 0); (void)errorcode;
> -
> -  // Initialize the mutex as a recursive mutex, if requested, or normal
> -  // otherwise.
> -  int kind = ( recursive  ? PTHREAD_MUTEX_RECURSIVE : PTHREAD_MUTEX_NORMAL );
> -  errorcode = pthread_mutexattr_settype(&attr, kind);
> -  assert(errorcode == 0);
> -
> -  // Initialize the mutex
> -  errorcode = pthread_mutex_init(mutex, &attr);
> -  assert(errorcode == 0);
> -
> -  // Destroy the attributes
> -  errorcode = pthread_mutexattr_destroy(&attr);
> -  assert(errorcode == 0);
> -
> -  // Assign the data member
> -  data_ = mutex;
> -}
> -
> -// Destruct a Mutex
> -MutexImpl::~MutexImpl()
> -{
> -  pthread_mutex_t* mutex = static_cast<pthread_mutex_t*>(data_);
> -  assert(mutex != nullptr);
> -  pthread_mutex_destroy(mutex);
> -  free(mutex);
> -}
> -
> -bool
> -MutexImpl::acquire()
> -{
> -  pthread_mutex_t* mutex = static_cast<pthread_mutex_t*>(data_);
> -  assert(mutex != nullptr);
> -
> -  int errorcode = pthread_mutex_lock(mutex);
> -  return errorcode == 0;
> -}
> -
> -bool
> -MutexImpl::release()
> -{
> -  pthread_mutex_t* mutex = static_cast<pthread_mutex_t*>(data_);
> -  assert(mutex != nullptr);
> -
> -  int errorcode = pthread_mutex_unlock(mutex);
> -  return errorcode == 0;
> -}
> -
> -bool
> -MutexImpl::tryacquire()
> -{
> -  pthread_mutex_t* mutex = static_cast<pthread_mutex_t*>(data_);
> -  assert(mutex != nullptr);
> -
> -  int errorcode = pthread_mutex_trylock(mutex);
> -  return errorcode == 0;
> -}
> -
> -}
> -
> -#elif defined(LLVM_ON_UNIX)
> -#include "Unix/Mutex.inc"
> -#elif defined( LLVM_ON_WIN32)
> -#include "Windows/Mutex.inc"
> -#else
> -#warning Neither LLVM_ON_UNIX nor LLVM_ON_WIN32 was set in Support/Mutex.cpp
> -#endif
> -#endif
> Index: lib/Support/PluginLoader.cpp
> ===================================================================
> --- lib/Support/PluginLoader.cpp
> +++ lib/Support/PluginLoader.cpp
> @@ -16,15 +16,16 @@
>  #include "llvm/Support/DynamicLibrary.h"
>  #include "llvm/Support/ManagedStatic.h"
>  #include "llvm/Support/Mutex.h"
> +#include "llvm/Support/MutexGuard.h"
>  #include "llvm/Support/raw_ostream.h"
>  #include <vector>
>  using namespace llvm;
>
>  static ManagedStatic<std::vector<std::string> > Plugins;
> -static ManagedStatic<sys::SmartMutex<true> > PluginsLock;
> +static ManagedStatic<sys::RecursiveDebugMutex> PluginsLock;
>
>  void PluginLoader::operator=(const std::string &Filename) {
> -  sys::SmartScopedLock<true> Lock(*PluginsLock);
> +  llvm::MutexGuard Lock(*PluginsLock);
>    std::string Error;
>    if (sys::DynamicLibrary::LoadLibraryPermanently(Filename.c_str(), &Error)) {
>      errs() << "Error opening '" << Filename << "': " << Error
> @@ -35,12 +36,12 @@
>  }
>
>  unsigned PluginLoader::getNumPlugins() {
> -  sys::SmartScopedLock<true> Lock(*PluginsLock);
> +  llvm::MutexGuard Lock(*PluginsLock);
>    return Plugins.isConstructed() ? Plugins->size() : 0;
>  }
>
>  std::string &PluginLoader::getPlugin(unsigned num) {
> -  sys::SmartScopedLock<true> Lock(*PluginsLock);
> +  llvm::MutexGuard Lock(*PluginsLock);
>    assert(Plugins.isConstructed() && num < Plugins->size() &&
>           "Asking for an out of bounds plugin");
>    return (*Plugins)[num];
> Index: lib/Support/Statistic.cpp
> ===================================================================
> --- lib/Support/Statistic.cpp
> +++ lib/Support/Statistic.cpp
> @@ -28,6 +28,7 @@
>  #include "llvm/Support/Format.h"
>  #include "llvm/Support/ManagedStatic.h"
>  #include "llvm/Support/Mutex.h"
> +#include "llvm/Support/MutexGuard.h"
>  #include "llvm/Support/raw_ostream.h"
>  #include <algorithm>
>  #include <cstring>
> @@ -63,14 +64,14 @@
>  }
>
>  static ManagedStatic<StatisticInfo> StatInfo;
> -static ManagedStatic<sys::SmartMutex<true> > StatLock;
> +static ManagedStatic<sys::RecursiveDebugMutex> StatLock;
>
>  /// RegisterStatistic - The first time a statistic is bumped, this method is
>  /// called.
>  void Statistic::RegisterStatistic() {
>    // If stats are enabled, inform StatInfo that this statistic should be
>    // printed.
> -  sys::SmartScopedLock<true> Writer(*StatLock);
> +  llvm::MutexGuard Writer(*StatLock);
>    if (!Initialized) {
>      if (Enabled)
>        StatInfo->addStatistic(this);
> Index: lib/Support/Threading.cpp
> ===================================================================
> --- lib/Support/Threading.cpp
> +++ lib/Support/Threading.cpp
> @@ -21,13 +21,13 @@
>
>  static bool multithreaded_mode = false;
>
> -static sys::Mutex* global_lock = nullptr;
> +static sys::RecursiveMutex* global_lock = nullptr;

Move the * to the right.

>
>  bool llvm::llvm_start_multithreaded() {
>  #if LLVM_ENABLE_THREADS != 0
>    assert(!multithreaded_mode && "Already multithreaded!");
>    multithreaded_mode = true;
> -  global_lock = new sys::Mutex(true);
> +  global_lock = new sys::RecursiveMutex;
>
>    // We fence here to ensure that all initialization is complete BEFORE we
>    // return from llvm_start_multithreaded().
> @@ -48,6 +48,7 @@
>
>    multithreaded_mode = false;
>    delete global_lock;
> +  global_lock = nullptr;
>  #endif
>  }
>
> Index: lib/Support/Timer.cpp
> ===================================================================
> --- lib/Support/Timer.cpp
> +++ lib/Support/Timer.cpp
> @@ -19,6 +19,7 @@
>  #include "llvm/Support/Format.h"
>  #include "llvm/Support/ManagedStatic.h"
>  #include "llvm/Support/Mutex.h"
> +#include "llvm/Support/MutexGuard.h"
>  #include "llvm/Support/Process.h"
>  #include "llvm/Support/raw_ostream.h"
>  using namespace llvm;
> @@ -39,7 +40,7 @@
>    return *LibSupportInfoOutputFilename;
>  }
>
> -static ManagedStatic<sys::SmartMutex<true> > TimerLock;
> +static ManagedStatic<sys::RecursiveDebugMutex> TimerLock;
>
>  namespace {
>    static cl::opt<bool>
> @@ -206,7 +207,7 @@
>    }
>
>    Timer &get(StringRef Name, StringRef GroupName) {
> -    sys::SmartScopedLock<true> L(*TimerLock);
> +    llvm::MutexGuard lock(*TimerLock);

Lock instead of lock.

>
>      std::pair<TimerGroup*, Name2TimerMap> &GroupEntry = Map[GroupName];
>
> @@ -226,7 +227,7 @@
>  static ManagedStatic<Name2PairMap> NamedGroupedTimers;
>
>  static Timer &getNamedRegionTimer(StringRef Name) {
> -  sys::SmartScopedLock<true> L(*TimerLock);
> +  llvm::MutexGuard lock(*TimerLock);

Lock instead of lock.

>
>    Timer &T = (*NamedTimers)[Name];
>    if (!T.isInitialized())
> @@ -254,7 +255,7 @@
>    : Name(name.begin(), name.end()), FirstTimer(nullptr) {
>
>    // Add the group to TimerGroupList.
> -  sys::SmartScopedLock<true> L(*TimerLock);
> +  llvm::MutexGuard lock(*TimerLock);

Lock instead of lock.

>    if (TimerGroupList)
>      TimerGroupList->Prev = &Next;
>    Next = TimerGroupList;
> @@ -269,15 +270,15 @@
>      removeTimer(*FirstTimer);
>
>    // Remove the group from the TimerGroupList.
> -  sys::SmartScopedLock<true> L(*TimerLock);
> +  llvm::MutexGuard lock(*TimerLock);

Lock instead of lock.

>    *Prev = Next;
>    if (Next)
>      Next->Prev = Prev;
>  }
>
>
>  void TimerGroup::removeTimer(Timer &T) {
> -  sys::SmartScopedLock<true> L(*TimerLock);
> +  llvm::MutexGuard lock(*TimerLock);

Lock instead of lock.

>
>    // If the timer was started, move its data to TimersToPrint.
>    if (T.Started)
> @@ -301,7 +302,7 @@
>  }
>
>  void TimerGroup::addTimer(Timer &T) {
> -  sys::SmartScopedLock<true> L(*TimerLock);
> +  llvm::MutexGuard lock(*TimerLock);

Lock instead of lock.

>
>    // Add the timer to our list.
>    if (FirstTimer)
> @@ -362,7 +363,7 @@
>
>  /// print - Print any started timers in this group and zero them.
>  void TimerGroup::print(raw_ostream &OS) {
> -  sys::SmartScopedLock<true> L(*TimerLock);
> +  llvm::MutexGuard lock(*TimerLock);

Lock instead of lock.

>
>    // See if any of our timers were started, if so add them to TimersToPrint and
>    // reset them.
> @@ -382,7 +383,7 @@
>
>  /// printAll - This static method prints all timers and clears them all out.
>  void TimerGroup::printAll(raw_ostream &OS) {
> -  sys::SmartScopedLock<true> L(*TimerLock);
> +  llvm::MutexGuard lock(*TimerLock);

Lock instead of lock.

>
>    for (TimerGroup *TG = TimerGroupList; TG; TG = TG->Next)
>      TG->print(OS);
> Index: lib/Support/Unix/Mutex.inc
> ===================================================================
> --- lib/Support/Unix/Mutex.inc
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -//===- llvm/Support/Unix/Mutex.inc - Unix Mutex Implementation ---*- C++ -*-===//
> -//
> -//                     The LLVM Compiler Infrastructure
> -//
> -// This file is distributed under the University of Illinois Open Source
> -// License. See LICENSE.TXT for details.
> -//
> -//===----------------------------------------------------------------------===//
> -//
> -// This file implements the Unix specific (non-pthread) Mutex class.
> -//
> -//===----------------------------------------------------------------------===//
> -
> -//===----------------------------------------------------------------------===//
> -//=== WARNING: Implementation here must contain only generic UNIX code that
> -//===          is guaranteed to work on *all* UNIX variants.
> -//===----------------------------------------------------------------------===//
> -
> -namespace llvm
> -{
> -using namespace sys;
> -
> -MutexImpl::MutexImpl( bool recursive)
> -{
> -}
> -
> -MutexImpl::~MutexImpl()
> -{
> -}
> -
> -bool
> -MutexImpl::release()
> -{
> -  return true;
> -}
> -
> -bool
> -MutexImpl::tryacquire( void )
> -{
> -  return true;
> -}
> -
> -}
> Index: lib/Support/Unix/Process.inc
> ===================================================================
> --- lib/Support/Unix/Process.inc
> +++ lib/Support/Unix/Process.inc
> @@ -266,7 +266,7 @@
>  static bool terminalHasColors(int fd) {
>  #ifdef HAVE_TERMINFO
>    // First, acquire a global lock because these C routines are thread hostile.
> -  static sys::Mutex M;
> +  static sys::RecursiveMutex M;
>    MutexGuard G(M);
>
>    int errret = 0;
> Index: lib/Support/Unix/Signals.inc
> ===================================================================
> --- lib/Support/Unix/Signals.inc
> +++ lib/Support/Unix/Signals.inc
> @@ -41,7 +41,7 @@
>
>  static RETSIGTYPE SignalHandler(int Sig);  // defined below.
>
> -static SmartMutex<true> SignalsMutex;
> +static llvm::sys::RecursiveDebugMutex SignalsMutex;
>
>  /// InterruptFunction - The function to call if ctrl-c is pressed.
>  static void (*InterruptFunction)() = nullptr;
> Index: lib/Support/Windows/DynamicLibrary.inc
> ===================================================================
> --- lib/Support/Windows/DynamicLibrary.inc
> +++ lib/Support/Windows/DynamicLibrary.inc
> @@ -71,7 +71,7 @@
>
>  DynamicLibrary DynamicLibrary::getPermanentLibrary(const char *filename,
>                                                     std::string *errMsg) {
> -  SmartScopedLock<true> lock(*SymbolsMutex);
> +  llvm::MutexGuard lock(*SymbolsMutex);

Lock instead of lock.

>
>    if (!filename) {
>      // When no file is specified, enumerate all DLLs and EXEs in the process.
> @@ -121,7 +121,7 @@
>  #undef EXPLICIT_SYMBOL2
>
>  void* DynamicLibrary::SearchForAddressOfSymbol(const char* symbolName) {
> -  SmartScopedLock<true> Lock(*SymbolsMutex);
> +  llvm::MutexGuard lock(*SymbolsMutex);

Lock instead of lock.

>
>    // First check symbols added via AddSymbol().
>    if (ExplicitSymbols.isConstructed()) {
> Index: lib/Support/Windows/Mutex.inc
> ===================================================================
> --- lib/Support/Windows/Mutex.inc
> +++ /dev/null
> @@ -1,58 +0,0 @@
> -//===- llvm/Support/Win32/Mutex.inc - Win32 Mutex Implementation -*- C++ -*-===//
> -//
> -//                     The LLVM Compiler Infrastructure
> -//
> -// This file is distributed under the University of Illinois Open Source
> -// License. See LICENSE.TXT for details.
> -//
> -//===----------------------------------------------------------------------===//
> -//
> -// This file implements the Win32 specific (non-pthread) Mutex class.
> -//
> -//===----------------------------------------------------------------------===//
> -
> -//===----------------------------------------------------------------------===//
> -//=== WARNING: Implementation here must contain only generic Win32 code that
> -//===          is guaranteed to work on *all* Win32 variants.
> -//===----------------------------------------------------------------------===//
> -
> -#include "WindowsSupport.h"
> -#include "llvm/Support/Mutex.h"
> -
> -namespace llvm {
> -using namespace sys;
> -
> -MutexImpl::MutexImpl(bool /*recursive*/)
> -{
> -  data_ = new CRITICAL_SECTION;
> -  InitializeCriticalSection((LPCRITICAL_SECTION)data_);
> -}
> -
> -MutexImpl::~MutexImpl()
> -{
> -  DeleteCriticalSection((LPCRITICAL_SECTION)data_);
> -  delete (LPCRITICAL_SECTION)data_;
> -  data_ = 0;
> -}
> -
> -bool
> -MutexImpl::acquire()
> -{
> -  EnterCriticalSection((LPCRITICAL_SECTION)data_);
> -  return true;
> -}
> -
> -bool
> -MutexImpl::release()
> -{
> -  LeaveCriticalSection((LPCRITICAL_SECTION)data_);
> -  return true;
> -}
> -
> -bool
> -MutexImpl::tryacquire()
> -{
> -  return TryEnterCriticalSection((LPCRITICAL_SECTION)data_);
> -}
> -
> -}
> Index: lib/Target/NVPTX/NVPTXUtilities.cpp
> ===================================================================
> --- lib/Target/NVPTX/NVPTXUtilities.cpp
> +++ lib/Target/NVPTX/NVPTXUtilities.cpp
> @@ -33,7 +33,7 @@
>  typedef std::map<const Module *, global_val_annot_t> per_module_annot_t;
>
>  ManagedStatic<per_module_annot_t> annotationCache;
> -static sys::Mutex Lock;
> +static sys::RecursiveMutex Lock;
>
>  void llvm::clearAnnotationCache(const llvm::Module *Mod) {
>    MutexGuard Guard(Lock);
> Index: unittests/IR/ValueMapTest.cpp
> ===================================================================
> --- unittests/IR/ValueMapTest.cpp
> +++ unittests/IR/ValueMapTest.cpp
> @@ -180,7 +180,7 @@
>  template<typename KeyT>
>  struct LockMutex : ValueMapConfig<KeyT> {
>    struct ExtraData {
> -    sys::Mutex *M;
> +    sys::NonRecursiveMutex *M;
>      bool *CalledRAUW;
>      bool *CalledDeleted;
>    };
> @@ -192,11 +192,11 @@
>      *Data.CalledDeleted = true;
>      EXPECT_FALSE(Data.M->tryacquire()) << "Mutex should already be locked.";
>    }
> -  static sys::Mutex *getMutex(const ExtraData &Data) { return Data.M; }
> +  static sys::NonRecursiveMutex *getMutex(const ExtraData &Data) { return Data.M; }
>  };
>  #if LLVM_ENABLE_THREADS
>  TYPED_TEST(ValueMapTest, LocksMutex) {
> -  sys::Mutex M(false);  // Not recursive.
> +  sys::NonRecursiveMutex M;
>    bool CalledRAUW = false, CalledDeleted = false;
>    typename LockMutex<TypeParam*>::ExtraData Data =
>      {&M, &CalledRAUW, &CalledDeleted};
>

~Aaron

On Thu, Jun 5, 2014 at 5:27 PM, Aaron Ballman <aaron.ballman at gmail.com> wrote:
> Okay, with that patch applied as well, this is passing all clang and
> llvm tests for me on Windows with MSVC.
>
> I'll do an actual review once I have the chance, unless someone else
> beats me to it. :-)
>
> ~Aaron
>
> On Thu, Jun 5, 2014 at 4:57 PM, Zachary Turner <zturner at google.com> wrote:
>> D3961 is updated with the change that should fix this deadlock.  (fingers
>> crossed).   Thanks for your patience!
>>
>>
>> On Thu, Jun 5, 2014 at 1:47 PM, Zachary Turner <zturner at google.com> wrote:
>>>
>>> Gah, my bad.  it looks like I do need to re-upload the clang review.  The
>>> original one is missing the change to ASTUnit to support locking from an
>>> atexit handler.
>>>
>>> I will update the patch soon, and hopefully that resolves it.
>>>
>>>
>>> On Thu, Jun 5, 2014 at 1:04 PM, Aaron Ballman <aaron.ballman at gmail.com>
>>> wrote:
>>>>
>>>> On Thu, Jun 5, 2014 at 3:30 PM, Zachary Turner <zturner at google.com>
>>>> wrote:
>>>> > Also make sure you're clang repo is up-to-date at least to r210225.
>>>> > That fixes the test failures we saw last time.
>>>> >
>>>> > http://reviews.llvm.org/D4033
>>>>
>>>> My repos are at r210280, but the clang tests still fail to complete
>>>> for me (Win 7, MSVC 2013, debug build). I don't know exactly where it
>>>> gets hung up, but my process list shows a whole lot of
>>>> c-index-test.exe which have pegged the CPU. I end tasked a handful of
>>>> them, and the reports came back as:
>>>>
>>>> 55>  -- Testing: 7271 tests, 32 threads --
>>>> 55>  FAIL: Clang :: Index/annotate-tokens-include.c (3206 of 7271)
>>>> 55>  ******************** TEST 'Clang ::
>>>> Index/annotate-tokens-include.c' FAILED ********************
>>>> 55>  Script:
>>>> 55>  --
>>>> 55>  E:/llvm/2013/Debug/bin\c-index-test.EXE
>>>>
>>>> -test-annotate-tokens=E:\llvm\llvm\tools\clang\test\Index\annotate-tokens-include.c:1:1:2:1
>>>> E:\llvm\llvm\tools\clang\test\Index\annotate-tokens-include.c |
>>>> E:/llvm/2013/Debug/bin\FileCheck.EXE
>>>> E:\llvm\llvm\tools\clang\test\Index\annotate-tokens-include.c
>>>> 55>  --
>>>> 55>  Exit Code: 1
>>>> 55>
>>>> 55>  Command Output (stdout):
>>>> 55>  --
>>>> 55>  Command 0: "E:/llvm/2013/Debug/bin\c-index-test.EXE"
>>>>
>>>> "-test-annotate-tokens=E:\llvm\llvm\tools\clang\test\Index\annotate-tokens-include.c:1:1:2:1"
>>>> "E:\llvm\llvm\tools\clang\test\Index\annotate-tokens-include.c"
>>>> 55>  Command 0 Result: 1
>>>> 55>  Command 0 Output:
>>>> 55>
>>>> 55>
>>>> 55>  Command 0 Stderr:
>>>> 55>
>>>> 55>
>>>> 55>  Command 1: "E:/llvm/2013/Debug/bin\FileCheck.EXE"
>>>> "E:\llvm\llvm\tools\clang\test\Index\annotate-tokens-include.c"
>>>> 55>  Command 1 Result: 0
>>>> 55>  Command 1 Output:
>>>> 55>
>>>> 55>
>>>> 55>  Command 1 Stderr:
>>>> 55>
>>>> 55>
>>>> 55>
>>>> 55>  --
>>>> 55>
>>>> 55>  ********************
>>>> 55>  FAIL: Clang :: Index/annotate-attribute.cpp (3207 of 7271)
>>>> 55>  ******************** TEST 'Clang :: Index/annotate-attribute.cpp'
>>>> FAILED ********************
>>>> 55>  Script:
>>>> 55>  --
>>>> 55>  E:/llvm/2013/Debug/bin\c-index-test.EXE -test-load-source all
>>>> E:\llvm\llvm\tools\clang\test\Index\annotate-attribute.cpp |
>>>> E:/llvm/2013/Debug/bin\FileCheck.EXE
>>>> E:\llvm\llvm\tools\clang\test\Index\annotate-attribute.cpp
>>>> 55>  --
>>>> 55>  Exit Code: 1
>>>> 55>
>>>> 55>  Command Output (stdout):
>>>> 55>  --
>>>> 55>  Command 0: "E:/llvm/2013/Debug/bin\c-index-test.EXE"
>>>> "-test-load-source" "all"
>>>> "E:\llvm\llvm\tools\clang\test\Index\annotate-attribute.cpp"
>>>> 55>  Command 0 Result: 1
>>>> 55>  Command 0 Output:
>>>> 55>
>>>> 55>
>>>> 55>  Command 0 Stderr:
>>>> 55>
>>>> 55>
>>>> 55>  Command 1: "E:/llvm/2013/Debug/bin\FileCheck.EXE"
>>>> "E:\llvm\llvm\tools\clang\test\Index\annotate-attribute.cpp"
>>>> 55>  Command 1 Result: 0
>>>> 55>  Command 1 Output:
>>>> 55>
>>>> 55>
>>>> 55>  Command 1 Stderr:
>>>> 55>
>>>> 55>
>>>> 55>
>>>> 55>  --
>>>> 55>
>>>> 55>  ********************
>>>> 55>  FAIL: Clang :: Index/annotate-tokens-pp.c (3208 of 7271)
>>>> 55>  ******************** TEST 'Clang :: Index/annotate-tokens-pp.c'
>>>> FAILED ********************
>>>> 55>  Script:
>>>> 55>  --
>>>> 55>  E:/llvm/2013/Debug/bin\c-index-test.EXE
>>>>
>>>> -test-annotate-tokens=E:\llvm\llvm\tools\clang\test\Index\annotate-tokens-pp.c:2:1:44:1
>>>> -IE:\llvm\llvm\tools\clang\test\Index/Inputs
>>>> E:\llvm\llvm\tools\clang\test\Index\annotate-tokens-pp.c |
>>>> E:/llvm/2013/Debug/bin\FileCheck.EXE
>>>> E:\llvm\llvm\tools\clang\test\Index\annotate-tokens-pp.c
>>>> 55>  env CINDEXTEST_EDITING=1 E:/llvm/2013/Debug/bin\c-index-test.EXE
>>>>
>>>> -test-annotate-tokens=E:\llvm\llvm\tools\clang\test\Index\annotate-tokens-pp.c:2:1:44:1
>>>> -IE:\llvm\llvm\tools\clang\test\Index/Inputs
>>>> E:\llvm\llvm\tools\clang\test\Index\annotate-tokens-pp.c |
>>>> E:/llvm/2013/Debug/bin\FileCheck.EXE
>>>> E:\llvm\llvm\tools\clang\test\Index\annotate-tokens-pp.c
>>>> 55>  --
>>>> 55>  Exit Code: 1
>>>> 55>
>>>> 55>  Command Output (stdout):
>>>> 55>  --
>>>> 55>  Command 0: "E:/llvm/2013/Debug/bin\c-index-test.EXE"
>>>>
>>>> "-test-annotate-tokens=E:\llvm\llvm\tools\clang\test\Index\annotate-tokens-pp.c:2:1:44:1"
>>>> "-IE:\llvm\llvm\tools\clang\test\Index/Inputs"
>>>> "E:\llvm\llvm\tools\clang\test\Index\annotate-tokens-pp.c"
>>>> 55>  Command 0 Result: 1
>>>> 55>  Command 0 Output:
>>>> 55>
>>>> 55>
>>>> 55>  Command 0 Stderr:
>>>> 55>
>>>> 55>
>>>> 55>  Command 1: "E:/llvm/2013/Debug/bin\FileCheck.EXE"
>>>> "E:\llvm\llvm\tools\clang\test\Index\annotate-tokens-pp.c"
>>>> 55>  Command 1 Result: 0
>>>> 55>  Command 1 Output:
>>>> 55>
>>>> 55>
>>>> 55>  Command 1 Stderr:
>>>> 55>
>>>> 55>
>>>> 55>
>>>> 55>  --
>>>> 55>
>>>> 55>  ********************
>>>> 55>  FAIL: Clang :: Index/annotate-comments.cpp (3210 of 7271)
>>>> 55>  ******************** TEST 'Clang :: Index/annotate-comments.cpp'
>>>> FAILED ********************
>>>> 55>  Script:
>>>> 55>  --
>>>> 55>  rm -rf
>>>> E:\llvm\2013\tools\clang\test\Index\Output\annotate-comments.cpp.tmp
>>>> 55>  mkdir
>>>> E:\llvm\2013\tools\clang\test\Index\Output\annotate-comments.cpp.tmp
>>>> 55>  E:/llvm/2013/Debug/bin/clang.EXE -cc1 -internal-isystem
>>>> E:\llvm\2013\Debug\bin\..\lib\clang\3.5.0\include -x c++ -std=c++11
>>>> -emit-pch -o
>>>> E:\llvm\2013\tools\clang\test\Index\Output\annotate-comments.cpp.tmp/out.pch
>>>> E:\llvm\llvm\tools\clang\test\Index\annotate-comments.cpp
>>>> 55>  E:/llvm/2013/Debug/bin/clang.EXE -cc1 -internal-isystem
>>>> E:\llvm\2013\Debug\bin\..\lib\clang\3.5.0\include -x c++ -std=c++11
>>>> -include-pch
>>>> E:\llvm\2013\tools\clang\test\Index\Output\annotate-comments.cpp.tmp/out.pch
>>>> -fsyntax-only E:\llvm\llvm\tools\clang\test\Index\annotate-comments.cpp
>>>> 55>  E:/llvm/2013/Debug/bin\c-index-test.EXE -test-load-source all
>>>>
>>>> -comments-xml-schema=E:\llvm\llvm\tools\clang\test\Index/../../bindings/xml/comment-xml-schema.rng
>>>> E:\llvm\llvm\tools\clang\test\Index\annotate-comments.cpp -std=c++11 >
>>>>
>>>> E:\llvm\2013\tools\clang\test\Index\Output\annotate-comments.cpp.tmp/out.c-index-direct
>>>> 55>  E:/llvm/2013/Debug/bin\c-index-test.EXE -test-load-tu
>>>>
>>>> E:\llvm\2013\tools\clang\test\Index\Output\annotate-comments.cpp.tmp/out.pch
>>>> all >
>>>> E:\llvm\2013\tools\clang\test\Index\Output\annotate-comments.cpp.tmp/out.c-index-pch
>>>> 55>  E:/llvm/2013/Debug/bin\FileCheck.EXE
>>>> E:\llvm\llvm\tools\clang\test\Index\annotate-comments.cpp
>>>> -check-prefix=WRONG <
>>>>
>>>> E:\llvm\2013\tools\clang\test\Index\Output\annotate-comments.cpp.tmp/out.c-index-direct
>>>> 55>  E:/llvm/2013/Debug/bin\FileCheck.EXE
>>>> E:\llvm\llvm\tools\clang\test\Index\annotate-comments.cpp
>>>> -check-prefix=WRONG <
>>>>
>>>> E:\llvm\2013\tools\clang\test\Index\Output\annotate-comments.cpp.tmp/out.c-index-pch
>>>> 55>  E:/llvm/2013/Debug/bin\FileCheck.EXE
>>>> E:\llvm\llvm\tools\clang\test\Index\annotate-comments.cpp <
>>>>
>>>> E:\llvm\2013\tools\clang\test\Index\Output\annotate-comments.cpp.tmp/out.c-index-direct
>>>> 55>  E:/llvm/2013/Debug/bin\FileCheck.EXE
>>>> E:\llvm\llvm\tools\clang\test\Index\annotate-comments.cpp <
>>>>
>>>> E:\llvm\2013\tools\clang\test\Index\Output\annotate-comments.cpp.tmp/out.c-index-pch
>>>> 55>  --
>>>> 55>  Exit Code: 1
>>>> 55>
>>>> 55>  Command Output (stdout):
>>>> 55>  --
>>>> 55>  Command 0: "rm" "-rf"
>>>> "E:\llvm\2013\tools\clang\test\Index\Output\annotate-comments.cpp.tmp"
>>>> 55>  Command 0 Result: 0
>>>> 55>  Command 0 Output:
>>>> 55>
>>>> 55>
>>>> 55>  Command 0 Stderr:
>>>> 55>
>>>> 55>
>>>> 55>  Command 1: "mkdir"
>>>> "E:\llvm\2013\tools\clang\test\Index\Output\annotate-comments.cpp.tmp"
>>>> 55>  Command 1 Result: 0
>>>> 55>  Command 1 Output:
>>>> 55>
>>>> 55>
>>>> 55>  Command 1 Stderr:
>>>> 55>
>>>> 55>
>>>> 55>  Command 2: "E:/llvm/2013/Debug/bin/clang.EXE" "-cc1"
>>>> "-internal-isystem"
>>>> "E:\llvm\2013\Debug\bin\..\lib\clang\3.5.0\include" "-x" "c++"
>>>> "-std=c++11" "-emit-pch" "-o"
>>>>
>>>> "E:\llvm\2013\tools\clang\test\Index\Output\annotate-comments.cpp.tmp/out.pch"
>>>> "E:\llvm\llvm\tools\clang\test\Index\annotate-comments.cpp"
>>>> 55>  Command 2 Result: 0
>>>> 55>  Command 2 Output:
>>>> 55>
>>>> 55>
>>>> 55>  Command 2 Stderr:
>>>> 55>
>>>> 55>
>>>> 55>  Command 3: "E:/llvm/2013/Debug/bin/clang.EXE" "-cc1"
>>>> "-internal-isystem"
>>>> "E:\llvm\2013\Debug\bin\..\lib\clang\3.5.0\include" "-x" "c++"
>>>> "-std=c++11" "-include-pch"
>>>>
>>>> "E:\llvm\2013\tools\clang\test\Index\Output\annotate-comments.cpp.tmp/out.pch"
>>>> "-fsyntax-only"
>>>> "E:\llvm\llvm\tools\clang\test\Index\annotate-comments.cpp"
>>>> 55>  Command 3 Result: 0
>>>> 55>  Command 3 Output:
>>>> 55>
>>>> 55>
>>>> 55>  Command 3 Stderr:
>>>> 55>
>>>> 55>
>>>> 55>  Command 4: "E:/llvm/2013/Debug/bin\c-index-test.EXE"
>>>> "-test-load-source" "all"
>>>>
>>>> "-comments-xml-schema=E:\llvm\llvm\tools\clang\test\Index/../../bindings/xml/comment-xml-schema.rng"
>>>> "E:\llvm\llvm\tools\clang\test\Index\annotate-comments.cpp"
>>>> "-std=c++11"
>>>> 55>  Command 4 Result: 1
>>>> 55>  Command 4 Output:
>>>> 55>  None
>>>> 55>
>>>> 55>  Command 4 Stderr:
>>>> 55>
>>>> 55>
>>>> 55>
>>>> 55>  --
>>>> 55>
>>>> 55>  ********************
>>>>
>>>> So I'm still not certain this is fully resolved yet.
>>>>
>>>> ~Aaron
>>>
>>>
>>



More information about the llvm-commits mailing list