[mlir][tensor] Improve verifiers: detect out-of-bounds accesses

[springerm]

Summary

• Disallow slices that run out of bounds.
• Disallow negative indices/offsets.
• Fix invalid test cases.

[springerm]

update

[jpienaar]

Does this match https://mlir.llvm.org/getting_started/DeveloperGuide/#ir-verifier ?

[springerm]

In D156061#4526283, @jpienaar wrote:

Does this match https://mlir.llvm.org/getting_started/DeveloperGuide/#ir-verifier ?

I'm going to remove verification for offsets/sizes that are SSA values. According to the developer guide we should only verify static_sizes and static_offsets.

[springerm]

address comments

[springerm]

update

[springerm]

udpate

[Hardcode84]

I'm going to remove verification for offsets/sizes that are SSA values. According to the developer guide we should only verify static_sizes and static_offsets.

I think static sizes/offsets can be generated from ssa ones by existing folders/canonicalization, so there are really no difference. IMO, we shouldn't fail verification on this as they can appear as result of valid transformations (e.g. in unreachable branches).

[nicolasvasilache]

Clearly this caught some UB cases and has value as a sanity check we would want to run manually.
However it does go against the principles outlined in the link posted by @jpienaar: turning UB into a verification error is a no-go as it breaks local invariants that need to hold.

You could have a dedicated pass which detects such UB, it would be similar to -test-memref-bound-check.
But I am afraid we cannot do significantly better as other invariants would break badly.

[rkayaith]

In D156061#4530658, @Hardcode84 wrote:

I think static sizes/offsets can be generated from ssa ones by existing folders/canonicalization, so there are really no difference.

those would have to be updated to only fold if the constant value is valid

[springerm]

In D156061#4534070, @rkayaith wrote:

In D156061#4530658, @Hardcode84 wrote:

I think static sizes/offsets can be generated from ssa ones by existing folders/canonicalization, so there are really no difference.

those would have to be updated to only fold if the constant value is valid

I think it would be OK to generate invalid ops during folding. (This can happen anyway, even if we do more aggressive checking of SSA values, because the verifier cannot take into account any possible foldings/canonicalizations that could happen.) garbage in => garbage out

We can check do extra runtime checks for SSA value offsets/sizes/strides as part of -generate-runtime-verification.

[rkayaith]

In D156061#4535845, @springerm wrote:

In D156061#4534070, @rkayaith wrote:

In D156061#4530658, @Hardcode84 wrote:

I think static sizes/offsets can be generated from ssa ones by existing folders/canonicalization, so there are really no difference.

those would have to be updated to only fold if the constant value is valid

I think it would be OK to generate invalid ops during folding. (This can happen anyway, even if we do more aggressive checking of SSA values, because the verifier cannot take into account any possible foldings/canonicalizations that could happen.) garbage in => garbage out

I thought one of the main points of https://mlir.llvm.org/getting_started/DeveloperGuide/#ir-verifier was to avoid this? if verifiers only verify local constraints, folders can check that they don't violate them.

[springerm]

In D156061#4535960, @rkayaith wrote:

I thought one of the main points of https://mlir.llvm.org/getting_started/DeveloperGuide/#ir-verifier was to avoid this? if verifiers only verify local constraints, folders can check that they don't violate them.

My thinking was: A folder/canonicalization may generate an op with invalid static offsets/sizes/strides (e.g., by promoting SSA values to attributes) and then next time the verifier is running, it will be caught (hopefully). Alternatively, the folder/canonicalization pattern could also just leave such ops alone that would be canonicalized/folded to ops that would not pass the verifier. (That would make those folders/patterns more complex and we would have to duplicate (or call) some verification code there.)

Also just to clarify: The purpose of this change is to clarify in the documentation that out-of-bounds accesses are invalid and add explicit verifiers for that. I think we already treated such ops as "invalid", it was not mentioned in the documentation. But such ops do not lower correctly and result in invalid IR during vectorization/bufferization.

[Hardcode84]

Consider following program

%0 = ... tensor<5xf32>
%1 = tensor.cast %0 tensor<5xf32> to tensor<?xf32>
%dim = dim %1 0 : tensor<?xf32>
%cond = <nontrivial calculation depending on %dim>
%res = scf.if %cond {
  %2 = tensor.extract %1 [42] : tensor<?xf32>
  yield %2
} else {
  yield %default
}

which can be canonicalized to

%0 = ... tensor<5xf32>
%dim = dim %0 0 : tensor<?xf32>
%cond = <nontrivial calculation depending on %dim>
%res = scf.if %cond {
  %2 = tensor.extract %0 [42] : tensor<5xf32>
  yield %2
} else {
  yield %default
}

Which is still a valid program, but now will be wrongfully rejected by verifier
Moreover, when we detected invalid indices, it can be transformed further to

%0 = ... tensor<5xf32>
%dim = dim %0 0 : tensor<?xf32>
%cond = <nontrivial calculation depending on %dim>
%res = scf.if %cond {
  %2 = poison
  yield %2
} else {
  yield %default
}

[springerm]

In D156061#4536205, @Hardcode84 wrote:

Consider following program

This is an interesting example. I'm not familiar with "poison". What does this mean for op verification? Does this mean that we should never verify anything? Can the same thing happen with things that the ExtractSliceOp/... verifiers are already checking at the moment?

[kuhar]

In D156061#4536205, @Hardcode84 wrote:

Which is still a valid program, but now will be wrongfully rejected by verifier

In D156061#4536297, @springerm wrote:

In D156061#4536205, @Hardcode84 wrote:

Consider following program

This is an interesting example. I'm not familiar with "poison". What does this mean for op verification? Does this mean that we should never verify anything? Can the same thing happen with things that the ExtractSliceOp/... verifiers are already checking at the moment?

-1, I would not call the transformed program valid. Local properties like non-ssa OOB indices should make ops invalid and fail in the verifier. And separately, any canonicalization/folds that would create invalid ops should instead either abort folding or replace them with poison/unreachable.

[kuhar]

LGTM % nits

mlir/include/mlir/Dialect/Tensor/IR/TensorOps.td

266

Did you want to highlight some difference by using 'should' and 'must'? If there is none, I'd prefer to use 'must' everywhere

739

same here

mlir/lib/Dialect/Tensor/IR/TensorOps.cpp

182–185

[springerm]

I’m going to add a check to the dynamic->static offset/sizes canonicalization so that no ops would be generated that are invalid

[Hardcode84]

In D156061#4536394, @kuhar wrote:

In D156061#4536205, @Hardcode84 wrote:

Which is still a valid program, but now will be wrongfully rejected by verifier

In D156061#4536297, @springerm wrote:

In D156061#4536205, @Hardcode84 wrote:

Consider following program

This is an interesting example. I'm not familiar with "poison". What does this mean for op verification? Does this mean that we should never verify anything? Can the same thing happen with things that the ExtractSliceOp/... verifiers are already checking at the moment?

-1, I would not call the transformed program valid. Local properties like non-ssa OOB indices should make ops invalid and fail in the verifier. And separately, any canonicalization/folds that would create invalid ops should instead either abort folding or replace them with poison/unreachable.

While idea of skipping invalid states entirely will work in principle, IMO it is much less composable. Not only we will need to add checks into indices canonicalizers, but also to cast canonicalizers, and, probably, to other places we are not even aware of yet.

mlir/include/mlir/Dialect/Tensor/IR/TensorOps.td

def Tensor_ExtractOp : Tensor_Op<"extract", [
     Pure,
     TypesMatchWith<"result type matches element type of tensor",
                    "tensor", "result",
                    "::llvm::cast<TensorType>($_self).getElementType()">]> {
   let summary = "element extraction operation";
   let description = [{
     The `tensor.extract` op reads a ranked tensor and returns one element as
     specified by the given indices. The result of the op is a value with the
-    same type as the elements of the tensor. The arity of indices must match
-    the rank of the accessed value. All indices should all be of `index` type.
+    same type as the elements of the tensor.
+
+    The arity of indices must match the rank of the accessed value. All indices
+    should all be of `index` type. They must be non-negative and within the
+    bounds of the tensor.
 
     Example:
 
     ```mlir
     %4 = tensor.extract %t[%1, %2] : tensor<4x4xi32>
     %5 = tensor.extract %rt[%1, %2] : tensor<?x?xi32>
     ```
   }];
def Tensor_ExtractSliceOp : Tensor_OpWithOffsetSizesAndStrides<"extract_slice", [
   let description = [{
     The "extract_slice" operation extract a tensor from another tensor as
     specified by the operation's offsets, sizes and strides arguments.
 
     The extract_slice operation supports the following arguments:
 
     * source: the "base" tensor from which to extract a slice.
     * offsets: tensor-rank number of offsets into the "base" tensor from which
-               to extract the slice.
+               to extract the slice. Offsets must be non-negative and within the
+               bounds of the source tensor.
     * sizes: tensor-rank number of sizes which specify the sizes of the result
-             tensor type.
+             tensor type. "offset + size" must not exceed the respective
+             source dimension size.
     * strides: tensor-rank number of strides specifying subsampling in each
                dimension.
 
     The representation based on offsets, sizes and strides support a
     partially-static specification via attributes specified through the
     `static_offsets`, `static_sizes` and `static_strides` arguments. A special
     sentinel value ShapedType::kDynamic encodes that the corresponding entry has
     a dynamic value.
def Tensor_InsertOp : Tensor_Op<"insert", [
   let description = [{
     The `tensor.insert` op inserts a scalar into a ranked tensor `dest` as
     specified by the operation's indices.
 
     It returns a copy of `dest` with the indexed position updated to the value
     of `scalar`.
 
     The arity of `indices `must match the rank of the tensor `dest`. All
-    indices should be of `index` type.
+    indices should be of `index` type. They must be non-negative and within the
+    bounds of the tensor.
 
     Example:
 
     ```mlir
     %4 = tensor.insert %t into %dest[%1, %2] : tensor<4x4xi32>
     %5 = tensor.insert %rt into %dest[%1, %2] : tensor<?x?xi32>
     ```
   }];
let description = [{
     It returns a copy of `dest` with the proper slice updated with the value
     of `source`.
 
     The insert_slice operation supports the following arguments:
 
     * source: the tensor that is inserted.
     * dest: the tensor into which the source tensor is inserted.
     * offsets: tensor-rank number of offsets into the `dest` tensor into which
-               the slice is inserted.
+               the slice is inserted. Offsets must be non-negative.
     * sizes: tensor-rank number of sizes which specify the sizes of the source
-             tensor type.
+             tensor type. "offset + size" must not exceed the respective
+             destination dimension size.
     * strides: tensor-rank number of strides that specify subsampling in each
                dimension.
 
     The representation based on offsets, sizes and strides support a
     partially-static specification via attributes specified through the
     `static_offsets`, `static_sizes` and `static_strides` arguments. A special
     sentinel value ShapedType::kDynamic encodes that the corresponding entry has
     a dynamic value.
let description = [{
     though it has no side effects, because it will get DCEd during
     canonicalization.
 
     The parallel_insert_slice operation supports the following arguments:
 
     * source: the tensor that is inserted.
     * dest: the tensor into which the source tensor is inserted.
     * offsets: tensor-rank number of offsets into the `dest` tensor into which
-               the slice is inserted.
+               the slice is inserted. Offsets must be non-negative.
     * sizes: tensor-rank number of sizes which specify the sizes of the source
-             tensor type.
+             tensor type. "offset + size" must not exceed the respective
+             destination dimension size.
     * strides: tensor-rank number of strides that specify subsampling in each
                dimension.
 
     The representation based on offsets, sizes and strides support a
     partially-static specification via attributes specified through the
     `static_offsets`, `static_sizes` and `static_strides` arguments. A special
     sentinel value ShapedType::kDynamic encodes that the corresponding entry has
     a dynamic value.

mlir/lib/Dialect/Tensor/IR/TensorOps.cpp

for (const auto &size : enumerate(mixedSizes)) {
     droppedDims.set(size.index());
   }
 
   assert(shapePos == static_cast<int64_t>(reducedShape.size()) &&
          "dimension mismatch");
   return droppedDims;
 }
 
+LogicalResult verifySliceOffsetsAndSizes(Operation *op, RankedTensorType type,
+                                         ArrayRef<int64_t> mixedOffsets,
+                                         ArrayRef<int64_t> mixedSizes) {
+  // No out-of-bounds accesses.
+  for (int64_t i = 0, e = type.getRank(); i < e; ++i) {
+    int64_t offset = mixedOffsets[i];
+    int64_t size = mixedSizes[i];
+    if (!type.isDynamicDim(i) && !ShapedType::isDynamic(size))
+      if (!ShapedType::isDynamic(offset) && offset + size > type.getDimSize(i))
+        return op->emitOpError("dimension #") << i << " runs out of bounds";
+  }
+
+  // No negative offsets.
+  for (const auto &it : llvm::enumerate(mixedOffsets))
+    if (!ShapedType::isDynamic(it.value()))
+      if (it.value() < 0)
+        return op->emitOpError("offset #") << it.index() << " is negative";
+
+  return success();
+}
+
 //===----------------------------------------------------------------------===//
 // BitcastOp
 //===----------------------------------------------------------------------===//
 
 bool BitcastOp::areCastCompatible(TypeRange inputs, TypeRange outputs) {
   if (inputs.size() != 1 || outputs.size() != 1)
     return false;
   Type a = inputs.front(), b = outputs.front();

 }
 
 /// Verifier for ExtractSliceOp.
 LogicalResult ExtractSliceOp::verify() {
   // Verify result type against inferred type.
   RankedTensorType expectedType = ExtractSliceOp::inferResultType(
       getSourceType(), getMixedOffsets(), getMixedSizes(), getMixedStrides());
   SliceVerificationResult result = isRankReducedType(expectedType, getType());
-  return produceSliceErrorMsg(result, *this, expectedType);
+  if (failed(produceSliceErrorMsg(result, *this, expectedType)))
+    return failure();
+  return verifySliceOffsetsAndSizes(*this, getSourceType(), getStaticOffsets(),
+                                    getStaticSizes());
 }
 
 llvm::SmallBitVector ExtractSliceOp::getDroppedDims() {
   return ::getDroppedDims(getType().getShape(), getMixedSizes());
 }
 
 FailureOr<Value>
 ExtractSliceOp::rankReduceIfNeeded(OpBuilder &b, Location loc, Value value,

 }
 
 /// Verifier for InsertSliceOp.
 LogicalResult InsertSliceOp::verify() {
   RankedTensorType expectedType;
   SliceVerificationResult result =
       verifyInsertSliceOp(getSourceType(), getType(), getStaticOffsets(),
                           getStaticSizes(), getStaticStrides(), &expectedType);
-  return produceSliceErrorMsg(result, *this, expectedType);
+  if (failed(produceSliceErrorMsg(result, *this, expectedType)))
+    return failure();
+  return verifySliceOffsetsAndSizes(*this, getDestType(), getStaticOffsets(),
+                                    getStaticSizes());
 }
 
 /// If we have two consecutive InsertSliceOp writing to the same slice, we
 /// can mutate the second InsertSliceOp's destination to the first one's.
 ///
 /// Example:
 ///
 /// ```mlir
LogicalResult ParallelInsertSliceOp::verify() {
   if (!isa<ParallelCombiningOpInterface>(getOperation()->getParentOp()))
     return this->emitError("expected ParallelCombiningOpInterface parent, got:")
            << *(getOperation()->getParentOp());
 
   RankedTensorType expectedType;
   SliceVerificationResult result =
       verifyInsertSliceOp(getSourceType(), getDestType(), getStaticOffsets(),
                           getStaticSizes(), getStaticStrides(), &expectedType);
-  return produceSliceErrorMsg(result, *this, expectedType);
+  if (failed(produceSliceErrorMsg(result, *this, expectedType)))
+    return failure();
+  return verifySliceOffsetsAndSizes(*this, getDestType(), getStaticOffsets(),
+                                    getStaticSizes());
 }
 
 void ParallelInsertSliceOp::getCanonicalizationPatterns(
     RewritePatternSet &results, MLIRContext *context) {
   results.add<InsertSliceOpConstantArgumentFolder<ParallelInsertSliceOp>,
               InsertSliceOpCastFolder<ParallelInsertSliceOp>,
               InsertSliceOpSourceCastInserter<ParallelInsertSliceOp>>(context);
 }

mlir/test/Dialect/Linalg/vectorize-tensor-extract-masked.mlir

  ^bb1(%arg1: !transform.any_op):
    %0 = transform.structured.match ops{["linalg.generic"]} in %arg1 : (!transform.any_op) -> !transform.any_op
    transform.structured.masked_vectorize %0 vector_sizes [1, 4] { vectorize_nd_extract } : !transform.any_op
 }
 
 // -----
 
 func.func @masked_vectorize_nd_tensor_extract_with_affine_apply_gather(%6: tensor<80x16xf32>, %arg0: index, %extracted_slice : tensor<1x3xf32>) -> tensor<1x3xf32> {
-  %c16 = arith.constant 16 : index
+  %c15 = arith.constant 15 : index
   %1 = linalg.generic {
     indexing_maps = [affine_map<(d0, d1) -> (d0, d1)>],
     iterator_types = ["parallel", "parallel"]
   } outs(%extracted_slice : tensor<1x3xf32>) {
   ^bb0(%out: f32):
     %2 = linalg.index 1 : index
     %3 = affine.apply affine_map<(d0, d1) -> (d0 + d1)>(%2, %arg0)
-    %extracted = tensor.extract %6[%3, %c16] : tensor<80x16xf32>
+    %extracted = tensor.extract %6[%3, %c15] : tensor<80x16xf32>
     linalg.yield %extracted : f32
   } -> tensor<1x3xf32>
   return %1 : tensor<1x3xf32>
 }
 
 // CHECK-LABEL:   func.func @masked_vectorize_nd_tensor_extract_with_affine_apply_gather
 // CHECK-DAG:       %[[VAL_4:.*]] = arith.constant 1 : index
 // CHECK-DAG:       %[[VAL_5:.*]] = arith.constant 3 : index

mlir/test/Dialect/Linalg/vectorize-tensor-extract.mlir

^bb1(%arg1: !transform.any_op):
    %1 = get_parent_op %0 {isolated_from_above} : (!transform.any_op) -> !transform.any_op
    %2 = transform.structured.vectorize %1 { vectorize_nd_extract } : (!transform.any_op) -> !transform.any_op
  }
 
 // -----
 
 // The vectorizer converts `affine.apply` so that the subsequent Ops can be vectorised based on the converted ops. Gather load.
 func.func @vectorize_nd_tensor_extract_with_affine_apply_gather(%6: tensor<80x16xf32>, %arg0: index, %extracted_slice : tensor<1x4xf32>) -> tensor<1x4xf32> {
-  %c16 = arith.constant 16 : index
+  %c15 = arith.constant 15 : index
   %1 = linalg.generic {
     indexing_maps = [affine_map<(d0, d1) -> (d0, d1)>],
     iterator_types = ["parallel", "parallel"]
   } outs(%extracted_slice : tensor<1x4xf32>) {
   ^bb0(%out: f32):
     %2 = linalg.index 1 : index
     %3 = affine.apply affine_map<(d0, d1) -> (d0 + d1)>(%2, %arg0)
-    %extracted = tensor.extract %6[%3, %c16] : tensor<80x16xf32>
+    %extracted = tensor.extract %6[%3, %c15] : tensor<80x16xf32>
     linalg.yield %extracted : f32
   } -> tensor<1x4xf32>
   return %1 : tensor<1x4xf32>
 }
 
 // CHECK-LABEL:   func.func @vectorize_nd_tensor_extract_with_affine_apply_gather(
 // CHECK-SAME:                                                                    %[[VAL_0:.*]]: tensor<80x16xf32>,
 // CHECK-SAME:                                                                    %[[VAL_1:.*]]: index,
 // CHECK-SAME:                                                                    %[[VAL_2:.*]]: tensor<1x4xf32>) -> tensor<1x4xf32> {
 // CHECK:           %[[VAL_3:.*]] = arith.constant dense<[0, 1, 2, 3]> : vector<4xindex>
 // CHECK:           %[[VAL_4:.*]] = arith.constant dense<true> : vector<1x4xi1>
 // CHECK:           %[[VAL_5:.*]] = arith.constant dense<0.000000e+00> : vector<1x4xf32>
 // CHECK:           %[[VAL_6:.*]] = arith.constant 0 : index
-// CHECK:           %[[VAL_7:.*]] = arith.constant dense<16> : vector<1x4xindex>
+// CHECK:           %[[VAL_7a:.*]] = arith.constant dense<16> : vector<1x4xindex>
+// CHECK:           %[[VAL_7b:.*]] = arith.constant dense<15> : vector<1x4xindex>
 // CHECK:           %[[VAL_8:.*]] = vector.broadcast %[[VAL_1]] : index to vector<4xindex>
 // CHECK:           %[[VAL_9:.*]] = arith.addi %[[VAL_8]], %[[VAL_3]] : vector<4xindex>
 // CHECK:           %[[VAL_10:.*]] = vector.broadcast %[[VAL_9]] : vector<4xindex> to vector<1x4xindex>
-// CHECK:           %[[VAL_11:.*]] = arith.muli %[[VAL_10]], %[[VAL_7]] : vector<1x4xindex>
-// CHECK:           %[[VAL_12:.*]] = arith.addi %[[VAL_11]], %[[VAL_7]] : vector<1x4xindex>
+// CHECK:           %[[VAL_11:.*]] = arith.muli %[[VAL_10]], %[[VAL_7a]] : vector<1x4xindex>
+// CHECK:           %[[VAL_12:.*]] = arith.addi %[[VAL_11]], %[[VAL_7b]] : vector<1x4xindex>
 // CHECK:           %[[VAL_13:.*]] = vector.gather %[[VAL_0]]{{\[}}%[[VAL_6]], %[[VAL_6]]] {{\[}}%[[VAL_12]]], %[[VAL_4]], %[[VAL_5]] : tensor<80x16xf32>, vector<1x4xindex>, vector<1x4xi1>, vector<1x4xf32> into vector<1x4xf32>
 // CHECK:           %[[VAL_14:.*]] = vector.transfer_write %[[VAL_13]], %[[VAL_2]]{{\[}}%[[VAL_6]], %[[VAL_6]]] {in_bounds = [true, true]} : vector<1x4xf32>, tensor<1x4xf32>
 // CHECK:           return %[[VAL_14]] : tensor<1x4xf32>
 // CHECK:         }
 
 transform.sequence failures(propagate) {
  ^bb1(%arg1: !transform.any_op):
    %0 = transform.structured.match ops{["linalg.generic"]} in %arg1 : (!transform.any_op) -> !transform.any_op

mlir/test/Dialect/Tensor/fold-tensor-subset-ops.mlir

func.func @insert_slice_of_transfer_write_rank_extending(%t1 : tensor<?x?x12xf32>, %v : vector<5x6xf32>, %s : index, %t2 : tensor<5x6xf32>) -> tensor<?x?x12xf32> {
   %c0 = arith.constant 0 : index
   %0 = vector.transfer_write %v, %t2[%c0, %c0] {in_bounds = [true, true]} : vector<5x6xf32>, tensor<5x6xf32>
   %1 = tensor.insert_slice %0 into %t1[4, 3, %s] [1, 5, 6] [1, 1, 1] : tensor<5x6xf32> into tensor<?x?x12xf32>
   return %1 : tensor<?x?x12xf32>
 }
 
 // -----
 
-//       CHECK: #[[$map:.*]] = affine_map<()[s0] -> (s0 + 2)>
 // CHECK-LABEL: func @insert_slice_of_insert_slice(
 //  CHECK-SAME:     %[[t:[0-9a-z]*]]: tensor<f32>
 //  CHECK-SAME:     %[[r1:[0-9a-z]*]]: tensor<1x14xf32>
 //  CHECK-SAME:     %[[pos:[0-9a-z]*]]: index
-//       CHECK:   %[[add:.*]] = affine.apply #[[$map]]()[%[[pos]]]
-//       CHECK:   tensor.insert_slice %[[t]] into %[[r1]][4, %[[add]]] [1, 1] [1, 1] : tensor<f32> into tensor<1x14xf32>
+//       CHECK:   tensor.insert_slice %[[t]] into %[[r1]][0, %[[pos]]] [1, 1] [1, 1] : tensor<f32> into tensor<1x14xf32>
 func.func @insert_slice_of_insert_slice(%t: tensor<f32>, %r0: tensor<1x1xf32>, %r1: tensor<1x14xf32>, %pos: index)
     -> tensor<1x14xf32> 
 {
-  %0 = tensor.insert_slice %t into %r0[1, 2] [1, 1] [1, 1] 
+  %0 = tensor.insert_slice %t into %r0[0, 0] [1, 1] [1, 1] 
     : tensor<f32> into tensor<1x1xf32>
-  %1 = tensor.insert_slice %0 into %r1[3, %pos] [1, 1] [1, 1] 
+  %1 = tensor.insert_slice %0 into %r1[0, %pos] [1, 1] [1, 1] 
     : tensor<1x1xf32> into tensor<1x14xf32>
   return %1 : tensor<1x14xf32>
 }
 
 // -----
 
 // CHECK-LABEL: func @insert_slice_of_insert_slice(
 //  CHECK-SAME:     %[[t:[0-9a-z]*]]: tensor<f32>
 //  CHECK-SAME:     %[[r1:[0-9a-z]*]]: tensor<1x14xf32>
 //  CHECK-SAME:     %[[pos:[0-9a-z]*]]: index
-//       CHECK:   tensor.insert_slice %[[t]] into %[[r1]][5, %[[pos]]] [1, 1] [1, 1] : tensor<f32> into tensor<1x14xf32>
+//       CHECK:   tensor.insert_slice %[[t]] into %[[r1]][0, %[[pos]]] [1, 1] [1, 1] : tensor<f32> into tensor<1x14xf32>
 func.func @insert_slice_of_insert_slice(%t: tensor<f32>, %r0: tensor<1xf32>, %r1: tensor<1x14xf32>, %pos: index)
     -> tensor<1x14xf32> 
 {
-  %0 = tensor.insert_slice %t into %r0[2] [1] [1] 
+  %0 = tensor.insert_slice %t into %r0[0] [1] [1] 
     : tensor<f32> into tensor<1xf32>
-  %1 = tensor.insert_slice %0 into %r1[3, %pos] [1, 1] [1, 1] 
+  %1 = tensor.insert_slice %0 into %r1[0, %pos] [1, 1] [1, 1] 
     : tensor<1xf32> into tensor<1x14xf32>
   return %1 : tensor<1x14xf32>
 }
 
 // -----
 
 // This test fails to fold because the size `4` and `%pos` do not match: 
 // this requires a copy

mlir/test/Dialect/Tensor/invalid.mlir

func.func @extract_slice_wrong_dynamic_type(%t: tensor<8x16x4xf32>, %idx : index) {
   %0 = tensor.extract_slice %t[0, 2, 0][4, 4, 4][1, 1, 1]
     : tensor<8x16x4xf32> to tensor<?x4x4xf32>
 
   return
 }
 
 // -----
 
+func.func @extract_slice_out_of_bounds(%t: tensor<5xf32>) {
+  // expected-error @+1 {{dimension #0 runs out of bounds}}
+  %0 = tensor.extract_slice %t[5][4][1] : tensor<5xf32> to tensor<4xf32>
+
+  return
+}
+
+// -----
+
+func.func @extract_slice_out_of_bounds_2(%t: tensor<5xf32>) {
+  // expected-error @+1 {{dimension #0 runs out of bounds}}
+  %0 = tensor.extract_slice %t[3][4][1] : tensor<5xf32> to tensor<4xf32>
+
+  return
+}
+
+// -----
+
+func.func @extract_slice_negative(%t: tensor<5xf32>) {
+  // expected-error @+1 {{offset #0 is negative}}
+  %0 = tensor.extract_slice %t[-1][4][1] : tensor<5xf32> to tensor<4xf32>
+
+  return
+}
+
+// -----
+
 func.func @insert_slice_wrong_result_rank(%t1: tensor<?xf32>, %t2: tensor<?x?xf32>, %idx : index) {
   // expected-error @+1 {{expected rank to be smaller or equal to the other rank.}}
   %0 = tensor.insert_slice %t2 into %t1[0][4][1] : tensor<?x?xf32> into tensor<?xf32>
 
   return
 }
 
 // -----
func.func @insert_slice_wrong_dynamic_type(%t1: tensor<?x4x4xf32>, %t2: tensor<8x16x4xf32>, %idx : index) {
   %0 = tensor.insert_slice %t1 into %t2[0, 2, 0][4, 4, 4][1, 1, 1]
     : tensor<?x4x4xf32> into tensor<8x16x4xf32>
 
   return
 }
 
 // -----
 
+func.func @insert_slice_out_of_bounds(%t1: tensor<4x4xf32>, %t2: tensor<8x16x2xf32>) {
+  // expected-error @+1 {{dimension #2 runs out of bounds}}
+  %0 = tensor.insert_slice %t1 into %t2[0, 0, 3][4, 4, 1][1, 1, 1]
+    : tensor<4x4xf32> into tensor<8x16x2xf32>
+
+  return
+}
+
+// -----
+
+func.func @insert_slice_out_of_bounds_2(%t1: tensor<4x4xf32>, %t2: tensor<8x16x2xf32>) {
+  // expected-error @+1 {{dimension #1 runs out of bounds}}
+  %0 = tensor.insert_slice %t1 into %t2[0, 15, 0][4, 4, 1][1, 1, 1]
+    : tensor<4x4xf32> into tensor<8x16x2xf32>
+
+  return
+}
+
+// -----
+
+func.func @insert_slice_negative(%t1: tensor<4x4xf32>, %t2: tensor<8x16x2xf32>) {
+  // expected-error @+1 {{offset #2 is negative}}
+  %0 = tensor.insert_slice %t1 into %t2[0, 0, -1][4, 4, 1][1, 1, 1]
+    : tensor<4x4xf32> into tensor<8x16x2xf32>
+
+  return
+}
+
+// -----
+
 func.func @illegal_expanding_reshape_dynamic_tensor
   (%arg0: tensor<?x?x?xf32>) -> tensor<?x?x?x4x?xf32> {
   // expected-error @+1 {{invalid to have a single dimension (2) expanded into multiple dynamic dims (2,4)}}
   %0 = tensor.expand_shape %arg0 [[0], [1], [2, 3, 4]]
       : tensor<?x?x?xf32> into tensor<?x?x?x4x?xf32>
   return %0 : tensor<?x?x?x4x?xf32>
 }