From 1145cebdab3774c70aa4cb74948e0849473b6388 Mon Sep 17 00:00:00 2001
From: Mahesh Ravishankar <ravishankarm@google.com>
Date: Wed, 20 Nov 2019 13:43:20 -0800
Subject: [PATCH] Verify subview op result has dynamic shape, when sizes are
 specified.

If the sizes are specified as arguments to the subview op, then the
shape must be dynamic as well.

PiperOrigin-RevId: 281591608
---
 mlir/lib/Dialect/StandardOps/Ops.cpp | 14 ++++++++++++++
 mlir/test/IR/invalid-ops.mlir        | 12 +++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/mlir/lib/Dialect/StandardOps/Ops.cpp b/mlir/lib/Dialect/StandardOps/Ops.cpp
index fcc472993fd3..c01bccc6340a 100644
--- a/mlir/lib/Dialect/StandardOps/Ops.cpp
+++ b/mlir/lib/Dialect/StandardOps/Ops.cpp
@@ -2663,6 +2663,20 @@ static LogicalResult verify(SubViewOp op) {
     return op.emitError("invalid to specify dynamic sizes when subview result "
                         "type is statically shaped and viceversa");
   }
+  if (op.getNumSizes() > 0) {
+    // Verify that non if the shape values of the result type are static.
+    if (llvm::any_of(subViewType.getShape(), [](int64_t dim) {
+          return dim != ShapedType::kDynamicSize;
+        })) {
+      // TODO: This is based on the assumption that number of size arguments are
+      // either 0, or the rank of the result type. It is possible to have more
+      // fine-grained verification where only particular dimensions are
+      // dynamic. That probably needs further changes to the shape op
+      // specification.
+      return op.emitError("expected shape of result type to be fully dynamic "
+                          "when sizes are specified");
+    }
+  }
 
   // Verify that if dynamic offsets are specified or base memref has dynamic
   // offset or base memref has dynamic strides, then the subview offset is
diff --git a/mlir/test/IR/invalid-ops.mlir b/mlir/test/IR/invalid-ops.mlir
index af4915eadf76..3b2aa347b01b 100644
--- a/mlir/test/IR/invalid-ops.mlir
+++ b/mlir/test/IR/invalid-ops.mlir
@@ -848,7 +848,7 @@ func @invalid_subview(%arg0 : index, %arg1 : index, %arg2 : index) {
   // expected-error@+1 {{expected result type to have dynamic strides}}
   %1 = subview %0[%arg0, %arg1, %arg2][%arg0, %arg1, %arg2][%arg0, %arg1, %arg2]
     : memref<8x16x4xf32> to
-      memref<?x?x4xf32, offset: ?, strides: [64, 4, 1]>
+      memref<?x?x?xf32, offset: ?, strides: [64, 4, 1]>
   return
 }
 
@@ -941,3 +941,13 @@ func @invalid_subview(%arg0 : index, %arg1 : memref<16x4xf32, offset: 0, strides
   // expected-error@+1 {{expected result type to have dynamic stride along a dimension if the base memref type has dynamic stride along that dimension}}
   %0 = subview %arg1[][][] : memref<16x4xf32, offset: 0, strides:[?, ?]> to memref<4x2xf32, offset:?, strides:[2, 1]>
 }
+
+// -----
+
+func @invalid_subview(%arg0 : index, %arg1 : memref<?x8x?xf32>) {
+  %c0 = constant 0 : index
+  %c1 = constant 1 : index
+  // expected-error@+1 {{expected shape of result type to be fully dynamic when sizes are specified}}
+  %0 = subview %arg1[%c0, %c0, %c0][%c1, %arg0, %c1][%c1, %c1, %c1] : memref<?x8x?xf32> to memref<?x8x?xf32, offset:?, strides:[?, ?, ?]>
+  return
+}