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

llvmlistbot at llvm.org llvmlistbot at llvm.org
Tue Jan 14 04:51:45 PST 2025


Author: Kirill Chibisov
Date: 2025-01-14T13:51:41+01:00
New Revision: be96bd74f8eae5637033e4e05fcbf2a693ce8334

URL: https://github.com/llvm/llvm-project/commit/be96bd74f8eae5637033e4e05fcbf2a693ce8334
DIFF: https://github.com/llvm/llvm-project/commit/be96bd74f8eae5637033e4e05fcbf2a693ce8334.diff

LOG: [mlir][emitc] Don't emit extra semicolon after bracket (#122464)

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.

Added: 
    mlir/test/Target/Cpp/no_extra_semicolon.mlir

Modified: 
    mlir/lib/Target/Cpp/TranslateToCpp.cpp
    mlir/test/Target/Cpp/declare_func.mlir
    mlir/test/Target/Cpp/for.mlir
    mlir/test/Target/Cpp/if.mlir
    mlir/test/Target/Cpp/switch.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Target/Cpp/TranslateToCpp.cpp b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
index d26adec500a113..a91f5ab9311401 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.
+  ///
+  /// For operations that should never be followed by a semicolon, like ForOp,
+  /// the `trailingSemicolon` argument is ignored and a 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,12 @@ LogicalResult CppEmitter::emitOperation(Operation &op, bool trailingSemicolon) {
        shouldBeInlined(cast<emitc::ExpressionOp>(op))))
     return success();
 
+  // Never emit a semicolon for some operations, especially if endening with
+  // `}`.
+  trailingSemicolon &=
+      !isa<cf::CondBranchOp, emitc::DeclareFuncOp, emitc::ForOp, emitc::IfOp,
+           emitc::IncludeOp, emitc::SwitchOp, emitc::VerbatimOp>(op);
+
   os << (trailingSemicolon ? ";\n" : "\n");
 
   return success();

diff  --git a/mlir/test/Target/Cpp/declare_func.mlir b/mlir/test/Target/Cpp/declare_func.mlir
index 00680d71824ae0..6901e135df389f 100644
--- a/mlir/test/Target/Cpp/declare_func.mlir
+++ b/mlir/test/Target/Cpp/declare_func.mlir
@@ -1,8 +1,10 @@
-// RUN: mlir-translate -mlir-to-cpp %s | FileCheck %s
+// RUN: mlir-translate -mlir-to-cpp %s | FileCheck --match-full-lines %s
 
 // CHECK: int32_t bar(int32_t [[V1:[^ ]*]]);
 emitc.declare_func @bar
-// CHECK: int32_t bar(int32_t [[V1:[^ ]*]]) {
+// CHECK:       int32_t bar(int32_t [[V1:[^ ]*]]) {
+// CHECK-NEXT:      return [[V1]];
+// CHECK-NEXT:  }
 emitc.func @bar(%arg0: i32) -> i32 {
     emitc.return %arg0 : i32
 }

diff  --git a/mlir/test/Target/Cpp/for.mlir b/mlir/test/Target/Cpp/for.mlir
index 6a446eaf4add3f..7cd3d5d646da61 100644
--- a/mlir/test/Target/Cpp/for.mlir
+++ b/mlir/test/Target/Cpp/for.mlir
@@ -1,5 +1,5 @@
-// RUN: mlir-translate -mlir-to-cpp %s | FileCheck %s -check-prefix=CPP-DEFAULT
-// RUN: mlir-translate -mlir-to-cpp -declare-variables-at-top %s | FileCheck %s -check-prefix=CPP-DECLTOP
+// RUN: mlir-translate -mlir-to-cpp %s | FileCheck --match-full-lines %s -check-prefix=CPP-DEFAULT
+// RUN: mlir-translate -mlir-to-cpp -declare-variables-at-top %s | FileCheck --match-full-lines %s -check-prefix=CPP-DECLTOP
 
 func.func @test_for(%arg0 : index, %arg1 : index, %arg2 : index) {
   %lb = emitc.expression : index {
@@ -160,5 +160,5 @@ func.func @test_for_yield_2() {
   return
 }
 // CPP-DEFAULT: void test_for_yield_2() {
-// CPP-DEFAULT: {{.*}}= M_PI
+// CPP-DEFAULT: {{.*}}= M_PI;
 // CPP-DEFAULT: for (size_t [[IN:.*]] = 0; [[IN]] < 10; [[IN]] += 1) {

diff  --git a/mlir/test/Target/Cpp/if.mlir b/mlir/test/Target/Cpp/if.mlir
index d3b792192c8b10..9a7e12b891bc92 100644
--- a/mlir/test/Target/Cpp/if.mlir
+++ b/mlir/test/Target/Cpp/if.mlir
@@ -1,5 +1,5 @@
-// RUN: mlir-translate -mlir-to-cpp %s | FileCheck %s -check-prefix=CPP-DEFAULT
-// RUN: mlir-translate -mlir-to-cpp -declare-variables-at-top %s | FileCheck %s -check-prefix=CPP-DECLTOP
+// RUN: mlir-translate -mlir-to-cpp %s | FileCheck --match-full-lines %s -check-prefix=CPP-DEFAULT
+// RUN: mlir-translate -mlir-to-cpp -declare-variables-at-top %s | FileCheck --match-full-lines %s -check-prefix=CPP-DECLTOP
 
 func.func @test_if(%arg0: i1, %arg1: f32) {
   emitc.if %arg0 {

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..4b1b55944434e2
--- /dev/null
+++ b/mlir/test/Target/Cpp/no_extra_semicolon.mlir
@@ -0,0 +1,20 @@
+// RUN: mlir-translate -mlir-to-cpp %s | FileCheck --match-full-lines %s
+// RUN: mlir-translate -mlir-to-cpp -declare-variables-at-top %s | FileCheck --match-full-lines %s
+
+func.func @no_extra_semicolon(%arg0: i1) {
+  emitc.if %arg0 {
+    emitc.include "myheader.h"
+    emitc.if %arg0 {
+    }
+    emitc.verbatim "return;"
+  }
+  return
+}
+// CHECK: void no_extra_semicolon(bool [[V0:[^ ]*]]) {
+// CHECK-NEXT: if ([[V0]]) {
+// CHECK-NEXT: #include "myheader.h"
+// CHECK-NEXT: if ([[V0]]) {
+// CHECK-NEXT: }
+// CHECK-NEXT: return;
+// CHECK-NEXT: }
+// CHECK-NEXT: return;

diff  --git a/mlir/test/Target/Cpp/switch.mlir b/mlir/test/Target/Cpp/switch.mlir
index 3339c026179492..1a8f5e2dfd2b61 100644
--- a/mlir/test/Target/Cpp/switch.mlir
+++ b/mlir/test/Target/Cpp/switch.mlir
@@ -1,5 +1,5 @@
-// RUN: mlir-translate -mlir-to-cpp %s | FileCheck %s -check-prefix=CPP-DEFAULT
-// RUN: mlir-translate -mlir-to-cpp -declare-variables-at-top %s | FileCheck %s -check-prefix=CPP-DECLTOP
+// RUN: mlir-translate -mlir-to-cpp %s | FileCheck --match-full-lines %s -check-prefix=CPP-DEFAULT
+// RUN: mlir-translate -mlir-to-cpp -declare-variables-at-top %s | FileCheck --match-full-lines %s -check-prefix=CPP-DECLTOP
 
 // CPP-DEFAULT-LABEL: void emitc_switch_ptr
diff _t() {
 // CPP-DEFAULT:         ptr
diff _t v1 = 1;


        


More information about the Mlir-commits mailing list