[CIR] Add the ability to detect if SwitchOp covers all the cases #171246
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
badumbatish
requested review from
andykaylor,
bcardosolopes,
lanza and
xlauko
as code owners
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clangir Author: Jasmine Tang (badumbatish) ChangesFull diff: https://github.com/llvm/llvm-project/pull/171246.diff 3 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 9bd24cf0bcf27..d6099388886d7 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -1047,6 +1047,11 @@ def CIR_SwitchOp : CIR_Op<"switch", [
conditionally executing multiple regions of code. The operand to an switch
is an integral condition value.
+ Besides accepting an int type of condition and regions of cir code, it also accepts
+ a boolean allEnumCasesCovered indicating if all cases of an enum is covered. Note that
+ having a default CaseOp inside the switch doesn't imply allEnumCasesCovered, the OG AST switch
+ needs to have each case spelled out.
+
The set of `cir.case` operations and their enclosing `cir.switch`
represent the semantics of a C/C++ switch statement. Users can use
`collectCases(llvm::SmallVector<CaseOp> &cases)` to collect the `cir.case`
@@ -1173,7 +1178,8 @@ def CIR_SwitchOp : CIR_Op<"switch", [
```
}];
- let arguments = (ins CIR_IntType:$condition);
+ let arguments = (ins CIR_IntType:$condition,
+ DefaultValuedAttr<BoolAttr, "false">:$allEnumCasesCovered);
let regions = (region AnyRegion:$body);
diff --git a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
index da7ab0691cb63..0ff19abaffc08 100644
--- a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
@@ -1105,6 +1105,8 @@ mlir::LogicalResult CIRGenFunction::emitSwitchStmt(const clang::SwitchStmt &s) {
terminateBody(builder, caseOp.getCaseRegion(), caseOp.getLoc());
terminateBody(builder, swop.getBody(), swop.getLoc());
+ swop.setAllEnumCasesCovered(s.isAllEnumCasesCovered());
+
return res;
}
diff --git a/clang/test/CIR/IR/switch.cir b/clang/test/CIR/IR/switch.cir
index 87d45bf1f5219..78c0273b8c97e 100644
--- a/clang/test/CIR/IR/switch.cir
+++ b/clang/test/CIR/IR/switch.cir
@@ -36,3 +36,33 @@ cir.func @s0() {
// CHECK-NEXT: }
// CHECK-NEXT: cir.yield
// CHECK-NEXT: }
+
+
+// Pretends that this is lowered from a C file and was tagged with allEnumCasesCovered = true
+cir.func @s1(%1 : !s32i) {
+ cir.switch (%1 : !s32i) {
+ cir.case (default, []) {
+ cir.return
+ }
+ cir.case (equal, [#cir.int<1> : !s32i]) {
+ cir.yield
+ }
+ cir.case (equal, [#cir.int<2> : !s32i]) {
+ cir.yield
+ }
+ cir.yield
+ } { allEnumCasesCovered = true}
+ cir.return
+}
+// CHECK: cir.switch (%arg0 : !s32i) {
+// CHECK-NEXT: cir.case(default, []) {
+// CHECK-NEXT: cir.return
+// CHECK-NEXT: }
+// CHECK-NEXT: cir.case(equal, [#cir.int<1> : !s32i]) {
+// CHECK-NEXT: cir.yield
+// CHECK-NEXT: }
+// CHECK-NEXT: cir.case(equal, [#cir.int<2> : !s32i]) {
+// CHECK-NEXT: cir.yield
+// CHECK-NEXT: }
+// CHECK-NEXT: cir.yield
+// CHECK-NEXT: } {allEnumCasesCovered = true}
|
Member
|
@llvm/pr-subscribers-clang Author: Jasmine Tang (badumbatish) ChangesFull diff: https://github.com/llvm/llvm-project/pull/171246.diff 3 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 9bd24cf0bcf27..d6099388886d7 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -1047,6 +1047,11 @@ def CIR_SwitchOp : CIR_Op<"switch", [
conditionally executing multiple regions of code. The operand to an switch
is an integral condition value.
+ Besides accepting an int type of condition and regions of cir code, it also accepts
+ a boolean allEnumCasesCovered indicating if all cases of an enum is covered. Note that
+ having a default CaseOp inside the switch doesn't imply allEnumCasesCovered, the OG AST switch
+ needs to have each case spelled out.
+
The set of `cir.case` operations and their enclosing `cir.switch`
represent the semantics of a C/C++ switch statement. Users can use
`collectCases(llvm::SmallVector<CaseOp> &cases)` to collect the `cir.case`
@@ -1173,7 +1178,8 @@ def CIR_SwitchOp : CIR_Op<"switch", [
```
}];
- let arguments = (ins CIR_IntType:$condition);
+ let arguments = (ins CIR_IntType:$condition,
+ DefaultValuedAttr<BoolAttr, "false">:$allEnumCasesCovered);
let regions = (region AnyRegion:$body);
diff --git a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
index da7ab0691cb63..0ff19abaffc08 100644
--- a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
@@ -1105,6 +1105,8 @@ mlir::LogicalResult CIRGenFunction::emitSwitchStmt(const clang::SwitchStmt &s) {
terminateBody(builder, caseOp.getCaseRegion(), caseOp.getLoc());
terminateBody(builder, swop.getBody(), swop.getLoc());
+ swop.setAllEnumCasesCovered(s.isAllEnumCasesCovered());
+
return res;
}
diff --git a/clang/test/CIR/IR/switch.cir b/clang/test/CIR/IR/switch.cir
index 87d45bf1f5219..78c0273b8c97e 100644
--- a/clang/test/CIR/IR/switch.cir
+++ b/clang/test/CIR/IR/switch.cir
@@ -36,3 +36,33 @@ cir.func @s0() {
// CHECK-NEXT: }
// CHECK-NEXT: cir.yield
// CHECK-NEXT: }
+
+
+// Pretends that this is lowered from a C file and was tagged with allEnumCasesCovered = true
+cir.func @s1(%1 : !s32i) {
+ cir.switch (%1 : !s32i) {
+ cir.case (default, []) {
+ cir.return
+ }
+ cir.case (equal, [#cir.int<1> : !s32i]) {
+ cir.yield
+ }
+ cir.case (equal, [#cir.int<2> : !s32i]) {
+ cir.yield
+ }
+ cir.yield
+ } { allEnumCasesCovered = true}
+ cir.return
+}
+// CHECK: cir.switch (%arg0 : !s32i) {
+// CHECK-NEXT: cir.case(default, []) {
+// CHECK-NEXT: cir.return
+// CHECK-NEXT: }
+// CHECK-NEXT: cir.case(equal, [#cir.int<1> : !s32i]) {
+// CHECK-NEXT: cir.yield
+// CHECK-NEXT: }
+// CHECK-NEXT: cir.case(equal, [#cir.int<2> : !s32i]) {
+// CHECK-NEXT: cir.yield
+// CHECK-NEXT: }
+// CHECK-NEXT: cir.yield
+// CHECK-NEXT: } {allEnumCasesCovered = true}
|
andykaylor
reviewed
Dec 9, 2025
Contributor
andykaylor
left a comment
andykaylor
left a comment
There was a problem hiding this comment.
This looks good. It could also be useful to know that the switch condition variable is an enum, for cases where it's not fully covered, but that could be a separate PR.
xlauko
reviewed
Dec 9, 2025
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/52/builds/13497 Here is the relevant piece of the build log for the reference |
Contributor
|
@badumbatish please apply requested changes completely next time, the desctription did not match the actual format. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.