[Mlir-commits] [mlir] [mlir][mlir-tblgen] Emit correct error message if method is pruned (PR #160334)
Justin Kim
llvmlistbot at llvm.org
Sun Sep 28 04:46:48 PDT 2025
https://github.com/JustinKim98 updated https://github.com/llvm/llvm-project/pull/160334
>From 0e28604f8687389ad2db09f83a029121d577e86f Mon Sep 17 00:00:00 2001
From: Justin Kim <jaewoo.kim at hyperaccel.ai>
Date: Wed, 24 Sep 2025 00:57:59 +0900
Subject: [PATCH 1/4] [mlir][mlir-tblgen] emit correct error message if method
is pruned
---
.../mlir/Dialect/Tosa/IR/TosaTypesBase.td | 3 +--
mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp | 19 +++++++++++++++++++
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/mlir/include/mlir/Dialect/Tosa/IR/TosaTypesBase.td b/mlir/include/mlir/Dialect/Tosa/IR/TosaTypesBase.td
index 553d69cc21d17..93ab120339d55 100644
--- a/mlir/include/mlir/Dialect/Tosa/IR/TosaTypesBase.td
+++ b/mlir/include/mlir/Dialect/Tosa/IR/TosaTypesBase.td
@@ -282,8 +282,7 @@ def Tosa_Shape : Tosa_Type<"shape", "shape"> {
!tosa.shape<0>
```
}];
- let parameters = (ins "int" : $rank);
- let builders = [TypeBuilder<(ins "int" : $rank)>];
+ let parameters = (ins "int":$rank);
let assemblyFormat = "`<` $rank `>`";
let genVerifyDecl = 1;
diff --git a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
index 3140f12c0b7e8..b27e4cdcd5b38 100644
--- a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
+++ b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
@@ -513,6 +513,17 @@ getCustomBuilderParams(std::initializer_list<MethodParameter> prefix,
return builderParams;
}
+static void errorIfPruned(size_t line, Method *m, const Twine &methodName,
+ const AttrOrTypeDef &def) {
+ if (m)
+ return;
+ PrintFatalError(def.getLoc(), "Unexpected overlap when generating `" +
+ methodName + "` for " + def.getName() +
+ " (from line " + Twine(line) + ")");
+}
+
+#define ERROR_IF_PRUNED(M, N, O) errorIfPruned(__LINE__, M, N, O)
+
void DefGen::emitCustomBuilder(const AttrOrTypeBuilder &builder) {
// Don't emit a body if there isn't one.
auto props = builder.getBody() ? Method::Static : Method::StaticDeclaration;
@@ -521,6 +532,10 @@ void DefGen::emitCustomBuilder(const AttrOrTypeBuilder &builder) {
returnType = *builderReturnType;
Method *m = defCls.addMethod(returnType, "get", props,
getCustomBuilderParams({}, builder));
+
+ // If method is pruned, report error and terminate.
+ ERROR_IF_PRUNED(m, "get", def);
+
if (!builder.getBody())
return;
@@ -552,6 +567,10 @@ void DefGen::emitCheckedCustomBuilder(const AttrOrTypeBuilder &builder) {
getCustomBuilderParams(
{{"::llvm::function_ref<::mlir::InFlightDiagnostic()>", "emitError"}},
builder));
+
+ // If method is pruned, report error and terminate.
+ ERROR_IF_PRUNED(m, "getChecked", def);
+
if (!builder.getBody())
return;
>From cac13aa6e594e59839488a64b97e07ea503611ac Mon Sep 17 00:00:00 2001
From: Justin Kim <jaewoo.kim at hyperaccel.ai>
Date: Thu, 25 Sep 2025 01:04:55 +0900
Subject: [PATCH 2/4] fix : check for pruned builder regardless of body
prescence
---
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 4fdde76a613bb..7e8e559baf878 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -3104,8 +3104,8 @@ void OpEmitter::genBuilder() {
std::optional<StringRef> body = builder.getBody();
auto properties = body ? Method::Static : Method::StaticDeclaration;
auto *method = opClass.addMethod("void", "build", properties, arguments);
- if (body)
- ERROR_IF_PRUNED(method, "build", op);
+
+ ERROR_IF_PRUNED(method, "build", op);
if (method)
method->setDeprecated(builder.getDeprecatedMessage());
>From 3525cc4c834c09164b7d59235a2a1728ea99b057 Mon Sep 17 00:00:00 2001
From: Justin Kim <jaewoo.kim at hyperaccel.ai>
Date: Thu, 25 Sep 2025 13:50:21 +0900
Subject: [PATCH 3/4] feat : add test for duplicated builders
---
.../attr-duplicated-builder-error.td | 39 +++++++++++++++++
.../attr-duplicated-custom-builders-error.td | 43 +++++++++++++++++++
2 files changed, 82 insertions(+)
create mode 100644 mlir/test/mlir-tblgen/attr-duplicated-builder-error.td
create mode 100644 mlir/test/mlir-tblgen/attr-duplicated-custom-builders-error.td
diff --git a/mlir/test/mlir-tblgen/attr-duplicated-builder-error.td b/mlir/test/mlir-tblgen/attr-duplicated-builder-error.td
new file mode 100644
index 0000000000000..8ac63cad9c6a8
--- /dev/null
+++ b/mlir/test/mlir-tblgen/attr-duplicated-builder-error.td
@@ -0,0 +1,39 @@
+// RUN: not mlir-tblgen -gen-attrdef-decls -I %S/../../include %s 2>&1 | FileCheck %s
+
+include "mlir/IR/OpBase.td"
+
+def Test_Dialect : Dialect {
+ let name = "test";
+ let cppNamespace = "::test";
+}
+
+class TestAttr<string attrName, string attrMnemonic, list<Trait> traits = []>
+ : AttrDef<Test_Dialect, attrName, traits> {
+ let mnemonic = attrMnemonic;
+}
+
+def TestAttr : TestAttr<"Test", "test"> {
+ let summary = "Test attrubute";
+ let description = "Test attribute";
+
+ let parameters = (ins AttrParameter<"std::int64_t", "arg">:$arg);
+ let builders = [AttrBuilder<(ins "std::int64_t":$arg), [{
+ return $_get($_ctxt, arg);
+ }]>];
+
+ let assemblyFormat = "`<` $arg `>`";
+
+ let skipDefaultBuilders = 0;
+ let genVerifyDecl = 1;
+ let genMnemonicAlias = 1;
+}
+
+def Test_TestAttrOp : Op<Test_Dialect, "test", []> {
+ let summary = "test operation with attribute";
+ let description = "test operation with attribute";
+
+ let arguments = (ins TestAttr:$testAttr);
+ let assemblyFormat = "$testAttr attr-dict";
+}
+
+// CHECK: attr-duplicated-builder-error.td:15:5: error: Unexpected overlap when generating `get` for TestAttr (from line 537)
diff --git a/mlir/test/mlir-tblgen/attr-duplicated-custom-builders-error.td b/mlir/test/mlir-tblgen/attr-duplicated-custom-builders-error.td
new file mode 100644
index 0000000000000..12615d0c66e97
--- /dev/null
+++ b/mlir/test/mlir-tblgen/attr-duplicated-custom-builders-error.td
@@ -0,0 +1,43 @@
+// RUN: not mlir-tblgen -gen-attrdef-decls -I %S/../../include %s 2>&1 | FileCheck %s
+
+include "mlir/IR/OpBase.td"
+
+def Test_Dialect : Dialect {
+ let name = "test";
+ let cppNamespace = "::test";
+}
+
+class TestAttr<string attrName, string attrMnemonic, list<Trait> traits = []>
+ : AttrDef<Test_Dialect, attrName, traits> {
+ let mnemonic = attrMnemonic;
+}
+
+def TestAttr : TestAttr<"Test", "test"> {
+ let summary = "Test attrubute";
+ let description = "Test attribute";
+
+ let parameters = (ins AttrParameter<"std::int64_t", "arg">:$arg);
+ let builders = [AttrBuilder<(ins "std::int64_t":$arg), [{
+ return $_get($_ctxt, arg);
+ }]>,
+ AttrBuilder<(ins "std::int64_t":$arg), [{
+ // Duplicated builder
+ return $_get($_ctxt, arg);
+ }]>];
+
+ let assemblyFormat = "`<` $arg `>`";
+
+ let skipDefaultBuilders = 1;
+ let genVerifyDecl = 1;
+ let genMnemonicAlias = 1;
+}
+
+def Test_TestAttrOp : Op<Test_Dialect, "test", []> {
+ let summary = "test operation with attribute";
+ let description = "test operation with attribute";
+
+ let arguments = (ins TestAttr:$testAttr);
+ let assemblyFormat = "$testAttr attr-dict";
+}
+
+// CHECK: attr-duplicated-custom-builders-error.td:15:5: error: Unexpected overlap when generating `get` for TestAttr (from line {{.*}})
>From 618b571e73183b04f8d04e60f9a2350af83dfab3 Mon Sep 17 00:00:00 2001
From: Justin Kim <jaewoo.kim at hyperaccel.ai>
Date: Sun, 28 Sep 2025 17:37:55 +0900
Subject: [PATCH 4/4] refactor : improve error message for conflicting builders
---
mlir/include/mlir/TableGen/Class.h | 4 ++
.../attr-duplicated-builder-error.td | 11 +++-
.../attr-duplicated-custom-builders-error.td | 11 +++-
mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp | 66 ++++++++++++++-----
4 files changed, 73 insertions(+), 19 deletions(-)
diff --git a/mlir/include/mlir/TableGen/Class.h b/mlir/include/mlir/TableGen/Class.h
index 10349676625d1..e6bedc7cc896d 100644
--- a/mlir/include/mlir/TableGen/Class.h
+++ b/mlir/include/mlir/TableGen/Class.h
@@ -789,6 +789,10 @@ class Class {
std::forward<Args>(args)...);
}
+ const std::vector<std::unique_ptr<Method>> &getMethods() const {
+ return methods;
+ }
+
/// Add a new field to the class. Class fields added this way are always
/// private.
template <typename TypeT, typename NameT>
diff --git a/mlir/test/mlir-tblgen/attr-duplicated-builder-error.td b/mlir/test/mlir-tblgen/attr-duplicated-builder-error.td
index 8ac63cad9c6a8..5f1c61a3a505d 100644
--- a/mlir/test/mlir-tblgen/attr-duplicated-builder-error.td
+++ b/mlir/test/mlir-tblgen/attr-duplicated-builder-error.td
@@ -36,4 +36,13 @@ def Test_TestAttrOp : Op<Test_Dialect, "test", []> {
let assemblyFormat = "$testAttr attr-dict";
}
-// CHECK: attr-duplicated-builder-error.td:15:5: error: Unexpected overlap when generating `get` for TestAttr (from line 537)
+// CHECK: attr-duplicated-builder-error.td:20:7: error: builder `get` conflicts with an existing builder.
+// CHECK-NEXT: let builders = [AttrBuilder<(ins "std::int64_t":$arg), [{
+// CHECK-NEXT: ^
+// CHECK-NEXT: note: A new builder with signature:
+// CHECK-NEXT: static TestAttr get(::mlir::MLIRContext *context, std::int64_t arg);
+// CHECK-EMPTY:
+// CHECK-NEXT: is shadowed by an existing builder with signature:
+// CHECK-NEXT: static TestAttr get(::mlir::MLIRContext *context, std::int64_t arg);
+// CHECK-EMPTY:
+// CHECK-NEXT: Please remove one of the conflicting definitions.
diff --git a/mlir/test/mlir-tblgen/attr-duplicated-custom-builders-error.td b/mlir/test/mlir-tblgen/attr-duplicated-custom-builders-error.td
index 12615d0c66e97..0e09f667c1ccd 100644
--- a/mlir/test/mlir-tblgen/attr-duplicated-custom-builders-error.td
+++ b/mlir/test/mlir-tblgen/attr-duplicated-custom-builders-error.td
@@ -40,4 +40,13 @@ def Test_TestAttrOp : Op<Test_Dialect, "test", []> {
let assemblyFormat = "$testAttr attr-dict";
}
-// CHECK: attr-duplicated-custom-builders-error.td:15:5: error: Unexpected overlap when generating `get` for TestAttr (from line {{.*}})
+// CHECK: attr-duplicated-custom-builders-error.td:20:7: error: builder `get` conflicts with an existing builder.
+// CHECK-NEXT: let builders = [AttrBuilder<(ins "std::int64_t":$arg), [{
+// CHECK-NEXT: ^
+// CHECK-NEXT: note: A new builder with signature:
+// CHECK-NEXT: static TestAttr get(::mlir::MLIRContext *context, std::int64_t arg);
+// CHECK-EMPTY:
+// CHECK-NEXT: is shadowed by an existing builder with signature:
+// CHECK-NEXT: static TestAttr get(::mlir::MLIRContext *context, std::int64_t arg);
+// CHECK-EMPTY:
+// CHECK-NEXT: Please remove one of the conflicting definitions.
diff --git a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
index b27e4cdcd5b38..b9115657d6bf3 100644
--- a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
+++ b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
@@ -513,16 +513,39 @@ getCustomBuilderParams(std::initializer_list<MethodParameter> prefix,
return builderParams;
}
-static void errorIfPruned(size_t line, Method *m, const Twine &methodName,
- const AttrOrTypeDef &def) {
- if (m)
- return;
- PrintFatalError(def.getLoc(), "Unexpected overlap when generating `" +
- methodName + "` for " + def.getName() +
- " (from line " + Twine(line) + ")");
+static std::string getSignature(const Method &m) {
+ std::string signature;
+ llvm::raw_string_ostream os(signature);
+ raw_indented_ostream indentedOs(os);
+ m.writeDeclTo(indentedOs);
+ return signature;
}
-#define ERROR_IF_PRUNED(M, N, O) errorIfPruned(__LINE__, M, N, O)
+static void emitDuplicatedBuilderError(const Method ¤tMethod,
+ StringRef methodName,
+ const Class &defCls,
+ const AttrOrTypeDef &def) {
+
+ // Try to search for method that makes `get` redundant.
+ auto loc = def.getDef()->getFieldLoc("builders");
+ for (auto &method : defCls.getMethods()) {
+ if (method->getName() == methodName &&
+ method->makesRedundant(currentMethod)) {
+ PrintError(loc, llvm::Twine("builder `") + methodName +
+ "` conflicts with an existing builder. ");
+ PrintFatalNote(llvm::Twine("A new builder with signature:\n") +
+ getSignature(currentMethod) +
+ "\nis shadowed by an existing builder with signature:\n" +
+ getSignature(*method) +
+ "\nPlease remove one of the conflicting "
+ "definitions.");
+ }
+ }
+
+ // This code shouldn't be reached, but leaving this here for potential future
+ // use.
+ PrintFatalError(loc, "Failed to generate builder " + methodName);
+}
void DefGen::emitCustomBuilder(const AttrOrTypeBuilder &builder) {
// Don't emit a body if there isn't one.
@@ -530,11 +553,16 @@ void DefGen::emitCustomBuilder(const AttrOrTypeBuilder &builder) {
StringRef returnType = def.getCppClassName();
if (std::optional<StringRef> builderReturnType = builder.getReturnType())
returnType = *builderReturnType;
- Method *m = defCls.addMethod(returnType, "get", props,
- getCustomBuilderParams({}, builder));
+
+ llvm::StringRef methodName = "get";
+ const auto parameters = getCustomBuilderParams({}, builder);
+ Method *m = defCls.addMethod(returnType, methodName, props, parameters);
// If method is pruned, report error and terminate.
- ERROR_IF_PRUNED(m, "get", def);
+ if (!m) {
+ auto curMethod = Method(returnType, methodName, props, parameters);
+ emitDuplicatedBuilderError(curMethod, methodName, defCls, def);
+ }
if (!builder.getBody())
return;
@@ -562,14 +590,18 @@ void DefGen::emitCheckedCustomBuilder(const AttrOrTypeBuilder &builder) {
StringRef returnType = def.getCppClassName();
if (std::optional<StringRef> builderReturnType = builder.getReturnType())
returnType = *builderReturnType;
- Method *m = defCls.addMethod(
- returnType, "getChecked", props,
- getCustomBuilderParams(
- {{"::llvm::function_ref<::mlir::InFlightDiagnostic()>", "emitError"}},
- builder));
+
+ llvm::StringRef methodName = "getChecked";
+ auto parameters = getCustomBuilderParams(
+ {{"::llvm::function_ref<::mlir::InFlightDiagnostic()>", "emitError"}},
+ builder);
+ Method *m = defCls.addMethod(returnType, methodName, props, parameters);
// If method is pruned, report error and terminate.
- ERROR_IF_PRUNED(m, "getChecked", def);
+ if (!m) {
+ auto curMethod = Method(returnType, methodName, props, parameters);
+ emitDuplicatedBuilderError(curMethod, methodName, defCls, def);
+ }
if (!builder.getBody())
return;
More information about the Mlir-commits
mailing list