[CIR] Introduce a flattening pass #516

gitoleg · 2024-03-20T14:30:01Z

We start our journey towards goto support and this is a first step on this way.

There are some discussion in #508 and according to the plan we do the following here:

a new pass called StructuredCFG that is a stub now but later will be responsible for the regions inlining. The pass works only if emit-cir is not passed in cmd line. Thus, the clang behavior is not changed here from the user's point of view.
The pass will be accomplished with goto solver later, so we talk about several passes that are mandatory for the lowering into llvm dialect. There are at least two clients of this pass that will be affected: clang itself and cir-opt, so we need a common point for them: and populateCIRFlatteningPasses and populateCIRToLLVMPasses guarantee that CIR will be in the correct state for all the clients, whatever new passes we will add later.
new option -emit-flat-cir to dump CIR after all the flattening and structuring and goto-solving.

github-actions · 2024-03-20T14:32:43Z

✅ With the latest revision this PR passed the C/C++ code formatter.

bcardosolopes

Awesome, mostly nitpicks comments

clang/include/clang/CIR/Dialect/Passes.td

clang/include/clang/Driver/Options.td

clang/include/clang/CIRFrontendAction/CIRGenAction.h

clang/include/clang/Driver/Options.td

clang/lib/CIR/Dialect/Transforms/CIRFlattening.cpp

bcardosolopes · 2024-03-22T05:26:34Z

clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp

@@ -916,7 +917,9 @@ struct ConvertCIRToLLVMPass
  }
  void runOnOperation() final;

