[Mlir-commits] [mlir] [mlir] [irdl] Optional attributes in irdl (PR #110981)

Alex Rice llvmlistbot at llvm.org
Thu Oct 3 03:38:42 PDT 2024


https://github.com/alexarice created https://github.com/llvm/llvm-project/pull/110981

Adds a variadicity array property to `irdl.attributes`, allowing optional attributes(/properties) to be defined. Also updates the tblgen-to-irdl script to support this.

@math-fehr 

>From ab9fab092f9bea15c711970c753b9d1a5484a3f8 Mon Sep 17 00:00:00 2001
From: Alex Rice <alexrice999 at hotmail.co.uk>
Date: Thu, 22 Aug 2024 16:29:27 +0100
Subject: [PATCH] Optional attributes

---
 mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td  |  5 +-
 mlir/lib/Dialect/IRDL/IR/IRDL.cpp             | 31 ++++++++---
 .../test/Dialect/IRDL/attributes-op.irdl.mlir | 11 ++--
 .../Dialect/IRDL/variadics-error.irdl.mlir    | 41 +++++++++++++-
 mlir/test/Dialect/IRDL/variadics.irdl.mlir    | 53 ++++++++++++-------
 .../tools/tblgen-to-irdl/OpDefinitionsGen.cpp | 14 +++--
 6 files changed, 119 insertions(+), 36 deletions(-)

diff --git a/mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td b/mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td
index c7fcb55120c827..f5b98ff5f1371e 100644
--- a/mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td
+++ b/mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td
@@ -321,9 +321,10 @@ def IRDL_AttributesOp : IRDL_Op<"attributes", [HasParent<"OperationOp">]> {
   }];
 
   let arguments = (ins Variadic<IRDL_AttributeType>:$attributeValues,
-                        StrArrayAttr:$attributeValueNames);
+                        StrArrayAttr:$attributeValueNames,
+                        VariadicityArrayAttr:$variadicity);
   let assemblyFormat = [{
-    custom<AttributesOp>($attributeValues, $attributeValueNames) attr-dict
+    custom<AttributesOp>($attributeValues, $attributeValueNames, $variadicity) attr-dict
   }];
 
   let hasVerifier = true;
