[mlir][NFC] Migrate to OpAsmAttrInterface for some Builtin Attributes for alias (#128191)

After the introduction of `OpAsmAttrInterface` for alias in #124721, the
natural thought to exercise it would be migrating the MLIR existing
alias generation method, i.e. `OpAsmDialectInterface`, to use the new
interface.

There is a `BuiltinOpAsmDialectInterface` that generates aliases for
`AffineMapAttr` and `IntegerSetAttr`, and these attributes could be
migrated to use `OpAsmAttrInterface`.

However, the tricky part is that `OpAsmAttrInterface` lives in
`OpImplementation.h`. If `BuiltinAttributes.h` includes that, it would
become a cyclic inclusion.

Note that only BuiltinAttribute/Type would face such issue as outside
user can just include `OpImplementation.h` (see downstream example
google/heir#1437)

The dependency is introduced by the fact that `OpAsmAttrInterface` uses
`OpAsmDialectInterface::AliasResult`.

The solution to is: Put the `AliasResult` in `OpAsmSupport.h` that all
interfaces can include that header safely. The API wont break as
`mlir::OpAsmDialectInterface::AliasResult` is a typedef of this class.

Author: ZenithalHourlyRate

mlir/include/mlir/IR/BuiltinAttributeInterfaces.h

@@ -279,6 +279,7 @@ verifyAffineMapAsLayout(AffineMap m, ArrayRef<int64_t> shape,
 //===----------------------------------------------------------------------===//
 
 #include "mlir/IR/BuiltinAttributeInterfaces.h.inc"
+#include "mlir/IR/OpAsmAttrInterface.h.inc"
 
 //===----------------------------------------------------------------------===//
 // ElementsAttr

mlir/include/mlir/IR/BuiltinAttributes.td

@@ -37,7 +37,8 @@ class Builtin_Attr<string name, string attrMnemonic, list<Trait> traits = [],
 //===----------------------------------------------------------------------===//
 
 def Builtin_AffineMapAttr : Builtin_Attr<"AffineMap", "affine_map", [
-    MemRefLayoutAttrInterface
+    MemRefLayoutAttrInterface,
+    OpAsmAttrInterface
   ]> {
   let summary = "An Attribute containing an AffineMap object";
   let description = [{
@@ -63,6 +64,16 @@ def Builtin_AffineMapAttr : Builtin_Attr<"AffineMap", "affine_map", [
   let extraClassDeclaration = [{
     using ValueType = AffineMap;
     AffineMap getAffineMap() const { return getValue(); }
+
+    //===------------------------------------------------------------------===//
+    // OpAsmAttrInterface Methods
+    //===------------------------------------------------------------------===//
+
+    /// Get a name to use when generating an alias for this attribute.
+    ::mlir::OpAsmAliasResult getAlias(::llvm::raw_ostream &os) const {
+      os << "map";
+      return ::mlir::OpAsmAliasResult::OverridableAlias;
+    }
   }];
   let skipDefaultBuilders = 1;
 }
@@ -755,7 +766,7 @@ def Builtin_IntegerAttr : Builtin_Attr<"Integer", "integer",
 // IntegerSetAttr
 //===----------------------------------------------------------------------===//
 
-def Builtin_IntegerSetAttr : Builtin_Attr<"IntegerSet", "integer_set"> {
+def Builtin_IntegerSetAttr : Builtin_Attr<"IntegerSet", "integer_set", [OpAsmAttrInterface]> {
   let summary = "An Attribute containing an IntegerSet object";
   let description = [{
     Syntax:
@@ -776,7 +787,19 @@ def Builtin_IntegerSetAttr : Builtin_Attr<"IntegerSet", "integer_set"> {
       return $_get(value.getContext(), value);
     }]>
   ];
-  let extraClassDeclaration = "using ValueType = IntegerSet;";
+  let extraClassDeclaration = [{
+    using ValueType = IntegerSet;
+
+    //===------------------------------------------------------------------===//
+    // OpAsmAttrInterface Methods
+    //===------------------------------------------------------------------===//
+
+    /// Get a name to use when generating an alias for this attribute.
+    ::mlir::OpAsmAliasResult getAlias(::llvm::raw_ostream &os) const {
+      os << "set";
+      return ::mlir::OpAsmAliasResult::OverridableAlias;
+    }
+  }];
   let skipDefaultBuilders = 1;
 }
 

mlir/include/mlir/IR/BuiltinTypeInterfaces.h

@@ -9,6 +9,7 @@
 #ifndef MLIR_IR_BUILTINTYPEINTERFACES_H
 #define MLIR_IR_BUILTINTYPEINTERFACES_H
 
+#include "mlir/IR/OpAsmSupport.h"
 #include "mlir/IR/Types.h"
 
 namespace llvm {
@@ -21,5 +22,6 @@ class MLIRContext;
 } // namespace mlir
 
 #include "mlir/IR/BuiltinTypeInterfaces.h.inc"
+#include "mlir/IR/OpAsmTypeInterface.h.inc"
 
 #endif // MLIR_IR_BUILTINTYPEINTERFACES_H

mlir/include/mlir/IR/OpAsmInterface.td

@@ -130,9 +130,9 @@ def OpAsmTypeInterface : TypeInterface<"OpAsmTypeInterface"> {
     InterfaceMethod<[{
         Get a name to use when generating an alias for this type.
       }],
-      "::mlir::OpAsmDialectInterface::AliasResult", "getAlias",
+      "::mlir::OpAsmAliasResult", "getAlias",
       (ins "::llvm::raw_ostream&":$os), "",
-      "return ::mlir::OpAsmDialectInterface::AliasResult::NoAlias;"
+      "return ::mlir::OpAsmAliasResult::NoAlias;"
     >,
   ];
 }
@@ -152,9 +152,9 @@ def OpAsmAttrInterface : AttrInterface<"OpAsmAttrInterface"> {
     InterfaceMethod<[{
         Get a name to use when generating an alias for this attribute.
       }],
-      "::mlir::OpAsmDialectInterface::AliasResult", "getAlias",
+      "::mlir::OpAsmAliasResult", "getAlias",
       (ins "::llvm::raw_ostream&":$os), "",
-      "return ::mlir::OpAsmDialectInterface::AliasResult::NoAlias;"
+      "return ::mlir::OpAsmAliasResult::NoAlias;"
     >,
   ];
 }

mlir/include/mlir/IR/OpAsmSupport.h

@@ -0,0 +1,52 @@
+//===- OpAsmSupport.h - OpAsm Interface Utilities ---------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines various classes and utilites for
+// OpAsm{Dialect,Type,Attr,Op}Interface
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef MLIR_IR_OPASMSUPPORT_H_
+#define MLIR_IR_OPASMSUPPORT_H_
+
+#include "mlir/IR/Block.h"
+#include "mlir/IR/Value.h"
+
+namespace mlir {
+
+//===--------------------------------------------------------------------===//
+// Utilities used by OpAsm{Dialect,Op,Type,Attr}Interface.
+//===--------------------------------------------------------------------===//
+
+/// A functor used to set the name of the result. See 'getAsmResultNames' below
+/// for more details.
+using OpAsmSetNameFn = function_ref<void(StringRef)>;
+
+/// A functor used to set the name of the start of a result group of an
+/// operation. See 'getAsmResultNames' below for more details.
+using OpAsmSetValueNameFn = function_ref<void(Value, StringRef)>;
+
+/// A functor used to set the name of blocks in regions directly nested under
+/// an operation.
+using OpAsmSetBlockNameFn = function_ref<void(Block *, StringRef)>;
+
+/// Holds the result of `OpAsm{Dialect,Attr,Type}Interface::getAlias` hook call.
+enum class OpAsmAliasResult {
+  /// The object (type or attribute) is not supported by the hook
+  /// and an alias was not provided.
+  NoAlias,
+  /// An alias was provided, but it might be overriden by other hook.
+  OverridableAlias,
+  /// An alias was provided and it should be used
+  /// (no other hooks will be checked).
+  FinalAlias
+};
+
+} // namespace mlir
+
+#endif // MLIR_IR_OPASMSUPPORT_H_

mlir/include/mlir/IR/OpImplementation.h

@@ -15,6 +15,7 @@
 
 #include "mlir/IR/BuiltinTypes.h"
 #include "mlir/IR/DialectInterface.h"
+#include "mlir/IR/OpAsmSupport.h"
 #include "mlir/IR/OpDefinition.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/SMLoc.h"
@@ -1730,38 +1731,12 @@ class OpAsmParser : public AsmParser {
 // Dialect OpAsm interface.
 //===--------------------------------------------------------------------===//
 
-/// A functor used to set the name of the result. See 'getAsmResultNames' below
-/// for more details.
-using OpAsmSetNameFn = function_ref<void(StringRef)>;
-
-/// A functor used to set the name of the start of a result group of an
-/// operation. See 'getAsmResultNames' below for more details.
-using OpAsmSetValueNameFn = function_ref<void(Value, StringRef)>;
-
-/// A functor used to set the name of blocks in regions directly nested under
-/// an operation.
-using OpAsmSetBlockNameFn = function_ref<void(Block *, StringRef)>;
-
 class OpAsmDialectInterface
     : public DialectInterface::Base<OpAsmDialectInterface> {
 public:
   OpAsmDialectInterface(Dialect *dialect) : Base(dialect) {}
 
-  //===------------------------------------------------------------------===//
-  // Aliases
-  //===------------------------------------------------------------------===//
-
-  /// Holds the result of `getAlias` hook call.
-  enum class AliasResult {
-    /// The object (type or attribute) is not supported by the hook
-    /// and an alias was not provided.
-    NoAlias,
-    /// An alias was provided, but it might be overriden by other hook.
-    OverridableAlias,
-    /// An alias was provided and it should be used
-    /// (no other hooks will be checked).
-    FinalAlias
-  };
+  using AliasResult = OpAsmAliasResult;
 
   /// Hooks for getting an alias identifier alias for a given symbol, that is
   /// not necessarily a part of this dialect. The identifier is used in place of
@@ -1827,9 +1802,7 @@ ParseResult parseDimensionList(OpAsmParser &parser,
 //===--------------------------------------------------------------------===//
 
 /// The OpAsmOpInterface, see OpAsmInterface.td for more details.
-#include "mlir/IR/OpAsmAttrInterface.h.inc"
 #include "mlir/IR/OpAsmOpInterface.h.inc"
-#include "mlir/IR/OpAsmTypeInterface.h.inc"
 
 namespace llvm {
 template <>

mlir/lib/IR/BuiltinDialect.cpp

@@ -48,14 +48,6 @@ struct BuiltinOpAsmDialectInterface : public OpAsmDialectInterface {
       : OpAsmDialectInterface(dialect), blobManager(mgr) {}
 
   AliasResult getAlias(Attribute attr, raw_ostream &os) const override {
-    if (llvm::isa<AffineMapAttr>(attr)) {
-      os << "map";
-      return AliasResult::OverridableAlias;
-    }
-    if (llvm::isa<IntegerSetAttr>(attr)) {
-      os << "set";
-      return AliasResult::OverridableAlias;
-    }
     if (llvm::isa<LocationAttr>(attr)) {
       os << "loc";
       return AliasResult::OverridableAlias;