-  virtual StringRef getArgument() const override { return "cir-to-llvm"; }
+  virtual StringRef getArgument() const override {
+    return "cir-to-llvm-internal";


what does internal means? I don't understand why this needs to be renamed.

internal here is a hint that this pass can't be run with any CIR input like it was before. Now the pre requirement is our new cir-flatten-cfg pass. I would deprecate createConvertCIRToLLVMPass of even remove it from the interface, but it;s up to you to decide.
And the pass need to be renamed since we register a pipeline in cir-opt with the same name - in order to guarantee any cir input is stiil valid as it was earlier - i.e. that first it will be processed by cir-flatten-cfg pass and then will go to the lowering.

I would deprecate createConvertCIRToLLVMPass of even remove it from the interface, but it;s up to you to decide.

I'm down for simplifying and unifying as much as possible, but can you address it in a follow up PR.

internal here is a hint that this pass can't be run with any CIR input like it was before. Now the pre requirement is our new cir-flatten-cfg pass

Perhaps it should be named cir-flat-to-llvm, or something like that.

Since it would be wrong to run cir-flat-to-llvm without running cir-flatten-cfg, we should make cir-flat-to-llvm pass to require cir-flatten-cfg pass to be run before.

Seems like all the changes for this comment should be addressed together in another PR.

clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp

clang/lib/Driver/Driver.cpp

clang/lib/CIR/Dialect/Transforms/StructuredCFG.cpp

gitoleg · 2024-03-22T13:58:10Z

clang/tools/cir-opt/cir-opt.cpp

-  ::mlir::registerPass([]() -> std::unique_ptr<::mlir::Pass> {
-    return cir::direct::createConvertCIRToLLVMPass();
-  });
+  mlir::PassPipelineRegistration<mlir::EmptyPipelineOptions> pipeline(


here we register the pipeline with the name cir-to-llvm. So we need to rename the pass in LowerToLLVM.cpp - otherwise we wouldn't be able to run either of them due to name conflict

gitoleg · 2024-03-22T14:42:30Z

@bcardosolopes done!

bcardosolopes

LGTM.

Awesome, I have a few requests for change, but they should come in another PR (see my inline comments). I'm gonna go ahead and merge this.

bcardosolopes · 2024-04-03T19:19:30Z

clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp

@@ -916,7 +917,9 @@ struct ConvertCIRToLLVMPass
  }
  void runOnOperation() final;

-  virtual StringRef getArgument() const override { return "cir-to-llvm"; }
+  virtual StringRef getArgument() const override {
+    return "cir-to-llvm-internal";


I would deprecate createConvertCIRToLLVMPass of even remove it from the interface, but it;s up to you to decide.

I'm down for simplifying and unifying as much as possible, but can you address it in a follow up PR.

internal here is a hint that this pass can't be run with any CIR input like it was before. Now the pre requirement is our new cir-flatten-cfg pass

Perhaps it should be named cir-flat-to-llvm, or something like that.

Since it would be wrong to run cir-flat-to-llvm without running cir-flatten-cfg, we should make cir-flat-to-llvm pass to require cir-flatten-cfg pass to be run before.

Seems like all the changes for this comment should be addressed together in another PR.

@foo

This PR perform flattening for `cir::IfOp` Basically, we just move the code from `LowerToLLVM.cpp` to `FlattenCFG.cpp`. There are several important things though I would like to highlight. 1) Consider the next code from the tests: ``` cir.func @foo(%arg0: !s32i) -> !s32i { %4 = cir.cast(int_to_bool, %arg0 : !s32i), !cir.bool cir.if %4 { %5 = cir.const(#cir.int<1> : !s32i) : !s32i cir.return %5 : !s32i } else { %5 = cir.const(#cir.int<0> : !s32i) : !s32i cir.return %5 : !s32i } cir.return %arg0 : !s32i } ``` The last `cir.return` becomes unreachable after flattening and hence is not reachable in the lowering. So we got the next error: ``` error: 'cir.return' op expects parent op to be one of 'cir.func, cir.scope, cir.if, cir.switch, cir.do, cir.while, cir.for' cir.return %arg0 : !s32i ``` the parent after lowering is `llvm.func`. And this is only the beginning - the more operations will be flatten, the more similar fails will happen. Thus, I added lowering for the unreachable code as well in `LowerToLLVM.cpp`. But may be you have another solution in your mind. 2) Please, pay attention on the flattening pass - I'm not that familiar with `mlir` builders as you are, so may be I'm doing something wrong. The idea was to start flattening from the most nested operations. 3) As you requested in #516, `cir-to-llvm-internal` is renamed to `cir-flat-to-llvm`. The only thing remain undone is related to the following: > Since it would be wrong to run cir-flat-to-llvm without running cir-flatten-cfg, we should make cir-flat-to-llvm pass to require cir-flatten-cfg pass to be run before. And I'm not sure I know how to do it exactly - is there something similar to pass dependencies from LLVM IR? 4) The part of `IfOp` lowering related to elimination of the vain casts for condition branch moved directly to the lowering of `BrCondOp` with some refactoring and guarding. 5) Just note, that now `cir-opt` is able to dump the flat cir as well: `cir-opt -cir-flat-cfg`

We start our journey towards `goto` support and this is a first step on this way. There are some discussion in #508 and according to the plan we do the following here: - a new pass called `cir-flatten-cfg` that is a stub now but later will be responsible for the regions inlining. The pass works only if `-emit-flat-cir` is passed in cmd line. Thus, the clang behavior is not changed here from the user's point of view. - The pass will be accomplished with `goto` solver later, so we talk about several passes that are mandatory for the lowering into `llvm` dialect. There are at least two clients of this pass that will be affected: `clang` itself and `cir-opt`, so we need a common point for them: and `populateCIRFlatteningPasses` and `populateCIRToLLVMPasses` guarantee that `CIR` will be in the correct state for all the clients, whatever new passes we will add later.

@foo

