Skip to content

[mlir] Add symbol user attribute interface. #153206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

[mlir] Add symbol user attribute interface. #153206

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5b317ea
[mlir] Add symbol user attribute interface.
jpienaar Aug 12, 2025
bd0bd41
Update mlir/include/mlir/IR/SymbolInterfaces.td
jpienaar Aug 13, 2025
3855fd5
Check all attributes rather than discardable only
jpienaar Aug 13, 2025
2fec0b1
Apply suggestions from code review
jpienaar Sep 18, 2025
d3bdf89
Update SymbolInterfaces.td
jpienaar Sep 18, 2025
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: 3 additions & 0 deletions mlir/include/mlir/IR/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
add_mlir_interface(SymbolInterfaces)
set(LLVM_TARGET_DEFINITIONS SymbolInterfaces.td)
mlir_tablegen(SymbolInterfacesAttrInterface.h.inc -gen-attr-interface-decls)
mlir_tablegen(SymbolInterfacesAttrInterface.cpp.inc -gen-attr-interface-defs)
add_mlir_interface(RegionKindInterface)

set(LLVM_TARGET_DEFINITIONS OpAsmInterface.td)
Expand Down
18 changes: 18 additions & 0 deletions mlir/include/mlir/IR/SymbolInterfaces.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,24 @@ def SymbolUserOpInterface : OpInterface<"SymbolUserOpInterface"> {
];
}

def SymbolUserAttrInterface : AttrInterface<"SymbolUserAttrInterface"> {
let description = [{
This interface describes an attribute that may use a `Symbol`. This
interface allows for users of symbols to hook into verification and other
symbol related utilities that are either costly or otherwise disallowed
within a traditional operation.
}];
let cppNamespace = "::mlir";

let methods = [
InterfaceMethod<"Verify the symbol uses held by this attribute of this operation.",
"::llvm::LogicalResult", "verifySymbolUses",
(ins "::mlir::Operation *":$op,
"::mlir::SymbolTableCollection &":$symbolTable)
>,
];
}

//===----------------------------------------------------------------------===//
// Symbol Traits
//===----------------------------------------------------------------------===//
Expand Down
1 change: 1 addition & 0 deletions mlir/include/mlir/IR/SymbolTable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,5 +499,6 @@ ParseResult parseOptionalVisibilityKeyword(OpAsmParser &parser,

/// Include the generated symbol interfaces.
#include "mlir/IR/SymbolInterfaces.h.inc"
#include "mlir/IR/SymbolInterfacesAttrInterface.h.inc"

#endif // MLIR_IR_SYMBOLTABLE_H
10 changes: 9 additions & 1 deletion mlir/lib/IR/SymbolTable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,14 @@ LogicalResult detail::verifySymbolTable(Operation *op) {
SymbolTableCollection symbolTable;
auto verifySymbolUserFn = [&](Operation *op) -> std::optional<WalkResult> {
if (SymbolUserOpInterface user = dyn_cast<SymbolUserOpInterface>(op))
return WalkResult(user.verifySymbolUses(symbolTable));
if (failed(user.verifySymbolUses(symbolTable)))
return WalkResult::interrupt();
for (auto &attr : op->getAttrs()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
for (auto &attr : op->getAttrs()) {
for (auto &attr : op->getDiscardableAttrs()) {

As we're splitting properties and attributes, I don't think we need more dependencies here!

But that also question the usefulness of this and how it'll work in practice: I assume an attribue that is part of the properties would also want to be verified.
However that should be handled through the SymbolUserOpInterface (which may just have to forward to the attribute).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is a big open:

But that also question the usefulness of this and how it'll work in practice: I assume an attribue that is part of the properties would also want to be verified.

Copy link
Member Author

Choose a reason for hiding this comment

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

Properties are bits that only has meaning for an operation today. There is no generic interpretation. If properties had interfaces, then one could query an interface here in the same way. I see this not as inhibiting nor prerequisite here. And indeed today the only way would be via SymbolUserOpInterface, one could additionally factor out common function C++ side that could be used in both.

In practice, this is very useful for composition between dialects where one isn't the sole owner. I have a few pipelines where this would be used where the alternatives require coupling, rearchitecting, some interface injection (which doesn't quite work), doing a bit of silly additions of attributes/adding registries or just having a worse error & debugging experience. So just covering discardable attributes suffices in improving error localization sufficiently for such cases.

if (auto user = dyn_cast<SymbolUserAttrInterface>(attr.getValue())) {
if (failed(user.verifySymbolUses(op, symbolTable)))
return WalkResult::interrupt();
}
}
return WalkResult::advance();
};

Expand Down Expand Up @@ -1132,3 +1139,4 @@ ParseResult impl::parseOptionalVisibilityKeyword(OpAsmParser &parser,

/// Include the generated symbol interfaces.
#include "mlir/IR/SymbolInterfaces.cpp.inc"
#include "mlir/IR/SymbolInterfacesAttrInterface.cpp.inc"
2 changes: 2 additions & 0 deletions utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ exports_files(glob(["include/**/*.td"]))
tbl_outs = {
"include/mlir/IR/" + name + ".h.inc": ["-gen-op-interface-decls"],
"include/mlir/IR/" + name + ".cpp.inc": ["-gen-op-interface-defs"],
"include/mlir/IR/" + name + "AttrInterface.h.inc": ["-gen-attr-interface-decls"],
"include/mlir/IR/" + name + "AttrInterface.cpp.inc": ["-gen-attr-interface-defs"],
},
tblgen = ":mlir-tblgen",
td_file = "include/mlir/IR/" + name + ".td",
Expand Down