[mlir] Add symbol user attribute interface. #153206
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Enables verification of attributes, independent of op, that references symbols. RFC: this enables verifying Attribute with symbol usage indpendent of operation attached to (e.g., the validity is on the Attribute independent of the operation).
joker-eph
reviewed
Aug 12, 2025
joker-eph
reviewed
Aug 12, 2025
Co-authored-by: Mehdi Amini <[email protected]>
jpienaar
requested review from
aaronmondal,
keith and
rupprecht
as code owners
|
@llvm/pr-subscribers-mlir-ods Author: Jacques Pienaar (jpienaar) ChangesEnables verification of attributes, independent of op, that references symbols. RFC: this enables verifying Attribute with symbol usage indpendent of operation attached to (e.g., the validity is on the Attribute independent of the operation). Want to get folks general idea here. Full diff: https://github.com/llvm/llvm-project/pull/153206.diff 5 Files Affected:
diff --git a/mlir/include/mlir/IR/CMakeLists.txt b/mlir/include/mlir/IR/CMakeLists.txt
index 846547ff131e3..d34bab2b76162 100644
--- a/mlir/include/mlir/IR/CMakeLists.txt
+++ b/mlir/include/mlir/IR/CMakeLists.txt
@@ -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)
diff --git a/mlir/include/mlir/IR/SymbolInterfaces.td b/mlir/include/mlir/IR/SymbolInterfaces.td
index bbfa30815bd4a..9793c8c1752fc 100644
--- a/mlir/include/mlir/IR/SymbolInterfaces.td
+++ b/mlir/include/mlir/IR/SymbolInterfaces.td
@@ -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
//===----------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/IR/SymbolTable.h b/mlir/include/mlir/IR/SymbolTable.h
index e4622354b8980..a174062d8d019 100644
--- a/mlir/include/mlir/IR/SymbolTable.h
+++ b/mlir/include/mlir/IR/SymbolTable.h
@@ -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
diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index 87b47992905e0..cf074b967d039 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -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()) {
+ if (auto user = dyn_cast<SymbolUserAttrInterface>(attr.getValue())) {
+ if (failed(user.verifySymbolUses(op, symbolTable)))
+ return WalkResult::interrupt();
+ }
+ }
return WalkResult::advance();
};
@@ -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"
diff --git a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
index f00c31003b185..25e148ac9f8cc 100644
--- a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
@@ -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",
|
|
@llvm/pr-subscribers-mlir Author: Jacques Pienaar (jpienaar) ChangesEnables verification of attributes, independent of op, that references symbols. RFC: this enables verifying Attribute with symbol usage indpendent of operation attached to (e.g., the validity is on the Attribute independent of the operation). Want to get folks general idea here. Full diff: https://github.com/llvm/llvm-project/pull/153206.diff 5 Files Affected:
diff --git a/mlir/include/mlir/IR/CMakeLists.txt b/mlir/include/mlir/IR/CMakeLists.txt
index 846547ff131e3..d34bab2b76162 100644
--- a/mlir/include/mlir/IR/CMakeLists.txt
+++ b/mlir/include/mlir/IR/CMakeLists.txt
@@ -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)
diff --git a/mlir/include/mlir/IR/SymbolInterfaces.td b/mlir/include/mlir/IR/SymbolInterfaces.td
index bbfa30815bd4a..9793c8c1752fc 100644
--- a/mlir/include/mlir/IR/SymbolInterfaces.td
+++ b/mlir/include/mlir/IR/SymbolInterfaces.td
@@ -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
//===----------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/IR/SymbolTable.h b/mlir/include/mlir/IR/SymbolTable.h
index e4622354b8980..a174062d8d019 100644
--- a/mlir/include/mlir/IR/SymbolTable.h
+++ b/mlir/include/mlir/IR/SymbolTable.h
@@ -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
diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index 87b47992905e0..cf074b967d039 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -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()) {
+ if (auto user = dyn_cast<SymbolUserAttrInterface>(attr.getValue())) {
+ if (failed(user.verifySymbolUses(op, symbolTable)))
+ return WalkResult::interrupt();
+ }
+ }
return WalkResult::advance();
};
@@ -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"
diff --git a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
index f00c31003b185..25e148ac9f8cc 100644
--- a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
@@ -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",
|
|
@llvm/pr-subscribers-mlir-core Author: Jacques Pienaar (jpienaar) ChangesEnables verification of attributes, independent of op, that references symbols. RFC: this enables verifying Attribute with symbol usage indpendent of operation attached to (e.g., the validity is on the Attribute independent of the operation). Want to get folks general idea here. Full diff: https://github.com/llvm/llvm-project/pull/153206.diff 5 Files Affected:
diff --git a/mlir/include/mlir/IR/CMakeLists.txt b/mlir/include/mlir/IR/CMakeLists.txt
index 846547ff131e3..d34bab2b76162 100644
--- a/mlir/include/mlir/IR/CMakeLists.txt
+++ b/mlir/include/mlir/IR/CMakeLists.txt
@@ -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)
diff --git a/mlir/include/mlir/IR/SymbolInterfaces.td b/mlir/include/mlir/IR/SymbolInterfaces.td
index bbfa30815bd4a..9793c8c1752fc 100644
--- a/mlir/include/mlir/IR/SymbolInterfaces.td
+++ b/mlir/include/mlir/IR/SymbolInterfaces.td
@@ -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
//===----------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/IR/SymbolTable.h b/mlir/include/mlir/IR/SymbolTable.h
index e4622354b8980..a174062d8d019 100644
--- a/mlir/include/mlir/IR/SymbolTable.h
+++ b/mlir/include/mlir/IR/SymbolTable.h
@@ -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
diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index 87b47992905e0..cf074b967d039 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -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()) {
+ if (auto user = dyn_cast<SymbolUserAttrInterface>(attr.getValue())) {
+ if (failed(user.verifySymbolUses(op, symbolTable)))
+ return WalkResult::interrupt();
+ }
+ }
return WalkResult::advance();
};
@@ -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"
diff --git a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
index f00c31003b185..25e148ac9f8cc 100644
--- a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
@@ -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",
|
joker-eph
reviewed
Sep 17, 2025
mlir/lib/IR/SymbolTable.cpp
Outdated
| return WalkResult(user.verifySymbolUses(symbolTable)); | ||
| if (failed(user.verifySymbolUses(symbolTable))) | ||
| return WalkResult::interrupt(); | ||
| for (auto &attr : op->getAttrs()) { |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Co-authored-by: Mehdi Amini <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Enables verification of attributes, independent of op, that references symbols.
RFC: this enables verifying Attribute with symbol usage indpendent of operation attached to (e.g., the validity is on the Attribute independent of the operation). Want to get folks general idea here.