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

[David Blaikie]

On Tue, Jun 17, 2014 at 9:14 AM, Zachary Turner <zturner at google.com> wrote:
> 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.

OK - I'll keep an eye out for the follow up patches to get an idea of
how/why/where different users specify different kinds of mutexes.
Thanks for the details

- David

>
>
> 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
>
>