llvm / clangir

A new (MLIR based) high-level IR for clang.
https://clangir.org
Other
351 stars 95 forks source link

[CIR] Add support and enable MLIR's mem2reg for flat CIR #659

Closed gitoleg closed 2 months ago

gitoleg commented 4 months ago

This is PR enables generic MLIR's mem2reg pass for the flat CIR code. I submit this draft PR if someone wants to play with mem2reg right now and to define the next incremental steps if necessary.

Briefly, what's important: 1) I used llvm dialect implementation for the inspiration 2) I did not implement interfaces for some of the operations, e.g. for cir.get_member, since looks like mem2reg doesn't handle them - it's more about SROA pass. 3) simple tests are added 4) mem2reg is disabled by default, so one need to use it explicitly, with the -fclangir-mem2reg option 5) There are a couple bugs fixed in the code . The most important is tablegen for BranchOp that assumed same sized operands for branches (it was extremely hard to detect!).

Note, that this mem2reg implementation work with flat CIR only. First of all, the MLIR's mem2reg knows how to propagate (and remove) memory slots only when we can build a graph of blocks (e.g. dom tree is a bread and butter of mem2reg). Also, given that phi nodes are represents as blocks with arguments in MLIR, there is no chance to transfer control flow and call a block from another region (and pass some values as well).

The last one is important and has some relation to canonicalizer - may be you remember I mentioned this problem once somewhere, when conditional branch verification fails on target block arguments number and branch operands number mismatch.

finally, short example to play with:

int return_42() {
    int y = 42;
    return y;  
}

First, without mem2reg enabled clang tmp.c -fclangir -emit-llvm -S -o - :

define dso_local i32 @return_42() #0 !dbg !3 {
  %1 = alloca i32, i64 1, align 4, !dbg !7
  %2 = alloca i32, i64 1, align 4, !dbg !8
  store i32 42, ptr %2, align 4, !dbg !8
  %3 = load i32, ptr %2, align 4, !dbg !9
  store i32 %3, ptr %1, align 4, !dbg !10
  %4 = load i32, ptr %1, align 4, !dbg !10
  ret i32 %4, !dbg !10
}

and with mem2reg enabled clang tmp.c -fclangir -fclangir-mem2reg -emit-llvm -S -o -:

define dso_local i32 @return_42() #0 !dbg !3 {
  ret i32 42, !dbg !7
}

So. What do you think? Which steps do you expect me to perform? I can try to split this PR into parts and add interfaces one-by-one, but I'n not sure the tests will work without all of them implemented.

bcardosolopes commented 3 months ago

Sorry I didn't had the time to look just yet, but should do that soon, can you update post rebase?

gitoleg commented 2 months ago

@bcardosolopes sorry for delay!
I updated the PR, there is a strange fail in the tests, I will fix it soon. But now you could look at the changes :)

bcardosolopes commented 2 months ago

Can you also update the description with a brief mention why this is only valid for flat CIR? (I know the answer, but it's good to leave some pointers for context)

gitoleg commented 2 months ago

@bcardosolopes so I think I did all you asked for. Please let me know if I missed something