[Mlir-commits] [llvm] [mlir] Revert "[mlir][IR] Add SymbolUserTypeInterface (#198435)" (PR #217959)
Will Dietz
llvmlistbot at llvm.org
Fri Aug 21 08:55:12 PDT 2026
https://github.com/dtzSiFive created https://github.com/llvm/llvm-project/pull/217959
Looking forward to this feature, but the verification cost presently is much too high: 2-3x+ end-to-end for CIRCT production workloads, as well as a noticeable bump to MaxRSS. A user has reported their workload is now 1.85x (#212160).
This verifier performance hit impacts all MLIR projects using the common SymbolTable regardless of whether they make use of this interface.
Presently there are no in-tree users.
Promising work to reduce this cost (#212160, #212354) is underway, but neither are ready yet.
Since the entire purpose of this interface is to support this verification, revert this entirely for now while the performance story is worked out.
This reverts commit 8c2d7cc3a9983459628c028d8537eaf79551f396.
>From 8f87ece9dd464ff30f867dcb8d486b300433fb5b Mon Sep 17 00:00:00 2001
From: Will Dietz <will.dietz at sifive.com>
Date: Thu, 20 Aug 2026 15:44:20 -0500
Subject: [PATCH] Revert "[mlir][IR] Add SymbolUserTypeInterface (#198435)"
Looking forward to this feature, but the verification cost
presently is much too high: 2-3x+ end-to-end for CIRCT
production workloads, as well as a noticeable bump to MaxRSS.
A user has reported their workload is now 1.85x (#212160).
This verifier performance hit impacts all MLIR projects
using the common SymbolTable regardless of whether they make
use of this interface.
Presently there are no in-tree users.
Promising work to reduce this cost (#212160, #212354) is
underway, but neither are ready yet.
Since the entire purpose of this interface is to support
this verification, revert this entirely for now while the
performance story is worked out.
This reverts commit 8c2d7cc3a9983459628c028d8537eaf79551f396.
---
mlir/include/mlir/IR/CMakeLists.txt | 2 -
mlir/include/mlir/IR/SymbolInterfaces.td | 21 +---
mlir/include/mlir/IR/SymbolTable.h | 1 -
mlir/lib/IR/SymbolTable.cpp | 57 -----------
.../IR/test-verifiers-symbol-user-type.mlir | 95 -------------------
mlir/test/lib/Dialect/Test/TestTypeDefs.td | 9 --
mlir/test/lib/Dialect/Test/TestTypes.cpp | 13 ---
mlir/test/lib/Dialect/Test/TestTypes.h | 1 -
.../llvm-project-overlay/mlir/BUILD.bazel | 45 ++++-----
9 files changed, 19 insertions(+), 225 deletions(-)
delete mode 100644 mlir/test/IR/test-verifiers-symbol-user-type.mlir
diff --git a/mlir/include/mlir/IR/CMakeLists.txt b/mlir/include/mlir/IR/CMakeLists.txt
index 54ebbd0b51bb5..15518901b901a 100644
--- a/mlir/include/mlir/IR/CMakeLists.txt
+++ b/mlir/include/mlir/IR/CMakeLists.txt
@@ -2,8 +2,6 @@ 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)
-mlir_tablegen(SymbolInterfacesTypeInterface.h.inc -gen-type-interface-decls)
-mlir_tablegen(SymbolInterfacesTypeInterface.cpp.inc -gen-type-interface-defs)
add_mlir_interface(RegionKindInterface)
add_mlir_type_interface(QuantStorageTypeInterface)
diff --git a/mlir/include/mlir/IR/SymbolInterfaces.td b/mlir/include/mlir/IR/SymbolInterfaces.td
index 292c355cbe157..ebe0c26637ad3 100644
--- a/mlir/include/mlir/IR/SymbolInterfaces.td
+++ b/mlir/include/mlir/IR/SymbolInterfaces.td
@@ -228,7 +228,7 @@ def SymbolUserAttrInterface : AttrInterface<"SymbolUserAttrInterface"> {
interface allows for users of symbols to hook into verification and other
symbol related utilities that are either costly or otherwise disallowed
within an operation (e.g., recreating symbol users per op verified rather
- than per symbol table, or querying symbols usage of siblings).
+ than per symbol table, or querying symbols usage of sibblings).
}];
let cppNamespace = "::mlir";
@@ -241,25 +241,6 @@ def SymbolUserAttrInterface : AttrInterface<"SymbolUserAttrInterface"> {
];
}
-def SymbolUserTypeInterface : TypeInterface<"SymbolUserTypeInterface"> {
- let description = [{
- This interface describes a type that may use a `Symbol`. This interface
- allows types to hook into verification that needs a symbol table, which is
- costly or otherwise disallowed within type construction and uniquing.
- `op` is the operation whose verification triggered the check and should be
- used as the anchor for symbol lookups.
- }];
- let cppNamespace = "::mlir";
-
- let methods = [
- InterfaceMethod<"Verify the symbol uses held by this type 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 e4790037d37b2..a174062d8d019 100644
--- a/mlir/include/mlir/IR/SymbolTable.h
+++ b/mlir/include/mlir/IR/SymbolTable.h
@@ -500,6 +500,5 @@ ParseResult parseOptionalVisibilityKeyword(OpAsmParser &parser,
/// Include the generated symbol interfaces.
#include "mlir/IR/SymbolInterfaces.h.inc"
#include "mlir/IR/SymbolInterfacesAttrInterface.h.inc"
-#include "mlir/IR/SymbolInterfacesTypeInterface.h.inc"
#endif // MLIR_IR_SYMBOLTABLE_H
diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index 100eba7146d81..078401c8380f4 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -476,51 +476,6 @@ raw_ostream &mlir::operator<<(raw_ostream &os,
// SymbolTable Trait Types
//===----------------------------------------------------------------------===//
-/// Verify the symbol uses held by the types owned by `op`: its operand,
-/// result, and block-argument types, and any types nested within its
-/// attributes. `op` is the anchor used for symbol lookups. `verifiedTypes`
-/// records the types already verified within the current symbol table so that
-/// each type, which may be uniqued and shared across many positions or
-/// operations, is verified at most once. Verification fails fast on the first
-/// invalid symbol use.
-static LogicalResult verifyOpTypeSymbolUses(Operation *op,
- SymbolTableCollection &symbolTable,
- SetVector<Type> &verifiedTypes) {
- // Walk `type` and any nested type parameters reachable from it, verifying
- // each not-yet-seen type and interrupting on the first failure.
- auto verify = [&](Type type) {
- return type.walk<WalkOrder::PreOrder>([&](Type nestedType) {
- if (!verifiedTypes.insert(nestedType))
- return WalkResult::advance();
- if (auto user = dyn_cast<SymbolUserTypeInterface>(nestedType))
- if (failed(user.verifySymbolUses(op, symbolTable)))
- return WalkResult::interrupt();
- return WalkResult::advance();
- });
- };
-
- for (Type type : op->getOperandTypes())
- if (verify(type).wasInterrupted())
- return failure();
- for (Type type : op->getResultTypes())
- if (verify(type).wasInterrupted())
- return failure();
- for (Region ®ion : op->getRegions())
- for (Block &block : region)
- for (BlockArgument argument : block.getArguments())
- if (verify(argument.getType()).wasInterrupted())
- return failure();
-
- // Verify types nested within the operation's attributes.
- WalkResult attrResult =
- op->getAttrDictionary().walk<WalkOrder::PreOrder>([&](Type type) {
- if (verify(type).wasInterrupted())
- return WalkResult::interrupt();
- return WalkResult::advance();
- });
- return failure(attrResult.wasInterrupted());
-}
-
LogicalResult detail::verifySymbolTable(Operation *op) {
if (op->getNumRegions() != 1)
return op->emitOpError()
@@ -551,27 +506,16 @@ LogicalResult detail::verifySymbolTable(Operation *op) {
// Verify any nested symbol user operations.
SymbolTableCollection symbolTable;
- // walkSymbolTable does not descend into nested symbol tables, so every
- // operation visited here shares the same nearest symbol table. A uniqued
- // attribute or type therefore resolves its symbol uses identically
- // regardless of which operation anchors the lookup, so each is verified at
- // most once across the whole scope.
- SetVector<Attribute> verifiedAttrs;
- SetVector<Type> verifiedTypes;
auto verifySymbolUserFn = [&](Operation *op) -> std::optional<WalkResult> {
if (SymbolUserOpInterface user = dyn_cast<SymbolUserOpInterface>(op))
if (failed(user.verifySymbolUses(symbolTable)))
return WalkResult::interrupt();
for (auto &attr : op->getDiscardableAttrs()) {
if (auto user = dyn_cast<SymbolUserAttrInterface>(attr.getValue())) {
- if (!verifiedAttrs.insert(attr.getValue()))
- continue;
if (failed(user.verifySymbolUses(op, symbolTable)))
return WalkResult::interrupt();
}
}
- if (failed(verifyOpTypeSymbolUses(op, symbolTable, verifiedTypes)))
- return WalkResult::interrupt();
return WalkResult::advance();
};
@@ -1193,4 +1137,3 @@ ParseResult impl::parseOptionalVisibilityKeyword(OpAsmParser &parser,
/// Include the generated symbol interfaces.
#include "mlir/IR/SymbolInterfaces.cpp.inc"
#include "mlir/IR/SymbolInterfacesAttrInterface.cpp.inc"
-#include "mlir/IR/SymbolInterfacesTypeInterface.cpp.inc"
diff --git a/mlir/test/IR/test-verifiers-symbol-user-type.mlir b/mlir/test/IR/test-verifiers-symbol-user-type.mlir
deleted file mode 100644
index b17b69e9f5ec7..0000000000000
--- a/mlir/test/IR/test-verifiers-symbol-user-type.mlir
+++ /dev/null
@@ -1,95 +0,0 @@
-// RUN: mlir-opt %s -verify-diagnostics -split-input-file
-
-module {
- func.func private @existing_symbol()
-
- "test.type_producer"() : () -> !test.symbol_ref<@existing_symbol>
-}
-
-// -----
-
-module {
- // expected-error at +1 {{'@non_existent_symbol' does not reference a valid symbol}}
- "test.type_producer"() : () -> !test.symbol_ref<@non_existent_symbol>
-}
-
-// -----
-
-module {
- func.func private @existing_symbol()
-
- %0 = "test.type_producer"() : () -> !test.symbol_ref<@existing_symbol>
- "test.type_consumer"(%0) : (!test.symbol_ref<@existing_symbol>) -> ()
-}
-
-// -----
-
-module {
- func.func private @existing_symbol()
-
- "test.type_producer"() : () -> tuple<!test.symbol_ref<@existing_symbol>>
-}
-
-// -----
-
-module {
- // expected-error at +1 {{'@non_existent_symbol' does not reference a valid symbol}}
- "test.type_producer"() : () -> tuple<!test.symbol_ref<@non_existent_symbol>>
-}
-
-// -----
-
-module {
- func.func private @existing_symbol()
-
- func.func private @uses_symbol_type(%arg0: !test.symbol_ref<@existing_symbol>)
-}
-
-// -----
-
-module {
- // expected-error at +1 {{'@non_existent_symbol' does not reference a valid symbol}}
- func.func private @uses_symbol_type(%arg0: !test.symbol_ref<@non_existent_symbol>)
-}
-
-// -----
-
-module {
- func.func private @existing_symbol()
-
- "test.one_region_op"() ({
- ^bb0(%arg0: !test.symbol_ref<@existing_symbol>):
- "test.valid"() : () -> ()
- }) : () -> ()
-}
-
-// -----
-
-module {
- // expected-error at +1 {{'@non_existent_symbol' does not reference a valid symbol}}
- "test.one_region_op"() ({
- ^bb0(%arg0: !test.symbol_ref<@non_existent_symbol>):
- "test.valid"() : () -> ()
- }) : () -> ()
-}
-
-// -----
-
-module {
- func.func private @existing_symbol()
-
- "test.typed_attr"() <{
- type = !test.symbol_ref<@existing_symbol>,
- attr = 0 : i32
- }> : () -> ()
-}
-
-// -----
-
-module {
- // expected-error at +1 {{'@non_existent_symbol' does not reference a valid symbol}}
- "test.typed_attr"() <{
- type = !test.symbol_ref<@non_existent_symbol>,
- attr = 0 : i32
- }> : () -> ()
-}
diff --git a/mlir/test/lib/Dialect/Test/TestTypeDefs.td b/mlir/test/lib/Dialect/Test/TestTypeDefs.td
index 9481aa1b35efd..58c340a935bdb 100644
--- a/mlir/test/lib/Dialect/Test/TestTypeDefs.td
+++ b/mlir/test/lib/Dialect/Test/TestTypeDefs.td
@@ -19,7 +19,6 @@ include "TestAttrDefs.td"
include "TestInterfaces.td"
include "mlir/IR/BuiltinTypes.td"
include "mlir/IR/BuiltinTypeInterfaces.td"
-include "mlir/IR/SymbolInterfaces.td"
include "mlir/Interfaces/DataLayoutInterfaces.td"
include "mlir/Dialect/Bufferization/IR/BufferizationTypeInterfaces.td"
@@ -85,14 +84,6 @@ def CompoundNestedOuterTypeQual : Test_Type<"CompoundNestedOuterQual"> {
let assemblyFormat = "`<` `i` qualified($inner) `>`";
}
-def TestSymbolUserType : Test_Type<"TestSymbolUser",
- [DeclareTypeInterfaceMethods<SymbolUserTypeInterface>]> {
- let mnemonic = "symbol_ref";
- let summary = "Test type that references a symbol";
- let parameters = (ins "::mlir::FlatSymbolRefAttr":$symbol);
- let assemblyFormat = "`<` $symbol `>`";
-}
-
// An example of how one could implement a standard integer.
def IntegerType : Test_Type<"TestInteger"> {
let mnemonic = "int";
diff --git a/mlir/test/lib/Dialect/Test/TestTypes.cpp b/mlir/test/lib/Dialect/Test/TestTypes.cpp
index 2ebc48c3c7e8b..7d1cfe8c9ba06 100644
--- a/mlir/test/lib/Dialect/Test/TestTypes.cpp
+++ b/mlir/test/lib/Dialect/Test/TestTypes.cpp
@@ -332,19 +332,6 @@ uint64_t TestTypeWithLayoutType::extractKind(DataLayoutEntryListRef params,
return 1;
}
-//===----------------------------------------------------------------------===//
-// TestSymbolUserType
-//===----------------------------------------------------------------------===//
-
-LogicalResult
-TestSymbolUserType::verifySymbolUses(Operation *op,
- SymbolTableCollection &symbolTable) const {
- if (!symbolTable.lookupNearestSymbolFrom<SymbolOpInterface>(op, getSymbol()))
- return op->emitOpError()
- << "'" << getSymbol() << "' does not reference a valid symbol";
- return success();
-}
-
//===----------------------------------------------------------------------===//
// Dynamic Types
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/lib/Dialect/Test/TestTypes.h b/mlir/test/lib/Dialect/Test/TestTypes.h
index cf3d1057de675..705fb86e9e9b3 100644
--- a/mlir/test/lib/Dialect/Test/TestTypes.h
+++ b/mlir/test/lib/Dialect/Test/TestTypes.h
@@ -24,7 +24,6 @@
#include "mlir/IR/Dialect.h"
#include "mlir/IR/DialectImplementation.h"
#include "mlir/IR/Operation.h"
-#include "mlir/IR/SymbolTable.h"
#include "mlir/IR/Types.h"
#include "mlir/Interfaces/DataLayoutInterfaces.h"
diff --git a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
index be7aba77e859c..262a69c333cf7 100644
--- a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
@@ -95,33 +95,24 @@ filegroup(
exports_files(glob(["include/**/*.td"]))
-gentbl_cc_library(
- name = "RegionKindInterfaceIncGen",
- tbl_outs = {
- "include/mlir/IR/RegionKindInterface.h.inc": ["-gen-op-interface-decls"],
- "include/mlir/IR/RegionKindInterface.cpp.inc": ["-gen-op-interface-defs"],
- "include/mlir/IR/RegionKindInterfaceAttrInterface.h.inc": ["-gen-attr-interface-decls"],
- "include/mlir/IR/RegionKindInterfaceAttrInterface.cpp.inc": ["-gen-attr-interface-defs"],
- },
- tblgen = ":mlir-tblgen",
- td_file = "include/mlir/IR/RegionKindInterface.td",
- deps = [":OpBaseTdFiles"],
-)
-
-gentbl_cc_library(
- name = "SymbolInterfacesIncGen",
- tbl_outs = {
- "include/mlir/IR/SymbolInterfaces.h.inc": ["-gen-op-interface-decls"],
- "include/mlir/IR/SymbolInterfaces.cpp.inc": ["-gen-op-interface-defs"],
- "include/mlir/IR/SymbolInterfacesAttrInterface.h.inc": ["-gen-attr-interface-decls"],
- "include/mlir/IR/SymbolInterfacesAttrInterface.cpp.inc": ["-gen-attr-interface-defs"],
- "include/mlir/IR/SymbolInterfacesTypeInterface.h.inc": ["-gen-type-interface-decls"],
- "include/mlir/IR/SymbolInterfacesTypeInterface.cpp.inc": ["-gen-type-interface-defs"],
- },
- tblgen = ":mlir-tblgen",
- td_file = "include/mlir/IR/SymbolInterfaces.td",
- deps = [":OpBaseTdFiles"],
-)
+[
+ gentbl_cc_library(
+ name = name + "IncGen",
+ 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",
+ deps = [":OpBaseTdFiles"],
+ )
+ for name in [
+ "RegionKindInterface",
+ "SymbolInterfaces",
+ ]
+]
gentbl_cc_library(
name = "OpAsmInterfaceIncGen",
More information about the Mlir-commits
mailing list