[Mlir-commits] [mlir] [memref] Support non-scalar copies in `reinterpret_cast` elision (PR #203873)

[Andrzej Warzyński]

================

----------------
banach-space wrote:

```suggestion
    // Track result dimensions that produce varying indices; unit dimensions are
    // always indexed at 0.
    for (auto [dim, resultSize] : llvm::enumerate(resType.getShape())) {
      if (resultSize != 1)
        dimsAndOffs.nonUnitDimsPos.push_back(static_cast<unsigned>(dim));
    }

    bool isScalarRes =
        llvm::all_of(resType.getShape(), [](int64_t size) { return size == 1; });

    // For non-scalar results, verify that strides do not introduce
    // non-contiguity that would require extra logic.
    if (!isScalarRes) {
      SmallVector<int64_t> srcIdentityStrides =
          computeStrides(srcType.getShape());
      ArrayRef<int64_t> rcResultStrides = rc.getStaticStrides();

      if (!llvm::all_of(llvm::zip_equal(srcIdentityStrides, rcResultStrides),
                        [](auto pair) {
                          auto [srcStride, resultStride] = pair;
                          return !ShapedType::isDynamic(resultStride) &&
                                 srcStride == resultStride;
                        }))
        return std::nullopt;
    }
```

Explanation:
* The loop to compute non-unit dimension can be safely hoisted outside ` if (!isScalarCopy) {}`.
* ` assert((srcIdentityStrides.size() == rcResultStrides.size()) ` is not required - further up you verify that `if (srcType.getRank() != resType.getRank())`. If ranks match, so do strides. Otherwise MemRefs would be ill-formed.
* `!isScalarCopy)` -> `isScalarRes`. We shouldn't mention `Copy` in this method. Apologies for that poor suggestion in my previous review.

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