[mlir][AsmPrinter] Print op properties directly in generic form

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,

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(

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

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.

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.

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

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();

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

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`)?

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",