[Mlir-commits] [mlir] daedea3 - [mlir][ods] Allow empty array ref parameter

[Jeff Niu]

Author: Jeff Niu
Date: 2022-09-20T11:07:56-07:00
New Revision: daedea39d182c7cac3cf9699ec1f58b471398c3b

URL: https://github.com/llvm/llvm-project/commit/daedea39d182c7cac3cf9699ec1f58b471398c3b
DIFF: https://github.com/llvm/llvm-project/commit/daedea39d182c7cac3cf9699ec1f58b471398c3b.diff

LOG: [mlir][ods] Allow empty array ref parameter

This patch "fixes" a longstanding issue where the assembly format for
ArrayRefParameter could not handle an empty list. This is because there
was no way to generically optionally parse the first element of the
array. The only solution was to write a (relatively simple) custom parser.

This patch implements "empty" ArrayRefParameters by using
inverted optional groups and an optional ArrayRefParameter.

Depends on D133816

Reviewed By: rriddle

Differential Revision: https://reviews.llvm.org/D133819

Added: 
    

Modified: 
    mlir/include/mlir/IR/AttrTypeBase.td
    mlir/test/IR/array-of-attr.mlir
    mlir/test/lib/Dialect/Test/TestAttrDefs.td

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/AttrTypeBase.td b/mlir/include/mlir/IR/AttrTypeBase.td
index 429624d70ee17..99bf7cb07e38b 100644
--- a/mlir/include/mlir/IR/AttrTypeBase.td
+++ b/mlir/include/mlir/IR/AttrTypeBase.td
@@ -329,6 +329,18 @@ class AttrParameter<string type, string desc, string accessorType = "">
 class TypeParameter<string type, string desc, string accessorType = "">
  : AttrOrTypeParameter<type, desc, accessorType>;
 
+// An optional parameter.
+class OptionalParameter<string type, string desc = ""> :
+    AttrOrTypeParameter<type, desc> {
+  let defaultValue = cppStorageType # "()";
+}
+
+// A parameter with a default value.
+class DefaultValuedParameter<string type, string value, string desc = ""> :
+    AttrOrTypeParameter<type, desc> {
+  let defaultValue = value;
+}
+
 // For StringRefs, which require allocation.
 class StringRefParameter<string desc = ""> :
     AttrOrTypeParameter<"::llvm::StringRef", desc> {
@@ -350,6 +362,15 @@ class ArrayRefParameter<string arrayOf, string desc = ""> :
   let cppStorageType = "::llvm::SmallVector<" # arrayOf # ">";
 }
 
+// Regular array parameters cannot be parsed when empty. This optional array
+// parameter can be used with optional groups to be parsed when empty.
+class OptionalArrayRefParameter<string arrayOf, string desc = ""> :
+    OptionalParameter<"::llvm::ArrayRef<" # arrayOf # ">", desc> {
+  let allocator = [{$_dst = $_allocator.copyInto($_self);}];
+  let cppStorageType = "::llvm::SmallVector<" # arrayOf # ">";
+  let comparator = "::llvm::makeArrayRef($_lhs) == ::llvm::makeArrayRef($_rhs)";
+}
+
 // For classes which require allocation and have their own allocateInto method.
 class SelfAllocationParameter<string type, string desc> :
     AttrOrTypeParameter<type, desc> {
@@ -367,18 +388,6 @@ class ArrayRefOfSelfAllocationParameter<string arrayOf, string desc> :
   }];
 }
 
-// An optional parameter.
-class OptionalParameter<string type, string desc = ""> :
-    AttrOrTypeParameter<type, desc> {
-  let defaultValue = cppStorageType # "()";
-}
-
-// A parameter with a default value.
-class DefaultValuedParameter<string type, string value, string desc = ""> :
-    AttrOrTypeParameter<type, desc> {
-  let defaultValue = value;
-}
-
 // This is a special attribute parameter that represents the "self" type of the
 // attribute. It is specially handled by the assembly format generator to derive
 // its value from the optional trailing type after each attribute.
@@ -406,9 +415,9 @@ class AttributeSelfTypeParameter<string desc,
 class ArrayOfAttr<Dialect dialect, string attrName, string attrMnemonic,
                   string eltName, list<Trait> traits = []>
     : AttrDef<dialect, attrName, traits> {
-  let parameters = (ins ArrayRefParameter<eltName>:$value);
+  let parameters = (ins OptionalArrayRefParameter<eltName>:$value);
   let mnemonic = attrMnemonic;
-  let assemblyFormat = "`[` $value `]`";
+  let assemblyFormat = "`[` (`]`) : ($value^ `]`)?";
 
   let returnType = "::llvm::ArrayRef<" # eltName # ">";
   let constBuilderCall = "$_builder.getAttr<" # attrName # "Attr>($0)";

diff  --git a/mlir/test/IR/array-of-attr.mlir b/mlir/test/IR/array-of-attr.mlir
index 8b85aa79ee992..1b6fe55205959 100644
--- a/mlir/test/IR/array-of-attr.mlir
+++ b/mlir/test/IR/array-of-attr.mlir
@@ -8,3 +8,7 @@ test.array_of_attr_op
     b = [0, 1, -42, 42],
     // CHECK-SAME: [a, b, b, a]
     c = [a, b, b, a]
+
+// CHECK: test.array_of_attr_op
+// CHECK-SAME: a = [], b = [], c = []
+test.array_of_attr_op a = [], b = [], c = []

diff  --git a/mlir/test/lib/Dialect/Test/TestAttrDefs.td b/mlir/test/lib/Dialect/Test/TestAttrDefs.td
index dba035779c0df..5ffcbccfc3963 100644
--- a/mlir/test/lib/Dialect/Test/TestAttrDefs.td
+++ b/mlir/test/lib/Dialect/Test/TestAttrDefs.td
@@ -270,7 +270,7 @@ def TestExtern1DI64ElementsAttr : Test_Attr<"TestExtern1DI64Elements", [
 // An array of nested attributes.
 def TestArrayOfUglyAttrs : ArrayOfAttr<Test_Dialect, "ArrayOfUglyAttrs",
     "array_of_ugly", "TestAttrUglyAttr"> {
-  let assemblyFormat = "`[` $value ` ` `]`";
+  let assemblyFormat = "`[` (`]`) : ($value^ ` ` `]`)?";
 }
 
 // An array of integers.