This PR perform flattening for `cir::IfOp` Basically, we just move the code from `LowerToLLVM.cpp` to `FlattenCFG.cpp`. There are several important things though I would like to highlight. 1) Consider the next code from the tests: ``` cir.func @foo(%arg0: !s32i) -> !s32i { %4 = cir.cast(int_to_bool, %arg0 : !s32i), !cir.bool cir.if %4 { %5 = cir.const(#cir.int<1> : !s32i) : !s32i cir.return %5 : !s32i } else { %5 = cir.const(#cir.int<0> : !s32i) : !s32i cir.return %5 : !s32i } cir.return %arg0 : !s32i } ``` The last `cir.return` becomes unreachable after flattening and hence is not reachable in the lowering. So we got the next error: ``` error: 'cir.return' op expects parent op to be one of 'cir.func, cir.scope, cir.if, cir.switch, cir.do, cir.while, cir.for' cir.return %arg0 : !s32i ``` the parent after lowering is `llvm.func`. And this is only the beginning - the more operations will be flatten, the more similar fails will happen. Thus, I added lowering for the unreachable code as well in `LowerToLLVM.cpp`. But may be you have another solution in your mind. 2) Please, pay attention on the flattening pass - I'm not that familiar with `mlir` builders as you are, so may be I'm doing something wrong. The idea was to start flattening from the most nested operations. 3) As you requested in #516, `cir-to-llvm-internal` is renamed to `cir-flat-to-llvm`. The only thing remain undone is related to the following: > Since it would be wrong to run cir-flat-to-llvm without running cir-flatten-cfg, we should make cir-flat-to-llvm pass to require cir-flatten-cfg pass to be run before. And I'm not sure I know how to do it exactly - is there something similar to pass dependencies from LLVM IR? 4) The part of `IfOp` lowering related to elimination of the vain casts for condition branch moved directly to the lowering of `BrCondOp` with some refactoring and guarding. 5) Just note, that now `cir-opt` is able to dump the flat cir as well: `cir-opt -cir-flat-cfg`

We start our journey towards `goto` support and this is a first step on this way. There are some discussion in #508 and according to the plan we do the following here: - a new pass called `cir-flatten-cfg` that is a stub now but later will be responsible for the regions inlining. The pass works only if `-emit-flat-cir` is passed in cmd line. Thus, the clang behavior is not changed here from the user's point of view. - The pass will be accomplished with `goto` solver later, so we talk about several passes that are mandatory for the lowering into `llvm` dialect. There are at least two clients of this pass that will be affected: `clang` itself and `cir-opt`, so we need a common point for them: and `populateCIRFlatteningPasses` and `populateCIRToLLVMPasses` guarantee that `CIR` will be in the correct state for all the clients, whatever new passes we will add later.

@foo

This PR perform flattening for `cir::IfOp` Basically, we just move the code from `LowerToLLVM.cpp` to `FlattenCFG.cpp`. There are several important things though I would like to highlight. 1) Consider the next code from the tests: ``` cir.func @foo(%arg0: !s32i) -> !s32i { %4 = cir.cast(int_to_bool, %arg0 : !s32i), !cir.bool cir.if %4 { %5 = cir.const(#cir.int<1> : !s32i) : !s32i cir.return %5 : !s32i } else { %5 = cir.const(#cir.int<0> : !s32i) : !s32i cir.return %5 : !s32i } cir.return %arg0 : !s32i } ``` The last `cir.return` becomes unreachable after flattening and hence is not reachable in the lowering. So we got the next error: ``` error: 'cir.return' op expects parent op to be one of 'cir.func, cir.scope, cir.if, cir.switch, cir.do, cir.while, cir.for' cir.return %arg0 : !s32i ``` the parent after lowering is `llvm.func`. And this is only the beginning - the more operations will be flatten, the more similar fails will happen. Thus, I added lowering for the unreachable code as well in `LowerToLLVM.cpp`. But may be you have another solution in your mind. 2) Please, pay attention on the flattening pass - I'm not that familiar with `mlir` builders as you are, so may be I'm doing something wrong. The idea was to start flattening from the most nested operations. 3) As you requested in #516, `cir-to-llvm-internal` is renamed to `cir-flat-to-llvm`. The only thing remain undone is related to the following: > Since it would be wrong to run cir-flat-to-llvm without running cir-flatten-cfg, we should make cir-flat-to-llvm pass to require cir-flatten-cfg pass to be run before. And I'm not sure I know how to do it exactly - is there something similar to pass dependencies from LLVM IR? 4) The part of `IfOp` lowering related to elimination of the vain casts for condition branch moved directly to the lowering of `BrCondOp` with some refactoring and guarding. 5) Just note, that now `cir-opt` is able to dump the flat cir as well: `cir-opt -cir-flat-cfg`

We start our journey towards `goto` support and this is a first step on this way. There are some discussion in llvm#508 and according to the plan we do the following here: - a new pass called `cir-flatten-cfg` that is a stub now but later will be responsible for the regions inlining. The pass works only if `-emit-flat-cir` is passed in cmd line. Thus, the clang behavior is not changed here from the user's point of view. - The pass will be accomplished with `goto` solver later, so we talk about several passes that are mandatory for the lowering into `llvm` dialect. There are at least two clients of this pass that will be affected: `clang` itself and `cir-opt`, so we need a common point for them: and `populateCIRFlatteningPasses` and `populateCIRToLLVMPasses` guarantee that `CIR` will be in the correct state for all the clients, whatever new passes we will add later.

@foo

This PR perform flattening for `cir::IfOp` Basically, we just move the code from `LowerToLLVM.cpp` to `FlattenCFG.cpp`. There are several important things though I would like to highlight. 1) Consider the next code from the tests: ``` cir.func @foo(%arg0: !s32i) -> !s32i { %4 = cir.cast(int_to_bool, %arg0 : !s32i), !cir.bool cir.if %4 { %5 = cir.const(#cir.int<1> : !s32i) : !s32i cir.return %5 : !s32i } else { %5 = cir.const(#cir.int<0> : !s32i) : !s32i cir.return %5 : !s32i } cir.return %arg0 : !s32i } ``` The last `cir.return` becomes unreachable after flattening and hence is not reachable in the lowering. So we got the next error: ``` error: 'cir.return' op expects parent op to be one of 'cir.func, cir.scope, cir.if, cir.switch, cir.do, cir.while, cir.for' cir.return %arg0 : !s32i ``` the parent after lowering is `llvm.func`. And this is only the beginning - the more operations will be flatten, the more similar fails will happen. Thus, I added lowering for the unreachable code as well in `LowerToLLVM.cpp`. But may be you have another solution in your mind. 2) Please, pay attention on the flattening pass - I'm not that familiar with `mlir` builders as you are, so may be I'm doing something wrong. The idea was to start flattening from the most nested operations. 3) As you requested in llvm#516, `cir-to-llvm-internal` is renamed to `cir-flat-to-llvm`. The only thing remain undone is related to the following: > Since it would be wrong to run cir-flat-to-llvm without running cir-flatten-cfg, we should make cir-flat-to-llvm pass to require cir-flatten-cfg pass to be run before. And I'm not sure I know how to do it exactly - is there something similar to pass dependencies from LLVM IR? 4) The part of `IfOp` lowering related to elimination of the vain casts for condition branch moved directly to the lowering of `BrCondOp` with some refactoring and guarding. 5) Just note, that now `cir-opt` is able to dump the flat cir as well: `cir-opt -cir-flat-cfg`

We start our journey towards `goto` support and this is a first step on this way. There are some discussion in #508 and according to the plan we do the following here: - a new pass called `cir-flatten-cfg` that is a stub now but later will be responsible for the regions inlining. The pass works only if `-emit-flat-cir` is passed in cmd line. Thus, the clang behavior is not changed here from the user's point of view. - The pass will be accomplished with `goto` solver later, so we talk about several passes that are mandatory for the lowering into `llvm` dialect. There are at least two clients of this pass that will be affected: `clang` itself and `cir-opt`, so we need a common point for them: and `populateCIRFlatteningPasses` and `populateCIRToLLVMPasses` guarantee that `CIR` will be in the correct state for all the clients, whatever new passes we will add later.

@foo

