Remove misused RAII gil_scoped_release/gil_scoped_acquire: without name they don't have any effect

[mehdi_amini]

Summary

I'm not sure what is the right fix here, but adding a name to all these
lead to many segfaults.

[mehdi_amini]

Same in IRCore.cpp

[stellaraccident]

Good catch - and sorry: I am not sure what I was thinking with those. As-is, we hold the GIL all the time, so they are fine to remove. I think I was attempting to make it future proof for cases where we may want to release the GIL for long running operations (i.e. transforms, etc). These cases represent re-entrancy where it would need to be managed carefully. Clearly wrong as-is, though so fine to remove.

mlir/lib/Bindings/Python/IRCore.cpp

void PyOperationBase::print(py::object fileObject, bool binary,
   if (largeElementsLimit)
     mlirOpPrintingFlagsElideLargeElementsAttrs(flags, *largeElementsLimit);
   if (enableDebugInfo)
     mlirOpPrintingFlagsEnableDebugInfo(flags, /*prettyForm=*/prettyDebugInfo);
   if (printGenericOpForm)
     mlirOpPrintingFlagsPrintGenericOpForm(flags);
 
   PyFileAccumulator accum(fileObject, binary);
-  py::gil_scoped_release();
   mlirOperationPrintWithFlags(operation, flags, accum.getCallback(),
                               accum.getUserData());
   mlirOpPrintingFlagsDestroy(flags);
 }
 
 py::object PyOperationBase::getAsm(bool binary,
                                    llvm::Optional<int64_t> largeElementsLimit,
                                    bool enableDebugInfo, bool prettyDebugInfo,

mlir/lib/Bindings/Python/IRModule.cpp

PyGlobals::PyGlobals() {
   // The default search path include {mlir.}dialects, where {mlir.} is the
   // package prefix configured at compile time.
   dialectSearchPrefixes.emplace_back(MAKE_MLIR_PYTHON_QUALNAME("dialects"));
 }
 
 PyGlobals::~PyGlobals() { instance = nullptr; }
 
 void PyGlobals::loadDialectModule(llvm::StringRef dialectNamespace) {
-  py::gil_scoped_acquire();
   if (loadedDialectModulesCache.contains(dialectNamespace))
     return;
   // Since re-entrancy is possible, make a copy of the search prefixes.
   std::vector<std::string> localSearchPrefixes = dialectSearchPrefixes;
   py::object loaded;
   for (std::string moduleName : localSearchPrefixes) {
     moduleName.push_back('.');
     moduleName.append(dialectNamespace.data(), dialectNamespace.size());
 
     try {
-      py::gil_scoped_release();
       loaded = py::module::import(moduleName.c_str());
     } catch (py::error_already_set &e) {
       if (e.matches(PyExc_ModuleNotFoundError)) {
         continue;
       }
       throw;
     }
     break;
   }
 
   // Note: Iterator cannot be shared from prior to loading, since re-entrancy
   // may have occurred, which may do anything.
   loadedDialectModulesCache.insert(dialectNamespace);
 }
 
 void PyGlobals::registerDialectImpl(const std::string &dialectNamespace,
                                     py::object pyClass) {
-  py::gil_scoped_acquire();
   py::object &found = dialectClassMap[dialectNamespace];
   if (found) {
     throw SetPyError(PyExc_RuntimeError, llvm::Twine("Dialect namespace '") +
                                              dialectNamespace +
                                              "' is already registered.");
   }
   found = std::move(pyClass);
 }
 
 void PyGlobals::registerOperationImpl(const std::string &operationName,
                                       py::object pyClass,
                                       py::object rawOpViewClass) {
-  py::gil_scoped_acquire();
   py::object &found = operationClassMap[operationName];
   if (found) {
     throw SetPyError(PyExc_RuntimeError, llvm::Twine("Operation '") +
                                              operationName +
                                              "' is already registered.");
   }
   found = std::move(pyClass);
   rawOpViewClassMap[operationName] = std::move(rawOpViewClass);
 }
 
 llvm::Optional<py::object>
 PyGlobals::lookupDialectClass(const std::string &dialectNamespace) {
-  py::gil_scoped_acquire();
   loadDialectModule(dialectNamespace);
   // Fast match against the class map first (common case).
   const auto foundIt = dialectClassMap.find(dialectNamespace);
   if (foundIt != dialectClassMap.end()) {
     if (foundIt->second.is_none())
       return llvm::None;
     assert(foundIt->second && "py::object is defined");
     return foundIt->second;
   }
 
   // Not found and loading did not yield a registration. Negative cache.
   dialectClassMap[dialectNamespace] = py::none();
   return llvm::None;
 }
 
 llvm::Optional<pybind11::object>
 PyGlobals::lookupRawOpViewClass(llvm::StringRef operationName) {
   {
-    py::gil_scoped_acquire();
     auto foundIt = rawOpViewClassMapCache.find(operationName);
     if (foundIt != rawOpViewClassMapCache.end()) {
       if (foundIt->second.is_none())
         return llvm::None;
       assert(foundIt->second && "py::object is defined");
       return foundIt->second;
     }
   }
 
   // Not found. Load the dialect namespace.
   auto split = operationName.split('.');
   llvm::StringRef dialectNamespace = split.first;
   loadDialectModule(dialectNamespace);
 
   // Attempt to find from the canonical map and cache.
   {
-    py::gil_scoped_acquire();
     auto foundIt = rawOpViewClassMap.find(operationName);
     if (foundIt != rawOpViewClassMap.end()) {
       if (foundIt->second.is_none())
         return llvm::None;
       assert(foundIt->second && "py::object is defined");
       // Positive cache.
       rawOpViewClassMapCache[operationName] = foundIt->second;
       return foundIt->second;
     }
     // Negative cache.
     rawOpViewClassMap[operationName] = py::none();
     return llvm::None;
   }
 }
 
 void PyGlobals::clearImportCache() {
-  py::gil_scoped_acquire();
   loadedDialectModulesCache.clear();
   rawOpViewClassMapCache.clear();
 }