RFC : Update to "General Design" section of Operation Canonicalizations in MLIR

[MaheshRavishankar]

@mehdi_amini

I have strong concerns with this: I actually am on the totally opposite view, it is on the folks who believe there is a problem with a canonicalization to explain what the problem is.
I don’t see any practical way to prove anything about canonicalization when most of the time this is about deciding on a particular convention.

Thats a surprising take. Lets use the framework above of thinking in terms of lattices. If you have a program in state a, canonicalization is expected to take your program to a state b such that b < a w.r.t to that lattice. So I think unless you do that you are not actually implementing a canonicalization. But that makes me think of a different phrasing of the problem. Let me come back to that.

@mehdi_amini

@qed (I misattributed to @sogartar earlier) nicely developed a principled reasoning about the vector dialect abstraction and why transpose is nicer to reason about here and here.

I mean one follows from the other. I agree Quinn spelled things out more clearly, so I am happy that argument connected for you.

@mehdi_amini

Having maintained a downstream backend in LLVM for a while, it is quite common that instcombine for example would be smarter and break our backend because our coverage of LLVM IR was incomplete, and we’d have to fix it in our codegen prepare by undoing some of the canonical form from instcombine. This seems WAI to me.
A downstream user being broken is a very weak signal, the kind of discussion that @qed brought are on the other hand highly valuable.

I dont know, are we comparing badges here? The crux of the matter here is that MLIR does not have a full pipeline that is intended to handle a wide range of programs. So a lot of subtle issues only show up in downstream users like IREE. When things break we do spend a lot of time diagnosing whether this is a downstream problem or an upstream problem. Given this hole in MLIR saying downstream user being broken is a weak signal is actually a problem. I am not trying to put IREE uses at a different bar from any other downstream user, but if I wear my MLIR contributor hat I have to acknowledge MLIR itself has no way of really catching how all the different transformations fit together, and signals from downstream users are extremely important (IREE or otherwise).

Different phrasing of issue with canonicalization

I think it is useful to lean into the framing of canonicalization as being a lattice and that canonicalization should essentially take you from one point in the lattice to a “lower point”. So the problem is
(a) We dont have a good definition of this lattice and how current canonicalizations help get “lower in the lattice”
(b) We dont need to have a single canonicalization. We need to break things up into separate fixed points and then it is much easier to reason about whether a pattern works in conjunction with other patterns added to a particular fixed point.
(c) Too often we have added patterns as canonicalizations as a convenience, when really it is a one-off pattern so that they “just get applied with the bucket of canonicalizations” without actually reasoning about whether it is a canonicalization.