ODS: Generate named accessors for raw attributes

Currently named accessors are generated for attributes returning a consumer
friendly type. But sometimes the attributes are used while transforming an
existing op and then the returned type has to be converted back into an
attribute or the raw `getAttr` needs to be used. Generate raw named accessor
for attributes to reference the raw attributes without having to use the string
interface for better compile time verification. This allows calling
`blahAttr()` instead of `getAttr("blah")`.

Raw here refers to returning the underlying storage attribute.

PiperOrigin-RevId: 284583426

mlir/g3doc/OpDefinitions.md

@@ -187,7 +187,6 @@ led by `ins`:
 let arguments = (ins
   <type-constraint>:$<operand-name>,
   ...
-
   <attr-constraint>:$<attr-name>,
   ...
 );
@@ -199,17 +198,18 @@ hierarchy. Similarly, `<attr-constraint>` is a TableGen `def` from the
 information.
 
 There is no requirements on the relative order of operands and attributes; they
-can mix freely. But it is recommended to put all operands ahead of attributes,
-and use an empty line to separate them to make it more visually distinguishable
-if possible. The relative order of operands themselves matters.
+can mix freely. The relative order of operands themselves matters. From each
+named argument a named getter will be generated that returns the argument with
+the return type (in the case of attributes the return type will be
+constructed from the storage type, while for operands it will be `Value`). Each
+attribute's raw value (e.g., as stored) can also be accessed via generated
+`<name>Attr` getters for use in transformation passes where the more user
+friendly return type is less suitable.
 
 All the arguments should be named to 1) provide documentation, 2) drive
 auto-generation of getter methods, 3) provide a handle to reference for other
 places like constraints.
 
-> * Place attributes after operands if possible
-> * Give operands and attribute proper names
-
 #### Variadic operands
 
 To declare a variadic operand, wrap the `TypeConstraint` for the operand with

mlir/test/mlir-tblgen/op-attribute.td

@@ -1,5 +1,5 @@
-// RUN: mlir-tblgen -gen-op-decls -I %S/../../include %s | FileCheck %s --check-prefix=DECL
-// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s --check-prefix=DEF
+// RUN: mlir-tblgen -gen-op-decls -I %S/../../include %s | FileCheck %s --check-prefix=DECL --dump-input-on-failure
+// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s --check-prefix=DEF --dump-input-on-failure
 
 include "mlir/IR/OpBase.td"
 
