[llvm-branch-commits] [mlir] [draft] Dialect Conversion without Rollback (PR #93412)

[Christian Ulmann via llvm-branch-commits]

Dinistro wrote:

> The final `OneShotConversionPatternRewriter` will support everything that `ConversionPatternRewriter` supports. A few helper functions are still missing in this PR:
> 
> * `applySignatureConversion`
> * `convertRegionTypes`
> * `convertNonEntryRegionTypes`
> * `replaceUsesOfBlockArgument` (removed by [[mlir][Transforms] Support `replaceAllUsesWith` in dialect conversion #84725](https://github.com/llvm/llvm-project/pull/84725))
> 
> These functions would have to become virtual, so that they can be overridden in `OneShotConversionPatternRewriter`.

I was mainly referring to the rollback logic. A pattern using the super class might assume rollback, which then breaks when it uses the specialised class. This violates the liskov substitution principle, which might be considered problematic.
I understand that we might deliberately do so, to ensure a smooth transition to the new infra, without requiring substantial changes.

I just fear that we will never be able to remove the inheritance if we build this in now.

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