[Mlir-commits] [mlir] [mlir][MemRef] Add position-based matching heuristics for rank-reduction with dynamic strides (PR #184334)

[Han-Chung Wang]

================
@@ -977,6 +1027,18 @@ computeMemRefRankReductionMask(MemRefType originalType, MemRefType reducedType,
           reducedType.getStridesAndOffset(candidateStrides, candidateOffset)))
     return failure();
 
+  // When strides are dynamic and multiple dimensions need to be dropped, we use
+  // position-based matching instead.
+  if (unusedDims.count() > 1 &&
+      (llvm::any_of(originalStrides, ShapedType::isDynamic) ||
+       llvm::any_of(candidateStrides, ShapedType::isDynamic))) {
----------------
hanhanW wrote:

This comment is generated by Claude, and I think it makes sense. The strides are not taken into account in your implementation.

The guard triggers when any stride is dynamic, not when all are. In mixed cases, strides carry real information that position-based matching ignores.

Example: `memref<1x1x?xf32, strided<[?, 4, 1]>>` with sizes `[1, 1, 4]` → `memref<1x4xf32, strided<[4, 1]>>`:
- Stride-based correctly returns `dropped={0}` (keeping dim `1` stride `4` matches result stride `4`)
- Position-based returns `dropped={1}` (leftmost match for result dim `0`)
- With `dropped={1}`, getCanonicalSubViewResultType picks strides `[?, 1]` instead of `[4, 1]`, losing static stride information.

I think we want to preserve the static infomation when possible.

Note that this is not a miscompile, but it produces unnecessarily dynamic types.

---

I think you can refactor the below code to `computeMemRefRankReductionMaskByStrides` and swap them in the order.

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