[PATCH] D133625: [mlir][MemRef] Simplify extract_strided_metadata(expand_shape)

[Nicolas Vasilache via Phabricator via llvm-commits]

nicolasvasilache added a comment.

Thanks for splitting it up this way, let's go with this for now

I will try to simplify some of the impl (at a tradeoff of adding a bit more IR that folds) as a separate PR but this is a great step towards removing logic from LLVM while also expanding the cases we can support.



================
Comment at: mlir/lib/Dialect/MemRef/Transforms/SimplifyExtractStridedMetadata.cpp:177
+static SmallVector<OpFoldResult>
+getExpandedSizes(memref::ExpandShapeOp expandShape, OpBuilder &builder,
+                 const SmallVectorImpl<OpFoldResult> &origSizes,
----------------
qcolombet wrote:
> qcolombet wrote:
> > nicolasvasilache wrote:
> > > I would make this a first-class citizen of expand: `SmallVector<OpFoldResult> memref::ExpandShapeOp::buildExpandedSizes(...)`.
> > > 
> > > Then as the op semantics evolve to take the result shape operands, we can update this.
> > That's not possible because the MemRef dialect would depend on Affine dialect if we were to do that.
> For the record, my plan was to move that into a common place (e.g., `ReshapeOpsUtils`), when we think it looks good.
Ah yes, I keep on forgetting this unfortunate layering .. could you please add a TODO?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133625/new/

https://reviews.llvm.org/D133625