[Mlir-commits] [mlir] 86d68e2 - [mlir] Gen removeAttr methods with tablegen

[Mehdi Amini]

Author: Felipe de Azevedo Piovezan
Date: 2021-01-05T18:48:09Z
New Revision: 86d68e288585964546d6382ecf71dcce10d018b7

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

LOG: [mlir] Gen removeAttr methods with tablegen

If an operation defines an optional attribute (OptionalAttr or
UnitAttr), transformations may wish to remove these attributes while
maintaining invariants established by the operation. Currently, the only
way to do this is by calling `Operation::removeAttr("attrName")`, which
requires developers to know the exact name of the attribute used by
table-gen. Furthermore, if the attribute name changes, this won't be
detected at compile time. Instead, `removeAttr` would return an empty
attribute and no errors would be raised, unless the caller checks for
the returned value.

This patch adds table gen support for generating `remove<AttrName>Attr`
methods for OptionalAttributes defined by operations.

Implementation choice: to preserve camelCase for the method's name, the
first character of an attribute called `myAttr` is changed to upper case
in order to preserve the coding style, so the final method would be
called `removeMyAttr`.

Reviewed By: mehdi_amini

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

Added: 
    

Modified: 
    mlir/test/mlir-tblgen/op-attribute.td
    mlir/test/mlir-tblgen/op-decl.td
    mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/test/mlir-tblgen/op-attribute.td b/mlir/test/mlir-tblgen/op-attribute.td
index 833f90dd28a2..b4c193c30c51 100644
--- a/mlir/test/mlir-tblgen/op-attribute.td
+++ b/mlir/test/mlir-tblgen/op-attribute.td
@@ -77,6 +77,12 @@ def AOp : NS_Op<"a_op", []> {
 // DEF:      void AOp::cAttrAttr(some-attr-kind attr) {
 // DEF-NEXT:   (*this)->setAttr("cAttr", attr);
 
+// Test remove methods
+// ---
+
+// DEF: Attribute AOp::removeCAttrAttr() {
+// DEF-NEXT: return (*this)->removeAttr("cAttr");
+
 // Test build methods
 // ---
 
@@ -265,6 +271,9 @@ def UnitAttrOp : NS_Op<"unit_attr_op", []> {
 // DEF: bool UnitAttrOp::attr() {
 // DEF:   return {{.*}} != nullptr
 
+// DEF: Attribute UnitAttrOp::removeAttrAttr() {
+// DEF-NEXT:   (*this)->removeAttr("attr");
+
 // DEF: build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, /*optional*/::mlir::UnitAttr attr)
 
 

diff  --git a/mlir/test/mlir-tblgen/op-decl.td b/mlir/test/mlir-tblgen/op-decl.td
index 13daca67c475..d4d1a8b012c6 100644
--- a/mlir/test/mlir-tblgen/op-decl.td
+++ b/mlir/test/mlir-tblgen/op-decl.td
@@ -81,6 +81,7 @@ def NS_AOp : NS_Op<"a_op", [IsolatedFromAbove, IsolatedFromAbove]> {
 // CHECK:   uint32_t attr1();
 // CHECK:   ::mlir::FloatAttr attr2Attr()
 // CHECK:   ::llvm::Optional< ::llvm::APFloat > attr2();
+// CHECK:   Attribute removeAttr2Attr();
 // CHECK:   static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, Value val);
 // CHECK:   static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, int integer = 0);
 // CHECK:   static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::Type r, ::mlir::TypeRange s, ::mlir::Value a, ::mlir::ValueRange b, ::mlir::IntegerAttr attr1, /*optional*/::mlir::FloatAttr attr2, unsigned someRegionsCount)

diff  --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 995b4cd05cd5..468fd7848d82 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -328,6 +328,9 @@ class OpEmitter {
   // Generates setter for the attributes.
   void genAttrSetters();
 
+  // Generates removers for optional attributes.
+  void genOptionalAttrRemovers();
+
   // Generates getters for named operands.
   void genNamedOperandGetters();
 
@@ -600,6 +603,7 @@ OpEmitter::OpEmitter(const Operator &op,
   genNamedSuccessorGetters();
   genAttrGetters();
   genAttrSetters();
+  genOptionalAttrRemovers();
   genBuilder();
   genParser();
   genPrinter();
@@ -777,6 +781,28 @@ void OpEmitter::genAttrSetters() {
   }
 }
 
+void OpEmitter::genOptionalAttrRemovers() {
+  // Generate methods for removing optional attributes, instead of having to
+  // use the string interface. Enables better compile time verification.
+  auto emitRemoveAttr = [&](StringRef name) {
+    auto upperInitial = name.take_front().upper();
+    auto suffix = name.drop_front();
+    auto *method = opClass.addMethodAndPrune(
+        "Attribute", ("remove" + upperInitial + suffix + "Attr").str());
+    if (!method)
+      return;
+    auto &body = method->body();
+    body << "  return (*this)->removeAttr(\"" << name << "\");";
+  };
+
+  for (const auto &namedAttr : op.getAttributes()) {
+    const auto &name = namedAttr.name;
+    const auto &attr = namedAttr.attr;
+    if (attr.isOptional())
+      emitRemoveAttr(name);
+  }
+}
+
 // Generates the code to compute the start and end index of an operand or result
 // range.
 template <typename RangeT>