-
Notifications
You must be signed in to change notification settings - Fork 110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Random question about the semantics of 'region' in CIR #978
Comments
The term "region" refers to exactly the same thing as it does in MLIR. Regions model syntactical scopes and structured control flow. I believe you're right that the current design of int test(int x) {
if (x)
goto label;
{
x = 10;
label:
return x;
}
} We get the following ClangIR output: cir.func @_Z4testi(%arg0: !s32i) -> !s32i {
%0 = cir.alloca !s32i, !cir.ptr<!s32i>, ["x", init] {alignment = 4 : i64}
%1 = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"] {alignment = 4 : i64}
cir.store %arg0, %0 : !s32i, !cir.ptr<!s32i>
cir.scope { // REGION #1
%2 = cir.load %0 : !cir.ptr<!s32i>, !s32i
%3 = cir.cast(int_to_bool, %2 : !s32i), !cir.bool
cir.if %3 {
cir.goto "label"
}
}
cir.scope { // REGION #2
%2 = cir.const #cir.int<10> : !s32i
cir.store %2, %0 : !s32i, !cir.ptr<!s32i>
cir.br ^bb1
^bb1: // pred: ^bb0
cir.label "label"
%3 = cir.load %0 : !cir.ptr<!s32i>, !s32i
cir.store %3, %1 : !s32i, !cir.ptr<!s32i>
%4 = cir.load %1 : !cir.ptr<!s32i>, !s32i
cir.return %4 : !s32i
}
cir.unreachable
} The [1] https://mlir.llvm.org/docs/LangRef/#control-flow-and-ssacfg-regions
|
Thanks. And do we have plans or ideas to fix such problems? Or do we have to continue by ignoring the use of |
Nope.
It works now because we don't have much analysis and optimizations on the structured CIR yet, and the problem does not exist anymore after we flatten the CIR. But once we start incorporating more analysis and optimizations on the non-flat CIR the current design may introduce troubles. |
Given the fact we have to support goto, maybe the simplest workaround we can do may be: add a |
Another approach: we run a special "goto-flattening" function pass immediately after CIRGen. For each |
Let me clarify a few observations:
|
Ok, tried to clarify a bit more: 0891e6f |
Also note that
comes with a tradeoff, e.g. you cannot do accurate modeling of all basic blocks interaction. But if you need an analysis that requires that, it will have to be run late in the pipeline (once flat CIR gives full visibility and completely models the correct final control-flow). |
Hey @ChuanqiXu9, I think I can provide a bit more insight here. CIR doesn't fully adhere to MLIR regions' rules in some cases. MLIR's SSACFG regions (which CIR uses) are Single-Entry-Multiple-Exit (SEME). This term can be misleading because it refers to multiple exit statements but not multiple exit destinations; a branching operation can only transfer control flow to the parent operation or a basic block within the same region. However, CIR regions don't strictly follow these rules. Operations like MLIR region's rules make control flow predictable and easy for the compiler to navigate, enabling generic control-flow passes that work on any dialect. So why doesn't CIR follow these rules? There are several reasons. Code generation would become more difficult, diverging from the original and requiring significant bookkeeping. Creating operations with regions that strictly follow these rules would make them too complex and harder to read. Abandoning regions altogether conflicts with CIR's main goal of providing a high-level IR for C/C++ and brushes aside one of MLIR's best abstractions. We don't currently need MLIR's generic passes, so supporting them isn't a priority; when we do, we can transform the IR later in the pipeline to conform to these rules (e.g., the flattening pass). And, most importantly, there is an ongoing work in MLIR to add control-flow support for these kinds of operations. Regarding As for your switch-case implementation, keep in mind that CIR's main goal is to be a high-level IR for C/C++, and |
@bcardosolopes @sitio-couto thanks for the great answers! They do help! I think I can close the issue. |
I know the concept of 'region' comes from MLIR directly. And I am developing #522 and the concept of 'region' in CIR makes me confusing about how to handle cases. Given this is not only limited to switch, I tried to open a new issue for this.
In my mind, a region is a group of blocks, where these blocks has a single entry and a single exit. And I am not sure if it is still true in CIR due to the existence of
goto
. I didn't see our handling ofgoto
changes any region information. So if I didn't misunderstand,goto
breaks the general semantics of 'region'. And I hope to get some input on this topic.I feel the possible answers may be:
a. then for 'goto':
i. I missed some places and actually CIR handles it well.
ii. We hope to fix the problem for
goto
some day.iii.
goto
is the devil and we don't know how to fix it. We will get what we get.goto
?CC: @Lancern @wenpen @gitoleg @bcardosolopes
The text was updated successfully, but these errors were encountered: