[Mlir-commits] [mlir] [mlir][memref] Compute subview dropped dims from inferred result type (PR #235093)

[Prakhar Dixit]

Prakhar-Dixit wrote:

> My coding agent found a potential correctness issue. Can you take a look at [this](https://gist.github.com/matthias-springer/9a09ee89aca939757eb94b139c593f6a)?

Thanks for catching this! I reproduced it on the PR branch, the folded load indexes source dim 3 with %s3, instead of dim 2 with %s2.

The root cause is in `computeMemRefRankReductionMaskByStrides`, it only counts stride occurrences and never checks which dim each stride belongs to. With the source strides [64, 32, 4, 1] the count used to fail, so we fell back to the size-based matching, which got it right. With the inferred strides [64, ?, ?, ?] the count now succeeds, but it drops the first unit dim with a ? stride (dim 1) instead of dim 3.

The counting is also wrong with static strides, independent of this PR. For example, the verifier rejects this valid subview (dropping dim 2 gives [8, 4], but the counting drops dim 0 and keeps [4, 8]):

```
%sv = memref.subview %m[0, 0, 0][1, 1, 1][1, 1, 1] :
  memref<1x1x1xf32, strided<[8, 4, 8]>> to memref<1x1xf32, strided<[8, 4]>>
```
I see two options:

1. A minimal guard: accept the stride-based mask only if the kept sizes match the result shape, otherwise fall back to the size-based matching. This fixes the regression.
2. Replace both heuristics with a single in-order matcher: each result dim takes the leftmost remaining source dim with the same size and a compatible stride, skipping only static unit dims. This fixes both cases, but it also changes what the verifier accepts.

Would you suggest adding the guard here and the matcher as a follow-up?

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