[Mlir-commits] [mlir] [mlir][emitc] Don't emit extra semicolon after bracket (PR #122464)

[Kirill Chibisov]

https://github.com/kchibisov updated https://github.com/llvm/llvm-project/pull/122464

>From 5520426df12d38d4be347461d11912ec31b208db Mon Sep 17 00:00:00 2001
From: Kirill Chibisov <contact at kchibisov.com>
Date: Fri, 10 Jan 2025 17:28:36 +0300
Subject: [PATCH] [mlir][emitc] Don't emit extra semicolon after bracket

Extra semicolons were emitted for operations that should never have
them, since not every place was checking whether semicolon would be
actually needed.

Thus change the emitOperation to ignore trailingSemicolon field for
such operations.
---
 mlir/lib/Target/Cpp/TranslateToCpp.cpp       | 20 ++++++++++----------
 mlir/test/Target/Cpp/no_extra_semicolon.mlir | 18 ++++++++++++++++++
 2 files changed, 28 insertions(+), 10 deletions(-)
 create mode 100644 mlir/test/Target/Cpp/no_extra_semicolon.mlir

diff --git a/mlir/lib/Target/Cpp/TranslateToCpp.cpp b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
index d26adec500a113..dba9a625382def 100644
--- a/mlir/lib/Target/Cpp/TranslateToCpp.cpp
+++ b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
@@ -120,6 +120,10 @@ struct CppEmitter {
   LogicalResult emitAttribute(Location loc, Attribute attr);
 
   /// Emits operation 'op' with/without training semicolon or returns failure.
+  ///
+  /// If the operation should not be followed by semicolon, like VerbatimOp,
+  /// the `trailingSemicolon` argument is ignore and semicolon is not
+  /// emitted.
   LogicalResult emitOperation(Operation &op, bool trailingSemicolon);
 
   /// Emits type 'type' or returns failure.
@@ -1036,16 +1040,7 @@ static LogicalResult printFunctionBody(CppEmitter &emitter,
         return failure();
     }
     for (Operation &op : block.getOperations()) {
-      // When generating code for an emitc.if or cf.cond_br op no semicolon
-      // needs to be printed after the closing brace.
-      // When generating code for an emitc.for and emitc.verbatim op, printing a
-      // trailing semicolon is handled within the printOperation function.
-      bool trailingSemicolon =
-          !isa<cf::CondBranchOp, emitc::DeclareFuncOp, emitc::ForOp,
-               emitc::IfOp, emitc::SwitchOp, emitc::VerbatimOp>(op);
-
-      if (failed(emitter.emitOperation(
-              op, /*trailingSemicolon=*/trailingSemicolon)))
+      if (failed(emitter.emitOperation(op, /*trailingSemicolon=*/true)))
         return failure();
     }
   }
@@ -1607,6 +1602,11 @@ LogicalResult CppEmitter::emitOperation(Operation &op, bool trailingSemicolon) {
        shouldBeInlined(cast<emitc::ExpressionOp>(op))))
     return success();
 
+  // Never emit semicolon for operations that end with } or opaque.
+  trailingSemicolon &=
+      !isa<cf::CondBranchOp, emitc::DeclareFuncOp, emitc::ForOp, emitc::IfOp,
+           emitc::SwitchOp, emitc::VerbatimOp, emitc::IncludeOp>(op);
+
   os << (trailingSemicolon ? ";\n" : "\n");
 
   return success();
diff --git a/mlir/test/Target/Cpp/no_extra_semicolon.mlir b/mlir/test/Target/Cpp/no_extra_semicolon.mlir
new file mode 100644
index 00000000000000..292232c71ebd8a
--- /dev/null
+++ b/mlir/test/Target/Cpp/no_extra_semicolon.mlir
@@ -0,0 +1,18 @@
+// RUN: mlir-translate -mlir-to-cpp %s | FileCheck %s
+// RUN: mlir-translate -mlir-to-cpp -declare-variables-at-top %s | FileCheck %s
+
+func.func @no_extra_semicolon(%arg0: i1) {
+  emitc.if %arg0 {
+    emitc.if %arg0 {
+    }
+    emitc.verbatim "return;"
+  }
+  return
+}
+// CHECK: void no_extra_semicolon(bool [[V0:[^ ]*]]) {
+// CHECK-NEXT: if ([[V0]]) {
+// CHECK-NEXT: if ([[V0]]) {
+// CHECK-NEXT: }
+// CHECK-NEXT: return;
+// CHECK-NEXT: }
+// CHECK-NEXT: return;