[Mlir-commits] [mlir] [mlir][vector] Add verification for incorrect vector.extract (PR #115824)

[Andrzej Warzyński]

================
@@ -1349,13 +1349,33 @@ LogicalResult vector::ExtractOp::verify() {
         "corresponding dynamic position) -- this can only happen due to an "
         "incorrect fold/rewrite");
   auto position = getMixedPosition();
-  if (position.size() > static_cast<unsigned>(getSourceVectorType().getRank()))
+  VectorType srcVecType = getSourceVectorType();
+  int64_t srcRank = srcVecType.getRank();
+  if (position.size() > static_cast<unsigned>(srcRank))
     return emitOpError(
         "expected position attribute of rank no greater than vector rank");
+
+  VectorType dstVecType = dyn_cast<VectorType>(getResult().getType());
+  if (dstVecType) {
+    int64_t srcRankMinusIndices = srcRank - getNumIndices();
+    int64_t dstRank = dstVecType.getRank();
+    if ((srcRankMinusIndices == 0 && dstRank != 1) ||
+        (srcRankMinusIndices != 0 && srcRankMinusIndices != dstRank)) {
+      return emitOpError(
+          "expected source rank minus number of indices to match "
+          "destination vector rank");
+    }
+  } else {
+    // Scalar result.
+    if (srcRank != getNumIndices())
+      return emitOpError("expected source rank to match number of indices "
+                         "for scalar result");
+  }
+
----------------
banach-space wrote:

> This logic works, but I prefer checking if the expected destination type matches the actual destination type.

No particular preference other than keeping `vector::ExtractOp::verify` and `vector::InsertOp::verify` consistent (as in, consistency within dialect > consistence across dialects). 

With that in mind, could this replicate `vector::InsertOp`: https://github.com/llvm/llvm-project/blob/d1aa0da7e28f70715bd92b2bc2809ac04a832aa8/mlir/lib/Dialect/Vector/IR/VectorOps.cpp#L2870-L2878
? As in:
```cpp
if (dstVecType && ...)
      return emitOpError("");
if (!dstVecType &&)
     return emitError("");
```

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