[all-commits] [llvm/llvm-project] bdc3e6: [MLIR][python bindings] invalidate ops after PassM...

[Maksim Levental via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: bdc3e6cb45203ba59e6654da2cb7212ef3a15854
      https://github.com/llvm/llvm-project/commit/bdc3e6cb45203ba59e6654da2cb7212ef3a15854
  Author: Maksim Levental <maksim.levental at gmail.com>
  Date:   2023-10-20 (Fri, 20 Oct 2023)

  Changed paths:
    M mlir/include/mlir-c/IR.h
    M mlir/lib/Bindings/Python/IRCore.cpp
    M mlir/lib/Bindings/Python/IRModule.h
    M mlir/lib/Bindings/Python/Pass.cpp
    M mlir/lib/CAPI/IR/IR.cpp
    M mlir/test/CAPI/ir.c
    M mlir/test/python/pass_manager.py

  Log Message:
  -----------
  [MLIR][python bindings] invalidate ops after PassManager run (#69746)

Fixes https://github.com/llvm/llvm-project/issues/69730 (also see
https://reviews.llvm.org/D155543).

There are  two things outstanding (why I didn't land before):

1. add some C API tests for `mlirOperationWalk`;
2. potentially refactor how the invalidation in `run` works; the first
version of the code looked like this:
    ```cpp
    if (invalidateOps) {
      auto *context = op.getOperation().getContext().get();
      MlirOperationWalkCallback invalidatingCallback =
          [](MlirOperation op, void *userData) {
            PyMlirContext *context =
                static_cast<PyMlirContext *>(userData);
            context->setOperationInvalid(op);
          };
      auto numRegions =
          mlirOperationGetNumRegions(op.getOperation().get());
      for (int i = 0; i < numRegions; ++i) {
        MlirRegion region =
            mlirOperationGetRegion(op.getOperation().get(), i);
        for (MlirBlock block = mlirRegionGetFirstBlock(region);
             !mlirBlockIsNull(block);
             block = mlirBlockGetNextInRegion(block))
          for (MlirOperation childOp =
                   mlirBlockGetFirstOperation(block);
               !mlirOperationIsNull(childOp);
               childOp = mlirOperationGetNextInBlock(childOp))
            mlirOperationWalk(childOp, invalidatingCallback, context,
                              MlirWalkPostOrder);
      }
    }
    ```
This is verbose and ugly but it has the important benefit of not
executing `mlirOperationEqual(rootOp->get(), op)` for every op
underneath the root op.

Supposing there's no desire for the slightly more efficient but highly
convoluted approach, I can land this "posthaste".
But, since we have eyes on this now, any suggestions or approaches (or
needs/concerns) are welcome.