[Mlir-commits] [mlir] c3c1230 - [mlir][NFC] Migrate to OpAsmAttrInterface for some Builtin Attributes for alias (#128191)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Wed Mar 5 09:20:18 PST 2025
Author: Hongren Zheng
Date: 2025-03-06T01:20:15+08:00
New Revision: c3c1230e731af9e989c4235e451d3d4b35adc512
URL: https://github.com/llvm/llvm-project/commit/c3c1230e731af9e989c4235e451d3d4b35adc512
DIFF: https://github.com/llvm/llvm-project/commit/c3c1230e731af9e989c4235e451d3d4b35adc512.diff
LOG: [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
https://github.com/google/heir/pull/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.
Added:
mlir/include/mlir/IR/OpAsmSupport.h
Modified:
mlir/include/mlir/IR/BuiltinAttributeInterfaces.h
mlir/include/mlir/IR/BuiltinAttributes.td
mlir/include/mlir/IR/BuiltinTypeInterfaces.h
mlir/include/mlir/IR/OpAsmInterface.td
mlir/include/mlir/IR/OpImplementation.h
mlir/lib/IR/BuiltinDialect.cpp
Removed:
################################################################################
diff --git a/mlir/include/mlir/IR/BuiltinAttributeInterfaces.h b/mlir/include/mlir/IR/BuiltinAttributeInterfaces.h
index c4a42020d1389..b969b60a66f16 100644
--- a/mlir/include/mlir/IR/BuiltinAttributeInterfaces.h
+++ b/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
diff --git a/mlir/include/mlir/IR/BuiltinAttributes.td b/mlir/include/mlir/IR/BuiltinAttributes.td
index 06f5e172a9909..6826d1a437775 100644
--- a/mlir/include/mlir/IR/BuiltinAttributes.td
+++ b/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;
}
diff --git a/mlir/include/mlir/IR/BuiltinTypeInterfaces.h b/mlir/include/mlir/IR/BuiltinTypeInterfaces.h
index e8011b5488dc9..5f14517d8dd71 100644
--- a/mlir/include/mlir/IR/BuiltinTypeInterfaces.h
+++ b/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
diff --git a/mlir/include/mlir/IR/OpAsmInterface.td b/mlir/include/mlir/IR/OpAsmInterface.td
index 1bd8eb04714c5..1838b248a2c7a 100644
--- a/mlir/include/mlir/IR/OpAsmInterface.td
+++ b/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;"
>,
];
}
diff --git a/mlir/include/mlir/IR/OpAsmSupport.h b/mlir/include/mlir/IR/OpAsmSupport.h
new file mode 100644
index 0000000000000..58528878d6a45
--- /dev/null
+++ b/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_
diff --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h
index d80bff2d78217..25c7d15eb8ed5 100644
--- a/mlir/include/mlir/IR/OpImplementation.h
+++ b/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 <>
diff --git a/mlir/lib/IR/BuiltinDialect.cpp b/mlir/lib/IR/BuiltinDialect.cpp
index 99796c5f1c371..2867d9b4ef68a 100644
--- a/mlir/lib/IR/BuiltinDialect.cpp
+++ b/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;
More information about the Mlir-commits
mailing list