[Mlir-commits] [mlir] [mlir][emitc] Don't emit extra semicolon after bracket (PR #122464)
Kirill Chibisov
llvmlistbot at llvm.org
Mon Jan 13 07:08:05 PST 2025
https://github.com/kchibisov updated https://github.com/llvm/llvm-project/pull/122464
>From 0fa37f596510a1749391ed83a9a9581b82b28852 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 1/2] [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/declare_func.mlir | 6 ++++--
mlir/test/Target/Cpp/for.mlir | 6 +++---
mlir/test/Target/Cpp/if.mlir | 4 ++--
mlir/test/Target/Cpp/no_extra_semicolon.mlir | 20 ++++++++++++++++++++
mlir/test/Target/Cpp/switch.mlir | 4 ++--
6 files changed, 41 insertions(+), 19 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/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_ptrdiff_t() {
// CPP-DEFAULT: ptrdiff_t v1 = 1;
>From 6a595a91e7f0192096b049a44592cb0f17dee47f Mon Sep 17 00:00:00 2001
From: Kirill Chibisov <contact at kchibisov.com>
Date: Mon, 13 Jan 2025 18:07:56 +0300
Subject: [PATCH 2/2] Update mlir/lib/Target/Cpp/TranslateToCpp.cpp
Co-authored-by: Gil Rapaport <aniragil at gmail.com>
---
mlir/lib/Target/Cpp/TranslateToCpp.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mlir/lib/Target/Cpp/TranslateToCpp.cpp b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
index dba9a625382def..138f839e908170 100644
--- a/mlir/lib/Target/Cpp/TranslateToCpp.cpp
+++ b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
@@ -121,8 +121,8 @@ struct CppEmitter {
/// 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
+ /// 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);
More information about the Mlir-commits
mailing list