[Mlir-commits] [mlir] [MLIR][Tensor] Remove FoldDimOf[Expand|Collapse]Shape Pattern (PR #134219)

[llvmlistbot at llvm.org]

MaheshRavishankar wrote:

> > . These pattern are also wrong
> 
> Wrong how?

Wrong cause they are crashing.

> 
> > and redundant
> 
> It's a bit ambiguous what you call redundant, if you compare to "other things that are not part of canonicalization" then it's a bit dubious to claim it "redundant" as it won't be part of the canonicalization run.

Redundant because these pattern are replacing `tensor.dim` of result in terms of `tensor.dim` of operands. Which is exactly what these patterns do (https://github.com/llvm/llvm-project/blob/b712068af26643d5fcbfd148b2084554b4e61650/mlir/include/mlir/Dialect/MemRef/Transforms/Transforms.h#L52) using the interface here (https://github.com/llvm/llvm-project/blob/a54736afd5b8f8ed25550a9f456afd36e49c04e0/mlir/lib/Dialect/Tensor/IR/TensorInferTypeOpInterfaceImpl.cpp#L129) . So its literally the same thing. 

> This is all an opinion, but I'd rather see you elaborate you're thinking to enlighten everyone, instead of just making a strong claim that you're not trying to substantiate right now (other than very unspecific terms like "wrong" and "redundant"). Because right now:

It isnt a strong claim. They literally do the same thing that another pattern in the code base does (redundant) and is crashing (wrong). I dont know what else you think would be needed to substantiate that this is wrong and redundant.

> 
> > So the resolution here is to remove this pattern.
> 
> may very well be the absolute right thing, however it's important to be able to convey **why** this is the right ultimate thing.

I dont want to get dragged into canonicalization vs. non-canonicalization discussion again and again. These two patterns are outliers since they are resolving `tensor.dim` operations of result in terms of `tensor.dim` operation of operands. The way the rest of the operations in tree handle that is through the interface (including the two ops here). No other ops have that. So I think we dont even need to get into canonicalization vs. not discussion. These patterns are outliers.

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