[Mlir-commits] [mlir] [mlir][tosa] Align Variable ops to match with TOSA v1.0 spec (PR #130680)

[llvmlistbot at llvm.org]

https://github.com/Jerry-Ge updated https://github.com/llvm/llvm-project/pull/130680

>From b3152506153da9a43f7f61d5e5f87b8225602386 Mon Sep 17 00:00:00 2001
From: Jerry Ge <jerry.ge at arm.com>
Date: Wed, 4 Dec 2024 00:29:10 +0000
Subject: [PATCH] [mlir][tosa] Align Variable ops to match with TOSA v1.0 spec

* updated AnyType:$value to Tosa_Tensor:$input1 and Tosa_Tensor:$output1 for VariableWrite and VriableRead Operators
* updated description discrepancies
* note: in the TOSA spec, we had var_shape attr, but it's already included
  in the TypeAttr:$type in MLIR

Signed-off-by: Jerry Ge <jerry.ge at arm.com>
Change-Id: I4cd0348cd4e306dbc2e0e53a89a9404d91fb44d4
---
 mlir/include/mlir/Dialect/Tosa/IR/TosaUtilOps.td  | 15 ++++++++-------
 .../TosaToMLProgram/TosaToMLProgram.cpp           |  2 +-
 .../Dialect/Tosa/Transforms/TosaValidation.cpp    |  5 ++---
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/mlir/include/mlir/Dialect/Tosa/IR/TosaUtilOps.td b/mlir/include/mlir/Dialect/Tosa/IR/TosaUtilOps.td
index 8a27e5ba39331..4dfd6651acd38 100644
--- a/mlir/include/mlir/Dialect/Tosa/IR/TosaUtilOps.td
+++ b/mlir/include/mlir/Dialect/Tosa/IR/TosaUtilOps.td
@@ -86,7 +86,8 @@ def Tosa_VariableOp : Tosa_Op<"variable", []> {
   let summary = "Defines a variable";
 
   let description = [{
-    Defines a new TOSA variable. This is a mutable value.
+    Defines a new TOSA variable.
+    This is a persistent mutable value across multiple TOSA graph invocations.
     Modifications are expressed using read/write semantics.
   }];
 
@@ -115,12 +116,12 @@ def Tosa_VariableWriteOp : Tosa_Op<"variable.write", []> {
   let summary = "write_buffer operator";
 
   let description = [{
-    Assigns a value to pseudo-buffer resource holding a mutable tensor.
+    Assigns a value to the pseudo-buffer resource holding a persistent mutable tensor.
   }];
 
   let arguments = (ins
     SymbolNameAttr:$name,
-    AnyType:$value
+    Tosa_Tensor:$input1
   );
 
   list<Availability> availability = [
@@ -129,7 +130,7 @@ def Tosa_VariableWriteOp : Tosa_Op<"variable.write", []> {
   ];
 
   let assemblyFormat = [{
-    $name attr-dict `,` $value `:` type($value)
+    $name attr-dict `,` $input1 `:` type($input1)
   }];
 }
 
@@ -140,7 +141,7 @@ def Tosa_VariableReadOp : Tosa_Op<"variable.read", []> {
   let summary = "read_buffer operator";
 
   let description = [{
-    Reads the value from a pseudo-buffer resource holding a mutable tensor.
+    Reads the value from a pseudo-buffer resource holding a persistent mutable tensor.
   }];
 
   let arguments = (ins
@@ -148,7 +149,7 @@ def Tosa_VariableReadOp : Tosa_Op<"variable.read", []> {
   );
 
   let results = (outs
-    AnyType:$value
+    Tosa_Tensor:$output1
   );
 
   list<Availability> availability = [
@@ -157,7 +158,7 @@ def Tosa_VariableReadOp : Tosa_Op<"variable.read", []> {
   ];
 
   let assemblyFormat = [{
-    $name attr-dict `:` type($value)
+    $name attr-dict `:` type($output1)
   }];
 }
 
diff --git a/mlir/lib/Conversion/TosaToMLProgram/TosaToMLProgram.cpp b/mlir/lib/Conversion/TosaToMLProgram/TosaToMLProgram.cpp
index d134d8cdf485e..310566e692202 100644
--- a/mlir/lib/Conversion/TosaToMLProgram/TosaToMLProgram.cpp
+++ b/mlir/lib/Conversion/TosaToMLProgram/TosaToMLProgram.cpp
@@ -45,7 +45,7 @@ class VariableWriteOpConverter
     auto globalSymbolRef =
         SymbolRefAttr::get(rewriter.getContext(), op.getName());
     auto newVariableWrite = rewriter.create<ml_program::GlobalStoreOp>(
-        op.getLoc(), globalSymbolRef, op.getValue());
+        op.getLoc(), globalSymbolRef, op.getInput1());
     rewriter.replaceOp(op, newVariableWrite);
     return success();
   }
diff --git a/mlir/lib/Dialect/Tosa/Transforms/TosaValidation.cpp b/mlir/lib/Dialect/Tosa/Transforms/TosaValidation.cpp
index 79c13793d7713..1eb5753767371 100644
--- a/mlir/lib/Dialect/Tosa/Transforms/TosaValidation.cpp
+++ b/mlir/lib/Dialect/Tosa/Transforms/TosaValidation.cpp
@@ -765,7 +765,7 @@ inline bool CompatibleTypes(const mlir::Type &type,
 
 bool TosaValidation::CheckVariable(Operation *op) {
   if (isa<mlir::tosa::VariableOp>(op)) {
-    auto nameAttr = cast<mlir::StringAttr>(op->getAttr("name"));
+    mlir::StringAttr nameAttr = cast<mlir::StringAttr>(op->getAttr("name"));
 
     if (variablesMap.count(nameAttr)) {
       op->emitOpError() << "name has already been declared";
@@ -784,8 +784,7 @@ bool TosaValidation::CheckVariable(Operation *op) {
 bool TosaValidation::CheckVariableReadOrWrite(Operation *op) {
   if (isa<mlir::tosa::VariableReadOp>(op) ||
       isa<mlir::tosa::VariableWriteOp>(op)) {
-    auto nameAttr = cast<mlir::StringAttr>(op->getAttr("name"));
-
+    mlir::StringAttr nameAttr = cast<mlir::StringAttr>(op->getAttr("name"));
     if (!variablesMap.count(nameAttr)) {
       op->emitOpError() << "name has not been declared";
       return false;