This PR perform flattening for `cir::IfOp` Basically, we just move the code from `LowerToLLVM.cpp` to `FlattenCFG.cpp`. There are several important things though I would like to highlight. 1) Consider the next code from the tests: ``` cir.func @foo(%arg0: !s32i) -> !s32i { %4 = cir.cast(int_to_bool, %arg0 : !s32i), !cir.bool cir.if %4 { %5 = cir.const(#cir.int<1> : !s32i) : !s32i cir.return %5 : !s32i } else { %5 = cir.const(#cir.int<0> : !s32i) : !s32i cir.return %5 : !s32i } cir.return %arg0 : !s32i } ``` The last `cir.return` becomes unreachable after flattening and hence is not reachable in the lowering. So we got the next error: ``` error: 'cir.return' op expects parent op to be one of 'cir.func, cir.scope, cir.if, cir.switch, cir.do, cir.while, cir.for' cir.return %arg0 : !s32i ``` the parent after lowering is `llvm.func`. And this is only the beginning - the more operations will be flatten, the more similar fails will happen. Thus, I added lowering for the unreachable code as well in `LowerToLLVM.cpp`. But may be you have another solution in your mind. 2) Please, pay attention on the flattening pass - I'm not that familiar with `mlir` builders as you are, so may be I'm doing something wrong. The idea was to start flattening from the most nested operations. 3) As you requested in #516, `cir-to-llvm-internal` is renamed to `cir-flat-to-llvm`. The only thing remain undone is related to the following: > Since it would be wrong to run cir-flat-to-llvm without running cir-flatten-cfg, we should make cir-flat-to-llvm pass to require cir-flatten-cfg pass to be run before. And I'm not sure I know how to do it exactly - is there something similar to pass dependencies from LLVM IR? 4) The part of `IfOp` lowering related to elimination of the vain casts for condition branch moved directly to the lowering of `BrCondOp` with some refactoring and guarding. 5) Just note, that now `cir-opt` is able to dump the flat cir as well: `cir-opt -cir-flat-cfg`

We start our journey towards `goto` support and this is a first step on this way. There are some discussion in llvm#508 and according to the plan we do the following here: - a new pass called `cir-flatten-cfg` that is a stub now but later will be responsible for the regions inlining. The pass works only if `-emit-flat-cir` is passed in cmd line. Thus, the clang behavior is not changed here from the user's point of view. - The pass will be accomplished with `goto` solver later, so we talk about several passes that are mandatory for the lowering into `llvm` dialect. There are at least two clients of this pass that will be affected: `clang` itself and `cir-opt`, so we need a common point for them: and `populateCIRFlatteningPasses` and `populateCIRToLLVMPasses` guarantee that `CIR` will be in the correct state for all the clients, whatever new passes we will add later.

@foo

This PR perform flattening for `cir::IfOp` Basically, we just move the code from `LowerToLLVM.cpp` to `FlattenCFG.cpp`. There are several important things though I would like to highlight. 1) Consider the next code from the tests: ``` cir.func @foo(%arg0: !s32i) -> !s32i { %4 = cir.cast(int_to_bool, %arg0 : !s32i), !cir.bool cir.if %4 { %5 = cir.const(#cir.int<1> : !s32i) : !s32i cir.return %5 : !s32i } else { %5 = cir.const(#cir.int<0> : !s32i) : !s32i cir.return %5 : !s32i } cir.return %arg0 : !s32i } ``` The last `cir.return` becomes unreachable after flattening and hence is not reachable in the lowering. So we got the next error: ``` error: 'cir.return' op expects parent op to be one of 'cir.func, cir.scope, cir.if, cir.switch, cir.do, cir.while, cir.for' cir.return %arg0 : !s32i ``` the parent after lowering is `llvm.func`. And this is only the beginning - the more operations will be flatten, the more similar fails will happen. Thus, I added lowering for the unreachable code as well in `LowerToLLVM.cpp`. But may be you have another solution in your mind. 2) Please, pay attention on the flattening pass - I'm not that familiar with `mlir` builders as you are, so may be I'm doing something wrong. The idea was to start flattening from the most nested operations. 3) As you requested in llvm#516, `cir-to-llvm-internal` is renamed to `cir-flat-to-llvm`. The only thing remain undone is related to the following: > Since it would be wrong to run cir-flat-to-llvm without running cir-flatten-cfg, we should make cir-flat-to-llvm pass to require cir-flatten-cfg pass to be run before. And I'm not sure I know how to do it exactly - is there something similar to pass dependencies from LLVM IR? 4) The part of `IfOp` lowering related to elimination of the vain casts for condition branch moved directly to the lowering of `BrCondOp` with some refactoring and guarding. 5) Just note, that now `cir-opt` is able to dump the flat cir as well: `cir-opt -cir-flat-cfg`

