[MemRef] Verify subiew offset, size & stride

[RoboTux]

Check that the number of static and dynamic offsets, sizes and strides
is valid. For static values it needs to match source memref rank, for dynamic
values it needs to be at most as much as the rank.

[llvmbot]

@llvm/pr-subscribers-mlir-memref

@llvm/pr-subscribers-mlir

Author: Thomas Preud'homme (RoboTux)

Changes

Check that the number of static and dynamic offsets, sizes and strides
is valid. For static values it needs to match source memref rank, for dynamic
values it needs to be at most as much as the rank.

---

Full diff: https://github.com/llvm/llvm-project/pull/167043.diff

1 Files Affected:

• (modified) mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp (+33)

diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 1c21a2f270da6..ea856057f9d85 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -3113,6 +3113,39 @@ LogicalResult SubViewOp::verify() {
   ArrayRef<int64_t> staticSizes = getStaticSizes();
   ArrayRef<int64_t> staticStrides = getStaticStrides();
 
+  // Check number of static offsets, sizes and strides match source rank.
+  int64_t baseRank = baseType.getRank();
+  size_t numStaticOffsets = staticOffsets.size();
+  if (static_cast<int64_t>(numStaticOffsets) != baseRank)
+    return emitError("number of static offsets (")
+           << numStaticOffsets << ") does not match source rank (" << baseRank
+           << ")";
+  size_t numStaticSizes = staticSizes.size();
+  if (static_cast<int64_t>(numStaticSizes) != baseRank)
+    return emitError("number of static sizes (")
+           << numStaticSizes << ") does not match source rank (" << baseRank
+           << ")";
+  size_t numStaticStrides = staticStrides.size();
+  if (static_cast<int64_t>(numStaticStrides) != baseRank)
+    return emitError("number of static strides (")
+           << numStaticStrides << ") does not match source rank (" << baseRank
+           << ")";
+
+  // Check number of dynamic offsets, sizes and strides is less of equal to the
+  // source rank.
+  size_t numOffsets = getOffsets().size();
+  if (static_cast<int64_t>(numOffsets) > baseRank)
+    return emitError("number of dynamic offsets (")
+           << numOffsets << ") bigger than rank (" << baseRank << ")";
+  size_t numSizes = getSizes().size();
+  if (static_cast<int64_t>(numSizes) > baseRank)
+    return emitError("number of dynamic sizes (")
+           << numSizes << ") bigger than rank (" << baseRank << ")";
+  size_t numStrides = getStrides().size();
+  if (static_cast<int64_t>(numStrides) > baseRank)
+    return emitError("number of dynamic strides (")
+           << numStrides << ") bigger than rank (" << baseRank << ")";
+
   // The base memref and the view memref should be in the same memory space.
   if (baseType.getMemorySpace() != subViewType.getMemorySpace())
     return emitError("different memory spaces specified for base memref "

[matthias-springer]

Isn't this already verified by the ViewLikeOpInterface verifier?

[RoboTux]

Err yes indeed, and I was not hitting it because the issue I had was just before the Subview is created. I thought this patch was catching the issue but I confused the output for an assert. My bad.