[Mlir-commits] [mlir] [mlir][AsmPrinter] Print op properties directly in generic form (PR #106996)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Mon Sep 2 07:20:02 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-gpu
@llvm/pr-subscribers-mlir-core
Author: Andrei Golubev (andrey-golubev)
<details>
<summary>Changes</summary>
As properties are always at least default-constructed (during Operation's creation), they should be printable at any point.
The AsmPrinter's code is overly cautious and always selects to print properties as attributes in the "generic op form". Instead, this decision could be moved to the operation. There's already some infrastructure around printing (with extension points available) that by default converts the properties to attributes (so the current behaviour is majorly unchaged).
This way, users not wishing to use "convert to property" fallback could provide a custom printing method that is going to be used during "generic op form" printing (e.g. when op verification fails). The assumption is that if we could convert properties to attributes in "failure" state, it should also be possible to print the properties directly.
---
Full diff: https://github.com/llvm/llvm-project/pull/106996.diff
10 Files Affected:
- (modified) mlir/include/mlir/Dialect/XeGPU/IR/XeGPUOps.td (+1-1)
- (modified) mlir/include/mlir/IR/ExtensibleDialect.h (+1)
- (modified) mlir/include/mlir/IR/OpDefinition.h (+14-15)
- (modified) mlir/include/mlir/IR/OperationSupport.h (+13)
- (modified) mlir/lib/IR/AsmPrinter.cpp (+2-4)
- (modified) mlir/lib/IR/MLIRContext.cpp (+10)
- (modified) mlir/lib/IR/Operation.cpp (+1)
- (modified) mlir/test/IR/properties.mlir (+5)
- (modified) mlir/test/lib/Dialect/Test/TestOps.td (+21)
- (modified) mlir/tools/mlir-tblgen/OpClass.cpp (+1)
``````````diff
diff --git a/mlir/include/mlir/Dialect/XeGPU/IR/XeGPUOps.td b/mlir/include/mlir/Dialect/XeGPU/IR/XeGPUOps.td
index c32c7541c39791..c88a976240bb26 100644
--- a/mlir/include/mlir/Dialect/XeGPU/IR/XeGPUOps.td
+++ b/mlir/include/mlir/Dialect/XeGPU/IR/XeGPUOps.td
@@ -31,7 +31,7 @@ class XeGPU_Op<string mnemonic, list<Trait> traits = []>:
::mlir::ArrayRef<::llvm::StringRef> elidedProps) {
Attribute propAttr = getPropertiesAsAttr(ctx, prop);
if (propAttr)
- p << "<" << propAttr << ">";
+ p << " <" << propAttr << ">";
}
static ::mlir::ParseResult parseProperties(::mlir::OpAsmParser &parser,
diff --git a/mlir/include/mlir/IR/ExtensibleDialect.h b/mlir/include/mlir/IR/ExtensibleDialect.h
index 494f3dfb05a04d..ffb9fa9ef467a2 100644
--- a/mlir/include/mlir/IR/ExtensibleDialect.h
+++ b/mlir/include/mlir/IR/ExtensibleDialect.h
@@ -496,6 +496,7 @@ class DynamicOpDefinition : public OperationName::Impl {
void copyProperties(OpaqueProperties lhs, OpaqueProperties rhs) final {}
bool compareProperties(OpaqueProperties, OpaqueProperties) final { return false; }
llvm::hash_code hashProperties(OpaqueProperties prop) final { return {}; }
+ void printProperties(Operation *, OpAsmPrinter &) final {}
private:
DynamicOpDefinition(
diff --git a/mlir/include/mlir/IR/OpDefinition.h b/mlir/include/mlir/IR/OpDefinition.h
index 59f094d6690991..b8788b22080150 100644
--- a/mlir/include/mlir/IR/OpDefinition.h
+++ b/mlir/include/mlir/IR/OpDefinition.h
@@ -1809,14 +1809,13 @@ class Op : public OpState, public Traits<ConcreteType>... {
/// Trait to check if printProperties(OpAsmPrinter, T, ArrayRef<StringRef>)
/// exist
- template <typename T, typename... Args>
- using has_print_properties =
- decltype(printProperties(std::declval<OpAsmPrinter &>(),
- std::declval<T>(),
- std::declval<ArrayRef<StringRef>>()));
- template <typename T>
+ template <typename ConcreteOp, typename Props>
+ using has_print_properties = decltype(ConcreteOp::printProperties(
+ std::declval<OpAsmPrinter &>(), std::declval<Props>(),
+ std::declval<ArrayRef<StringRef>>()));
+ template <typename ConcreteOp, typename Props>
using detect_has_print_properties =
- llvm::is_detected<has_print_properties, T>;
+ llvm::is_detected<has_print_properties, ConcreteOp, Props>;
/// Trait to check if parseProperties(OpAsmParser, T) exist
template <typename T, typename... Args>
@@ -1981,16 +1980,16 @@ class Op : public OpState, public Traits<ConcreteType>... {
/// Print the operation properties with names not included within
/// 'elidedProps'. Unless overridden, this method will try to dispatch to a
- /// `printProperties` free-function if it exists, and otherwise by converting
- /// the properties to an Attribute.
- template <typename T>
+ /// `T::printProperties` if it exists, and otherwise by converting the
+ /// properties to an Attribute.
+ template <typename T = ConcreteType>
static void printProperties(MLIRContext *ctx, OpAsmPrinter &p,
- const T &properties,
+ const InferredProperties<T> &properties,
ArrayRef<StringRef> elidedProps = {}) {
- if constexpr (detect_has_print_properties<T>::value)
- return printProperties(p, properties, elidedProps);
- genericPrintProperties(
- p, ConcreteType::getPropertiesAsAttr(ctx, properties), elidedProps);
+ if constexpr (detect_has_print_properties<T, InferredProperties<T>>::value)
+ return T::printProperties(p, properties, elidedProps);
+ genericPrintProperties(p, T::getPropertiesAsAttr(ctx, properties),
+ elidedProps);
}
/// Parses 'prop-dict' for the operation. Unless overridden, the method will
diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index 1b93f3d3d04fe8..51ec61ea15b3c4 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -142,6 +142,7 @@ class OperationName {
virtual void copyProperties(OpaqueProperties, OpaqueProperties) = 0;
virtual bool compareProperties(OpaqueProperties, OpaqueProperties) = 0;
virtual llvm::hash_code hashProperties(OpaqueProperties) = 0;
+ virtual void printProperties(Operation *, OpAsmPrinter &) = 0;
};
public:
@@ -223,6 +224,7 @@ class OperationName {
void copyProperties(OpaqueProperties, OpaqueProperties) final;
bool compareProperties(OpaqueProperties, OpaqueProperties) final;
llvm::hash_code hashProperties(OpaqueProperties) final;
+ void printProperties(Operation *, OpAsmPrinter &) final;
};
public:
@@ -455,6 +457,10 @@ class OperationName {
return getImpl()->hashProperties(properties);
}
+ void printOpProperties(Operation *op, OpAsmPrinter &printer) const {
+ getImpl()->printProperties(op, printer);
+ }
+
/// Return the dialect this operation is registered to if the dialect is
/// loaded in the context, or nullptr if the dialect isn't loaded.
Dialect *getDialect() const {
@@ -668,6 +674,13 @@ class RegisteredOperationName : public OperationName {
return {};
}
+ void printProperties(Operation *op, OpAsmPrinter &printer) final {
+ if constexpr (hasProperties) {
+ auto concreteOp = cast<ConcreteOp>(op);
+ ConcreteOp::printProperties(concreteOp->getContext(), printer,
+ concreteOp.getProperties());
+ }
+ }
};
/// Lookup the registered operation information for the given operation.
diff --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp
index 02acc8c3f4659e..0db6bccf4cfeaf 100644
--- a/mlir/lib/IR/AsmPrinter.cpp
+++ b/mlir/lib/IR/AsmPrinter.cpp
@@ -3558,10 +3558,8 @@ void OperationPrinter::printGenericOp(Operation *op, bool printOpName) {
}
// Print the properties.
- if (Attribute prop = op->getPropertiesAsAttribute()) {
- os << " <";
- Impl::printAttribute(prop);
- os << '>';
+ if (op->getPropertiesStorageSize()) {
+ op->getName().printOpProperties(op, *this);
}
// Print regions.
diff --git a/mlir/lib/IR/MLIRContext.cpp b/mlir/lib/IR/MLIRContext.cpp
index 5c93747438ecdb..cbf544e34a9f0a 100644
--- a/mlir/lib/IR/MLIRContext.cpp
+++ b/mlir/lib/IR/MLIRContext.cpp
@@ -908,6 +908,16 @@ llvm::hash_code
OperationName::UnregisteredOpModel::hashProperties(OpaqueProperties prop) {
return llvm::hash_combine(*prop.as<Attribute *>());
}
+void OperationName::UnregisteredOpModel::printProperties(
+ Operation *op, OpAsmPrinter &printer) {
+ auto asAttr = getPropertiesAsAttr(op);
+ // Note: printAttribute(nullptr) would insert <<NULL ATTRIBUTE>> which changes
+ // the current behavior
+ if (asAttr == nullptr)
+ return;
+ printer << ' ';
+ printer.printAttribute(asAttr);
+}
//===----------------------------------------------------------------------===//
// RegisteredOperationName
diff --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp
index 8f28d61c732cdc..e2f393ec3d207c 100644
--- a/mlir/lib/IR/Operation.cpp
+++ b/mlir/lib/IR/Operation.cpp
@@ -803,6 +803,7 @@ void OpState::genericPrintProperties(OpAsmPrinter &p, Attribute properties,
ArrayRef<StringRef> elidedProps) {
if (!properties)
return;
+ p << ' ';
auto dictAttr = dyn_cast_or_null<::mlir::DictionaryAttr>(properties);
if (dictAttr && !elidedProps.empty()) {
ArrayRef<NamedAttribute> attrs = dictAttr.getValue();
diff --git a/mlir/test/IR/properties.mlir b/mlir/test/IR/properties.mlir
index 418b81dcbb034f..5d64aa8b6024da 100644
--- a/mlir/test/IR/properties.mlir
+++ b/mlir/test/IR/properties.mlir
@@ -79,3 +79,8 @@ test.with_optional_properties nested = some<none>
// CHECK-SAME: ints = [1, 2] strings = ["a", "b"] nested = {{\[}}[1, 2], [3, 4]] opt = [-1, -2] explicitOptions = [none, 0] explicitUnits = [unit, unit_absent]
// GENERIC: "test.with_array_properties"()
test.with_array_properties ints = [1, 2] strings = ["a", "b"] nested = [[1, 2], [3, 4]] opt = [-1, -2] explicitOptions = [none, 0] explicitUnits = [unit, unit_absent] [] thats_has_default
+
+// CHECK: test.with_print_properties_hook a = 42{{$}}
+// GENERIC: "test.with_print_properties_hook"()
+// GENERIC-SAME: <{printing_through_custom_hook = true, a = 42}> : () -> ()
+test.with_print_properties_hook a = 42
diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index 9e19966414d1d7..f146564b90a288 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -3150,6 +3150,27 @@ def TestOpWithVersionedProperties : TEST_Op<"with_versioned_properties"> {
}];
}
+def TestOpWithPrintPropertiesHook : TEST_Op<"with_print_properties_hook"> {
+ let assemblyFormat = "`a` `=` $a attr-dict";
+ let arguments = (ins
+ I64Property:$a
+ );
+ let extraClassDeclaration = [{
+ static void printProperties(::mlir::OpAsmPrinter &p,
+ const Properties &prop,
+ ::mlir::ArrayRef<::llvm::StringRef>);
+ }];
+ let extraClassDefinition = [{
+ void TestOpWithPrintPropertiesHook::printProperties(
+ ::mlir::OpAsmPrinter &p, const Properties &prop,
+ ::mlir::ArrayRef<::llvm::StringRef>) {
+ // Note: we need to comply with MLIR's asm parser, so "pretend" we're
+ // printing an attribute sequence
+ p << "<{printing_through_custom_hook = true, a = " << prop.a << "}>";
+ }
+ }];
+}
+
def TestOpWithDefaultValuedProperties : TEST_Op<"with_default_valued_properties"> {
let assemblyFormat = [{
($a^) : (`na`)?
diff --git a/mlir/tools/mlir-tblgen/OpClass.cpp b/mlir/tools/mlir-tblgen/OpClass.cpp
index 60fa1833ce625e..c3f5d5f1bf486f 100644
--- a/mlir/tools/mlir-tblgen/OpClass.cpp
+++ b/mlir/tools/mlir-tblgen/OpClass.cpp
@@ -26,6 +26,7 @@ OpClass::OpClass(StringRef name, std::string extraClassDeclaration,
/// Inherit functions from Op.
declare<UsingDeclaration>("Op::Op");
declare<UsingDeclaration>("Op::print");
+ declare<UsingDeclaration>("Op::printProperties");
/// Type alias for the adaptor class.
declare<UsingDeclaration>("Adaptor", className + "Adaptor");
declare<UsingDeclaration>("GenericAdaptor",
``````````
</details>
https://github.com/llvm/llvm-project/pull/106996
More information about the Mlir-commits
mailing list