[Mlir-commits] [mlir] [mlir][vector] Add pattern to rewrite contiguous ExtractStridedSlice into Extract (PR #111541)

[Mehdi Amini]

================
@@ -235,6 +235,11 @@ void populateVectorExtractStridedSliceToExtractInsertChainPatterns(
     std::function<bool(ExtractStridedSliceOp)> controlFn = nullptr,
     PatternBenefit benefit = 1);
 
+/// Pattern to rewrite simple cases of N-D extract_strided_slice, where the
+/// slice is contiguous, into extract and shape_cast.
+void populateVectorContiguousExtractStridedSliceToExtractPatterns(
+    RewritePatternSet &patterns, PatternBenefit benefit = 1);
----------------
joker-eph wrote:

>  Basically question is if this pattern will unlock other canonicalizations,

I don't think this is a necessary criteria, for example @bjacob above mentioned:

- extract is more constrained than extract_strided_slice, so it is more likely to have a good lowering.
- a extract_strided_slice producing a vector with leading unit dims, followed by a shape_cast dropping the unit dims. That shape_cast was hitting the fallback lowering pattern, ShapeCastOpRewritePattern. 


These are enough of a motivation to justify a canonicalization to me. Then on top of this is the question of whether the transformation is potentially losing semantics that can't be trivially reconstructed (such aspect would likely make it clearly not suitable for canonicalization).

> @joker-eph whats your solution for such cases. 

Make it a canonicalization.

> So one option is, we dont add it to MLIR itself, but keep it downstream. 

If this can't be grouped in a cohesive pass that achieve something meaningful that we can reason about, then yeah please keep all these patterns out-of-tree.


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