[Mlir-commits] [mlir] [mlir][Transforms][NFC] Remove `InlineBlockRewrite` (PR #83262)

[Matthias Springer]

================
@@ -1649,10 +1598,18 @@ void ConversionPatternRewriter::inlineBlockBefore(Block *source, Block *dest,
          "expected 'source' to have no predecessors");
 #endif // NDEBUG
 
-  impl->notifyBlockBeingInlined(dest, source, before);
+  // Replace all uses of block arguments.
+  // TODO: Support `replaceAllUsesWith` in the dialect conversion. Then this
+  // function no longer has to be overridden and can be turned into a
+  // non-virtual function.
   for (auto it : llvm::zip(source->getArguments(), argValues))
     replaceUsesOfBlockArgument(std::get<0>(it), std::get<1>(it));
-  dest->getOperations().splice(before, source->getOperations());
+
+  // Move op by op.
----------------
matthias-springer wrote:

If you just think about listener support, yes. I could check whether a listener is attached or not. If not, move ops with a `splice`, otherwise one-by-one. I can start a CI run with IREE, to see if there is any performance implication of moving ops at one-by-one vs. in bulk. They run some pretty large models that take multiple seconds to compile.

There was another change that I was planning in the near future: supporting `RewriterBase::replaceAllUsesWith`. Then we would not need any override for `inlineBlockBefore` in `ConversionPatternRewriter` and we could make `inlineBlockBefore` non-virtual. Imo, the fewer virtual functions we have in `RewriterBase`, the better. (From an API design perspective. It won't improve performance.) That refactoring would only be possible if we move ops one-by-one; that's what the `RewriterBase::inlineBlockBefore` implementation does if a listener is attached. (A `ConversionPatternRewriter` *always* has a listener attached. (I mean the rewriter, not the dialect conversion.)


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