[Mlir-commits] [mlir] [mlir][ODS] Reuse collective builders for zero-result ops (NFC) (PR #226059)

[Mehdi Amini]

https://github.com/joker-eph updated https://github.com/llvm/llvm-project/pull/226059

>From 189a2e4c5c8cccceee2226f5388a750a1840ff2f Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Wed, 23 Sep 2026 08:34:29 -0700
Subject: [PATCH 1/2] [mlir][ODS] Reuse collective builders for zero-result ops

Forward the no-result separate-argument builder to its collective
counterpart instead of generating the same operand, attribute, and
region construction twice. This cuts OpenMPDialect.cpp compiler work
by 1.5% and its machine-code text by about 71 KB.

Assisted-by: Codex
(cherry picked from commit b452f58f7c29795f5bcee4426940ac8b80729825)
---
 mlir/test/mlir-tblgen/op-decl-and-defs.td   | 2 ++
 mlir/test/mlir-tblgen/op-operand.td         | 8 ++++++--
 mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp | 9 +++++++++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/mlir/test/mlir-tblgen/op-decl-and-defs.td b/mlir/test/mlir-tblgen/op-decl-and-defs.td
index 32c4bb88d188b..bd39c4a013ba3 100644
--- a/mlir/test/mlir-tblgen/op-decl-and-defs.td
+++ b/mlir/test/mlir-tblgen/op-decl-and-defs.td
@@ -205,6 +205,8 @@ def NS_BOp : NS_Op<"op_with_no_operand", []> {
 
 // CHECK-LABEL: NS::BOp declarations
 // CHECK: OpTrait::ZeroOperands
+// DEFS-LABEL: void BOp::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState) {
+// DEFS-NEXT:   build(odsBuilder, odsState, ::mlir::TypeRange{});
 
 def NS_COp : NS_Op<"op_with_one_operand", []> {
   let arguments = (ins I32:$operand);
diff --git a/mlir/test/mlir-tblgen/op-operand.td b/mlir/test/mlir-tblgen/op-operand.td
index ab8d721ed5427..95ddf957ee1c5 100644
--- a/mlir/test/mlir-tblgen/op-operand.td
+++ b/mlir/test/mlir-tblgen/op-operand.td
@@ -37,7 +37,9 @@ def OpB : NS_Op<"one_variadic_operand_op", []> {
 // CHECK-LABEL: OpB::build
 // CHECK:         ::mlir::ValueRange input
 // CHECK-NOT:     assert
-// CHECK:         odsState.addOperands(input);
+// CHECK:         build(odsBuilder, odsState, ::mlir::TypeRange{}, input);
+// CHECK-LABEL: OpB::build
+// CHECK:         odsState.addOperands(operands);
 
 def OpD : NS_Op<"mix_variadic_and_normal_inputs_op", [SameVariadicOperandSize]> {
   let arguments = (ins Variadic<AnyTensor>:$input1, AnyTensor:$input2, Variadic<AnyTensor>:$input3);
@@ -55,6 +57,8 @@ def OpD : NS_Op<"mix_variadic_and_normal_inputs_op", [SameVariadicOperandSize]>
 // DECL-NEXT: return ::llvm::cast<::mlir::TypedValue<::mlir::TensorType>>(*getODSOperands(1).begin());
 
 // CHECK-LABEL: OpD::build
-// CHECK-NEXT: odsState.addOperands(input1);
+// CHECK-NEXT: build(odsBuilder, odsState, ::mlir::TypeRange{}, input1, input2, input3);
+// CHECK-LABEL: OpD::build
+// CHECK: odsState.addOperands(input1);
 // CHECK-NEXT: odsState.addOperands(input2);
 // CHECK-NEXT: odsState.addOperands(input3);
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index aa9ff159903bc..6bda98d00e50f 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -2485,6 +2485,15 @@ void OpEmitter::genSeparateArgParamBuilder() {
     genInlineCreateBody(paramList);
 
     auto &body = m->body();
+    // The no-result builder can reuse the collective builder. Both forms
+    // otherwise emit identical operand, attribute, and region handling.
+    if (paramKind == TypeParamKind::Separate && op.getNumResults() == 0) {
+      body << "  build(odsBuilder, odsState, ::mlir::TypeRange{}";
+      for (const MethodParameter &param : llvm::drop_begin(paramList, 2))
+        body << ", " << param.getName();
+      body << ");\n";
+      return;
+    }
     genCodeForAddingArgAndRegionForBuilder(body, inferredAttributes,
                                            /*isRawValueAttr=*/attrType ==
                                                AttrParamKind::UnwrappedValue);

>From c025a0de3527ffbba9649f20dabb5d714483f302 Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Thu, 24 Sep 2026 01:28:47 -0700
Subject: [PATCH 2/2] [mlir][ODS] Update attribute builder test for delegation

The no-result separate-argument builder now forwards to the collective
builder, so the previous property assignment check appears only in the
collective overload. Check both the forwarding call and the property
conversion in emitted order.

Assisted-by: Codex
---
 mlir/test/mlir-tblgen/op-attribute.td | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/mlir/test/mlir-tblgen/op-attribute.td b/mlir/test/mlir-tblgen/op-attribute.td
index ebba86d32d82f..3613a46de9445 100644
--- a/mlir/test/mlir-tblgen/op-attribute.td
+++ b/mlir/test/mlir-tblgen/op-attribute.td
@@ -143,12 +143,19 @@ def AOp : NS_Op<"a_op", []> {
 // Test build methods
 // ---
 
+// DEF:      void AOp::build(
+// DEF:        build(odsBuilder, odsState, ::mlir::TypeRange{}, aAttr, bAttr, cAttr, dAttr);
+
 // DEF:      void AOp::build(
 // DEF:        odsState.getOrAddProperties<Properties>().aAttr = aAttr;
 // DEF:        odsState.getOrAddProperties<Properties>().bAttr = bAttr;
 // DEF:        if (cAttr) {
 // DEF-NEXT:     odsState.getOrAddProperties<Properties>().cAttr = cAttr;
 
+// DEF:      void AOp::build(
+// DEF:        some-return-type aAttr, some-return-type bAttr, /*optional*/some-attr-kind cAttr
+// DEF:        build(odsBuilder, odsState, ::mlir::TypeRange{}, aAttr, bAttr, cAttr, dAttr);
+
 // DEF:        odsState.getOrAddProperties<Properties>().aAttr = some-const-builder-call(odsBuilder, aAttr);
 // DEF-NEXT:   odsState.getOrAddProperties<Properties>().bAttr = some-const-builder-call(odsBuilder, bAttr);
 // DEF-NEXT:   if (cAttr) {
@@ -157,10 +164,6 @@ def AOp : NS_Op<"a_op", []> {
 // DEF-NOT:    if (dAttr)
 // DEF:        odsState.getOrAddProperties<Properties>().dAttr = some-const-builder-call(odsBuilder, dAttr);
 
-// DEF:      void AOp::build(
-// DEF:        some-return-type aAttr, some-return-type bAttr, /*optional*/some-attr-kind cAttr
-// DEF:        odsState.getOrAddProperties<Properties>().aAttr = some-const-builder-call(odsBuilder, aAttr);
-
 // DEF:      void AOp::build(
 // DEF:        ::llvm::ArrayRef<::mlir::NamedAttribute> attributes
 // DEF:      buildPropertiesAndDiscardableAttributes(odsState, attributes);