[mlir][bufferization] Modify setting illegal from dialect to a CloneOp

[sotoshigoto]

Summary

Fix an error that prevented bufferization operations other than CloneOp
from being legalized. The BufferizationToMemRefPass was only
transforming CloneOp, but the illegally registered BufferizationDialect
was causing an error when attempting to legalize other bufferization
operations. This patch addresses the issue by properly registering the
BufferizationDialect.

fixes https://github.com/llvm/llvm-project/issues/60104

[springerm]

I think this is not the right way to fix the issue. As far as I know, -convert-x-to-y passes usually remove all occurrences of ops from dialect x. So it would be surprising if we leave some of the ops around. Looking at #60104, the issue was remaining to_memref/to_tensor ops. We have other pass to lower/fold these, e.g., -finalizing-bufferize. -bufferization-to-memref can be run afterwards.

[sotoshigoto]

Thanks to this comment and I read a your comment on this issue. So I think that the name of this pass, convert-bufferization-to-memref, may be misleading. This pass requires that it runs after other bufferization passes, so perhaps it would be more appropriate to rename it or move it to -finalizing-bufferize.
Alternatively, I'm wondering if this conversion pass will eventually be removed, since it is recommended to use One-shot Bufferize instead of Conversion-based Bufferization. I am still trying to understand the details and context of Bufferization. If this PR is not needed, I will close it.

[springerm]

In D144446#4143371, @sotoshigoto wrote:

Thanks to this comment and I read a your comment on this issue. So I think that the name of this pass, convert-bufferization-to-memref, may be misleading. This pass requires that it runs after other bufferization passes, so perhaps it would be more appropriate to rename it or move it to -finalizing-bufferize.
Alternatively, I'm wondering if this conversion pass will eventually be removed, since it is recommended to use One-shot Bufferize instead of Conversion-based Bufferization. I am still trying to understand the details and context of Bufferization. If this PR is not needed, I will close it.

Thx for looking into this. It can indeed be a bit misleading because the term bufferization in convert-bufferization-to-memref refers to the dialect name bufferization and not the process of "bufferizing tensor IR".

One-Shot Bufferize takes care of buffer deallocation in simple cases. It will never produce bufferization.clone ops. So this pass is not needed when using One-Shot Bufferize. There are cases that One-Shot Bufferize cannot handle yet: yielding allocs from a block. In that case, One-Shot Bufferize must be run with create-deallocs=0 and the -buffer-deallocation pass must be run afterwards. That's the pass that introduces clone ops.

There's a person that's currently looking into alternatives for the buffer deallocation pass. bufferization.clone may then disappear entirely. And maybe also this pass. Merging finalizing-bufferize and convert-bufferization-to-memref also sounds interesting. finalizing-bufferize may insert buffer allocs+copies, which are memrefs. So this can be seen as converting bufferization to memref. Other than that, I think this is WAI and we're going to reevaluate when we have a replacement for buffer-deallocation.

[sotoshigoto]

Thank you for your comment! I understand that there may be cases where One-Shot Bufferize is not yet able to handle, and I appreciate the information on alternatives for the buffer deallocation pass.
Based on this, I will go ahead and close this PR.

mlir/lib/Conversion/BufferizationToMemRef/BufferizationToMemRef.cpp

struct BufferizationToMemRefPass
 
   void runOnOperation() override {
     RewritePatternSet patterns(&getContext());
     populateBufferizationToMemRefConversionPatterns(patterns);
 
     ConversionTarget target(getContext());
     target.addLegalDialect<memref::MemRefDialect>();
     target.addLegalOp<arith::ConstantOp>();
-    target.addIllegalDialect<bufferization::BufferizationDialect>();
+    // In the current state, This pass only transforms a CloneOp in
+    // Bufferization dialect.
+    target.addIllegalOp<bufferization::CloneOp>();
 
     if (failed(applyPartialConversion(getOperation(), target,
                                       std::move(patterns))))
       signalPassFailure();
   }
 };
 } // namespace
 
 std::unique_ptr<Pass> mlir::createBufferizationToMemRefPass() {
   return std::make_unique<BufferizationToMemRefPass>();
 }