Skip to content

Conversation

@joker-eph
Copy link
Collaborator

This allows people writing custom C++ assembly functions to reuse the "prop-dict" parser.

Fix #145028

@joker-eph joker-eph requested review from Copilot and jpienaar June 26, 2025 21:04
@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Jun 26, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes an issue where the parsed attribute properties setter was not emitted for operations with custom assembly formats, ensuring that custom C++ assembly functions can reuse the "prop-dict" parser.

  • Generates the parsed attribute properties setter when the operation does not expose an assembly format.
  • Updates test files to verify that the new setter is generated.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
mlir/tools/mlir-tblgen/OpFormatGen.cpp Added a conditional block to generate the parsed properties setter when op.hasAssemblyFormat() is false.
mlir/test/mlir-tblgen/op-decl-and-defs.td Added a test check comment for verifying the generated setPropertiesFromParsedAttr function.

@llvmbot
Copy link
Member

llvmbot commented Jun 26, 2025

@llvm/pr-subscribers-mlir-core

Author: Mehdi Amini (joker-eph)

Changes

This allows people writing custom C++ assembly functions to reuse the "prop-dict" parser.

Fix #145028


Full diff: https://github.com/llvm/llvm-project/pull/145973.diff

2 Files Affected:

  • (modified) mlir/test/mlir-tblgen/op-decl-and-defs.td (+1)
  • (modified) mlir/tools/mlir-tblgen/OpFormatGen.cpp (+7-1)
diff --git a/mlir/test/mlir-tblgen/op-decl-and-defs.td b/mlir/test/mlir-tblgen/op-decl-and-defs.td
index a3dce9b31f8d2..3ccefd4d82366 100644
--- a/mlir/test/mlir-tblgen/op-decl-and-defs.td
+++ b/mlir/test/mlir-tblgen/op-decl-and-defs.td
@@ -125,6 +125,7 @@ def NS_AOp : NS_Op<"a_op", [IsolatedFromAbove, IsolatedFromAbove]> {
 // CHECK:   ::llvm::LogicalResult verifyInvariants();
 // CHECK:   static void getCanonicalizationPatterns(::mlir::RewritePatternSet &results, ::mlir::MLIRContext *context);
 // CHECK:   ::llvm::LogicalResult fold(FoldAdaptor adaptor, ::llvm::SmallVectorImpl<::mlir::OpFoldResult> &results);
+// CHECK:   static ::llvm::LogicalResult setPropertiesFromParsedAttr(Properties &prop, ::mlir::Attribute attr, ::llvm::function_ref<::mlir::InFlightDiagnostic()> emitError);
 // CHECK:   // Display a graph for debugging purposes.
 // CHECK:   void displayGraph();
 // CHECK: };
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index 11edf2523f1aa..0c62e803ece8a 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -3848,8 +3848,14 @@ void mlir::tblgen::generateOpFormat(const Operator &constOp, OpClass &opClass,
   // TODO: Operator doesn't expose all necessary functionality via
   // the const interface.
   Operator &op = const_cast<Operator &>(constOp);
-  if (!op.hasAssemblyFormat())
+  if (!op.hasAssemblyFormat()) {
+    // We still need to generate the parsed attribute properties setter for
+    // allowing it to be reused in custom assembly implementations.
+    OperationFormat format(op, hasProperties);
+    format.hasPropDict = true;
+    genParsedAttrPropertiesSetter(format, op, opClass);
     return;
+  }
 
   // Parse the format description.
   llvm::SourceMgr mgr;

@llvmbot
Copy link
Member

llvmbot commented Jun 26, 2025

@llvm/pr-subscribers-mlir

Author: Mehdi Amini (joker-eph)

Changes

This allows people writing custom C++ assembly functions to reuse the "prop-dict" parser.

Fix #145028


Full diff: https://github.com/llvm/llvm-project/pull/145973.diff

2 Files Affected:

  • (modified) mlir/test/mlir-tblgen/op-decl-and-defs.td (+1)
  • (modified) mlir/tools/mlir-tblgen/OpFormatGen.cpp (+7-1)
diff --git a/mlir/test/mlir-tblgen/op-decl-and-defs.td b/mlir/test/mlir-tblgen/op-decl-and-defs.td
index a3dce9b31f8d2..3ccefd4d82366 100644
--- a/mlir/test/mlir-tblgen/op-decl-and-defs.td
+++ b/mlir/test/mlir-tblgen/op-decl-and-defs.td
@@ -125,6 +125,7 @@ def NS_AOp : NS_Op<"a_op", [IsolatedFromAbove, IsolatedFromAbove]> {
 // CHECK:   ::llvm::LogicalResult verifyInvariants();
 // CHECK:   static void getCanonicalizationPatterns(::mlir::RewritePatternSet &results, ::mlir::MLIRContext *context);
 // CHECK:   ::llvm::LogicalResult fold(FoldAdaptor adaptor, ::llvm::SmallVectorImpl<::mlir::OpFoldResult> &results);
+// CHECK:   static ::llvm::LogicalResult setPropertiesFromParsedAttr(Properties &prop, ::mlir::Attribute attr, ::llvm::function_ref<::mlir::InFlightDiagnostic()> emitError);
 // CHECK:   // Display a graph for debugging purposes.
 // CHECK:   void displayGraph();
 // CHECK: };
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index 11edf2523f1aa..0c62e803ece8a 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -3848,8 +3848,14 @@ void mlir::tblgen::generateOpFormat(const Operator &constOp, OpClass &opClass,
   // TODO: Operator doesn't expose all necessary functionality via
   // the const interface.
   Operator &op = const_cast<Operator &>(constOp);
-  if (!op.hasAssemblyFormat())
+  if (!op.hasAssemblyFormat()) {
+    // We still need to generate the parsed attribute properties setter for
+    // allowing it to be reused in custom assembly implementations.
+    OperationFormat format(op, hasProperties);
+    format.hasPropDict = true;
+    genParsedAttrPropertiesSetter(format, op, opClass);
     return;
+  }
 
   // Parse the format description.
   llvm::SourceMgr mgr;

@peledins-zimperium
Copy link

Thank you!

@joker-eph joker-eph force-pushed the setPropertiesFromParsedAttr branch 2 times, most recently from 6f4ba63 to 29d1160 Compare June 30, 2025 11:30
…lyFormat is set

This allows people writing custom C++ assembly functions to reuse the
"prop-dict" parser.

Fix llvm#145028
@joker-eph joker-eph force-pushed the setPropertiesFromParsedAttr branch from 29d1160 to 66f1ada Compare June 30, 2025 13:17
@joker-eph joker-eph merged commit 5f91b69 into llvm:main Jun 30, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mlir:core MLIR Core Infrastructure mlir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[mlir] parseProperties not usable when hasCustomAssemblyFormat

4 participants