[Mlir-commits] [mlir] [MLIR] Incorrect result for RuntimeVerifiableOpInterface on MemRef::R… (PR #96580)

[Matthias Springer]

================
@@ -203,8 +224,20 @@ std::pair<Value, Value> computeLinearBounds(OpBuilder &builder, Location loc,
   return computeLinearBounds(builder, loc, offset, strides, sizes);
 }
 
+/// Returns two Values representing the bounds of the memref. The bounds are
+/// returned as a half open interval -- [low, high].
+std::pair<Value, Value>
+computeLinearInclusiveBounds(OpBuilder &builder, Location loc,
+                             TypedValue<BaseMemRefType> memref) {
+  auto runtimeMetadata = builder.create<ExtractStridedMetadataOp>(loc, memref);
+  auto offset = runtimeMetadata.getConstifiedMixedOffset();
+  auto strides = runtimeMetadata.getConstifiedMixedStrides();
+  auto sizes = runtimeMetadata.getConstifiedMixedSizes();
+  return computeLinearInclusiveBounds(builder, loc, offset, strides, sizes);
+}
+
 /// Verifies that the linear bounds of a reinterpret_cast op are within the
-/// linear bounds of the base memref: low >= baseLow && high <= baseHigh
+/// linear bounds of the base memref: low >= baseLow && high <= baseHigh.
----------------
matthias-springer wrote:

I'm starting to wonder if this is something that should be verified at all. Why would `high > baseHigh` make the op invalid? Maybe the programmer knows that the allocation is larger than the current MemRef type indicates. We do not verify this for fully static memref types in the op verifier of `memref.reinterpret_cast` either.

I think there's a bug in the program logic here and your commit is fixing it, but I think the right solution is to delete this entire interface impl. I think there's nothing to verify for `memref.reinterpret_cast`.

Alternatively: Tighten the op semantics in `MemRefOps.td` and make clear that `reinterpret_cast` ops that "extend" the linear index range are invalid.


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