Skip to content
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

[CIR][Codegen] IfOp flattening #537

Merged
merged 10 commits into from
Apr 15, 2024
Merged

Conversation

gitoleg
Copy link
Collaborator

@gitoleg gitoleg commented Apr 8, 2024

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.

  1. 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.

  2. As you requested in [CIR] Introduce a flattening pass #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?

  1. 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.

  2. Just note, that now cir-opt is able to dump the flat cir as well: cir-opt -cir-flat-cfg

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM, one comment before we move forward.

The last cir.return becomes unreachable after flattening and hence is not reachable in the lowering

I see two options:

  1. Add a cleanup pass that removes unrecheable blocks before LLVM lowering. I believe https://mlir.llvm.org/docs/Passes/#-canonicalize does that.
  2. Lowering should also look at unrechable blocks?

Thus, I added lowering for the unreachable code as well in LowerToLLVM.cpp

(2) sounds good, it's correct, but (1) would be more compiler friendly (we likely don't want to spend compilation time lowering pieces we don't care about). Before we go on with collect_unreachable, can you check whether adding canonicalize to the lowering pipeline could fix the problem?

And I'm not sure I know how to do it exactly - is there something similar to pass dependencies from LLVM IR?

I believe so but I haven't tried to use just yet, I could be wrong.

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.

Ok.

Just note, that now cir-opt is able to dump the flat cir as well: cir-opt -cir-flat-cfg

Nice.

@gitoleg
Copy link
Collaborator Author

gitoleg commented Apr 10, 2024

@bcardosolopes

At the first glance the canonicalizer pass is exactly what we need. And I would use it.
But in the same time I have some doubts.

First of all, ~30 lowering tests fails - maybe it's not a problem, but anyway just want to let you know. For example, the next test (alloca.cir);

cir.func @foo(%arg0: !s32i) {
    %0 = cir.alloca !s32i, cir.ptr <!s32i>, %arg0 : !s32i, ["tmp"] {alignment = 16 : i64}
    cir.return
  }

fails because %0 is not used, hence it's removed by the pass.

Next, take a look how the llvm dialect looks like for the example from the PR message (after the pass applied):

module {
  llvm.func @foo(%arg0: i32) -> i32 attributes {cir.extra_attrs = #fn_attr} {
    %0 = llvm.mlir.constant(1 : i32) : i32
    %1 = llvm.mlir.constant(0 : i32) : i32
    %2 = llvm.mlir.constant(0 : i32) : i32
    %3 = llvm.icmp "ne" %arg0, %2 : i32
    llvm.cond_br %3, ^bb1(%0 : i32), ^bb1(%1 : i32)

  ^bb1(%4: i32):  // 2 preds: ^bb0, ^bb0     /// <------  awesome!! Just a single exit block with operands. 
    llvm.return %4 : i32
  }
}

But I ran the codebase I use for work with CIR (e.g. gcc code) and got around 1000 new errors like:
branch has 0 operands for successor #0 but target block has 1. I suppose the reason is exactly in the blocks with operands added. Looks like there is a bug on the mlir's side somewhere. It's also fixable, but we need to do it as well.

I see two options:

So let's come back again to the choice - which one do you prefer? if it's still the first one - I start to investigate a new bug.
Another way is to write our own clean up pass for unreachable blocks, though we need to be careful.

so, what do you think?

@sitio-couto
Copy link
Collaborator

@bcardosolopes & @gitoleg, I have a question in mind regarding this discussion:

What is our current approach to dealing with unreachable code? Do we want CIRGen to emit it into CIR for some reason, or would it be preferable to eliminate it as soon as possible to prevent passes from dealing with errors related to it?

fails because %0 is not used, hence it's removed by the pass.

@gitoleg it might be better to add this DCE pass right after codegen instead so that it does not affect any of the existing CIR to LLVM lowering tests.

@gitoleg
Copy link
Collaborator Author

gitoleg commented Apr 10, 2024

@sitio-couto
There is a question :) Either we lower it and solve all the problems in LowerToLLVM.cpp or make some clean up before.

it might be better to add this DCE pass right after codegen

The problem is that unreachable code may be introduced by flattening as well. Previously, when all the region inlining was done during the lowering, the unreachable code also could be introduced but it was llvm dialect code, so no complains from the verification happened. Now, when unreachable code is CIR one, it won't be lowered because the code processed in the dominance order where unreachable blocks doesn't participate. Hence, the problem. And now we need to decide how to handle the unreachable code.

@bcardosolopes
Copy link
Member

bcardosolopes commented Apr 10, 2024

@gitoleg @sitio-couto

First of all, ~30 lowering tests fails - maybe it's not a problem, but anyway just want to let you know. For example, the next test (alloca.cir);

Yea, this is not a new problem, before we had our merge cleanup pass, I tried to use the canonicalizer but it was too aggressive. We can probably fix that by making sure more operations has side-effects, so it wouldn't eliminate as much.
Not asking you to that for this PR, but this is a problem we'll have to tackle sooner or later. Can you create an issue to track this?

But I ran the codebase I use for work with CIR (e.g. gcc code) and got around 1000 new errors like:
branch has 0 operands for successor #0 but target block has 1. I suppose the reason is exactly in the blocks with operands added. Looks like there is a bug on the mlir's side somewhere. It's also fixable, but we need to do it as well.

Oh =(
Can you also add this information to the issue you create? The issue should contain two main points:

  • More operations need to be marked with side-effects so they won't be removed.
  • The apparent bug in MLIR with the branch issue.

What is our current approach to dealing with unreachable code? Do we want CIRGen to emit it into CIR for some reason, or would it be preferable to eliminate it as soon as possible to prevent passes from dealing with errors related to it?

Unreachable code should live until before lowering prepare: at some point we want to use CIR for producing unrecheable warnings in clang.

... it might be better to add this DCE pass right after codegen instead so that it does not affect any of the existing CIR to LLVM lowering tests.

See my reply above.

So let's come back again to the choice - which one do you prefer?

I think we should use collect_unreachable for now, and create a second issue to delete it once we solve the problems from issue 1. Also add comments to that function to illustrate.

@gitoleg
Copy link
Collaborator Author

gitoleg commented Apr 12, 2024

@bcardosolopes
I create issues #542 and #543 and also added comments for collect_unreachable function

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job on the extra explanation comment, LGTM

@bcardosolopes bcardosolopes merged commit e50db8b into llvm:main Apr 15, 2024
6 checks passed
@gitoleg gitoleg mentioned this pull request Apr 26, 2024
lanza pushed a commit that referenced this pull request Apr 29, 2024
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 pushed a commit that referenced this pull request Apr 29, 2024
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`
eZWALT pushed a commit to eZWALT/clangir that referenced this pull request Apr 29, 2024
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`
lanza pushed a commit that referenced this pull request Apr 29, 2024
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`
bruteforceboy pushed a commit to bruteforceboy/clangir that referenced this pull request Oct 2, 2024
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`
Hugobros3 pushed a commit to shady-gang/clangir that referenced this pull request Oct 2, 2024
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`
keryell pushed a commit to keryell/clangir that referenced this pull request Oct 19, 2024
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`
lanza pushed a commit that referenced this pull request Nov 5, 2024
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`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants