[Mlir-commits] [flang] [mlir] [clang] [compiler-rt] [llvm] [clang-tools-extra] [mlir] Verify non-negative `offset` and `size` (PR #72059)
Rik Huijzer
llvmlistbot at llvm.org
Mon Nov 13 06:27:22 PST 2023
https://github.com/rikhuijzer updated https://github.com/llvm/llvm-project/pull/72059
>From 8f90ae9113229faf5ab4b859e5c70f23853f4db8 Mon Sep 17 00:00:00 2001
From: Rik Huijzer <github at huijzer.xyz>
Date: Sat, 11 Nov 2023 20:43:31 +0100
Subject: [PATCH 01/11] [mlir][memref] Verify subview offsets and sizes
---
mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 12 +++++++++++-
mlir/test/Dialect/MemRef/invalid.mlir | 16 ++++++++++++++++
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 215a8f5e7d18be0..dc3e04be83fc1c9 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2842,8 +2842,18 @@ static LogicalResult produceSubViewErrorMsg(SliceVerificationResult result,
llvm_unreachable("unexpected subview verification result");
}
-/// Verifier for SubViewOp.
LogicalResult SubViewOp::verify() {
+ for (int64_t offset : getStaticOffsets()) {
+ if (offset < 0 && !ShapedType::isDynamic(offset))
+ return emitError("expected subview offsets to be non-negative, but got ")
+ << offset;
+ }
+ for (int64_t size : getStaticSizes()) {
+ if (size < 1 && !ShapedType::isDynamic(size))
+ return emitError("expected subview sizes to be positive, but got ")
+ << size;
+ }
+
MemRefType baseType = getSourceType();
MemRefType subViewType = getType();
diff --git a/mlir/test/Dialect/MemRef/invalid.mlir b/mlir/test/Dialect/MemRef/invalid.mlir
index cb5977e302a993f..17f5fbea3bda17a 100644
--- a/mlir/test/Dialect/MemRef/invalid.mlir
+++ b/mlir/test/Dialect/MemRef/invalid.mlir
@@ -611,6 +611,22 @@ func.func @invalid_view(%arg0 : index, %arg1 : index, %arg2 : index) {
// -----
+func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, strided<[1024, 1], offset: 2304>> {
+ // expected-error at +1 {{expected subview offsets to be non-negative, but got -1}}
+ %0 = memref.subview %input[-1, 256] [2, 256] [1, 1] : memref<4x1024xf32> to memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+ return %0 : memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+}
+
+// -----
+
+func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, strided<[1024, 1], offset: 2304>> {
+ // expected-error at +1 {{expected subview sizes to be positive, but got 0}}
+ %0 = memref.subview %input[2, 256] [0, 256] [1, 1] : memref<4x1024xf32> to memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+ return %0 : memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+}
+
+// -----
+
func.func @invalid_subview(%arg0 : index, %arg1 : index, %arg2 : index) {
%0 = memref.alloc() : memref<8x16x4xf32>
// expected-error at +1 {{expected mixed offsets rank to match mixed sizes rank (2 vs 3) so the rank of the result type is well-formed}}
>From 32adf97c8d19e29e4673c772bd190c0eb0b413e9 Mon Sep 17 00:00:00 2001
From: Rik Huijzer <github at huijzer.xyz>
Date: Sun, 12 Nov 2023 09:10:00 +0100
Subject: [PATCH 02/11] Allow zero sizes
---
mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 5 +++--
mlir/test/Dialect/MemRef/invalid.mlir | 4 ++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index dc3e04be83fc1c9..a24467a321a70c5 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2849,8 +2849,8 @@ LogicalResult SubViewOp::verify() {
<< offset;
}
for (int64_t size : getStaticSizes()) {
- if (size < 1 && !ShapedType::isDynamic(size))
- return emitError("expected subview sizes to be positive, but got ")
+ if (size < 0 && !ShapedType::isDynamic(size))
+ return emitError("expected subview sizes to be non-negative, but got ")
<< size;
}
@@ -3103,6 +3103,7 @@ struct SubViewReturnTypeCanonicalizer {
MemRefType operator()(SubViewOp op, ArrayRef<OpFoldResult> mixedOffsets,
ArrayRef<OpFoldResult> mixedSizes,
ArrayRef<OpFoldResult> mixedStrides) {
+
// Infer a memref type without taking into account any rank reductions.
MemRefType nonReducedType = cast<MemRefType>(SubViewOp::inferResultType(
op.getSourceType(), mixedOffsets, mixedSizes, mixedStrides));
diff --git a/mlir/test/Dialect/MemRef/invalid.mlir b/mlir/test/Dialect/MemRef/invalid.mlir
index 17f5fbea3bda17a..38c0bcc3f2491c2 100644
--- a/mlir/test/Dialect/MemRef/invalid.mlir
+++ b/mlir/test/Dialect/MemRef/invalid.mlir
@@ -620,8 +620,8 @@ func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, stri
// -----
func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, strided<[1024, 1], offset: 2304>> {
- // expected-error at +1 {{expected subview sizes to be positive, but got 0}}
- %0 = memref.subview %input[2, 256] [0, 256] [1, 1] : memref<4x1024xf32> to memref<2x256xf32, strided<[1024, 1], offset: 2304>>
+ // expected-error at +1 {{expected subview sizes to be non-negative, but got -1}}
+ %0 = memref.subview %input[2, 256] [-1, 256] [1, 1] : memref<4x1024xf32> to memref<2x256xf32, strided<[1024, 1], offset: 2304>>
return %0 : memref<2x256xf32, strided<[1024, 1], offset: 2304>>
}
>From 30fc408fec59c656c90b3bd927b1cd6e93b18e3d Mon Sep 17 00:00:00 2001
From: Rik Huijzer <github at huijzer.xyz>
Date: Sun, 12 Nov 2023 15:28:55 +0100
Subject: [PATCH 03/11] Catch negative size earlier
This now does catch the negative size inside the interface so that an
`op.emitError` can be thrown. That works, but then continues to
return an empty result?
Instead, the interface can probably be refactored first because it's
very restrictive in its current form.
---
mlir/include/mlir/Interfaces/ViewLikeInterface.h | 14 ++++++++++++++
mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 15 +++++++++++++++
mlir/lib/Dialect/Tensor/IR/TensorOps.cpp | 5 ++++-
3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/mlir/include/mlir/Interfaces/ViewLikeInterface.h b/mlir/include/mlir/Interfaces/ViewLikeInterface.h
index a114e9af126f112..80d5b656cccb066 100644
--- a/mlir/include/mlir/Interfaces/ViewLikeInterface.h
+++ b/mlir/include/mlir/Interfaces/ViewLikeInterface.h
@@ -19,6 +19,7 @@
#include "mlir/IR/BuiltinTypes.h"
#include "mlir/IR/OpImplementation.h"
#include "mlir/IR/PatternMatch.h"
+#include <_types/_uint64_t.h>
namespace mlir {
@@ -72,6 +73,19 @@ class OpWithOffsetSizesAndStridesConstantArgumentFolder final
failed(foldDynamicIndexList(mixedStrides)))
return failure();
+ SmallVector<int64_t> staticOffsets, staticSizes, staticStrides;
+ SmallVector<Value> dynamicOffsets, dynamicSizes, dynamicStrides;
+ dispatchIndexOpFoldResults(mixedOffsets, dynamicOffsets, staticOffsets);
+ dispatchIndexOpFoldResults(mixedSizes, dynamicSizes, staticSizes);
+ dispatchIndexOpFoldResults(mixedStrides, dynamicStrides, staticStrides);
+
+ for (int64_t size : staticSizes) {
+ if (size < 0 && !ShapedType::isDynamic(size)) {
+ return op.emitError("expected non-negative size, but got ")
+ << size;;
+ }
+ }
+
// Create the new op in canonical form.
auto resultType =
ResultTypeFn()(op, mixedOffsets, mixedSizes, mixedStrides);
diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index a24467a321a70c5..477f9e050cafd1b 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2843,6 +2843,8 @@ static LogicalResult produceSubViewErrorMsg(SliceVerificationResult result,
}
LogicalResult SubViewOp::verify() {
+ llvm::outs() << "SubViewOp::verify\n";
+
for (int64_t offset : getStaticOffsets()) {
if (offset < 0 && !ShapedType::isDynamic(offset))
return emitError("expected subview offsets to be non-negative, but got ")
@@ -3103,6 +3105,19 @@ struct SubViewReturnTypeCanonicalizer {
MemRefType operator()(SubViewOp op, ArrayRef<OpFoldResult> mixedOffsets,
ArrayRef<OpFoldResult> mixedSizes,
ArrayRef<OpFoldResult> mixedStrides) {
+ SmallVector<int64_t> staticOffsets, staticSizes, staticStrides;
+ SmallVector<Value> dynamicOffsets, dynamicSizes, dynamicStrides;
+ dispatchIndexOpFoldResults(mixedOffsets, dynamicOffsets, staticOffsets);
+ dispatchIndexOpFoldResults(mixedSizes, dynamicSizes, staticSizes);
+ dispatchIndexOpFoldResults(mixedStrides, dynamicStrides, staticStrides);
+
+ for (int64_t size : staticSizes) {
+ if (size < 0 && !ShapedType::isDynamic(size)) {
+ llvm::dbgs() << "expected subview sizes to be non-negative, but got "
+ << size << "\n";
+ return {};
+ }
+ }
// Infer a memref type without taking into account any rank reductions.
MemRefType nonReducedType = cast<MemRefType>(SubViewOp::inferResultType(
diff --git a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
index 6fc45379111fc34..507123463ec941a 100644
--- a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
+++ b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
@@ -1163,6 +1163,8 @@ static void operandsAndShape(TensorType resultType,
}
LogicalResult GenerateOp::verify() {
+ llvm::outs() << "GenerateOp::verify()\n";
+
// Ensure that the tensor type has as many dynamic dimensions as are
// specified by the operands.
RankedTensorType resultType = llvm::cast<RankedTensorType>(getType());
@@ -1173,6 +1175,7 @@ LogicalResult GenerateOp::verify() {
SmallVector<Value> newOperands;
SmallVector<int64_t> newShape;
operandsAndShape(resultType, getDynamicExtents(), newOperands, newShape);
+
for (int64_t newdim : newShape) {
if (newdim < 0 && !ShapedType::isDynamic(newdim))
return emitError("tensor dimensions must be non-negative");
@@ -1242,7 +1245,7 @@ struct StaticTensorGenerate : public OpRewritePattern<GenerateOp> {
for (int64_t newdim : newShape) {
// This check also occurs in the verifier, but we need it here too
- // since intermediate passes may have some replaced dynamic dimensions
+ // since intermediate passes may have replaced some dynamic dimensions
// by constants.
if (newdim < 0 && !ShapedType::isDynamic(newdim))
return failure();
>From 7323aa03ffe2d541fb0e555755edc11c132e3f72 Mon Sep 17 00:00:00 2001
From: Rik Huijzer <github at huijzer.xyz>
Date: Sun, 12 Nov 2023 17:48:11 +0100
Subject: [PATCH 04/11] Add assertion in outer `inferResultType
---
.../mlir/Interfaces/ViewLikeInterface.h | 13 ---------
mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 29 +++++++++----------
mlir/lib/Dialect/Tensor/IR/TensorOps.cpp | 2 --
3 files changed, 13 insertions(+), 31 deletions(-)
diff --git a/mlir/include/mlir/Interfaces/ViewLikeInterface.h b/mlir/include/mlir/Interfaces/ViewLikeInterface.h
index 80d5b656cccb066..7867fb2429ca3df 100644
--- a/mlir/include/mlir/Interfaces/ViewLikeInterface.h
+++ b/mlir/include/mlir/Interfaces/ViewLikeInterface.h
@@ -73,19 +73,6 @@ class OpWithOffsetSizesAndStridesConstantArgumentFolder final
failed(foldDynamicIndexList(mixedStrides)))
return failure();
- SmallVector<int64_t> staticOffsets, staticSizes, staticStrides;
- SmallVector<Value> dynamicOffsets, dynamicSizes, dynamicStrides;
- dispatchIndexOpFoldResults(mixedOffsets, dynamicOffsets, staticOffsets);
- dispatchIndexOpFoldResults(mixedSizes, dynamicSizes, staticSizes);
- dispatchIndexOpFoldResults(mixedStrides, dynamicStrides, staticStrides);
-
- for (int64_t size : staticSizes) {
- if (size < 0 && !ShapedType::isDynamic(size)) {
- return op.emitError("expected non-negative size, but got ")
- << size;;
- }
- }
-
// Create the new op in canonical form.
auto resultType =
ResultTypeFn()(op, mixedOffsets, mixedSizes, mixedStrides);
diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 477f9e050cafd1b..6f2f08897784569 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2621,6 +2621,19 @@ Type SubViewOp::inferResultType(MemRefType sourceMemRefType,
dispatchIndexOpFoldResults(offsets, dynamicOffsets, staticOffsets);
dispatchIndexOpFoldResults(sizes, dynamicSizes, staticSizes);
dispatchIndexOpFoldResults(strides, dynamicStrides, staticStrides);
+
+ // Double-check the offsets, sizes, and strides after constant folding.
+ // This allows throwing a more informative assertion message than
+ // what would be thrown at a later point.
+ for (int64_t offset : staticOffsets) {
+ if (!ShapedType::isDynamic(offset))
+ assert(offset >= 0 && "expected subview offsets to be non-negative");
+ }
+ for (int64_t size : staticSizes) {
+ if (!ShapedType::isDynamic(size))
+ assert(size >= 0 && "expected subview sizes to be non-negative");
+ }
+
return SubViewOp::inferResultType(sourceMemRefType, staticOffsets,
staticSizes, staticStrides);
}
@@ -2843,8 +2856,6 @@ static LogicalResult produceSubViewErrorMsg(SliceVerificationResult result,
}
LogicalResult SubViewOp::verify() {
- llvm::outs() << "SubViewOp::verify\n";
-
for (int64_t offset : getStaticOffsets()) {
if (offset < 0 && !ShapedType::isDynamic(offset))
return emitError("expected subview offsets to be non-negative, but got ")
@@ -3105,20 +3116,6 @@ struct SubViewReturnTypeCanonicalizer {
MemRefType operator()(SubViewOp op, ArrayRef<OpFoldResult> mixedOffsets,
ArrayRef<OpFoldResult> mixedSizes,
ArrayRef<OpFoldResult> mixedStrides) {
- SmallVector<int64_t> staticOffsets, staticSizes, staticStrides;
- SmallVector<Value> dynamicOffsets, dynamicSizes, dynamicStrides;
- dispatchIndexOpFoldResults(mixedOffsets, dynamicOffsets, staticOffsets);
- dispatchIndexOpFoldResults(mixedSizes, dynamicSizes, staticSizes);
- dispatchIndexOpFoldResults(mixedStrides, dynamicStrides, staticStrides);
-
- for (int64_t size : staticSizes) {
- if (size < 0 && !ShapedType::isDynamic(size)) {
- llvm::dbgs() << "expected subview sizes to be non-negative, but got "
- << size << "\n";
- return {};
- }
- }
-
// Infer a memref type without taking into account any rank reductions.
MemRefType nonReducedType = cast<MemRefType>(SubViewOp::inferResultType(
op.getSourceType(), mixedOffsets, mixedSizes, mixedStrides));
diff --git a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
index 507123463ec941a..3c4fc1b58ed26cf 100644
--- a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
+++ b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
@@ -1163,8 +1163,6 @@ static void operandsAndShape(TensorType resultType,
}
LogicalResult GenerateOp::verify() {
- llvm::outs() << "GenerateOp::verify()\n";
-
// Ensure that the tensor type has as many dynamic dimensions as are
// specified by the operands.
RankedTensorType resultType = llvm::cast<RankedTensorType>(getType());
>From 1859a9b3e0969202d7f97c78ed083f5f179e2632 Mon Sep 17 00:00:00 2001
From: Rik Huijzer <github at huijzer.xyz>
Date: Sun, 12 Nov 2023 17:52:53 +0100
Subject: [PATCH 05/11] Update comment
---
mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 6f2f08897784569..69ca8275e736636 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2622,9 +2622,7 @@ Type SubViewOp::inferResultType(MemRefType sourceMemRefType,
dispatchIndexOpFoldResults(sizes, dynamicSizes, staticSizes);
dispatchIndexOpFoldResults(strides, dynamicStrides, staticStrides);
- // Double-check the offsets, sizes, and strides after constant folding.
- // This allows throwing a more informative assertion message than
- // what would be thrown at a later point.
+ // TODO: Handle these situations gracefully in the canonicalizer.
for (int64_t offset : staticOffsets) {
if (!ShapedType::isDynamic(offset))
assert(offset >= 0 && "expected subview offsets to be non-negative");
@@ -2633,7 +2631,6 @@ Type SubViewOp::inferResultType(MemRefType sourceMemRefType,
if (!ShapedType::isDynamic(size))
assert(size >= 0 && "expected subview sizes to be non-negative");
}
-
return SubViewOp::inferResultType(sourceMemRefType, staticOffsets,
staticSizes, staticStrides);
}
>From 68778fb9e1b7bc2dec5d0609c0adbb5b0a1875e1 Mon Sep 17 00:00:00 2001
From: Rik Huijzer <github at huijzer.xyz>
Date: Sun, 12 Nov 2023 17:58:21 +0100
Subject: [PATCH 06/11] Revert some changes
---
mlir/include/mlir/Interfaces/ViewLikeInterface.h | 1 -
mlir/lib/Dialect/Tensor/IR/TensorOps.cpp | 1 -
2 files changed, 2 deletions(-)
diff --git a/mlir/include/mlir/Interfaces/ViewLikeInterface.h b/mlir/include/mlir/Interfaces/ViewLikeInterface.h
index 7867fb2429ca3df..a114e9af126f112 100644
--- a/mlir/include/mlir/Interfaces/ViewLikeInterface.h
+++ b/mlir/include/mlir/Interfaces/ViewLikeInterface.h
@@ -19,7 +19,6 @@
#include "mlir/IR/BuiltinTypes.h"
#include "mlir/IR/OpImplementation.h"
#include "mlir/IR/PatternMatch.h"
-#include <_types/_uint64_t.h>
namespace mlir {
diff --git a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
index 3c4fc1b58ed26cf..16e4c99a82a5615 100644
--- a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
+++ b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
@@ -1240,7 +1240,6 @@ struct StaticTensorGenerate : public OpRewritePattern<GenerateOp> {
SmallVector<Value> newOperands;
SmallVector<int64_t> newShape;
operandsAndShape(resultType, dynamicExtents, newOperands, newShape);
-
for (int64_t newdim : newShape) {
// This check also occurs in the verifier, but we need it here too
// since intermediate passes may have replaced some dynamic dimensions
>From 6ef2ddaf31a5acea6a65e46a07483da2cf2b99a4 Mon Sep 17 00:00:00 2001
From: Rik Huijzer <github at huijzer.xyz>
Date: Sun, 12 Nov 2023 17:59:04 +0100
Subject: [PATCH 07/11] Revert some changes
---
mlir/lib/Dialect/Tensor/IR/TensorOps.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
index 16e4c99a82a5615..ab915c0e786aeb5 100644
--- a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
+++ b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
@@ -1173,7 +1173,6 @@ LogicalResult GenerateOp::verify() {
SmallVector<Value> newOperands;
SmallVector<int64_t> newShape;
operandsAndShape(resultType, getDynamicExtents(), newOperands, newShape);
-
for (int64_t newdim : newShape) {
if (newdim < 0 && !ShapedType::isDynamic(newdim))
return emitError("tensor dimensions must be non-negative");
@@ -1240,6 +1239,7 @@ struct StaticTensorGenerate : public OpRewritePattern<GenerateOp> {
SmallVector<Value> newOperands;
SmallVector<int64_t> newShape;
operandsAndShape(resultType, dynamicExtents, newOperands, newShape);
+
for (int64_t newdim : newShape) {
// This check also occurs in the verifier, but we need it here too
// since intermediate passes may have replaced some dynamic dimensions
>From 4036cd14622b16d260764d5f6b7d7090bf10a775 Mon Sep 17 00:00:00 2001
From: Rik Huijzer <github at huijzer.xyz>
Date: Sun, 12 Nov 2023 18:49:45 +0100
Subject: [PATCH 08/11] Remove comment
---
mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 69ca8275e736636..86d6f6bf6ad5388 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2622,7 +2622,6 @@ Type SubViewOp::inferResultType(MemRefType sourceMemRefType,
dispatchIndexOpFoldResults(sizes, dynamicSizes, staticSizes);
dispatchIndexOpFoldResults(strides, dynamicStrides, staticStrides);
- // TODO: Handle these situations gracefully in the canonicalizer.
for (int64_t offset : staticOffsets) {
if (!ShapedType::isDynamic(offset))
assert(offset >= 0 && "expected subview offsets to be non-negative");
>From e4e40f2529e54c28e30f511c420e9f80eceafc65 Mon Sep 17 00:00:00 2001
From: Rik Huijzer <github at huijzer.xyz>
Date: Mon, 13 Nov 2023 12:01:04 +0100
Subject: [PATCH 09/11] Move logic to `verifyOffsetSizeAndStrideOp`
---
mlir/include/mlir/Interfaces/ViewLikeInterface.td | 1 +
mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 11 -----------
mlir/lib/Interfaces/ViewLikeInterface.cpp | 11 +++++++++++
mlir/test/Dialect/MemRef/invalid.mlir | 4 ++--
4 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/mlir/include/mlir/Interfaces/ViewLikeInterface.td b/mlir/include/mlir/Interfaces/ViewLikeInterface.td
index 8029380a2c8f1dc..ea5bb1b5ac48535 100644
--- a/mlir/include/mlir/Interfaces/ViewLikeInterface.td
+++ b/mlir/include/mlir/Interfaces/ViewLikeInterface.td
@@ -56,6 +56,7 @@ def OffsetSizeAndStrideOpInterface : OpInterface<"OffsetSizeAndStrideOpInterface
for each dynamic offset (resp. size, stride).
5. `offsets`, `sizes` and `strides` operands are specified in this order
at operand index starting at `getOffsetSizeAndStrideStartOperandIndex`.
+ 6. `offsets` and `sizes` operands are non-negative.
This interface is useful to factor out common behavior and provide support
for carrying or injecting static behavior through the use of the static
diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 901784d7e369c8e..b66b413fbf89872 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2852,17 +2852,6 @@ static LogicalResult produceSubViewErrorMsg(SliceVerificationResult result,
}
LogicalResult SubViewOp::verify() {
- for (int64_t offset : getStaticOffsets()) {
- if (offset < 0 && !ShapedType::isDynamic(offset))
- return emitError("expected subview offsets to be non-negative, but got ")
- << offset;
- }
- for (int64_t size : getStaticSizes()) {
- if (size < 0 && !ShapedType::isDynamic(size))
- return emitError("expected subview sizes to be non-negative, but got ")
- << size;
- }
-
MemRefType baseType = getSourceType();
MemRefType subViewType = getType();
diff --git a/mlir/lib/Interfaces/ViewLikeInterface.cpp b/mlir/lib/Interfaces/ViewLikeInterface.cpp
index 9247f571d9ab2c3..7c5905010eb4137 100644
--- a/mlir/lib/Interfaces/ViewLikeInterface.cpp
+++ b/mlir/lib/Interfaces/ViewLikeInterface.cpp
@@ -66,6 +66,17 @@ mlir::detail::verifyOffsetSizeAndStrideOp(OffsetSizeAndStrideOpInterface op) {
if (failed(verifyListOfOperandsOrIntegers(
op, "stride", maxRanks[2], op.getStaticStrides(), op.getStrides())))
return failure();
+
+ for (int64_t offset : op.getStaticOffsets()) {
+ if (offset < 0 && !ShapedType::isDynamic(offset))
+ return op->emitError("expected offsets to be non-negative, but got ")
+ << offset;
+ }
+ for (int64_t size : op.getStaticSizes()) {
+ if (size < 0 && !ShapedType::isDynamic(size))
+ return op->emitError("expected sizes to be non-negative, but got ")
+ << size;
+ }
return success();
}
diff --git a/mlir/test/Dialect/MemRef/invalid.mlir b/mlir/test/Dialect/MemRef/invalid.mlir
index 38c0bcc3f2491c2..55b759cbb3ce7cd 100644
--- a/mlir/test/Dialect/MemRef/invalid.mlir
+++ b/mlir/test/Dialect/MemRef/invalid.mlir
@@ -612,7 +612,7 @@ func.func @invalid_view(%arg0 : index, %arg1 : index, %arg2 : index) {
// -----
func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, strided<[1024, 1], offset: 2304>> {
- // expected-error at +1 {{expected subview offsets to be non-negative, but got -1}}
+ // expected-error at +1 {{expected offsets to be non-negative, but got -1}}
%0 = memref.subview %input[-1, 256] [2, 256] [1, 1] : memref<4x1024xf32> to memref<2x256xf32, strided<[1024, 1], offset: 2304>>
return %0 : memref<2x256xf32, strided<[1024, 1], offset: 2304>>
}
@@ -620,7 +620,7 @@ func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, stri
// -----
func.func @invalid_subview(%input: memref<4x1024xf32>) -> memref<2x256xf32, strided<[1024, 1], offset: 2304>> {
- // expected-error at +1 {{expected subview sizes to be non-negative, but got -1}}
+ // expected-error at +1 {{expected sizes to be non-negative, but got -1}}
%0 = memref.subview %input[2, 256] [-1, 256] [1, 1] : memref<4x1024xf32> to memref<2x256xf32, strided<[1024, 1], offset: 2304>>
return %0 : memref<2x256xf32, strided<[1024, 1], offset: 2304>>
}
>From 6a2f5b8808f5c3715400ed5e3393492eee996675 Mon Sep 17 00:00:00 2001
From: Rik Huijzer <github at huijzer.xyz>
Date: Mon, 13 Nov 2023 15:25:29 +0100
Subject: [PATCH 10/11] Remove outdated assertion
---
mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index b66b413fbf89872..5ee7f39a955c7c2 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2621,15 +2621,6 @@ Type SubViewOp::inferResultType(MemRefType sourceMemRefType,
dispatchIndexOpFoldResults(offsets, dynamicOffsets, staticOffsets);
dispatchIndexOpFoldResults(sizes, dynamicSizes, staticSizes);
dispatchIndexOpFoldResults(strides, dynamicStrides, staticStrides);
-
- for (int64_t offset : staticOffsets) {
- if (!ShapedType::isDynamic(offset))
- assert(offset >= 0 && "expected subview offsets to be non-negative");
- }
- for (int64_t size : staticSizes) {
- if (!ShapedType::isDynamic(size))
- assert(size >= 0 && "expected subview sizes to be non-negative");
- }
return SubViewOp::inferResultType(sourceMemRefType, staticOffsets,
staticSizes, staticStrides);
}
>From d59fe3a566c33b66276cd0eb3b7c5b18246c8963 Mon Sep 17 00:00:00 2001
From: Rik Huijzer <github at huijzer.xyz>
Date: Mon, 13 Nov 2023 15:27:03 +0100
Subject: [PATCH 11/11] Put old comment back to avoid file touch
---
mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 5ee7f39a955c7c2..aba46dc0a0d33ca 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2842,6 +2842,7 @@ static LogicalResult produceSubViewErrorMsg(SliceVerificationResult result,
llvm_unreachable("unexpected subview verification result");
}
+/// Verifier for SubViewOp.
LogicalResult SubViewOp::verify() {
MemRefType baseType = getSourceType();
MemRefType subViewType = getType();
More information about the Mlir-commits
mailing list