[Mlir-commits] [mlir] [mlir] Introduce wrapInZeroTripCheck in LoopLikeOpInterface (PR #80331)

[llvmlistbot at llvm.org]

MaheshRavishankar wrote:

> > > Late to the party here, but I think these transformations dont belong in the interface. Looking at the `LoopLikeOpInterface` it is actually doing some very heavy weight transformations that not all loop ops that implement this interface can handle. I tried to do this too in an earlier PR, and it was rightly pointed out that those kind of transformations dont go into the interface definition.
> > > Instead of adding oto the interface definition, maybe these are just transformation methods that are written on the interface. My read of interfaces are that they are a wrapper around the core operation to have a unified way to interpret operations. Adding transformations of this kind to an interface seem odd to me.
> > > I am not pointing out something really off in this specific PR, but it is contributing to an aspect of the interface that I wanted to flag.
> > 
> > 
> > My initial thought is to create a common interface on loop ops so users can combine with utilities like `moveLoopInvariantCode` to hoist loop invariants and be op-agnostic:
> > https://github.com/llvm/llvm-project/blob/7880b2c8586eade00a4aa5ac11007317a61e376c/mlir/lib/Transforms/Utils/LoopInvariantCodeMotionUtils.cpp#L106-L115
> > 
> > I also have the same question when writing this as an interface. Do you have an example where you think this transformation should live?
> 
> IMO, I can probably write this as a transformation helper like this one:
> 
> https://github.com/llvm/llvm-project/blob/8b0f47bfa4b6aa1bafa10261444c93aba5a2d31d/mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp#L990-L993


This is what I was thinking of. Thanks for addressing.

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