-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[emitc] Fix the translation switchop with argument of expressionop #123701
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[emitc] Fix the translation switchop with argument of expressionop #123701
Conversation
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-emitc Author: Jianjian Guan (jacquesguan) ChangesFull diff: https://github.com/llvm/llvm-project/pull/123701.diff 2 Files Affected:
diff --git a/mlir/lib/Target/Cpp/TranslateToCpp.cpp b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
index a91f5ab9311401..01de0e41f20353 100644
--- a/mlir/lib/Target/Cpp/TranslateToCpp.cpp
+++ b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
@@ -475,7 +475,10 @@ static LogicalResult printOperation(CppEmitter &emitter,
emitc::SwitchOp switchOp) {
raw_indented_ostream &os = emitter.ostream();
- os << "\nswitch (" << emitter.getOrCreateName(switchOp.getArg()) << ") {";
+ os << "\nswitch (";
+ if (failed(emitter.emitOperand(switchOp.getArg())))
+ return failure();
+ os << ") {";
for (auto pair : llvm::zip(switchOp.getCases(), switchOp.getCaseRegions())) {
os << "\ncase " << std::get<0>(pair) << ": {\n";
diff --git a/mlir/test/Target/Cpp/switch.mlir b/mlir/test/Target/Cpp/switch.mlir
index 1a8f5e2dfd2b61..222562b5c57e2b 100644
--- a/mlir/test/Target/Cpp/switch.mlir
+++ b/mlir/test/Target/Cpp/switch.mlir
@@ -882,3 +882,77 @@ func.func @emitc_switch_ui64() {
}
return
}
+
+// CPP-DEFAULT-LABEL: void emitc_switch_expression() {
+// CPP-DEFAULT: int64_t v1 = 42;
+// CPP-DEFAULT: int64_t v2 = 24;
+// CPP-DEFAULT: switch ((v1 + v2) * v2) {
+// CPP-DEFAULT: case 2: {
+// CPP-DEFAULT: int32_t v3 = func_b();
+// CPP-DEFAULT: break;
+// CPP-DEFAULT: }
+// CPP-DEFAULT: case 5: {
+// CPP-DEFAULT: int32_t v4 = func_a();
+// CPP-DEFAULT: break;
+// CPP-DEFAULT: }
+// CPP-DEFAULT: default: {
+// CPP-DEFAULT: float v5 = 4.200000000e+01f;
+// CPP-DEFAULT: func2(v5);
+// CPP-DEFAULT: break;
+// CPP-DEFAULT: }
+// CPP-DEFAULT: }
+// CPP-DEFAULT: return;
+// CPP-DEFAULT: }
+
+// CPP-DECLTOP-LABEL: void emitc_switch_expression() {
+// CPP-DECLTOP: int64_t v1;
+// CPP-DECLTOP: int64_t v2;
+// CPP-DECLTOP: float v3;
+// CPP-DECLTOP: int32_t v4;
+// CPP-DECLTOP: int32_t v5;
+// CPP-DECLTOP: v1 = 42;
+// CPP-DECLTOP: v2 = 24;
+// CPP-DECLTOP: switch ((v1 + v2) * v2) {
+// CPP-DECLTOP: case 2: {
+// CPP-DECLTOP: v4 = func_b();
+// CPP-DECLTOP: break;
+// CPP-DECLTOP: }
+// CPP-DECLTOP: case 5: {
+// CPP-DECLTOP: v5 = func_a();
+// CPP-DECLTOP: break;
+// CPP-DECLTOP: }
+// CPP-DECLTOP: default: {
+// CPP-DECLTOP: v3 = 4.200000000e+01f;
+// CPP-DECLTOP: func2(v3);
+// CPP-DECLTOP: break;
+// CPP-DECLTOP: }
+// CPP-DECLTOP: }
+// CPP-DECLTOP: return;
+// CPP-DECLTOP: }
+
+func.func @emitc_switch_expression() {
+ %x = "emitc.constant"(){value = 42 : i64} : () -> i64
+ %y = "emitc.constant"(){value = 24 : i64} : () -> i64
+
+ %0 = emitc.expression : i64 {
+ %a = emitc.add %x, %y : (i64, i64) -> i64
+ %b = emitc.mul %a, %y : (i64, i64) -> i64
+ emitc.yield %b : i64
+ }
+
+ emitc.switch %0 : i64
+ case 2 {
+ %1 = emitc.call_opaque "func_b" () : () -> i32
+ emitc.yield
+ }
+ case 5 {
+ %2 = emitc.call_opaque "func_a" () : () -> i32
+ emitc.yield
+ }
+ default {
+ %3 = "emitc.constant"(){value = 42.0 : f32} : () -> f32
+ emitc.call_opaque "func2" (%3) : (f32) -> ()
+ emitc.yield
+ }
+ return
+}
|
Thanks for the patch! |
Thanks for comment, I retitle this pr and add the description to mark it as a bugfix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Do you have commit access or should I merge it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix!
Thanks, I have it, and merge this pr right now. |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/94/builds/3992 Here is the relevant piece of the build log for the reference
|
Now a
emitc.switch
with argument ofemitc.expression
wouldn't emit its argument to cpp. This patch fix it.