[ClangIR][CIRGen] Introduce CaseOp and refactor SwitchOp #1006
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Great to see improvements in this area, thanks for working on this. I'm gonna take a closer look on Monday but I like the overall direction from your description! By the way, seems like there's a formatting issue. |
bcardosolopes
changed the title
ChuanqiXu9
force-pushed
the
refacroting_switch_op
branch
from
36e4d28 to
95614bc
Compare
|
The format check failed still. What is the version of clang-format in the CI? I am using the same clang-format just built in the same true of CIR. |
|
Per https://github.com/llvm/clangir/actions/runs/11566254518/workflow?pr=1006#L63, seems like clang-format 18.0.7. |
Thanks. I don't have clang18 so I tried to format it manually this time. |
|
I updated the docs: https://llvm.github.io/clangir/Dialect/ops.html#cirswitch-cirswitchop. We can also hopefully close out all our switch-related issues now after verifying them :) |
Close #522 This solves the issue we can't handle `case` in nested scopes and we can't handle if the switch body is not a compound statement. The core idea of the patch is to introduce the `cir.case` operation to the language. Then we can get the cases by traversing the body of the `cir.switch` operation easily instead of counting the regions and the attributes. Every `cir.case` operation has a region and now the `cir.switch` has only one region too. But to make the analysis and optimizations easier, I add a new concept `simple form` here. That a simple `cir.switch` operation is: all the `cir.case` operation owned by the `cir.switch` lives in the top level blocks of the `cir.switch` region and there is no other operations except the ending `cir.yield`. This solves the previous `simplified for common-case` vs `general solution` discussion in #522. After implemented this, I feel the correct answer to it is, we want a general solution for constructing and lowering the operations but we like simple and common case for analysis and optimizations. We just mixed the different phases. For other semantics, see `CIROps.td`. For lowering, we can make it generally by lower the cases one by one and finally lower the switch itself. Although this patch has 1000+ lines of changes, I feel it is relatively neat especially it erases some odd behaviors before. Tested with Spec2017's C benchmarks except 500.perlbench_r.
Close #522
This solves the issue we can't handle
casein nested scopes and we can't handle if the switch body is not a compound statement.The core idea of the patch is to introduce the
cir.caseoperation to the language. Then we can get the cases by traversing the body of thecir.switchoperation easily instead of counting the regions and the attributes.Every
cir.caseoperation has a region and now thecir.switchhas only one region too. But to make the analysis and optimizations easier, I add a new conceptsimple formhere. That a simplecir.switchoperation is: all thecir.caseoperation owned by thecir.switchlives in the top level blocks of thecir.switchregion and there is no other operations except the endingcir.yield. This solves the previoussimplified for common-casevsgeneral solutiondiscussion in #522. After implemented this, I feel the correct answer to it is, we want a general solution for constructing and lowering the operations but we like simple and common case for analysis and optimizations. We just mixed the different phases.For other semantics, see
CIROps.td.For lowering, we can make it generally by lower the cases one by one and finally lower the switch itself.
Although this patch has 1000+ lines of changes, I feel it is relatively neat especially it erases some odd behaviors before.
Tested with Spec2017's C benchmarks except 500.perlbench_r.