Skip to content

Commit

Permalink
[CIR][CodeGen] Handling multiple stmt followed after a switch case (l…
Browse files Browse the repository at this point in the history
…lvm#878)

Close llvm#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<SwitchCase>(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<CompoundStmt>(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?
  • Loading branch information
ChuanqiXu9 authored and smeenai committed Oct 9, 2024
1 parent 0a1dff1 commit 9dcdf81
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
4 changes: 2 additions & 2 deletions clang/lib/CIR/CodeGen/CIRGenStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -996,18 +996,18 @@ mlir::LogicalResult CIRGenFunction::buildSwitchBody(
for (auto *c : compoundStmt->body()) {
if (auto *switchCase = dyn_cast<SwitchCase>(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<CompoundStmt>(c));
lastCaseBlock = builder.getBlock();
} else {
llvm_unreachable("statement doesn't belong to any case region, NYI");
}

lastCaseBlock = builder.getBlock();

if (res.failed())
break;
}
Expand Down
22 changes: 22 additions & 0 deletions clang/test/CIR/CodeGen/goto.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,3 +288,25 @@ void foo() {
// NOFLAT: cir.scope {
// NOFLAT: cir.label "label"
// NOFLAT: %0 = cir.alloca !ty_S, !cir.ptr<!ty_S>, ["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

0 comments on commit 9dcdf81

Please sign in to comment.