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

[llvmlistbot at llvm.org]

MaheshRavishankar wrote:

> I had a closer look at the thread and I think this issue is just the tip of the iceberg.
> 
> Although I sympathize with what you are trying to do, I think this is just one of the many issues that you will face because you are technically changing the type of the memref. In other words, you will need to either prevent all transformation based on the memref type or recover the information after the fact.
> 
> Indeed when you go from:
> 
> ```
> %1 = memref.alloc(%d1) : memref<?xf32>
> ```
> 
> To:
> 
> ```
> %1 = memref.subview %alloc[%d0] [%d1] [1] : memref<?xf32> to memref<?xf32, strided=<[1], offset = ?>>
> ```
> 
> You changed the memref from `memref<?xf32>` to `memref<?xf32, strided=<[1], offset = ?>>` and I think you are in for a wild ride at this point. Essentially, all the uses of `getStridesAndOffset` in the MLIR code base may do something adverse to what you want to achieve and I don't believe you can reasonably disable all of that.
> 
> All in all, I don't think that what you are trying to achieve is correct as in you are changing a type without proper casting. Instead, I think you should use dynamic offsets in all your memrefs from the get go when you lower from tensors.

Good point on `getStridesAndOffset`. That is a big footgun indeed. Like I said, we found a work-around for this downstream (which includes your suggestion of just using dynamic offsets to begin with), but this is a footgun that anyone building something reasonably complex with MLIR is going to hit. The only thing I would amend to your last sentence though is that the transformation that I described above is not incorrect, and even if I do all the proper casting, the folder has broken the link between offset uses in a way that cannot be recovered. So this is really an MLIR bug which looks like we dont have a way to fix.

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