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

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-emitc

@llvm/pr-subscribers-mlir

Author: Kirill Chibisov (kchibisov)

<details>
<summary>Changes</summary>

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.

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


2 Files Affected:

- (modified) mlir/lib/Target/Cpp/TranslateToCpp.cpp (+10-10) 
- (added) mlir/test/Target/Cpp/no_extra_semicolon.mlir (+18) 


``````````diff
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..e6e0c58daf4e42
--- /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 test_if(bool [[V0:[^ ]*]]) {
+// CHECK-NEXT: if ([[V0]]) {
+// CHECK-NEXT: if ([[V0]]) {
+// CHECK-NEXT: }
+// CHECK-NEXT: return;
+// CHECK-NEXT: }
+// CHECK-NEXT: return;

``````````

</details>


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