From c7dab559bae8eeb0b14038aa592f9fb03025bcc1 Mon Sep 17 00:00:00 2001
From: Alex Zinenko <zinenko@google.com>
Date: Mon, 29 Jul 2019 14:12:15 -0700
Subject: [PATCH] RewriterGen: properly handle zero-result ops

RewriterGen was emitting invalid C++ code if the pattern required to create a
zero-result operation due to the absence of a special case that would avoid
generating a spurious comma.  Handle this case.  Also add rewriter tests for
zero-argument operations.

PiperOrigin-RevId: 260576998
---
 mlir/test/lib/TestDialect/TestOps.td   | 10 ++++++++++
 mlir/test/mlir-tblgen/pattern.mlir     | 14 ++++++++++++++
 mlir/tools/mlir-tblgen/RewriterGen.cpp |  6 +++---
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/mlir/test/lib/TestDialect/TestOps.td b/mlir/test/lib/TestDialect/TestOps.td
index db52057..d6a2135 100644
--- a/mlir/test/lib/TestDialect/TestOps.td
+++ b/mlir/test/lib/TestDialect/TestOps.td
@@ -245,6 +245,16 @@ def : Pat<(OpD $input), (OpF $input), [], (addBenefit 10)>;
 def : Pat<(OpG $input), (OpB $input, ConstantAttr<I32Attr, "20">:$attr)>;
 def : Pat<(OpG (OpG $input)), (OpB $input, ConstantAttr<I32Attr, "34">:$attr)>;
 
+// Test patterns for zero-result op.
+def OpH : TEST_Op<"op_h">, Arguments<(ins I32:$arg)>, Results<(outs)>;
+def OpI : TEST_Op<"op_i">, Arguments<(ins I32:$arg)>, Results<(outs)>;
+def : Pat<(OpH $input), (OpI $input)>;
+
+// Test patterns for zero-input op.
+def OpJ : TEST_Op<"op_j">, Arguments<(ins)>, Results<(outs I32:$res)>;
+def OpK : TEST_Op<"op_k">, Arguments<(ins)>, Results<(outs I32:$res)>;
+def : Pat<(OpJ), (OpK)>;
+
 // Test NativeCodeCall.
 def OpNativeCodeCall1 : TEST_Op<"native_code_call1"> {
   let arguments = (ins
diff --git a/mlir/test/mlir-tblgen/pattern.mlir b/mlir/test/mlir-tblgen/pattern.mlir
index 731cd59..d905bba8 100644
--- a/mlir/test/mlir-tblgen/pattern.mlir
+++ b/mlir/test/mlir-tblgen/pattern.mlir
@@ -10,6 +10,20 @@ func @verifyFusedLocs(%arg0 : i32) -> i32 {
   return %result : i32
 }
 
+// CHECK-LABEL: verifyZeroResult
+func @verifyZeroResult(%arg0 : i32) {
+  // CHECK: "test.op_i"(%arg0) : (i32) -> ()
+  "test.op_h"(%arg0) : (i32) -> ()
+  return
+}
+
+// CHECK-LABEL verifyZeroArg
+func @verifyZeroArg() -> i32 {
+  // CHECK: "test.op_k"() : () -> i32
+  %0 = "test.op_j"() : () -> i32
+  return %0 : i32
+}
+
 // CHECK-LABEL: verifyBenefit
 func @verifyBenefit(%arg0 : i32) -> i32 {
   %0 = "test.op_d"(%arg0) : (i32) -> i32
diff --git a/mlir/tools/mlir-tblgen/RewriterGen.cpp b/mlir/tools/mlir-tblgen/RewriterGen.cpp
index cf4dd85..5dfafdd 100644
--- a/mlir/tools/mlir-tblgen/RewriterGen.cpp
+++ b/mlir/tools/mlir-tblgen/RewriterGen.cpp
@@ -806,9 +806,9 @@ std::string PatternEmitter::emitOpCreate(DagNode tree, int resultIndex,
         formatValuePack("op->getResult({1})->getType()", valuePackName,
                         resultOp.getNumResults(), resultIndex);
 
-    os.indent(4) << formatv("auto {0} = rewriter.create<{1}>(loc, {2}",
-                            valuePackName, resultOp.getQualCppClassName(),
-                            resultTypes);
+    os.indent(4) << formatv("auto {0} = rewriter.create<{1}>(loc",
+                            valuePackName, resultOp.getQualCppClassName())
+                 << (resultTypes.empty() ? "" : ", ") << resultTypes;
   }
 
   // Create the builder call for the result.
-- 
2.7.4