Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions mlir/include/mlir/Dialect/Tosa/IR/TosaTypesBase.td
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
39 changes: 39 additions & 0 deletions mlir/test/mlir-tblgen/attr-duplicated-builder-error.td
Original file line number Diff line number Diff line change
@@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the "from line 537" referring to here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's line number of error location in AttrOrTypeDefGen.cpp.
I followed this to be consistent with error handling logic in OpDefinitionsGen.cpp.
However, if you feel error message looks terse, I can improve it to provide more information, and omit line number of the original source code.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that seems preferable for the end-user. Thanks!

43 changes: 43 additions & 0 deletions mlir/test/mlir-tblgen/attr-duplicated-custom-builders-error.td
Original file line number Diff line number Diff line change
@@ -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 {{.*}})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error is quite terse, can we improve it? Can we point at the two methods that are shadowing each other?

Copy link
Contributor Author

@JustinKim98 JustinKim98 Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to do so, but custom method itself doesn't store the exact location in the file where it was defined.
But I can still point to location of let builders = ...

19 changes: 19 additions & 0 deletions mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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;

Expand Down
4 changes: 2 additions & 2 deletions mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down