[Mlir-commits] [mlir] [mlir] remove folder on MemRef::ExtractStridedMetadataOp (PR #88043)

[llvmlistbot at llvm.org]

MaheshRavishankar wrote:

> I don't follow the logic: my understanding is that you're using the wrong abstractions, you introduce lower-level concepts with extractstridedmetadata and you're losing high-level information, but then you want to do a high-level transformation and complain that it is hard to do. Well that's work-as-intended as far as I can tell.

I am sympathetic to this argument. Yes this is a low level construct. But the flip side of this argument is that there is an implicit phase ordering introduced here. Certain transformations are "not safe" after you introduce `memref.extract_strided_metadata`. Sometimes you cannot avoid having such constraints and you live with it. Here you can remove this potential footgun by just dropping the folding. (more on this below).


> The problem to me is in your "buffer coalesing pass": you can't arbitrary modify the memref without seeing all the users: the existence of `ExtractStridedMetadataOp` reminds me "pointer escaping" operations in LLVM. Any optimization in LLVM that interacts with pointer has to figure out if a pointer might escape, and be very conservative about the transformation in such cases. Basically "buffer coalesing pass" needs to "prove" that a memref isn't decomposed with a `ExtractStridedMetadataOp` before doing the transformation. ("proving" here can also be done by construction instead of IR analysis: the compiler architect can assemble a pipeline where `ExtractStridedMetadataOp` aren't introduced before a given stage, and "buffer coalesing pass" is applied before this stage).

That is a bit convoluted. You cant do this optimization because extract strided metadata folds away things, but if it didnt fold away things then you can do this optimization. I would rather drop the folding and avoid introducing phase ordering issues and avoid having to do complicated analysis to see transitive uses and see if it is safe to do it or not. In some cases you will need to do this analysis, but here if we just drop the folder, then we dont need to do complicated analysis.

In my book this is yet another case of folders/canonicalizers being way too aggressive, and applied/executed in a way that is is not good for the ecosystem. This particular case where the pointer and offset are decoupled is really a big footgun (that I can gaurantee you everyone is going to hit at some point). You can say "you need to be aware of the footgun" or you can avoid having this footgun.  I know how I can work around this footgun, but the goal here is to not have such footguns.

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