We start our journey towards `goto` support and this is a first step on this way. There are some discussion in llvm#508 and according to the plan we do the following here: - a new pass called `cir-flatten-cfg` that is a stub now but later will be responsible for the regions inlining. The pass works only if `-emit-flat-cir` is passed in cmd line. Thus, the clang behavior is not changed here from the user's point of view. - The pass will be accomplished with `goto` solver later, so we talk about several passes that are mandatory for the lowering into `llvm` dialect. There are at least two clients of this pass that will be affected: `clang` itself and `cir-opt`, so we need a common point for them: and `populateCIRFlatteningPasses` and `populateCIRToLLVMPasses` guarantee that `CIR` will be in the correct state for all the clients, whatever new passes we will add later.

@foo

This PR perform flattening for `cir::IfOp` Basically, we just move the code from `LowerToLLVM.cpp` to `FlattenCFG.cpp`. There are several important things though I would like to highlight. 1) Consider the next code from the tests: ``` cir.func @foo(%arg0: !s32i) -> !s32i { %4 = cir.cast(int_to_bool, %arg0 : !s32i), !cir.bool cir.if %4 { %5 = cir.const(#cir.int<1> : !s32i) : !s32i cir.return %5 : !s32i } else { %5 = cir.const(#cir.int<0> : !s32i) : !s32i cir.return %5 : !s32i } cir.return %arg0 : !s32i } ``` The last `cir.return` becomes unreachable after flattening and hence is not reachable in the lowering. So we got the next error: ``` error: 'cir.return' op expects parent op to be one of 'cir.func, cir.scope, cir.if, cir.switch, cir.do, cir.while, cir.for' cir.return %arg0 : !s32i ``` the parent after lowering is `llvm.func`. And this is only the beginning - the more operations will be flatten, the more similar fails will happen. Thus, I added lowering for the unreachable code as well in `LowerToLLVM.cpp`. But may be you have another solution in your mind. 2) Please, pay attention on the flattening pass - I'm not that familiar with `mlir` builders as you are, so may be I'm doing something wrong. The idea was to start flattening from the most nested operations. 3) As you requested in llvm#516, `cir-to-llvm-internal` is renamed to `cir-flat-to-llvm`. The only thing remain undone is related to the following: > Since it would be wrong to run cir-flat-to-llvm without running cir-flatten-cfg, we should make cir-flat-to-llvm pass to require cir-flatten-cfg pass to be run before. And I'm not sure I know how to do it exactly - is there something similar to pass dependencies from LLVM IR? 4) The part of `IfOp` lowering related to elimination of the vain casts for condition branch moved directly to the lowering of `BrCondOp` with some refactoring and guarding. 5) Just note, that now `cir-opt` is able to dump the flat cir as well: `cir-opt -cir-flat-cfg`

We start our journey towards `goto` support and this is a first step on this way. There are some discussion in llvm#508 and according to the plan we do the following here: - a new pass called `cir-flatten-cfg` that is a stub now but later will be responsible for the regions inlining. The pass works only if `-emit-flat-cir` is passed in cmd line. Thus, the clang behavior is not changed here from the user's point of view. - The pass will be accomplished with `goto` solver later, so we talk about several passes that are mandatory for the lowering into `llvm` dialect. There are at least two clients of this pass that will be affected: `clang` itself and `cir-opt`, so we need a common point for them: and `populateCIRFlatteningPasses` and `populateCIRToLLVMPasses` guarantee that `CIR` will be in the correct state for all the clients, whatever new passes we will add later.

@foo