@@ -32,18 +32,24 @@ def AOp : NS_Op<"a_op", []> {
 // Test getter methods
 // ---
 
+// DEF:      some-attr-kind AOp::aAttrAttr()
+// DEF-NEXT:   this->getAttr("aAttr").cast<some-attr-kind>()
 // DEF:      some-return-type AOp::aAttr() {
-// DEF-NEXT:   auto attr = this->getAttr("aAttr").cast<some-attr-kind>();
+// DEF-NEXT:   auto attr = aAttrAttr()
 // DEF-NEXT:   return attr.some-convert-from-storage();
 
+// DEF:      some-attr-kind AOp::bAttrAttr()
+// DEF-NEXT:   return this->getAttr("bAttr").dyn_cast_or_null<some-attr-kind>()
 // DEF:      some-return-type AOp::bAttr() {
-// DEF-NEXT:   auto attr = this->getAttr("bAttr").dyn_cast_or_null<some-attr-kind>();
+// DEF-NEXT:   auto attr = bAttrAttr();
 // DEF-NEXT:   if (!attr)
 // DEF-NEXT:       return some-const-builder-call(mlir::Builder(this->getContext()), 4.2).some-convert-from-storage();
 // DEF-NEXT:   return attr.some-convert-from-storage();
 
+// DEF:      some-attr-kind AOp::cAttrAttr()
+// DEF-NEXT:   return this->getAttr("cAttr").dyn_cast_or_null<some-attr-kind>()
 // DEF:      Optional<some-return-type> AOp::cAttr() {
-// DEF-NEXT:   auto attr = this->getAttr("cAttr").dyn_cast_or_null<some-attr-kind>();
+// DEF-NEXT:   auto attr = cAttrAttr()
 // DEF-NEXT:   return attr ? Optional<some-return-type>(attr.some-convert-from-storage()) : (llvm::None);
 
 // Test build methods

mlir/test/mlir-tblgen/op-decl.td

@@ -63,7 +63,9 @@ def NS_AOp : NS_Op<"a_op", [NoSideEffect]> {
 // CHECK:   Operation::result_range getODSResults(unsigned index);
 // CHECK:   Value *r();
 // CHECK:   Region &someRegion();
+// CHECK:   IntegerAttr attr1Attr()
 // CHECK:   APInt attr1();
+// CHECK:   FloatAttr attr2Attr()
 // CHECK:   Optional< APFloat > attr2();
 // CHECK:   static void build(Value *val);
 // CHECK:   static void build(Builder *tblgen_builder, OperationState &tblgen_state, Type r, ArrayRef<Type> s, Value *a, ValueRange b, IntegerAttr attr1, /*optional*/FloatAttr attr2);

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

@@ -464,8 +464,7 @@ void OpClass::writeDeclTo(raw_ostream &os) const {
     os << extraClassDeclaration << "\n";
 
   if (hasPrivateMethod) {
-    os << '\n';
-    os << "private:\n";
+    os << "\nprivate:\n";
     for (const auto &method : methods) {
       if (method.isPrivate()) {
         method.writeDeclTo(os);
@@ -668,29 +667,19 @@ void OpEmitter::emitDef(raw_ostream &os) { opClass.writeDefTo(os); }
 void OpEmitter::genAttrGetters() {
   FmtContext fctx;
   fctx.withBuilder("mlir::Builder(this->getContext())");
-  for (auto &namedAttr : op.getAttributes()) {
-    const auto &name = namedAttr.name;
-    const auto &attr = namedAttr.attr;
-
-    auto &method = opClass.newMethod(attr.getReturnType(), name);
-    auto &body = method.body();
 
   // Emit the derived attribute body.
-    if (attr.isDerivedAttr()) {
+  auto emitDerivedAttr = [&](StringRef name, Attribute attr) {
+    auto &method = opClass.newMethod(attr.getReturnType(), name);
+    auto &body = method.body();
     body << "  " << attr.getDerivedCodeBody() << "\n";
-      continue;
-    }
+  };
 
-    // Emit normal emitter.
-
-    // Return the queried attribute with the correct return type.
-    auto attrVal =
-        (attr.hasDefaultValue() || attr.isOptional())
-            ? formatv("this->getAttr(\"{0}\").dyn_cast_or_null<{1}>()", name,
-                      attr.getStorageType())
-            : formatv("this->getAttr(\"{0}\").cast<{1}>()", name,
-                      attr.getStorageType());
-    body << "  auto attr = " << attrVal << ";\n";
+  // Emit with return type specified.
+  auto emitAttrWithReturnType = [&](StringRef name, Attribute attr) {
+    auto &method = opClass.newMethod(attr.getReturnType(), name);
+    auto &body = method.body();
+    body << "  auto attr = " << name << "Attr();\n";
     if (attr.hasDefaultValue()) {
       // Returns the default value if not set.
       // TODO: this is inefficient, we are recreating the attribute for every
@@ -705,6 +694,32 @@ void OpEmitter::genAttrGetters() {
     body << "  return "
          << tgfmt(attr.getConvertFromStorageCall(), &fctx.withSelf("attr"))
          << ";\n";
+  };
+
+  // Generate raw named accessor type. This is a wrapper class that allows
+  // referring to the attributes via accessors instead of having to use
+  // the string interface for better compile time verification.
+  auto emitAttrWithStorageType = [&](StringRef name, Attribute attr) {
+    auto &method =
+        opClass.newMethod(attr.getStorageType(), (name + "Attr").str());
+    auto &body = method.body();
+    body << "  return this->getAttr(\"" << name << "\").";
+    if (attr.isOptional() || attr.hasDefaultValue())
+      body << "dyn_cast_or_null<";
+    else
+      body << "cast<";
+    body << attr.getStorageType() << ">();";
+  };
+
+  for (auto &namedAttr : op.getAttributes()) {
+    const auto &name = namedAttr.name;
+    const auto &attr = namedAttr.attr;
+    if (attr.isDerivedAttr()) {
+      emitDerivedAttr(name, attr);
+    } else {
+      emitAttrWithStorageType(name, attr);
+      emitAttrWithReturnType(name, attr);
+    }
   }
 }