From 9dcdf81bfb45605021a57a7136e773734f3cf231 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Wed, 25 Sep 2024 01:47:05 +0800 Subject: [PATCH] [CIR][CodeGen] Handling multiple stmt followed after a switch case (#878) Close https://github.com/llvm/clangir/issues/876 We've already considered the case that there are random stmt after a switch case: ``` for (auto *c : compoundStmt->body()) { if (auto *switchCase = dyn_cast(c)) { res = buildSwitchCase(*switchCase, condType, caseAttrs); } else if (lastCaseBlock) { // This means it's a random stmt following up a case, just // emit it as part of previous known case. mlir::OpBuilder::InsertionGuard guardCase(builder); builder.setInsertionPointToEnd(lastCaseBlock); res = buildStmt(c, /*useCurrentScope=*/!isa(c)); } else { llvm_unreachable("statement doesn't belong to any case region, NYI"); } lastCaseBlock = builder.getBlock(); if (res.failed()) break; } ``` However, maybe this is an oversight, in the branch of ` if (lastCaseBlock)`, the insertion point will be updated automatically when the RAII object `guardCase` destroys, then we can assign the correct value for `lastCaseBlock` later. So we will see the weird code pattern in the issue side. BTW, I found the codes in CIRGenStmt.cpp are far more less similar with the ones other code gen places. Is this intentional? And what is the motivation and guide lines here? --- clang/lib/CIR/CodeGen/CIRGenStmt.cpp | 4 ++-- clang/test/CIR/CodeGen/goto.cpp | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp index 543b744cc5d0..426da35b5238 100644 --- a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp @@ -996,18 +996,18 @@ mlir::LogicalResult CIRGenFunction::buildSwitchBody( for (auto *c : compoundStmt->body()) { if (auto *switchCase = dyn_cast(c)) { res = buildSwitchCase(*switchCase, condType, caseAttrs); + lastCaseBlock = builder.getBlock(); } else if (lastCaseBlock) { // This means it's a random stmt following up a case, just // emit it as part of previous known case. mlir::OpBuilder::InsertionGuard guardCase(builder); builder.setInsertionPointToEnd(lastCaseBlock); res = buildStmt(c, /*useCurrentScope=*/!isa(c)); + lastCaseBlock = builder.getBlock(); } else { llvm_unreachable("statement doesn't belong to any case region, NYI"); } - lastCaseBlock = builder.getBlock(); - if (res.failed()) break; } diff --git a/clang/test/CIR/CodeGen/goto.cpp b/clang/test/CIR/CodeGen/goto.cpp index 5a8d598d95cd..81eb4ec43e65 100644 --- a/clang/test/CIR/CodeGen/goto.cpp +++ b/clang/test/CIR/CodeGen/goto.cpp @@ -288,3 +288,25 @@ void foo() { // NOFLAT: cir.scope { // NOFLAT: cir.label "label" // NOFLAT: %0 = cir.alloca !ty_S, !cir.ptr, ["agg.tmp0"] + +extern "C" void action1(); +extern "C" void action2(); +extern "C" void multiple_non_case(int v) { + switch (v) { + default: + action1(); + l2: + action2(); + break; + } +} + +// NOFLAT: cir.func @multiple_non_case +// NOFLAT: cir.switch +// NOFLAT: case (default) +// NOFLAT: cir.call @action1() +// NOFLAT: cir.br ^[[BB1:[a-zA-Z0-9]+]] +// NOFLAT: ^[[BB1]]: +// NOFLAT: cir.label +// NOFLAT: cir.call @action2() +// NOFLAT: cir.break