diff --git a/mlir/lib/Dialect/IRDL/IR/IRDL.cpp b/mlir/lib/Dialect/IRDL/IR/IRDL.cpp
index c5c44d97ce0911..2c37a5bb82d74a 100644
--- a/mlir/lib/Dialect/IRDL/IR/IRDL.cpp
+++ b/mlir/lib/Dialect/IRDL/IR/IRDL.cpp
@@ -107,11 +107,18 @@ LogicalResult ResultsOp::verify() {
 LogicalResult AttributesOp::verify() {
   size_t namesSize = getAttributeValueNames().size();
   size_t valuesSize = getAttributeValues().size();
+  size_t numVariadicities = getVariadicity().size();
+
+  if (numVariadicities != valuesSize)
+    return emitOpError()
+           << "the number of attributes and their variadicities must be "
+              "the same, but got "
+           << valuesSize << " and " << numVariadicities << " respectively";
 
   if (namesSize != valuesSize)
     return emitOpError()
            << "the number of attribute names and their constraints must be "
-              "the same but got "
+              "the same, but got "
            << namesSize << " and " << valuesSize << " respectively";
 
   return success();
@@ -249,13 +256,18 @@ static void printValuesWithVariadicity(OpAsmPrinter &p, Operation *op,
 static ParseResult
 parseAttributesOp(OpAsmParser &p,
                   SmallVectorImpl<OpAsmParser::UnresolvedOperand> &attrOperands,
-                  ArrayAttr &attrNamesAttr) {
+                  ArrayAttr &attrNamesAttr,
+                  VariadicityArrayAttr &variadicityAttr) {
   Builder &builder = p.getBuilder();
+  MLIRContext *ctx = builder.getContext();
   SmallVector<Attribute> attrNames;
+  SmallVector<VariadicityAttr> variadicities;
+
   if (succeeded(p.parseOptionalLBrace())) {
     auto parseOperands = [&]() {
       if (p.parseAttribute(attrNames.emplace_back()) || p.parseEqual() ||
-          p.parseOperand(attrOperands.emplace_back()))
+          parseValueWithVariadicity(p, attrOperands.emplace_back(),
+                                    variadicities.emplace_back()))
         return failure();
       return success();
     };
@@ -263,16 +275,23 @@ parseAttributesOp(OpAsmParser &p,
       return failure();
   }
   attrNamesAttr = builder.getArrayAttr(attrNames);
+  variadicityAttr = VariadicityArrayAttr::get(ctx, variadicities);
   return success();
 }
 
 static void printAttributesOp(OpAsmPrinter &p, AttributesOp op,
-                              OperandRange attrArgs, ArrayAttr attrNames) {
+                              OperandRange attrArgs, ArrayAttr attrNames,
+                              VariadicityArrayAttr variadicityAttr) {
   if (attrNames.empty())
     return;
   p << "{";
-  interleaveComma(llvm::seq<int>(0, attrNames.size()), p,
-                  [&](int i) { p << attrNames[i] << " = " << attrArgs[i]; });
+  interleaveComma(llvm::seq<int>(0, attrNames.size()), p, [&](int i) {
+    Variadicity variadicity = variadicityAttr[i].getValue();
+    p << attrNames[i] << " = ";
+    if (variadicity != Variadicity::single)
+      p << stringifyVariadicity(variadicity) << " ";
+    p << attrArgs[i];
+  });
   p << '}';
 }
 
diff --git a/mlir/test/Dialect/IRDL/attributes-op.irdl.mlir b/mlir/test/Dialect/IRDL/attributes-op.irdl.mlir
index 89a059803a86fd..4f38aa80e6e8d0 100644
--- a/mlir/test/Dialect/IRDL/attributes-op.irdl.mlir
+++ b/mlir/test/Dialect/IRDL/attributes-op.irdl.mlir
@@ -1,11 +1,12 @@
 // RUN: mlir-opt %s -verify-diagnostics -split-input-file
+
 irdl.dialect @errors {
   irdl.operation @attrs1 {
     %0 = irdl.is i32
     %1 = irdl.is i64
 
-    // expected-error at +1 {{'irdl.attributes' op the number of attribute names and their constraints must be the same but got 1 and 2 respectively}}
-    "irdl.attributes"(%0, %1) <{attributeValueNames = ["attr1"]}> : (!irdl.attribute, !irdl.attribute) -> ()
+    // expected-error at +1 {{'irdl.attributes' op the number of attribute names and their constraints must be the same, but got 1 and 2 respectively}}
+    "irdl.attributes"(%0, %1) <{attributeValueNames = ["attr1"], variadicity = #irdl<variadicity_array[ single, single]>}> : (!irdl.attribute, !irdl.attribute) -> ()
   }
 }
 
@@ -15,8 +16,8 @@ irdl.dialect @errors {
   irdl.operation @attrs2 {
     %0 = irdl.is i32
     %1 = irdl.is i64
-   
-    // expected-error at +1 {{'irdl.attributes' op the number of attribute names and their constraints must be the same but got 2 and 1 respectively}}
-    "irdl.attributes"(%0) <{attributeValueNames = ["attr1", "attr2"]}> : (!irdl.attribute) -> ()
+
+    // expected-error at +1 {{'irdl.attributes' op the number of attribute names and their constraints must be the same, but got 2 and 1 respectively}}
+    "irdl.attributes"(%0) <{attributeValueNames = ["attr1", "attr2"], variadicity = #irdl<variadicity_array[ single]>}> : (!irdl.attribute) -> ()
   }
 }
diff --git a/mlir/test/Dialect/IRDL/variadics-error.irdl.mlir b/mlir/test/Dialect/IRDL/variadics-error.irdl.mlir
index 67fa94ea8b7427..51d9e5aeec2319 100644
--- a/mlir/test/Dialect/IRDL/variadics-error.irdl.mlir
+++ b/mlir/test/Dialect/IRDL/variadics-error.irdl.mlir
@@ -14,7 +14,7 @@ irdl.dialect @errors {
 irdl.dialect @errors {
   irdl.operation @operands2 {
     %0 = irdl.is i32
-   
+
     // expected-error at +1 {{'irdl.operands' op the number of operands and their variadicities must be the same, but got 1 and 2 respectively}}
     "irdl.operands"(%0) <{variadicity = #irdl<variadicity_array[single, single]>}> : (!irdl.attribute) -> ()
   }
@@ -36,8 +36,45 @@ irdl.dialect @errors {
 irdl.dialect @errors {
   irdl.operation @results2 {
     %0 = irdl.is i32
-   
+
     // expected-error at +1 {{'irdl.results' op the number of operands and their variadicities must be the same, but got 1 and 2 respectively}}
     "irdl.results"(%0) <{variadicity = #irdl<variadicity_array[single, single]>}> : (!irdl.attribute) -> ()
   }
 }
+
+// -----
+
+irdl.dialect @errors {
+  irdl.operation @no_var {
+    %0 = irdl.is i32
+    %1 = irdl.is i64
+
+    // expected-error at +1 {{'irdl.attributes' op requires attribute 'variadicity'}}
+    "irdl.attributes"(%0) <{attributeValueNames = ["attr1"]}> : (!irdl.attribute) -> ()
+  }
+}
+
+// -----
+
+irdl.dialect @errors {
+  irdl.operation @attrs1 {
+    %0 = irdl.is i32
+    %1 = irdl.is i64
+
+    // expected-error at +1 {{'irdl.attributes' op the number of attributes and their variadicities must be the same, but got 2 and 1 respectively}}
+    "irdl.attributes"(%0, %1) <{attributeValueNames = ["attr1", "attr2"], variadicity = #irdl<variadicity_array[ single]>}> : (!irdl.attribute, !irdl.attribute) -> ()
+  }
+}
+
+// -----
+
+irdl.dialect @errors {
+  irdl.operation @attrs2 {
+    %0 = irdl.is i32
+    %1 = irdl.is i64
+
+    // expected-error at +1 {{'irdl.attributes' op the number of attributes and their variadicities must be the same, but got 1 and 2 respectively}}
+    "irdl.attributes"(%0) <{attributeValueNames = ["attr1"], variadicity = #irdl<variadicity_array[ single, single]>}> : (!irdl.attribute) -> ()
+  }
+}
+
diff --git a/mlir/test/Dialect/IRDL/variadics.irdl.mlir b/mlir/test/Dialect/IRDL/variadics.irdl.mlir
index 64c5e1878f08eb..c61e764c2e118d 100644
--- a/mlir/test/Dialect/IRDL/variadics.irdl.mlir
+++ b/mlir/test/Dialect/IRDL/variadics.irdl.mlir
@@ -13,9 +13,9 @@ irdl.dialect @testvar {
   }
 
   // CHECK-LABEL: irdl.operation @var_operand {
-  // CHECK-NEXT:    %[[v0:[^ ]*]] = irdl.is i16 
-  // CHECK-NEXT:    %[[v1:[^ ]*]] = irdl.is i32 
-  // CHECK-NEXT:    %[[v2:[^ ]*]] = irdl.is i64 
+  // CHECK-NEXT:    %[[v0:[^ ]*]] = irdl.is i16
+  // CHECK-NEXT:    %[[v1:[^ ]*]] = irdl.is i32
+  // CHECK-NEXT:    %[[v2:[^ ]*]] = irdl.is i64
   // CHECK-NEXT:    irdl.operands(%[[v0]], variadic %[[v1]], %[[v2]])
   // CHECK-NEXT:  }
   irdl.operation @var_operand {
@@ -26,9 +26,9 @@ irdl.dialect @testvar {
   }
 
   // CHECK-LABEL: irdl.operation @opt_operand {
-  // CHECK-NEXT:    %[[v0:[^ ]*]] = irdl.is i16 
-  // CHECK-NEXT:    %[[v1:[^ ]*]] = irdl.is i32 
-  // CHECK-NEXT:    %[[v2:[^ ]*]] = irdl.is i64 
+  // CHECK-NEXT:    %[[v0:[^ ]*]] = irdl.is i16
+  // CHECK-NEXT:    %[[v1:[^ ]*]] = irdl.is i32
+  // CHECK-NEXT:    %[[v2:[^ ]*]] = irdl.is i64
   // CHECK-NEXT:    irdl.operands(%[[v0]], optional %[[v1]], %[[v2]])
   // CHECK-NEXT:  }
   irdl.operation @opt_operand {
@@ -39,9 +39,9 @@ irdl.dialect @testvar {
   }
 
   // CHECK-LABEL: irdl.operation @var_and_opt_operand {
-  // CHECK-NEXT:    %[[v0:[^ ]*]] = irdl.is i16 
-  // CHECK-NEXT:    %[[v1:[^ ]*]] = irdl.is i32 
-  // CHECK-NEXT:    %[[v2:[^ ]*]] = irdl.is i64 
+  // CHECK-NEXT:    %[[v0:[^ ]*]] = irdl.is i16
+  // CHECK-NEXT:    %[[v1:[^ ]*]] = irdl.is i32
+  // CHECK-NEXT:    %[[v2:[^ ]*]] = irdl.is i64
   // CHECK-NEXT:    irdl.operands(variadic %[[v0]], optional %[[v1]], %[[v2]])
   // CHECK-NEXT:  }
   irdl.operation @var_and_opt_operand {
@@ -62,9 +62,9 @@ irdl.dialect @testvar {
   }
 
   // CHECK-LABEL: irdl.operation @var_result {
-  // CHECK-NEXT:    %[[v0:[^ ]*]] = irdl.is i16 
-  // CHECK-NEXT:    %[[v1:[^ ]*]] = irdl.is i32 
-  // CHECK-NEXT:    %[[v2:[^ ]*]] = irdl.is i64 
+  // CHECK-NEXT:    %[[v0:[^ ]*]] = irdl.is i16
+  // CHECK-NEXT:    %[[v1:[^ ]*]] = irdl.is i32
+  // CHECK-NEXT:    %[[v2:[^ ]*]] = irdl.is i64
   // CHECK-NEXT:    irdl.results(%[[v0]], variadic %[[v1]], %[[v2]])
   // CHECK-NEXT:  }
   irdl.operation @var_result {
@@ -75,9 +75,9 @@ irdl.dialect @testvar {
   }
 
   // CHECK-LABEL: irdl.operation @opt_result {
-  // CHECK-NEXT:    %[[v0:[^ ]*]] = irdl.is i16 
-  // CHECK-NEXT:    %[[v1:[^ ]*]] = irdl.is i32 
-  // CHECK-NEXT:    %[[v2:[^ ]*]] = irdl.is i64 
+  // CHECK-NEXT:    %[[v0:[^ ]*]] = irdl.is i16
+  // CHECK-NEXT:    %[[v1:[^ ]*]] = irdl.is i32
+  // CHECK-NEXT:    %[[v2:[^ ]*]] = irdl.is i64
   // CHECK-NEXT:    irdl.results(%[[v0]], optional %[[v1]], %[[v2]])
   // CHECK-NEXT:  }
   irdl.operation @opt_result {
@@ -88,9 +88,9 @@ irdl.dialect @testvar {
   }
 
   // CHECK-LABEL: irdl.operation @var_and_opt_result {
-  // CHECK-NEXT:    %[[v0:[^ ]*]] = irdl.is i16 
-  // CHECK-NEXT:    %[[v1:[^ ]*]] = irdl.is i32 
-  // CHECK-NEXT:    %[[v2:[^ ]*]] = irdl.is i64 
+  // CHECK-NEXT:    %[[v0:[^ ]*]] = irdl.is i16
+  // CHECK-NEXT:    %[[v1:[^ ]*]] = irdl.is i32
+  // CHECK-NEXT:    %[[v2:[^ ]*]] = irdl.is i64
   // CHECK-NEXT:    irdl.results(variadic %[[v0]], optional %[[v1]], %[[v2]])
   // CHECK-NEXT:  }
   irdl.operation @var_and_opt_result {
@@ -99,4 +99,21 @@ irdl.dialect @testvar {
     %2 = irdl.is i64
     irdl.results(variadic %0, optional %1, %2)
   }
+
+  // CHECK-LABEL: irdl.operation @var_attr {
+  // CHECK-NEXT:    %[[v0:[^ ]*]] = irdl.is i16
+  // CHECK-NEXT:    %[[v1:[^ ]*]] = irdl.is i32
+  // CHECK-NEXT:    %[[v2:[^ ]*]] = irdl.is i64
+  // CHECK-NEXT:    irdl.attributes {"optional" = optional %[[v0]], "single" = %[[v1]], "single_no_word" = %[[v2]]}
+  // CHECK-NEXT:  }
+  irdl.operation @var_attr {
+    %0 = irdl.is i16
+    %1 = irdl.is i32
+    %2 = irdl.is i64
+    irdl.attributes {
+      "optional" = optional %0,
+      "single" = single %1,
+      "single_no_word" = %2
+    }
+  }
 }
diff --git a/mlir/tools/tblgen-to-irdl/OpDefinitionsGen.cpp b/mlir/tools/tblgen-to-irdl/OpDefinitionsGen.cpp
index ace1029b4e7ff0..637598e2d43bac 100644
--- a/mlir/tools/tblgen-to-irdl/OpDefinitionsGen.cpp
+++ b/mlir/tools/tblgen-to-irdl/OpDefinitionsGen.cpp
@@ -422,11 +422,18 @@ irdl::OperationOp createIRDLOperation(OpBuilder &builder,
 
   SmallVector<Value> attributes;
   SmallVector<Attribute> attrNames;
+  SmallVector<irdl::VariadicityAttr> attrVariadicity;
   for (auto namedAttr : tblgenOp.getAttributes()) {
+    irdl::VariadicityAttr var;
     if (namedAttr.attr.isOptional())
-      continue;
+      var = consBuilder.getAttr<irdl::VariadicityAttr>(
+          irdl::Variadicity::optional);
+    else
+      var =
+          consBuilder.getAttr<irdl::VariadicityAttr>(irdl::Variadicity::single);
     attributes.push_back(createAttrConstraint(consBuilder, namedAttr.attr));
     attrNames.push_back(StringAttr::get(ctx, namedAttr.name));
+    attrVariadicity.push_back(var);
   }
 
   SmallVector<Value> regions;
@@ -443,8 +450,9 @@ irdl::OperationOp createIRDLOperation(OpBuilder &builder,
     consBuilder.create<irdl::ResultsOp>(UnknownLoc::get(ctx), results,
                                         resultVariadicity);
   if (!attributes.empty())
-    consBuilder.create<irdl::AttributesOp>(UnknownLoc::get(ctx), attributes,
-                                           ArrayAttr::get(ctx, attrNames));
+    consBuilder.create<irdl::AttributesOp>(
+        UnknownLoc::get(ctx), attributes, ArrayAttr::get(ctx, attrNames),
+        irdl::VariadicityArrayAttr::get(ctx, attrVariadicity));
   if (!regions.empty())
     consBuilder.create<irdl::RegionsOp>(UnknownLoc::get(ctx), regions);
 



More information about the Mlir-commits mailing list