[Mlir-commits] [mlir] Fix data race in PyOperation (PR #130612)

[llvmlistbot at llvm.org]

github-actions[bot] wrote:

<!--LLVM CODE FORMAT COMMENT: {clang-format}-->


:warning: C/C++ code formatter, clang-format found issues in your code. :warning:

<details>
<summary>
You can test this locally with the following command:
</summary>

``````````bash
git-clang-format --diff aace6a2f9d8bffd84a225ef76633421ff541a5d0 b31cf08684c74756eb9896fc66d2e2a357e57dec --extensions h,cpp -- mlir/lib/Bindings/Python/IRCore.cpp mlir/lib/Bindings/Python/IRModule.h
``````````

</details>

<details>
<summary>
View the diff from clang-format here.
</summary>

``````````diff
diff --git a/mlir/lib/Bindings/Python/IRCore.cpp b/mlir/lib/Bindings/Python/IRCore.cpp
index 517f13df0f..beadab7872 100644
--- a/mlir/lib/Bindings/Python/IRCore.cpp
+++ b/mlir/lib/Bindings/Python/IRCore.cpp
@@ -754,14 +754,14 @@ void PyMlirContext::clearOperationsInside(MlirOperation op) {
 }
 
 void _clearOperationAndInsideHelper(
-  PyOperation &op, MlirOperationWalkCallback invalidatingCallback
-) {
-  mlirOperationWalk(op, invalidatingCallback, &op.getContext(), MlirWalkPreOrder);
+    PyOperation &op, MlirOperationWalkCallback invalidatingCallback) {
+  mlirOperationWalk(op, invalidatingCallback, &op.getContext(),
+                    MlirWalkPreOrder);
 }
 
 void PyMlirContext::_clearOperationAndInsideLocked(PyOperationBase &op) {
   MlirOperationWalkCallback invalidatingCallbackLocked = [](MlirOperation op,
-                                                      void *userData) {
+                                                            void *userData) {
     PyMlirContextRef &contextRef = *static_cast<PyMlirContextRef *>(userData);
     contextRef->_clearOperationLocked(op);
     return MlirWalkResult::MlirWalkResultAdvance;
@@ -1205,8 +1205,8 @@ PyOperation::PyOperation(PyMlirContextRef contextRef, MlirOperation operation)
     : BaseContextObject(std::move(contextRef)), operation(operation) {}
 
 PyOperation::~PyOperation() {
-  // This lock helps to serialize the access to ~PyOperation and PyOperation::forOperation
-  // when we should invalidate existing PyOperation
+  // This lock helps to serialize the access to ~PyOperation and
+  // PyOperation::forOperation when we should invalidate existing PyOperation
   nb::ft_lock_guard lock(getContext()->liveOperationsMutex);
   {
     nb::ft_lock_guard lock2(opMutex);
@@ -1256,41 +1256,41 @@ PyOperationRef PyOperation::createInstance(PyMlirContextRef contextRef,
   return unownedOperation;
 }
 
-
 bool _nb_try_inc_ref(PyObject *obj) {
-    // See https://github.com/python/cpython/blob/d05140f9f77d7dfc753dd1e5ac3a5962aaa03eff/Include/internal/pycore_object.h#L761
-    uint32_t local = _Py_atomic_load_uint32_relaxed(&obj->ob_ref_local);
-    local += 1;
-    if (local == 0) {
-        // immortal
-        return true;
-    }
-    if (_Py_IsOwnedByCurrentThread(obj)) {
-        _Py_atomic_store_uint32_relaxed(&obj->ob_ref_local, local);
+  // See
+  // https://github.com/python/cpython/blob/d05140f9f77d7dfc753dd1e5ac3a5962aaa03eff/Include/internal/pycore_object.h#L761
+  uint32_t local = _Py_atomic_load_uint32_relaxed(&obj->ob_ref_local);
+  local += 1;
+  if (local == 0) {
+    // immortal
+    return true;
+  }
+  if (_Py_IsOwnedByCurrentThread(obj)) {
+    _Py_atomic_store_uint32_relaxed(&obj->ob_ref_local, local);
 #ifdef Py_REF_DEBUG
-        _Py_INCREF_IncRefTotal();
+    _Py_INCREF_IncRefTotal();
 #endif
-        return true;
+    return true;
+  }
+  Py_ssize_t shared = _Py_atomic_load_ssize_relaxed(&obj->ob_ref_shared);
+  for (;;) {
+    // If the shared refcount is zero and the object is either merged
+    // or may not have weak references, then we cannot incref it.
+    if (shared == 0 || shared == _Py_REF_MERGED) {
+      return false;
     }
-    Py_ssize_t shared = _Py_atomic_load_ssize_relaxed(&obj->ob_ref_shared);
-    for (;;) {
-        // If the shared refcount is zero and the object is either merged
-        // or may not have weak references, then we cannot incref it.
-        if (shared == 0 || shared == _Py_REF_MERGED) {
-            return false;
-        }
 
-        if (_Py_atomic_compare_exchange_ssize(
-                &obj->ob_ref_shared, &shared, shared + (1 << _Py_REF_SHARED_SHIFT))) {
+    if (_Py_atomic_compare_exchange_ssize(&obj->ob_ref_shared, &shared,
+                                          shared +
+                                              (1 << _Py_REF_SHARED_SHIFT))) {
 #ifdef Py_REF_DEBUG
-            _Py_INCREF_IncRefTotal();
+      _Py_INCREF_IncRefTotal();
 #endif
-            return true;
-        }
+      return true;
     }
+  }
 }
 
-
 PyOperationRef PyOperation::forOperation(PyMlirContextRef contextRef,
                                          MlirOperation operation,
                                          nb::object parentKeepAlive) {
@@ -1312,8 +1312,8 @@ PyOperationRef PyOperation::forOperation(PyMlirContextRef contextRef,
   // Check whether pyRef is ongoing to be destroyed such that refcount increment
   // wont keep it from deletion.
   // If after incrementing the reference count its value is 1,
-  // it means that python object is under removal and ~PyOperation should be called.
-  // Thus, we should create new PyOperationRef.
+  // it means that python object is under removal and ~PyOperation should be
+  // called. Thus, we should create new PyOperationRef.
   if (_nb_try_inc_ref(pyRef.ptr())) {
     return PyOperationRef(existing, std::move(pyRef));
   }
@@ -1328,7 +1328,7 @@ PyOperationRef PyOperation::forOperation(PyMlirContextRef contextRef,
 
   // Create.
   PyOperationRef result = createInstance(std::move(contextRef), operation,
-                                          std::move(parentKeepAlive));
+                                         std::move(parentKeepAlive));
   liveOperations[operation.ptr] =
       std::make_pair(result.getObject(), result.get());
   return result;

``````````

</details>


https://github.com/llvm/llvm-project/pull/130612