This PR perform flattening for `cir::IfOp` Basically, we just move the code from `LowerToLLVM.cpp` to `FlattenCFG.cpp`. There are several important things though I would like to highlight. 1) Consider the next code from the tests: ``` cir.func @foo(%arg0: !s32i) -> !s32i { %4 = cir.cast(int_to_bool, %arg0 : !s32i), !cir.bool cir.if %4 { %5 = cir.const(#cir.int<1> : !s32i) : !s32i cir.return %5 : !s32i } else { %5 = cir.const(#cir.int<0> : !s32i) : !s32i cir.return %5 : !s32i } cir.return %arg0 : !s32i } ``` The last `cir.return` becomes unreachable after flattening and hence is not reachable in the lowering. So we got the next error: ``` error: 'cir.return' op expects parent op to be one of 'cir.func, cir.scope, cir.if, cir.switch, cir.do, cir.while, cir.for' cir.return %arg0 : !s32i ``` the parent after lowering is `llvm.func`. And this is only the beginning - the more operations will be flatten, the more similar fails will happen. Thus, I added lowering for the unreachable code as well in `LowerToLLVM.cpp`. But may be you have another solution in your mind. 2) Please, pay attention on the flattening pass - I'm not that familiar with `mlir` builders as you are, so may be I'm doing something wrong. The idea was to start flattening from the most nested operations. 3) As you requested in llvm#516, `cir-to-llvm-internal` is renamed to `cir-flat-to-llvm`. The only thing remain undone is related to the following: > Since it would be wrong to run cir-flat-to-llvm without running cir-flatten-cfg, we should make cir-flat-to-llvm pass to require cir-flatten-cfg pass to be run before. And I'm not sure I know how to do it exactly - is there something similar to pass dependencies from LLVM IR? 4) The part of `IfOp` lowering related to elimination of the vain casts for condition branch moved directly to the lowering of `BrCondOp` with some refactoring and guarding. 5) Just note, that now `cir-opt` is able to dump the flat cir as well: `cir-opt -cir-flat-cfg`

We start our journey towards `goto` support and this is a first step on this way. There are some discussion in #508 and according to the plan we do the following here: - a new pass called `cir-flatten-cfg` that is a stub now but later will be responsible for the regions inlining. The pass works only if `-emit-flat-cir` is passed in cmd line. Thus, the clang behavior is not changed here from the user's point of view. - The pass will be accomplished with `goto` solver later, so we talk about several passes that are mandatory for the lowering into `llvm` dialect. There are at least two clients of this pass that will be affected: `clang` itself and `cir-opt`, so we need a common point for them: and `populateCIRFlatteningPasses` and `populateCIRToLLVMPasses` guarantee that `CIR` will be in the correct state for all the clients, whatever new passes we will add later.

@foo

This PR perform flattening for `cir::IfOp` Basically, we just move the code from `LowerToLLVM.cpp` to `FlattenCFG.cpp`. There are several important things though I would like to highlight. 1) Consider the next code from the tests: ``` cir.func @foo(%arg0: !s32i) -> !s32i { %4 = cir.cast(int_to_bool, %arg0 : !s32i), !cir.bool cir.if %4 { %5 = cir.const(#cir.int<1> : !s32i) : !s32i cir.return %5 : !s32i } else { %5 = cir.const(#cir.int<0> : !s32i) : !s32i cir.return %5 : !s32i } cir.return %arg0 : !s32i } ``` The last `cir.return` becomes unreachable after flattening and hence is not reachable in the lowering. So we got the next error: ``` error: 'cir.return' op expects parent op to be one of 'cir.func, cir.scope, cir.if, cir.switch, cir.do, cir.while, cir.for' cir.return %arg0 : !s32i ``` the parent after lowering is `llvm.func`. And this is only the beginning - the more operations will be flatten, the more similar fails will happen. Thus, I added lowering for the unreachable code as well in `LowerToLLVM.cpp`. But may be you have another solution in your mind. 2) Please, pay attention on the flattening pass - I'm not that familiar with `mlir` builders as you are, so may be I'm doing something wrong. The idea was to start flattening from the most nested operations. 3) As you requested in #516, `cir-to-llvm-internal` is renamed to `cir-flat-to-llvm`. The only thing remain undone is related to the following: > Since it would be wrong to run cir-flat-to-llvm without running cir-flatten-cfg, we should make cir-flat-to-llvm pass to require cir-flatten-cfg pass to be run before. And I'm not sure I know how to do it exactly - is there something similar to pass dependencies from LLVM IR? 4) The part of `IfOp` lowering related to elimination of the vain casts for condition branch moved directly to the lowering of `BrCondOp` with some refactoring and guarding. 5) Just note, that now `cir-opt` is able to dump the flat cir as well: `cir-opt -cir-flat-cfg`

