[Mlir-commits] [mlir] [mlir][memref] Add a new `ReifyResultShapes` pass (PR #145927)

[llvmlistbot at llvm.org]

MaheshRavishankar wrote:

> > This is my source of confusion. As far as I know this is meant to extract information about the shape of the result of the, but this is actually changing the operation itself. This seems like something that cannot be done just based on the interface/clone. The change in the result type might make the operation invalid (according to its verifier). This kind of rewrite cannot really be done just on the interface.
> 
> As usual, confluence is a non-negotiable property for the system to avoid crumbling upon its own weight. Otherwise we have a flawed design with undocumented internal assumption of when it is or isn't safe to use. What I am hearing is there are confluence issues with ReifyRankedShapedTypeOpInterface wrt various op verifiers, making ReifyRankedShapedTypeOpInterface unsafe to use?

I dont think this is true. The `ReifyRankedShapedTypeOpInterface` is perfectly safe to use if you use it as a "retreival" mechanism. What is off here is for these operations you are trying to get the shape of the output that ends up being inconsistent with the shape of the output as recorded by the operations. That is not an issue with the interface, but rather issue with the ops itself. You then cannot use the interface to blanket update the result shape cause there is literally no way the interface can know what the op verifier sees as an internal consistent state. 

> 
> When I look [here](https://github.com/llvm/llvm-project/commit/092372da15e5165be14cdbb7cac3cf4976fd82d0#diff-8cc8879b3d74154e46c94a24f4f1650e9233547350fc425092f8317ba63a6b62L55) I see an alteration of the interface that seems related to confluence. Why was the assertion removed in this case?

Again, its an issue with the operation. You are right though, maybe that assertion should not have been removed. Also assertion are just a poor way of handling this. This needed to be an error.

I think the way forward here is : add a pattern that is specific for these ops that will make the shape of the result consistent with what the interface returns. For now this could go in as a canonicalization cause that is consistent with how static shape information is propagated today in the code base.  Then you dont need this pass.

I think thats what you had tried initially, sorry maybe I misread the PR that led to this thread splintering into different directions. If you have a link to that PR, I can take a look again.


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