clangir
clangir copied to clipboard
[CIR][CIRGen] Improve switch support for unrecheable code
Support non-block case
and statementw that don't belong to any case
region, fix #520 #521
Actually the body of a switch statement can be neither of case
, default
, or compound:
int f(int x) {
switch (x)
return 1;
return 2;
}
This is accepted by clang (f
returns 2
unconditionally) and since you're working on this I believe we can further resolve this in this PR.
@Lancern Added CaseOpKind_NE
and buildCaseNoneStmt()
to handle the unreachable statements.
Thanks for pointing out it!
Since you are already working on this and they are all related, this PR should also fix the other issues and include testscases from #521 and #522
Well, things are a little tricky in the case when the body of a switch
statement is not one of case
, default
, or compound. These statements are NOT simply unreachable, consider:
void foo(int x) {
switch (x)
while (condition()) {
case 42:
do_something();
}
}
The while
statement is indeed reachable when x
is 42. In such a case switch
behaves just like a goto
.
It becomes a little complex when we consider case
across scope, the current definition of SwitchOp
is not enough to express the semantics.
The definition assume the size of case attributes is same with regions, unfortunately the region may be nested.
For example, there should be 2 case attribute with only 1 SwitchOp
region (or said 2 nested region) in following code.
switch (x)
case 9: {
x++;
case 7:
x++;
}
I'm not sure what a reasonable SwitchOp
definition should be. A preliminary idea is using label and branch somehow, then the cir is like
cir.scope {
cir.switch (...) [
9: ^bb0
7: ^bb1
] {
^bb0:
cir.scope {
...
^bb1:
...
}
}
}
Otherwise I noticed goto
didn't support branch across scope as well, maybe the problem is related?
Do you have any suggestions?
Since switch
statements are more or less like a syntax sugar for goto
, we may learn some ideas from the way goto
is implemented in CIR: #508 .
It becomes a little complex when we consider
case
across scope
Oh right, you don't need to solve this problem for the moment, let's focus on the testcases that don't involve scope crossing. @gitoleg is working on https://github.com/llvm/clangir/pull/508, once that lands we can do incremental work to fix it.
Let me know when this comes out of Draft state and I'll take a look again
Appreciate for the suggestions! This pr is ready for review now. @Lancern @bcardosolopes
Comments addressed. Found some new corner-case, so rewrite some function and add a few UT.
Let me know once this is ready for review again!
Hi, this pr is ready for review, thanks~
@wenpen still working on this? I don't usually look at draft PRs, just trying to make sure if there's something I should be looking here.
@bcardosolopes Yes, just be a little busy recently, I will update the pr and request review form you later days, thanks~
I'm going to resume reviewing this, sorry for the delay!
I landed https://github.com/llvm/clangir/pull/611 which has some comments related to this PR (cc: @piggynl)