[llvm] r211080 - Expose ValueMap's mutex type as a typedef instead of a sys::Mutex.

Zachary Turner zturner at google.com
Tue Jun 17 09:14:30 PDT 2014


This is so I can change smaller amounts of code incrementally, but with
sys::Mutex, whether or not the mutex is recursive is a runtime parameter to
the constructor, and with the std mutexes, recursive and non-recursive
mutexes are entirely different types.  It turns out that ValueMap is being
used with both recursive and non-recursive mutexes, so the only way to
support this is to templatize the mutex type.


On Mon, Jun 16, 2014 at 10:38 PM, David Blaikie <dblaikie at gmail.com> wrote:

> I'm assuming this is so you can change smaller amounts of code/work
> incrementally, and at the end everything will be std::murex and this new
> template parameter can be removed?
> On Jun 16, 2014 5:27 PM, "Zachary Turner" <zturner at google.com> wrote:
>
>> Author: zturner
>> Date: Mon Jun 16 19:17:38 2014
>> New Revision: 211080
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=211080&view=rev
>> Log:
>> Expose ValueMap's mutex type as a typedef instead of a sys::Mutex.
>>
>> This enables static polymorphism of the mutex type, which is
>> necessary in order to replace the standard mutex implementation
>> with a different type.
>>
>> Modified:
>>     llvm/trunk/include/llvm/IR/ValueMap.h
>>     llvm/trunk/unittests/IR/ValueMapTest.cpp
>>
>> Modified: llvm/trunk/include/llvm/IR/ValueMap.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/ValueMap.h?rev=211080&r1=211079&r2=211080&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/IR/ValueMap.h (original)
>> +++ llvm/trunk/include/llvm/IR/ValueMap.h Mon Jun 16 19:17:38 2014
>> @@ -45,8 +45,10 @@ class ValueMapConstIterator;
>>  /// This class defines the default behavior for configurable aspects of
>>  /// ValueMap<>.  User Configs should inherit from this class to be as
>> compatible
>>  /// as possible with future versions of ValueMap.
>> -template<typename KeyT>
>> +template<typename KeyT, typename MutexT = sys::Mutex>
>>  struct ValueMapConfig {
>> +  typedef MutexT mutex_type;
>> +
>>    /// If FollowRAUW is true, the ValueMap will update mappings on RAUW.
>> If it's
>>    /// false, the ValueMap will leave the original mapping in place.
>>    enum { FollowRAUW = true };
>> @@ -67,7 +69,7 @@ struct ValueMapConfig {
>>    /// 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 mutex_type *getMutex(const ExtraDataT &/*Data*/) { return
>> nullptr; }
>>  };
>>
>>  /// See the file comment.
>> @@ -212,7 +214,7 @@ public:
>>    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);
>> +    typename Config::mutex_type *M = Config::getMutex(Copy.Map->Data);
>>      if (M)
>>        M->acquire();
>>      Config::onDelete(Copy.Map->Data, Copy.Unwrap());  // May destroy
>> *this.
>> @@ -225,7 +227,7 @@ public:
>>             "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);
>> +    typename Config::mutex_type *M = Config::getMutex(Copy.Map->Data);
>>      if (M)
>>        M->acquire();
>>
>>
>> Modified: llvm/trunk/unittests/IR/ValueMapTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/ValueMapTest.cpp?rev=211080&r1=211079&r2=211080&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/IR/ValueMapTest.cpp (original)
>> +++ llvm/trunk/unittests/IR/ValueMapTest.cpp Mon Jun 16 19:17:38 2014
>> @@ -177,10 +177,10 @@ TYPED_TEST(ValueMapTest, ConfiguredColli
>>    // TODO: Implement this when someone needs it.
>>  }
>>
>> -template<typename KeyT>
>> -struct LockMutex : ValueMapConfig<KeyT> {
>> +template<typename KeyT, typename MutexT>
>> +struct LockMutex : ValueMapConfig<KeyT, MutexT> {
>>    struct ExtraData {
>> -    sys::Mutex *M;
>> +    mutex_type *M;
>>      bool *CalledRAUW;
>>      bool *CalledDeleted;
>>    };
>> @@ -192,15 +192,15 @@ struct LockMutex : ValueMapConfig<KeyT>
>>      *Data.CalledDeleted = true;
>>      EXPECT_FALSE(Data.M->tryacquire()) << "Mutex should already be
>> locked.";
>>    }
>> -  static sys::Mutex *getMutex(const ExtraData &Data) { return Data.M; }
>> +  static mutex_type *getMutex(const ExtraData &Data) { return Data.M; }
>>  };
>>  #if LLVM_ENABLE_THREADS
>>  TYPED_TEST(ValueMapTest, LocksMutex) {
>>    sys::Mutex M(false);  // Not recursive.
>>    bool CalledRAUW = false, CalledDeleted = false;
>> -  typename LockMutex<TypeParam*>::ExtraData Data =
>> -    {&M, &CalledRAUW, &CalledDeleted};
>> -  ValueMap<TypeParam*, int, LockMutex<TypeParam*> > VM(Data);
>> +  typedef LockMutex<TypeParam*, sys::Mutex> ConfigType;
>> +  typename ConfigType::ExtraData Data = {&M, &CalledRAUW,
>> &CalledDeleted};
>> +  ValueMap<TypeParam*, int, ConfigType> VM(Data);
>>    VM[this->BitcastV.get()] = 7;
>>    this->BitcastV->replaceAllUsesWith(this->AddV.get());
>>    this->AddV.reset();
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140617/dea34b22/attachment.html>


More information about the llvm-commits mailing list