[SelectionDAG] use <mutex> instead of LLVM home-grown primitive

[davide]

Summary

I hope to get rid of SmartMutex et similia in the short to medium term, now that C++ has a decent infrastructure for locking.
Some comments (on the patch):
!) SmartMutex by default allows recursion but I'm not entirely sure we need that here? Anyway, to keep the semantic equivalent to the existing code I switched to std::recursive_mutex.

2. I'm not entirely sure if we want to guard the mutex code with #if defined(LLVM_ENABLE_THREADS) to make it a no-op in the single threaded case. I wasn't able to measure a substantial impact when benchmarking, but, that goes without saying, your mileage may vary and, of course, feedback welcome.

[davide]

ping

[• rafael]

Gosh, this code is horrible.

It uses a global for a set of structs that include a pointer to a LLVM type. The type itself is not global, it is part of the LLVMContext.

So while this change is probably fine, how much harder would it be to avoid the lock completely by moving the guarded globals to LLVMContext? You might need to use a "void *CodeGenData" pointer to work around the library layering.

[rnk]

It's actually important for performance to compile out this mutex when threading is disabled. We do this in Chromium and apparently it matters.

If you implement Rafael's suggestion you can avoid the lock entirely.

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalAlias.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MathExtras.h"
-#include "llvm/Support/Mutex.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetInstrInfo.h"
 #include "llvm/Target/TargetIntrinsicInfo.h"
 #include "llvm/Target/TargetLowering.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Target/TargetRegisterInfo.h"
 #include "llvm/Target/TargetSubtargetInfo.h"
 #include <algorithm>
 #include <cmath>
+#include <mutex>
 #include <utility>
 
 using namespace llvm;
 
 /// makeVTList - Return an instance of the SDVTList struct initialized with the
 /// specified members.
 static SDVTList makeVTList(const EVT *VTs, unsigned NumVTs) {
   SDVTList Res = {VTs, NumVTs};

       for (unsigned i = 0; i < MVT::LAST_VALUETYPE; ++i)
         VTs.push_back(MVT((MVT::SimpleValueType)i));
     }
   };
 }
 
 static ManagedStatic<std::set<EVT, EVT::compareRawBits> > EVTs;
 static ManagedStatic<EVTArray> SimpleVTArray;
-static ManagedStatic<sys::SmartMutex<true> > VTMutex;
+static ManagedStatic<std::recursive_mutex> VTMutex;
 
 /// getValueTypeList - Return a pointer to the specified value type.
 ///
 const EVT *SDNode::getValueTypeList(EVT VT) {
   if (VT.isExtended()) {
-    sys::SmartScopedLock<true> Lock(*VTMutex);
+    std::lock_guard<std::recursive_mutex> Lock(*VTMutex);
     return &(*EVTs->insert(VT).first);
   } else {
     assert(VT.getSimpleVT() < MVT::LAST_VALUETYPE &&
            "Value type out of range!");
     return &SimpleVTArray->VTs[VT.getSimpleVT().SimpleTy];
   }
 }