lanza force-pushed the main branch from 43551d4 to 60e2694 Compare March 21, 2024 01:05

gitoleg force-pushed the flattening-prepare branch from 8b680a2 to 4a1a234 Compare March 21, 2024 09:00

bcardosolopes requested changes Mar 22, 2024

View reviewed changes

bcardosolopes changed the title ~~[CIR] Flattening prepare~~ [CIR] Introduce a flattening pass Mar 22, 2024

gitoleg commented Mar 22, 2024

View reviewed changes

lanza force-pushed the main branch from ed3955a to 8b7417c Compare March 23, 2024 05:07

gitoleg force-pushed the flattening-prepare branch from 7bf2c8f to c9c262e Compare March 25, 2024 14:25

lanza force-pushed the main branch from 8b7417c to 330c983 Compare March 30, 2024 04:29

gitoleg added 16 commits April 1, 2024 16:17

[CIR] A skeleton for the structured cfg pass

ce7c238

adds the pass right after the lowering prepare

9d4c51d

Refactoring And Unifrom flat cir passes

92b5586

emit-flat-cir option

3565272

kind of test fixing

058d28d

silly test

6594249

refactoring

d2c6640

refactoring

d97f9a1

test updated

3fa07d7

clang-format ...

8bd1e7b

minor fix

5d4c149

flat cir -> cir flat

1714c8a

StructuredCFG -> FlattenCFG

399f8bd

minor fixes

f324404

minor bug fix

daec97f

clang-format ...

5a876d9

gitoleg force-pushed the flattening-prepare branch from c9c262e to 5a876d9 Compare April 1, 2024 13:20

bcardosolopes approved these changes Apr 3, 2024

View reviewed changes

bcardosolopes merged commit 9978d5e into llvm:main Apr 3, 2024
6 checks passed

gitoleg mentioned this pull request Apr 8, 2024

[CIR][Codegen] IfOp flattening #537

Merged

gitoleg mentioned this pull request Apr 26, 2024

[CIR] Goto #508

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CIR] Introduce a flattening pass #516

[CIR] Introduce a flattening pass #516

gitoleg commented Mar 20, 2024 •

edited

Loading

github-actions bot commented Mar 20, 2024 •

edited

Loading

bcardosolopes left a comment

bcardosolopes Mar 22, 2024

gitoleg Mar 22, 2024

bcardosolopes Apr 3, 2024

gitoleg Mar 22, 2024

gitoleg commented Mar 22, 2024

bcardosolopes left a comment

bcardosolopes Apr 3, 2024

[CIR] Introduce a flattening pass #516

[CIR] Introduce a flattening pass #516

Conversation

gitoleg commented Mar 20, 2024 • edited Loading

github-actions bot commented Mar 20, 2024 • edited Loading

bcardosolopes left a comment

Choose a reason for hiding this comment

bcardosolopes Mar 22, 2024

Choose a reason for hiding this comment

gitoleg Mar 22, 2024

Choose a reason for hiding this comment

bcardosolopes Apr 3, 2024

Choose a reason for hiding this comment

gitoleg Mar 22, 2024

Choose a reason for hiding this comment

gitoleg commented Mar 22, 2024

bcardosolopes left a comment

Choose a reason for hiding this comment

bcardosolopes Apr 3, 2024

Choose a reason for hiding this comment

gitoleg commented Mar 20, 2024 •

edited

Loading

github-actions bot commented Mar 20, 2024 •

edited

Loading