[CIR] Add the ability to detect enum argument for switch op

[badumbatish]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clangir

Author: Jasmine Tang (badumbatish)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/172236.diff

3 Files Affected:

• (modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+3-1)
• (modified) clang/lib/CIR/CodeGen/CIRGenStmt.cpp (+1)
• (modified) clang/test/CIR/CodeGen/switch.cpp (+2-2)

diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 868b813458aae..3b6f35ae96497 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -1216,7 +1216,8 @@ def CIR_SwitchOp : CIR_Op<"switch", [
 
   let arguments = (ins 
     CIR_IntType:$condition,
-    UnitAttr:$allEnumCasesCovered
+    UnitAttr:$allEnumCasesCovered,
+    UnitAttr:$handling_enum
   );
 
   let regions = (region AnyRegion:$body);
@@ -1230,6 +1231,7 @@ def CIR_SwitchOp : CIR_Op<"switch", [
   let assemblyFormat = [{
     `(` $condition `:` qualified(type($condition)) `)` 
      (`allEnumCasesCovered` $allEnumCasesCovered^)?
+     (`handling_enum` $handling_enum^)?
     $body
     attr-dict
   }];
diff --git a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
index c7a95b34a0d6b..4a84376e8afd2 100644
--- a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
@@ -1106,6 +1106,7 @@ mlir::LogicalResult CIRGenFunction::emitSwitchStmt(const clang::SwitchStmt &s) {
     terminateBody(builder, caseOp.getCaseRegion(), caseOp.getLoc());
   terminateBody(builder, swop.getBody(), swop.getLoc());
 
+  swop.setHandlingEnum(s.getCond()->IgnoreParenImpCasts()->getType()->isEnumeralType());
   swop.setAllEnumCasesCovered(s.isAllEnumCasesCovered());
 
   return res;
diff --git a/clang/test/CIR/CodeGen/switch.cpp b/clang/test/CIR/CodeGen/switch.cpp
index b7bd2da5e39b8..8ca58e8ca1a5a 100644
--- a/clang/test/CIR/CodeGen/switch.cpp
+++ b/clang/test/CIR/CodeGen/switch.cpp
@@ -1282,7 +1282,7 @@ void testSwitchCoverAllCase(M m) {
     break;
   }
 }
-// CIR: cir.switch(%[[ARG:.*]] : !s32i) allEnumCasesCovered {
+// CIR: cir.switch(%[[ARG:.*]] : !s32i) allEnumCasesCovered handling_enum {
 
 void testSwitchNotCoverAllCase(M m) {
   switch (m) {
@@ -1291,4 +1291,4 @@ void testSwitchNotCoverAllCase(M m) {
     break;
   }
 }
-// CIR: cir.switch(%[[ARG:.*]] : !s32i) {
+// CIR: cir.switch(%[[ARG:.*]] : !s32i) handling_enum {

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff origin/main HEAD --extensions cpp -- clang/lib/CIR/CodeGen/CIRGenStmt.cpp clang/test/CIR/CodeGen/switch.cpp --diff_from_common_commit

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.

diff --git a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
index 4a84376e8..12e46624b 100644
--- a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
@@ -1106,7 +1106,8 @@ mlir::LogicalResult CIRGenFunction::emitSwitchStmt(const clang::SwitchStmt &s) {
     terminateBody(builder, caseOp.getCaseRegion(), caseOp.getLoc());
   terminateBody(builder, swop.getBody(), swop.getLoc());
 
-  swop.setHandlingEnum(s.getCond()->IgnoreParenImpCasts()->getType()->isEnumeralType());
+  swop.setHandlingEnum(
+      s.getCond()->IgnoreParenImpCasts()->getType()->isEnumeralType());
   swop.setAllEnumCasesCovered(s.isAllEnumCasesCovered());
 
   return res;

[HendrikHuebner]

Why do you want to add this? Perhaps add a PR description explaining why you are proposing this change. I don't think we are currently preserving any information about which values in Clang IR were originally enumeration types, since the underlying type is used (e.g. u32i). If we want to preserve this information, it seems like we need a more general solution.

[andykaylor]

This looks good. I just have a suggestion for making it less verbose.

[andykaylor]

Suggested change

	UnitAttr:$handling_enum
	UnitAttr:$is_enum

[andykaylor]

Suggested change

	(`handling_enum` $handling_enum^)?
	(`enum` $is_enum^)?

[andykaylor]

Why do you want to add this? Perhaps add a PR description explaining why you are proposing this change. I don't think we are currently preserving any information about which values in Clang IR were originally enumeration types, since the underlying type is used (e.g. u32i). If we want to preserve this information, it seems like we need a more general solution.

A description is definitely needed. I think this is useful for static analysis. As the test shows, we are able to see during code generation that the switch is being done on an enumerated type. This change was a follow-up I suggested to an earlier change that annotated cases where the switch was fully covering an enumerated type.

It may be useful in the future to preserve more information about when enum types are being used. There are some limited cases where that would allow us to make assumptions about the range of possible values, and I can imagine added an option that allows us to go beyond language standard rules to assume strict enum type semantics everywhere.