[Mlir-commits] [mlir] [mlir][IR] Require inherent symbol attributes for Symbol operations (PR #218920)
Mehdi Amini
llvmlistbot at llvm.org
Wed Aug 26 06:07:16 PDT 2026
https://github.com/joker-eph created https://github.com/llvm/llvm-project/pull/218920
Require SymbolTable operations to implement SymbolOpInterface and store symbol names and visibility as inherent attributes.
Add missing symbol properties/interfaces to GPU, OpenACC, OpenMP, EmitC, and Toy operations, and update affected tests and the GPU Python builder.
Assisted-by: Codex
>From 46af535a3cde274e01df801ff5d73232ced36616 Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Wed, 26 Aug 2026 04:48:51 -0700
Subject: [PATCH] [mlir][IR] Require inherent symbol attributes
Require SymbolTable operations to implement SymbolOpInterface and store symbol
names and visibility as inherent attributes.
Add missing symbol properties/interfaces to GPU, OpenACC, OpenMP, EmitC, and
Toy operations, and update affected tests and the GPU Python builder.
Assisted-by: Codex
---
mlir/examples/toy/Ch2/include/toy/Ops.td | 1 +
mlir/examples/toy/Ch3/include/toy/Ops.td | 1 +
mlir/examples/toy/Ch4/include/toy/Ops.td | 1 +
mlir/examples/toy/Ch5/include/toy/Ops.td | 1 +
mlir/examples/toy/Ch6/include/toy/Ops.td | 1 +
mlir/examples/toy/Ch7/include/toy/Ops.td | 1 +
mlir/include/mlir/Dialect/EmitC/IR/EmitC.td | 1 +
mlir/include/mlir/Dialect/GPU/IR/GPUOps.td | 4 +-
.../mlir/Dialect/OpenACC/OpenACCOps.td | 2 +-
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 3 +-
mlir/include/mlir/IR/SymbolInterfaces.td | 3 +-
mlir/include/mlir/IR/SymbolTable.h | 6 +-
.../Conversion/FuncToEmitC/FuncToEmitC.cpp | 4 +-
mlir/lib/IR/BuiltinAttributes.cpp | 7 +-
mlir/lib/IR/SymbolTable.cpp | 74 +++++++++++--------
.../lib/Interfaces/FunctionImplementation.cpp | 11 +--
mlir/lib/Pass/IRPrinting.cpp | 11 +--
mlir/python/mlir/dialects/gpu/__init__.py | 10 +--
.../Dialect/OpenMP/cli-canonical_loop.mlir | 9 +--
mlir/unittests/Transforms/Canonicalizer.cpp | 5 +-
20 files changed, 80 insertions(+), 76 deletions(-)
diff --git a/mlir/examples/toy/Ch2/include/toy/Ops.td b/mlir/examples/toy/Ch2/include/toy/Ops.td
index 91bf83a54df1a..57ac98143972e 100644
--- a/mlir/examples/toy/Ch2/include/toy/Ops.td
+++ b/mlir/examples/toy/Ch2/include/toy/Ops.td
@@ -134,6 +134,7 @@ def FuncOp : Toy_Op<"func", [
let arguments = (ins
SymbolNameAttr:$sym_name,
+ OptionalAttr<StrAttr>:$sym_visibility,
TypeAttrOf<FunctionType>:$function_type,
OptionalAttr<DictArrayAttr>:$arg_attrs,
OptionalAttr<DictArrayAttr>:$res_attrs
diff --git a/mlir/examples/toy/Ch3/include/toy/Ops.td b/mlir/examples/toy/Ch3/include/toy/Ops.td
index 027b076af9e63..4989918e98093 100644
--- a/mlir/examples/toy/Ch3/include/toy/Ops.td
+++ b/mlir/examples/toy/Ch3/include/toy/Ops.td
@@ -133,6 +133,7 @@ def FuncOp : Toy_Op<"func", [
let arguments = (ins
SymbolNameAttr:$sym_name,
+ OptionalAttr<StrAttr>:$sym_visibility,
TypeAttrOf<FunctionType>:$function_type,
OptionalAttr<DictArrayAttr>:$arg_attrs,
OptionalAttr<DictArrayAttr>:$res_attrs
diff --git a/mlir/examples/toy/Ch4/include/toy/Ops.td b/mlir/examples/toy/Ch4/include/toy/Ops.td
index 6c6b73937aaf8..1301ba0ab233c 100644
--- a/mlir/examples/toy/Ch4/include/toy/Ops.td
+++ b/mlir/examples/toy/Ch4/include/toy/Ops.td
@@ -162,6 +162,7 @@ def FuncOp : Toy_Op<"func", [
let arguments = (ins
SymbolNameAttr:$sym_name,
+ OptionalAttr<StrAttr>:$sym_visibility,
TypeAttrOf<FunctionType>:$function_type,
OptionalAttr<DictArrayAttr>:$arg_attrs,
OptionalAttr<DictArrayAttr>:$res_attrs
diff --git a/mlir/examples/toy/Ch5/include/toy/Ops.td b/mlir/examples/toy/Ch5/include/toy/Ops.td
index 6a136ec76e3d1..348a5e48bd0f0 100644
--- a/mlir/examples/toy/Ch5/include/toy/Ops.td
+++ b/mlir/examples/toy/Ch5/include/toy/Ops.td
@@ -162,6 +162,7 @@ def FuncOp : Toy_Op<"func", [
let arguments = (ins
SymbolNameAttr:$sym_name,
+ OptionalAttr<StrAttr>:$sym_visibility,
TypeAttrOf<FunctionType>:$function_type,
OptionalAttr<DictArrayAttr>:$arg_attrs,
OptionalAttr<DictArrayAttr>:$res_attrs
diff --git a/mlir/examples/toy/Ch6/include/toy/Ops.td b/mlir/examples/toy/Ch6/include/toy/Ops.td
index 897b36d6135f4..464c13e6663b6 100644
--- a/mlir/examples/toy/Ch6/include/toy/Ops.td
+++ b/mlir/examples/toy/Ch6/include/toy/Ops.td
@@ -162,6 +162,7 @@ def FuncOp : Toy_Op<"func", [
let arguments = (ins
SymbolNameAttr:$sym_name,
+ OptionalAttr<StrAttr>:$sym_visibility,
TypeAttrOf<FunctionType>:$function_type,
OptionalAttr<DictArrayAttr>:$arg_attrs,
OptionalAttr<DictArrayAttr>:$res_attrs
diff --git a/mlir/examples/toy/Ch7/include/toy/Ops.td b/mlir/examples/toy/Ch7/include/toy/Ops.td
index 9151396c8aac7..658d6947db359 100644
--- a/mlir/examples/toy/Ch7/include/toy/Ops.td
+++ b/mlir/examples/toy/Ch7/include/toy/Ops.td
@@ -186,6 +186,7 @@ def FuncOp : Toy_Op<"func", [
let arguments = (ins
SymbolNameAttr:$sym_name,
+ OptionalAttr<StrAttr>:$sym_visibility,
TypeAttrOf<FunctionType>:$function_type,
OptionalAttr<DictArrayAttr>:$arg_attrs,
OptionalAttr<DictArrayAttr>:$res_attrs
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
index 0e7ea45d6d091..56490bbe46b60 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
@@ -898,6 +898,7 @@ def EmitC_FuncOp : EmitC_Op<"func", [
```
}];
let arguments = (ins SymbolNameAttr:$sym_name,
+ OptionalAttr<StrAttr>:$sym_visibility,
TypeAttrOf<FunctionType>:$function_type,
OptionalAttr<StrArrayAttr>:$specifiers,
OptionalAttr<DictArrayAttr>:$arg_attrs,
diff --git a/mlir/include/mlir/Dialect/GPU/IR/GPUOps.td b/mlir/include/mlir/Dialect/GPU/IR/GPUOps.td
index 1066dcb3b7308..0945b2dd167cb 100644
--- a/mlir/include/mlir/Dialect/GPU/IR/GPUOps.td
+++ b/mlir/include/mlir/Dialect/GPU/IR/GPUOps.td
@@ -419,7 +419,9 @@ def GPU_GPUFuncOp : GPU_Op<"func", [
attribution.
}];
- let arguments = (ins TypeAttrOf<FunctionType>:$function_type,
+ let arguments = (ins SymbolNameAttr:$sym_name,
+ OptionalAttr<StrAttr>:$sym_visibility,
+ TypeAttrOf<FunctionType>:$function_type,
OptionalAttr<DictArrayAttr>:$arg_attrs,
OptionalAttr<DictArrayAttr>:$res_attrs,
OptionalAttr<DictArrayAttr>:$workgroup_attrib_attrs,
diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index aab852d735042..fbe6dbdee5657 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -3330,7 +3330,7 @@ def OpenACC_DeclareOp : OpenACC_Op<"declare",
// 2.15.1 Routine Directive
//===----------------------------------------------------------------------===//
-def OpenACC_RoutineOp : OpenACC_Op<"routine", [IsolatedFromAbove]> {
+def OpenACC_RoutineOp : OpenACC_Op<"routine", [IsolatedFromAbove, Symbol]> {
let summary = "acc routine operation";
let description = [{
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index 024c6aa2fc1e3..cd2672e02917e 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -30,7 +30,8 @@ include "mlir/IR/SymbolInterfaces.td"
// 2.19.4 Data-Sharing Attribute Clauses
//===----------------------------------------------------------------------===//
-def PrivateClauseOp : OpenMP_Op<"private", [IsolatedFromAbove, RecipeInterface]> {
+def PrivateClauseOp
+ : OpenMP_Op<"private", [IsolatedFromAbove, RecipeInterface, Symbol]> {
let summary = "Provides declaration of [first]private logic.";
let description = [{
This operation provides a declaration of how to implement the
diff --git a/mlir/include/mlir/IR/SymbolInterfaces.td b/mlir/include/mlir/IR/SymbolInterfaces.td
index 75b47a99fcd9e..5486befe07a46 100644
--- a/mlir/include/mlir/IR/SymbolInterfaces.td
+++ b/mlir/include/mlir/IR/SymbolInterfaces.td
@@ -42,8 +42,7 @@ def Symbol : OpInterface<"SymbolOpInterface"> {
InterfaceMethod<"Sets the name of this symbol.",
"void", "setName", (ins "::mlir::StringAttr":$name), [{}],
/*defaultImplementation=*/[{
- this->getOperation()->setAttr(
- mlir::SymbolTable::getSymbolAttrName(), name);
+ mlir::SymbolTable::setSymbolName(this->getOperation(), name);
}]
>,
InterfaceMethod<[{
diff --git a/mlir/include/mlir/IR/SymbolTable.h b/mlir/include/mlir/IR/SymbolTable.h
index 9d3fdb98248d5..494a19d1dbab6 100644
--- a/mlir/include/mlir/IR/SymbolTable.h
+++ b/mlir/include/mlir/IR/SymbolTable.h
@@ -450,10 +450,8 @@ namespace OpTrait {
/// A trait used to provide symbol table functionalities to a region operation.
/// This operation must hold exactly 1 region. Once attached, all operations
/// that are directly within the region, i.e not including those within child
-/// regions, that contain a 'SymbolTable::getSymbolAttrName()' StringAttr will
-/// be verified to ensure that the names are uniqued. These operations must also
-/// adhere to the constraints defined by the `Symbol` trait, even if they do not
-/// inherit from it.
+/// regions, and implement `SymbolOpInterface` will be verified to ensure that
+/// their names are uniqued.
template <typename ConcreteType>
class SymbolTable : public TraitBase<ConcreteType, SymbolTable> {
public:
diff --git a/mlir/lib/Conversion/FuncToEmitC/FuncToEmitC.cpp b/mlir/lib/Conversion/FuncToEmitC/FuncToEmitC.cpp
index 96a5a77bce341..bd4d9e18338ad 100644
--- a/mlir/lib/Conversion/FuncToEmitC/FuncToEmitC.cpp
+++ b/mlir/lib/Conversion/FuncToEmitC/FuncToEmitC.cpp
@@ -297,9 +297,7 @@ class FuncOpConversion final : public OpConversionPattern<func::FuncOp> {
newFuncOp.setArgAttrsAttr(funcOp.getArgAttrsAttr());
newFuncOp.setResAttrsAttr(funcOp.getResAttrsAttr());
- if (StringAttr visibility = funcOp.getSymVisibilityAttr())
- newFuncOp->setDiscardableAttr(
- SymbolOpInterface::getDefaultVisibilityAttrName(), visibility);
+ newFuncOp.setVisibility(funcOp.getVisibility());
// Copy over all attributes other than the function name and type.
for (const auto &namedAttr :
diff --git a/mlir/lib/IR/BuiltinAttributes.cpp b/mlir/lib/IR/BuiltinAttributes.cpp
index 0f84f7a682fcf..4dc39ad3d0e93 100644
--- a/mlir/lib/IR/BuiltinAttributes.cpp
+++ b/mlir/lib/IR/BuiltinAttributes.cpp
@@ -348,10 +348,9 @@ FlatSymbolRefAttr SymbolRefAttr::get(StringAttr value) {
}
FlatSymbolRefAttr SymbolRefAttr::get(Operation *symbol) {
- auto symName =
- symbol->getAttrOfType<StringAttr>(SymbolTable::getSymbolAttrName());
- assert(symName && "value does not have a valid symbol name");
- return SymbolRefAttr::get(symName);
+ auto symbolOp = dyn_cast<SymbolOpInterface>(symbol);
+ assert(symbolOp && "value does not implement SymbolOpInterface");
+ return SymbolRefAttr::get(symbolOp.getNameAttr());
}
StringAttr SymbolRefAttr::getLeafReference() const {
diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index cdf5a7d7c1469..4922640d2dfc5 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -25,10 +25,11 @@ static bool isPotentiallyUnknownSymbolTable(Operation *op) {
/// Returns the string name of the given symbol, or null if this is not a
/// symbol.
static StringAttr getNameIfSymbol(Operation *op) {
- return op->getAttrOfType<StringAttr>(SymbolTable::getSymbolAttrName());
-}
-static StringAttr getNameIfSymbol(Operation *op, StringAttr symbolAttrNameId) {
- return op->getAttrOfType<StringAttr>(symbolAttrNameId);
+ if (!isa<SymbolOpInterface>(op))
+ return {};
+ return dyn_cast_or_null<StringAttr>(
+ op->getInherentAttr(SymbolTable::getSymbolAttrName())
+ .value_or(Attribute{}));
}
/// Computes the nested symbol reference attribute for the symbol 'symbolName'
@@ -40,7 +41,6 @@ collectValidReferencesFor(Operation *symbol, StringAttr symbolName,
Operation *within,
SmallVectorImpl<SymbolRefAttr> &results) {
assert(within->isAncestor(symbol) && "expected 'within' to be an ancestor");
- MLIRContext *ctx = symbol->getContext();
auto leafRef = FlatSymbolRefAttr::get(symbolName);
results.push_back(leafRef);
@@ -52,14 +52,12 @@ collectValidReferencesFor(Operation *symbol, StringAttr symbolName,
// Collect references until 'symbolTableOp' reaches 'within'.
SmallVector<FlatSymbolRefAttr, 1> nestedRefs(1, leafRef);
- StringAttr symbolNameId =
- StringAttr::get(ctx, SymbolTable::getSymbolAttrName());
do {
// Each parent of 'symbol' should define a symbol table.
if (!symbolTableOp->hasTrait<OpTrait::SymbolTable>())
return failure();
// Each parent of 'symbol' should also be a symbol.
- StringAttr symbolTableName = getNameIfSymbol(symbolTableOp, symbolNameId);
+ StringAttr symbolTableName = getNameIfSymbol(symbolTableOp);
if (!symbolTableName)
return failure();
results.push_back(SymbolRefAttr::get(symbolTableName, nestedRefs));
@@ -123,10 +121,8 @@ SymbolTable::SymbolTable(Operation *symbolTableOp)
assert(symbolTableOp->getRegion(0).hasOneBlock() &&
"expected operation to have a single block");
- StringAttr symbolNameId = StringAttr::get(symbolTableOp->getContext(),
- SymbolTable::getSymbolAttrName());
for (auto &op : symbolTableOp->getRegion(0).front()) {
- StringAttr name = getNameIfSymbol(&op, symbolNameId);
+ StringAttr name = getNameIfSymbol(&op);
if (!name)
continue;
@@ -292,14 +288,21 @@ SymbolTable::renameToUnique(Operation *op, ArrayRef<SymbolTable *> others) {
/// Returns the name of the given symbol operation.
StringAttr SymbolTable::getSymbolName(Operation *symbol) {
- StringAttr name = getNameIfSymbol(symbol);
+ assert(isa<SymbolOpInterface>(symbol) &&
+ "expected operation to implement SymbolOpInterface");
+ StringAttr name = dyn_cast_or_null<StringAttr>(
+ symbol->getInherentAttr(getSymbolAttrName()).value_or(Attribute{}));
assert(name && "expected valid symbol name");
return name;
}
/// Sets the name of the given symbol operation.
void SymbolTable::setSymbolName(Operation *symbol, StringAttr name) {
- symbol->setAttr(getSymbolAttrName(), name);
+ assert(isa<SymbolOpInterface>(symbol) &&
+ "expected operation to implement SymbolOpInterface");
+ StringAttr attrName =
+ StringAttr::get(symbol->getContext(), getSymbolAttrName());
+ symbol->setInherentAttr(attrName, name);
}
/// Returns the visibility of the given symbol operation.
@@ -373,10 +376,8 @@ Operation *SymbolTable::lookupSymbolIn(Operation *symbolTableOp,
return nullptr;
// Look for a symbol with the given name.
- StringAttr symbolNameId = StringAttr::get(symbolTableOp->getContext(),
- SymbolTable::getSymbolAttrName());
for (auto &op : region.front())
- if (getNameIfSymbol(&op, symbolNameId) == symbol)
+ if (getNameIfSymbol(&op) == symbol)
return &op;
return nullptr;
}
@@ -472,8 +473,7 @@ LogicalResult detail::verifySymbolTable(Operation *op) {
for (auto &block : op->getRegion(0)) {
for (auto &op : block) {
// Check for a symbol name attribute.
- auto nameAttr =
- op.getAttrOfType<StringAttr>(mlir::SymbolTable::getSymbolAttrName());
+ StringAttr nameAttr = getNameIfSymbol(&op);
if (!nameAttr)
continue;
@@ -493,7 +493,7 @@ LogicalResult detail::verifySymbolTable(Operation *op) {
if (SymbolUserOpInterface user = dyn_cast<SymbolUserOpInterface>(op))
if (failed(user.verifySymbolUses(symbolTable)))
return WalkResult::interrupt();
- for (auto &attr : op->getDiscardableAttrs()) {
+ for (auto &attr : op->getDiscardableAttrDictionary().getValue()) {
if (auto user = dyn_cast<SymbolUserAttrInterface>(attr.getValue())) {
if (failed(user.verifySymbolUses(op, symbolTable)))
return WalkResult::interrupt();
@@ -509,14 +509,15 @@ LogicalResult detail::verifySymbolTable(Operation *op) {
LogicalResult detail::verifySymbol(Operation *op) {
// Verify the name attribute.
- if (!op->getAttrOfType<StringAttr>(mlir::SymbolTable::getSymbolAttrName()))
+ if (!op->getInherentAttr(mlir::SymbolTable::getSymbolAttrName())
+ .value_or(Attribute{}))
return op->emitOpError() << "requires string attribute '"
<< mlir::SymbolTable::getSymbolAttrName() << "'";
// Verify the visibility attribute.
StringRef visAttrName =
mlir::SymbolOpInterface::getDefaultVisibilityAttrName();
- if (Attribute vis = op->getAttr(visAttrName)) {
+ if (Attribute vis = op->getInherentAttr(visAttrName).value_or(Attribute{})) {
StringAttr visStrAttr = llvm::dyn_cast<StringAttr>(vis);
if (!visStrAttr)
return op->emitOpError()
@@ -534,8 +535,9 @@ LogicalResult detail::verifySymbol(Operation *op) {
}
SymbolTable::Visibility detail::defaultGetSymbolVisibility(Operation *symbol) {
- StringAttr vis = symbol->getAttrOfType<StringAttr>(
- SymbolOpInterface::getDefaultVisibilityAttrName());
+ StringAttr vis = dyn_cast_or_null<StringAttr>(
+ symbol->getInherentAttr(SymbolOpInterface::getDefaultVisibilityAttrName())
+ .value_or(Attribute{}));
// If the attribute doesn't exist, assume public.
if (!vis)
return SymbolTable::Visibility::Public;
@@ -550,11 +552,12 @@ SymbolTable::Visibility detail::defaultGetSymbolVisibility(Operation *symbol) {
void detail::defaultSetSymbolVisibility(Operation *symbol,
SymbolTable::Visibility vis) {
StringRef attrName = SymbolOpInterface::getDefaultVisibilityAttrName();
+ StringAttr attrNameAttr = StringAttr::get(symbol->getContext(), attrName);
// If the visibility is public, just drop the attribute as this is the
// default.
if (vis == SymbolTable::Visibility::Public) {
- symbol->removeAttr(attrName);
+ symbol->setInherentAttr(attrNameAttr, {});
return;
}
@@ -565,7 +568,8 @@ void detail::defaultSetSymbolVisibility(Operation *symbol,
StringRef visName =
vis == SymbolTable::Visibility::Private ? "private" : "nested";
- symbol->setAttr(attrName, StringAttr::get(symbol->getContext(), visName));
+ symbol->setInherentAttr(attrNameAttr,
+ StringAttr::get(symbol->getContext(), visName));
}
//===----------------------------------------------------------------------===//
@@ -578,14 +582,20 @@ void detail::defaultSetSymbolVisibility(Operation *symbol,
static WalkResult
walkSymbolRefs(Operation *op,
function_ref<WalkResult(SymbolTable::SymbolUse)> callback) {
- return op->getAttrDictionary().walk<WalkOrder::PreOrder>(
- [&](SymbolRefAttr symbolRef) {
- if (callback({op, symbolRef}).wasInterrupted())
- return WalkResult::interrupt();
+ auto walk = [&](Attribute attr) {
+ return attr.walk<WalkOrder::PreOrder>([&](SymbolRefAttr symbolRef) {
+ if (callback({op, symbolRef}).wasInterrupted())
+ return WalkResult::interrupt();
- // Don't walk nested references.
- return WalkResult::skip();
- });
+ // Don't walk nested references.
+ return WalkResult::skip();
+ });
+ };
+ if (walk(op->getRawDictionaryAttrs()).wasInterrupted())
+ return WalkResult::interrupt();
+ if (Attribute properties = op->getPropertiesAsAttribute())
+ return walk(properties);
+ return WalkResult::advance();
}
/// Walk all of the uses, for any symbol, that are nested within the given
diff --git a/mlir/lib/Interfaces/FunctionImplementation.cpp b/mlir/lib/Interfaces/FunctionImplementation.cpp
index 94d4cc08aa0eb..2d39a181f5c90 100644
--- a/mlir/lib/Interfaces/FunctionImplementation.cpp
+++ b/mlir/lib/Interfaces/FunctionImplementation.cpp
@@ -179,15 +179,16 @@ void function_interface_impl::printFunctionOp(
OpAsmPrinter &p, FunctionOpInterface op, bool isVariadic,
StringRef typeAttrName, StringAttr argAttrsName, StringAttr resAttrsName) {
// Print the operation and the function name.
- auto funcName =
- op->getAttrOfType<StringAttr>(SymbolTable::getSymbolAttrName())
- .getValue();
+ auto symbol = cast<SymbolOpInterface>(op.getOperation());
+ StringRef funcName = symbol.getName();
p << ' ';
StringRef visibilityAttrName =
SymbolOpInterface::getDefaultVisibilityAttrName();
- if (auto visibility = op->getAttrOfType<StringAttr>(visibilityAttrName))
- p << visibility.getValue() << ' ';
+ Attribute visibility =
+ op->getInherentAttr(visibilityAttrName).value_or(Attribute{});
+ if (auto value = dyn_cast_or_null<StringAttr>(visibility))
+ p << value.getValue() << ' ';
p.printSymbolName(funcName);
ArrayRef<Type> argTypes = op.getArgumentTypes();
diff --git a/mlir/lib/Pass/IRPrinting.cpp b/mlir/lib/Pass/IRPrinting.cpp
index 006ce3cbe794a..61a6adb0f1a24 100644
--- a/mlir/lib/Pass/IRPrinting.cpp
+++ b/mlir/lib/Pass/IRPrinting.cpp
@@ -70,9 +70,8 @@ static void printIRHeader(raw_ostream &out, StringRef title, Pass *pass,
pass->printAsTextualPipeline(out);
if (printModuleScope) {
out << " ('" << op->getName() << "' operation";
- if (auto symbolName =
- op->getAttrOfType<StringAttr>(SymbolTable::getSymbolAttrName()))
- out << ": @" << symbolName.getValue();
+ if (auto symbol = dyn_cast<SymbolOpInterface>(op))
+ out << ": @" << symbol.getName();
out << ")";
}
out << " //----- //\n";
@@ -230,10 +229,8 @@ getOpAndSymbolNames(Operation *op, StringRef passName,
++counters.try_emplace(op, -1).first->second;
while (iter) {
countPrefix.push_back(counters[iter]);
- StringAttr symbolNameAttr =
- iter->getAttrOfType<StringAttr>(SymbolTable::getSymbolAttrName());
- std::string symbolName =
- symbolNameAttr ? symbolNameAttr.str() : "no-symbol-name";
+ auto symbol = dyn_cast<SymbolOpInterface>(iter);
+ std::string symbolName = symbol ? symbol.getName().str() : "no-symbol-name";
llvm::replace(symbolName, '/', '_');
llvm::replace(symbolName, '\\', '_');
diff --git a/mlir/python/mlir/dialects/gpu/__init__.py b/mlir/python/mlir/dialects/gpu/__init__.py
index b75bd525c9487..2ce4e3a24bf6b 100644
--- a/mlir/python/mlir/dialects/gpu/__init__.py
+++ b/mlir/python/mlir/dialects/gpu/__init__.py
@@ -86,7 +86,10 @@ def __init__(
if not isinstance(function_type, TypeAttr)
else function_type
)
+ if not isinstance(sym_name, (str, StringAttr)):
+ raise ValueError("sym_name must be a string or a StringAttr")
super().__init__(
+ sym_name,
function_type,
arg_attrs=arg_attrs,
res_attrs=res_attrs,
@@ -96,13 +99,6 @@ def __init__(
ip=ip,
)
- if isinstance(sym_name, str):
- self.attributes[self.SYM_NAME_ATTR_NAME] = StringAttr.get(sym_name)
- elif isinstance(sym_name, StringAttr):
- self.attributes[self.SYM_NAME_ATTR_NAME] = sym_name
- else:
- raise ValueError("sym_name must be a string or a StringAttr")
-
if kernel:
self.attributes[self.KERNEL_ATTR_NAME] = UnitAttr.get()
diff --git a/mlir/test/Dialect/OpenMP/cli-canonical_loop.mlir b/mlir/test/Dialect/OpenMP/cli-canonical_loop.mlir
index 0e9385ee75c47..76d450424ffda 100644
--- a/mlir/test/Dialect/OpenMP/cli-canonical_loop.mlir
+++ b/mlir/test/Dialect/OpenMP/cli-canonical_loop.mlir
@@ -236,9 +236,8 @@ func.func @omp_newcli_unused() -> () {
}
-// CHECK-LABEL: @omp_canonloop_multiregion_isolatedfromabove(
-func.func @omp_canonloop_multiregion_isolatedfromabove() -> () {
- omp.private {type = firstprivate} @x.privatizer : !llvm.ptr init {
+// CHECK-LABEL: omp.private {{.*}} @x.privatizer
+omp.private {type = firstprivate} @x.privatizer : !llvm.ptr init {
^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr):
%c42_i32 = arith.constant 42: i32
// CHECK: omp.canonical_loop %iv : i32 in range(%c42_i32) {
@@ -273,10 +272,6 @@ func.func @omp_canonloop_multiregion_isolatedfromabove() -> () {
}
// CHECK: omp.yield
omp.yield
- }
-
- // CHECK: return
- return
}
diff --git a/mlir/unittests/Transforms/Canonicalizer.cpp b/mlir/unittests/Transforms/Canonicalizer.cpp
index 4b94e0602b509..74144ea39603d 100644
--- a/mlir/unittests/Transforms/Canonicalizer.cpp
+++ b/mlir/unittests/Transforms/Canonicalizer.cpp
@@ -79,8 +79,9 @@ TEST(CanonicalizerTest, TestDisablePatterns) {
OwningOpRef<ModuleOp> module = parseSourceString<ModuleOp>(code, &context);
ASSERT_TRUE(succeeded(mgr.run(*module)));
- EXPECT_TRUE(module->lookupSymbol("B"));
- EXPECT_FALSE(module->lookupSymbol("A"));
+ Block &body = module->getBodyRegion().front();
+ ASSERT_EQ(body.getOperations().size(), 1u);
+ EXPECT_EQ(body.front().getResult(0).getType(), Float32Type::get(&context));
}
} // end anonymous namespace
More information about the Mlir-commits
mailing list