[llvm/Support] Make llvm::sys::RWMutex Lockable

[medismailben]

This patch extends the llvm::sys::RWMutex class to fullfill the Lockable requirement to include attempted locking, by implementing a bool try_lock member function.

As the name suggests, this method will try to acquire to lock in a non-blocking fashion and release it immediately. If it managed to acquire the lock, returns true, otherwise returns false.

[llvmbot]

@llvm/pr-subscribers-llvm-support

Author: Med Ismail Bennani (medismailben)

Changes

This patch extends the llvm::sys::RWMutex class to fullfill the Lockable requirement to include attempted locking, by implementing a bool try_lock member function.

As the name suggests, this method will try to acquire to lock in a non-blocking fashion and release it immediately. If it managed to acquire the lock, returns true, otherwise returns false.

---

Full diff: https://github.com/llvm/llvm-project/pull/90667.diff

2 Files Affected:

• (modified) llvm/include/llvm/Support/RWMutex.h (+6)
• (modified) llvm/lib/Support/RWMutex.cpp (+13)

diff --git a/llvm/include/llvm/Support/RWMutex.h b/llvm/include/llvm/Support/RWMutex.h
index 32987c3b98f1cb..ac9c542625bd79 100644
--- a/llvm/include/llvm/Support/RWMutex.h
+++ b/llvm/include/llvm/Support/RWMutex.h
@@ -75,6 +75,10 @@ class RWMutexImpl {
   /// Unconditionally release the lock in write mode.
   bool unlock();
 
+  /// Attempts to acquire the lock. Returns immediately.
+  /// @returns true on successful lock acquisition, false otherwise.
+  bool try_lock();
+
   //@}
   /// @name Platform Dependent Data
   /// @{
@@ -148,6 +152,8 @@ template <bool mt_only> class SmartRWMutex {
     --writers;
     return true;
   }
+
+  bool try_lock() { return impl.try_lock(); }
 };
 
 typedef SmartRWMutex<false> RWMutex;
diff --git a/llvm/lib/Support/RWMutex.cpp b/llvm/lib/Support/RWMutex.cpp
index 5accf73e5f9404..d93c7a22271c57 100644
--- a/llvm/lib/Support/RWMutex.cpp
+++ b/llvm/lib/Support/RWMutex.cpp
@@ -28,6 +28,7 @@ bool RWMutexImpl::lock_shared() { return true; }
 bool RWMutexImpl::unlock_shared() { return true; }
 bool RWMutexImpl::lock() { return true; }
 bool RWMutexImpl::unlock() { return true; }
+bool RWMutexImpl::try_lock() { return true; }
 
 #else
 
@@ -107,6 +108,14 @@ RWMutexImpl::unlock()
   return errorcode == 0;
 }
 
+bool RWMutexImpl::try_lock() {
+  pthread_rwlock_t *rwlock = static_cast<pthread_rwlock_t *>(data_);
+  assert(rwlock != nullptr);
+
+  int errorcode = pthread_rwlock_tryrdlock(rwlock);
+  return errorcode == 0;
+}
+
 #else
 
 RWMutexImpl::RWMutexImpl() : data_(new MutexImpl(false)) { }
@@ -131,6 +140,10 @@ bool RWMutexImpl::unlock() {
   return static_cast<MutexImpl *>(data_)->release();
 }
 
+bool RWMutexImpl::try_lock() {
+  return static_cast<MutexImpl *>(data_)->tryacquire();
+}
+
 #endif
 #endif
 #endif

[bulbazord]

LGTM

[JDevlieghere]

It looks like this is trying to implement something like is_locked. try_lock has fairly well defined semantics for std::mutex and pthreads. In both cases this acquires the lock if it succeeds. Our implementation shouldn't behave differently.

[JDevlieghere]

This LGTM!

[dwblaikie]

as an aside - per the instructions here ( https://llvm.org/docs/GitHub.html#updating-pull-requests ) please don't force-push to pull request reviews, as it makes it difficult to trace back comments and discussion through the revisions of a patch (I think, especially, inline comments can get lost/hidden/hard to find when force pushing)

[medismailben]

as an aside - per the instructions here ( https://llvm.org/docs/GitHub.html#updating-pull-requests ) please don't force-push to pull request reviews, as it makes it difficult to trace back comments and discussion through the revisions of a patch (I think, especially, inline comments can get lost/hidden/hard to find when force pushing)

Sure! I'll keep that in mind next time.