[Mlir-commits] [mlir] [MLIR][TableGen] Fix EnumAttr not recognized as enum in OpFormatGen (PR #189046)
Mehdi Amini
llvmlistbot at llvm.org
Fri Aug 14 15:55:22 PDT 2026
https://github.com/joker-eph updated https://github.com/llvm/llvm-project/pull/189046
>From 63c472686cb646566931f47dcf7b74675dd18be9 Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Fri, 27 Mar 2026 08:20:37 -0700
Subject: [PATCH] [MLIR][TableGen] Fix EnumAttr not recognized as enum in
OpFormatGen
`Attribute::isEnumAttr()` checked `isSubClassOf("EnumAttrInfo")`, but
the newer `EnumAttr` class (from `mlir/include/mlir/IR/EnumAttr.td`)
extends `AttrDef`, not `EnumAttrInfo`. This caused `canFormatEnumAttr`
in `OpFormatGen.cpp` to return `false` for `EnumAttr`-backed attributes,
forcing them to use the generic format path instead of the enum keyword
format path.
The fix has two parts:
1. Update `Attribute::isEnumAttr()` to also check `isSubClassOf("EnumAttr")`.
2. Add `getEnumInfoRecord()` helper in `OpFormatGen.cpp` that, for
`EnumAttr`-based attributes, retrieves the `enum` sub-field (which is
the actual `EnumInfo` record) rather than the attribute def itself.
Update `canFormatEnumAttr`, `genEnumAttrParser`, and `genEnumAttrPrinter`
to use this helper.
Add a test to `op-format.td` verifying that an `EnumAttr`-wrapped enum
generates the enum keyword format (symbolize/stringify calls).
Fixes #180455
Assisted-by: Claude Code
Assisted-by: Codex
---
mlir/lib/TableGen/Attribute.cpp | 4 +-
mlir/test/IR/enum-attr-invalid.mlir | 9 ++--
mlir/test/mlir-tblgen/op-format.td | 74 ++++++++++++++++++++++++++
mlir/tools/mlir-tblgen/OpFormatGen.cpp | 41 ++++++++++++--
4 files changed, 116 insertions(+), 12 deletions(-)
diff --git a/mlir/lib/TableGen/Attribute.cpp b/mlir/lib/TableGen/Attribute.cpp
index d53ccea576b8b..ec7f4faafeb4e 100644
--- a/mlir/lib/TableGen/Attribute.cpp
+++ b/mlir/lib/TableGen/Attribute.cpp
@@ -52,7 +52,9 @@ bool Attribute::isSymbolRefAttr() const {
return isSubClassOf("SymbolRefAttr") || isSubClassOf("FlatSymbolRefAttr");
}
-bool Attribute::isEnumAttr() const { return isSubClassOf("EnumAttrInfo"); }
+bool Attribute::isEnumAttr() const {
+ return isSubClassOf("EnumAttrInfo") || isSubClassOf("EnumAttr");
+}
StringRef Attribute::getStorageType() const {
const auto *init = def->getValueInit("storageType");
diff --git a/mlir/test/IR/enum-attr-invalid.mlir b/mlir/test/IR/enum-attr-invalid.mlir
index 2f240a56c9874..7f9a80737480e 100644
--- a/mlir/test/IR/enum-attr-invalid.mlir
+++ b/mlir/test/IR/enum-attr-invalid.mlir
@@ -1,17 +1,15 @@
// RUN: mlir-opt -verify-diagnostics -split-input-file %s
func.func @test_invalid_enum_case() -> () {
- // expected-error at +2 {{expected test::TestEnum to be one of: first, second, third}}
- // expected-error at +1 {{failed to parse TestEnumAttr}}
+ // expected-error at +1 {{expected string or keyword containing one of the following enum values for attribute 'value' [first, second, third]}}
test.op_with_enum #test<enum fourth>
}
// -----
func.func @test_invalid_enum_case() -> () {
- // expected-error at +1 {{expected test::TestEnum to be one of: first, second, third}}
+ // expected-error at +1 {{expected string or keyword containing one of the following enum values for attribute 'value' [first, second, third]}}
test.op_with_enum fourth
- // expected-error at +1 {{failed to parse TestEnumAttr}}
}
// -----
@@ -24,8 +22,7 @@ func.func @test_invalid_attr() -> () {
// -----
func.func @test_parse_invalid_attr() -> () {
- // expected-error at +2 {{expected valid keyword}}
- // expected-error at +1 {{failed to parse TestEnumAttr parameter 'value'}}
+ // expected-error at +1 {{expected string or keyword containing one of the following enum values for attribute 'value' [first, second, third]}}
test.op_with_enum 1 : index
}
diff --git a/mlir/test/mlir-tblgen/op-format.td b/mlir/test/mlir-tblgen/op-format.td
index 6c7e6961abfa7..8c6359b89d88a 100644
--- a/mlir/test/mlir-tblgen/op-format.td
+++ b/mlir/test/mlir-tblgen/op-format.td
@@ -1,6 +1,7 @@
// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s
include "mlir/IR/OpBase.td"
+include "mlir/IR/EnumAttr.td"
def TestDialect : Dialect {
let name = "test";
@@ -102,6 +103,79 @@ def CustomStringLiteralD : TestFormat_Op<[{
custom<Foo>(prop-dict) attr-dict
}]>;
+//===----------------------------------------------------------------------===//
+// EnumAttr formatting
+//===----------------------------------------------------------------------===//
+
+// Test that EnumAttr (backed by EnumInfo, not EnumAttrInfo) is recognized as
+// an enum attribute and uses the enum-keyword format path.
+
+def TestEnumCase0 : I32EnumCase<"Case0", 0>;
+def TestEnumCase1 : I32EnumCase<"Case1", 1>;
+
+def TestEnum : I32Enum<"TestEnum", "a test enum", [TestEnumCase0, TestEnumCase1]> {
+ let cppNamespace = "::test";
+}
+
+def TestEnumAttr : EnumAttr<TestDialect, TestEnum, "enum">;
+
+// Default-valued optional attributes have a non-optional getter.
+
+// CHECK-LABEL: DefaultOptionalEnumAttrOp::print
+// CHECK: auto caseValue = getAttr();
+def DefaultOptionalEnumAttrOp : TestFormat_Op<"($attr^)? attr-dict">,
+ Arguments<(ins DefaultValuedOptionalAttr<
+ TestEnumAttr, "::test::TestEnum::Case0">:$attr)>;
+
+// CHECK-LABEL: EnumAttrOp::parse
+// CHECK: symbolizeTestEnum
+// CHECK-LABEL: EnumAttrOp::print
+// CHECK: stringifyTestEnum
+def EnumAttrOp : TestFormat_Op<"$attr attr-dict">,
+ Arguments<(ins TestEnumAttr:$attr)>;
+
+// Unquoted bit enums use a separator-aware attribute parser instead of the
+// operation-level enum parser, which only accepts one keyword or string.
+
+def TestBitEnumNone : I32BitEnumCaseNone<"None">;
+def TestBitEnumBit0 : I32BitEnumCaseBit<"Bit0", 0>;
+def TestBitEnumBit1 : I32BitEnumCaseBit<"Bit1", 1>;
+
+def TestBitEnum : I32BitEnum<"TestBitEnum", "a test bit enum",
+ [TestBitEnumNone, TestBitEnumBit0,
+ TestBitEnumBit1]> {
+ let cppNamespace = "::test";
+ let printBitEnumQuoted = 0;
+}
+
+def TestBitEnumAttr : EnumAttr<TestDialect, TestBitEnum, "bit_enum">;
+
+// CHECK-LABEL: EnumAttrUnquotedBitOp::parse
+// CHECK: parseCustomAttributeWithFallback
+// CHECK-NOT: symbolizeTestBitEnum
+// CHECK-LABEL: EnumAttrUnquotedBitOp::print
+// CHECK: printStrippedAttrOrType(getAttrAttr())
+def EnumAttrUnquotedBitOp : TestFormat_Op<"$attr attr-dict">,
+ Arguments<(ins TestBitEnumAttr:$attr)>;
+
+// Test that legacy EnumAttrInfo-based attributes (I32EnumAttr) also use the
+// enum-keyword format path.
+
+def LegacyEnumCase0 : I32EnumAttrCase<"LCase0", 0>;
+def LegacyEnumCase1 : I32EnumAttrCase<"LCase1", 1>;
+
+def LegacyTestEnum : I32EnumAttr<"LegacyTestEnum", "a legacy test enum",
+ [LegacyEnumCase0, LegacyEnumCase1]> {
+ let cppNamespace = "::test";
+}
+
+// CHECK-LABEL: LegacyEnumAttrOp::parse
+// CHECK: symbolizeLegacyTestEnum
+// CHECK-LABEL: LegacyEnumAttrOp::print
+// CHECK: stringifyLegacyTestEnum
+def LegacyEnumAttrOp : TestFormat_Op<"$attr attr-dict">,
+ Arguments<(ins LegacyTestEnum:$attr)>;
+
//===----------------------------------------------------------------------===//
// Optional Groups
//===----------------------------------------------------------------------===//
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index 548851503c445..470d27cfc6060 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -447,13 +447,43 @@ struct OperationFormat {
// Parser Gen
//===----------------------------------------------------------------------===//
+/// Returns the Record to use when constructing an EnumInfo for the given
+/// attribute. For legacy EnumAttrInfo-based attributes, this is the attribute
+/// def itself (which extends both EnumInfo and Attr). For newer EnumAttr-based
+/// attributes (which extend AttrDef), this is the `enum` sub-field.
+static const llvm::Record *getEnumInfoRecord(const Attribute &attr) {
+ if (attr.isSubClassOf("EnumAttr"))
+ return attr.getDef().getValueAsDef("enum");
+ return &attr.getDef();
+}
+
/// Returns true if we can format the given attribute as an enum in the
/// parser format.
static bool canFormatEnumAttr(const NamedAttribute *attr) {
Attribute baseAttr = attr->attr.getBaseAttr();
if (!baseAttr.isEnumAttr())
return false;
- EnumInfo enumInfo(&baseAttr.getDef());
+
+ // For newer EnumAttr-based attributes (which extend AttrDef), only apply
+ // enum keyword formatting when the attribute uses the default "$value"
+ // assembly format. If it has a custom format (e.g., `<` $value `>`), the
+ // attribute's own AttrDef parser/printer handles formatting — using the
+ // keyword path here would conflict with that custom format.
+ if (baseAttr.isSubClassOf("EnumAttr")) {
+ llvm::StringRef asmFmt =
+ baseAttr.getDef().getValueAsString("assemblyFormat");
+ if (asmFmt != "$value")
+ return false;
+ }
+
+ EnumInfo enumInfo(getEnumInfoRecord(baseAttr));
+
+ // Unquoted bit enums may consist of multiple keywords separated by a comma
+ // or vertical bar. Their attribute parser handles this syntax, whereas the
+ // operation-level enum parser expects a single keyword or string.
+ if (baseAttr.isSubClassOf("EnumAttr") && enumInfo.isBitEnum() &&
+ !enumInfo.printBitEnumQuoted())
+ return false;
// The attribute must have a valid underlying type and a constant builder.
return !enumInfo.getUnderlyingType().empty() &&
@@ -1181,7 +1211,7 @@ static void genEnumAttrParser(const NamedAttribute *var, MethodBody &body,
FmtContext &attrTypeCtx, bool parseAsOptional,
bool useProperties, StringRef opCppClassName) {
Attribute baseAttr = var->attr.getBaseAttr();
- EnumInfo enumInfo(&baseAttr.getDef());
+ EnumInfo enumInfo(getEnumInfoRecord(baseAttr));
std::vector<EnumCase> cases = enumInfo.getAllCases();
// Generate the code for building an attribute for this enum.
@@ -2340,12 +2370,13 @@ static MethodBody &genTypeOperandPrinter(FormatElement *arg, const Operator &op,
static void genEnumAttrPrinter(const NamedAttribute *var, const Operator &op,
MethodBody &body) {
Attribute baseAttr = var->attr.getBaseAttr();
- const EnumInfo enumInfo(&baseAttr.getDef());
+ const EnumInfo enumInfo(getEnumInfoRecord(baseAttr));
std::vector<EnumCase> cases = enumInfo.getAllCases();
+ bool dereferenceGetter =
+ var->attr.isOptional() && !var->attr.hasDefaultValue();
body << formatv(enumAttrBeginPrinterCode,
- (var->attr.isOptional() ? "*" : "") +
- op.getGetterName(var->name),
+ (dereferenceGetter ? "*" : "") + op.getGetterName(var->name),
enumInfo.getSymbolToStringFnName());
// Get a string containing all of the cases that can't be represented with a
More information about the Mlir-commits
mailing list