From 9c06985f308dd529959812bfb18d3a5fb7dbad53 Mon Sep 17 00:00:00 2001
From: Sihao Liu <sihao@cs.ucla.edu>
Date: Thu, 1 May 2025 20:02:19 -0700
Subject: [PATCH] Revert "Remove multidim/dynamic size checks from memref to
 Handshake"

This reverts commit d763224f84d09a146b153141114d496c7743e5dd.
---
 .../CFToHandshake/CFToHandshake.cpp           |  6 ++--
 lib/Dialect/Handshake/HandshakeOps.cpp        | 10 +++----
 test/Conversion/CFToHandshake/invalid.mlir    | 29 +++++++++++++++++++
 test/Dialect/Handshake/errors.mlir            | 16 ++++++++++
 4 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/lib/Conversion/CFToHandshake/CFToHandshake.cpp b/lib/Conversion/CFToHandshake/CFToHandshake.cpp
index 3412dae24a..33c728d94f 100644
--- a/lib/Conversion/CFToHandshake/CFToHandshake.cpp
+++ b/lib/Conversion/CFToHandshake/CFToHandshake.cpp
@@ -268,9 +268,9 @@ void removeBasicBlocks(handshake::FuncOp funcOp) {
 }
 
 static LogicalResult isValidMemrefType(Location loc, mlir::MemRefType type) {
-  // if (type.getNumDynamicDims() != 0 || type.getShape().size() != 1)
-  //   return emitError(loc) << "memref's must be both statically sized and "
-  //                            "unidimensional.";
+  if (type.getNumDynamicDims() != 0 || type.getShape().size() != 1)
+    return emitError(loc) << "memref's must be both statically sized and "
+                             "unidimensional.";
   return success();
 }
 
diff --git a/lib/Dialect/Handshake/HandshakeOps.cpp b/lib/Dialect/Handshake/HandshakeOps.cpp
index ca7a69d109..b20a666810 100644
--- a/lib/Dialect/Handshake/HandshakeOps.cpp
+++ b/lib/Dialect/Handshake/HandshakeOps.cpp
@@ -986,11 +986,11 @@ std::string handshake::MemoryOp::getResultName(unsigned int idx) {
 LogicalResult MemoryOp::verify() {
   auto memrefType = getMemRefType();
 
-  // if (memrefType.getNumDynamicDims() != 0)
-  //   return emitOpError()
-  //          << "memref dimensions for handshake.memory must be static.";
-  // if (memrefType.getShape().size() != 1)
-  //   return emitOpError() << "memref must have only a single dimension.";
+  if (memrefType.getNumDynamicDims() != 0)
+    return emitOpError()
+           << "memref dimensions for handshake.memory must be static.";
+  if (memrefType.getShape().size() != 1)
+    return emitOpError() << "memref must have only a single dimension.";
 
   unsigned opStCount = getStCount();
   unsigned opLdCount = getLdCount();
diff --git a/test/Conversion/CFToHandshake/invalid.mlir b/test/Conversion/CFToHandshake/invalid.mlir
index 136d758f58..b0b04b0505 100644
--- a/test/Conversion/CFToHandshake/invalid.mlir
+++ b/test/Conversion/CFToHandshake/invalid.mlir
@@ -1,5 +1,34 @@
 // RUN: circt-opt -lower-cf-to-handshake %s -split-input-file -verify-diagnostics
 
+func.func @multidim() -> i32 {
+  // expected-error @+1 {{memref's must be both statically sized and unidimensional.}}
+  %0 = memref.alloc() : memref<2x2xi32>
+  %idx = arith.constant 0 : index
+  %1 = memref.load %0[%idx, %idx] : memref<2x2xi32>
+  return %1 : i32
+}
+
+// -----
+
+func.func @dynsize(%dyn : index) -> i32{
+  // expected-error @+1 {{memref's must be both statically sized and unidimensional.}}
+  %0 = memref.alloc(%dyn) : memref<?xi32>
+  %idx = arith.constant 0 : index
+  %1 = memref.load %0[%idx] : memref<?xi32>
+  return %1 : i32
+}
+
+// -----
+
+func.func @singleton() -> (){
+  // expected-error @+1 {{memref's must be both statically sized and unidimensional.}}
+  %0 = memref.alloc() : memref<i32>
+  %1 = memref.load %0[] : memref<i32>
+  return
+}
+
+// -----
+
 func.func @non_canon_loop(%arg0 : memref<100xi32>, %arg1 : i32) -> i32 {
   // expected-error @below {{expected cmerges to have two operands}}
   %c0_i32 = arith.constant 0 : i32
diff --git a/test/Dialect/Handshake/errors.mlir b/test/Dialect/Handshake/errors.mlir
index b371ccfe26..ab9a861b2a 100644
--- a/test/Dialect/Handshake/errors.mlir
+++ b/test/Dialect/Handshake/errors.mlir
@@ -103,6 +103,14 @@ handshake.func @invalid_instance_op(%ctrl : none) -> none {
 
 // -----
 
+handshake.func @invalid_multidim_memory(%ctrl : none) -> none {
+  // expected-error @+1 {{'handshake.memory' op memref must have only a single dimension.}}
+  memory [ld = 0, st = 0] () {id = 0 : i32, lsq = false} : memref<10x10xi8>, () -> ()
+  return %ctrl : none
+}
+
+// -----
+
 handshake.func @invalid_missing_lsq(%ctrl : none) -> none {
   // expected-error @+1 {{'handshake.memory' op requires attribute 'lsq'}}
   memory [ld = 0, st = 0] () {id = 0 : i32} : memref<10xi8>, () -> ()
@@ -111,6 +119,14 @@ handshake.func @invalid_missing_lsq(%ctrl : none) -> none {
 
 // -----
 
+handshake.func @invalid_dynamic_memory(%ctrl : none) -> none {
+  // expected-error @+1 {{'handshake.memory' op memref dimensions for handshake.memory must be static.}}
+  memory [ld = 0, st = 0] () {id = 0 : i32, lsq = false} : memref<?xi8>, () -> ()
+  return %ctrl : none
+}
+
+// -----
+
 // expected-error @+1 {{'handshake.func' op attribute 'argNames' has 2 entries but is expected to have 3.}}
 handshake.func @invalid_num_argnames(%a : i32, %b : i32, %c : none) -> none attributes {argNames = ["a", "b"]} {
   return %c : none