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

[Nicolas Vasilache via Phabricator via llvm-commits]

nicolasvasilache added inline comments.
Herald added a subscriber: zero9178.


================
Comment at: mlir/lib/Dialect/MemRef/Transforms/SimplifyExtractStridedMetadata.cpp:170
+
+/// Computed the expanded sizes of the given \p expandShape for the
+/// \p groupId-th reassociation group.
----------------
nit: typo


================
Comment at: mlir/lib/Dialect/MemRef/Transforms/SimplifyExtractStridedMetadata.cpp:177
+static SmallVector<OpFoldResult>
+getExpandedSizes(memref::ExpandShapeOp expandShape, OpBuilder &builder,
+                 const SmallVectorImpl<OpFoldResult> &origSizes,
----------------
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.


================
Comment at: mlir/lib/Dialect/MemRef/Transforms/SimplifyExtractStridedMetadata.cpp:178
+getExpandedSizes(memref::ExpandShapeOp expandShape, OpBuilder &builder,
+                 const SmallVectorImpl<OpFoldResult> &origSizes,
+                 unsigned groupId) {
----------------
ArrayRef ?


================
Comment at: mlir/lib/Dialect/MemRef/Transforms/SimplifyExtractStridedMetadata.cpp:226
+SmallVector<OpFoldResult>
+getExpandedStrides(memref::ExpandShapeOp expandShape, OpBuilder &builder,
+                   const SmallVectorImpl<OpFoldResult> &origSizes,
----------------
same for making this a first-class citizen of expand


================
Comment at: mlir/lib/Dialect/MemRef/Transforms/SimplifyExtractStridedMetadata.cpp:227
+getExpandedStrides(memref::ExpandShapeOp expandShape, OpBuilder &builder,
+                   const SmallVectorImpl<OpFoldResult> &origSizes,
+                   const SmallVectorImpl<OpFoldResult> &origStrides,
----------------
ArrayRef ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133625