andykaylor wrote:
I'm not entirely convinced this is the direction we want to go long-term, so
I'd like to solicit opinions on that question specifically.
This matches the handling for this case in the incubator, which is closely
modeled after the same handling in classic codegen. Classic codegen does more
with the BranchFixups, which may just be missing implementation.
The alternative for CIR would be to explicity represent EHStackScope entries in
CIR, as @bcardosolopes has previously suggested, and apply the branching during
FlattenCFG. So for example, this patch leads to the following CIR (before
CIRCanonicalize) for the added VLA test:
```
cir.func dso_local @_Z2f5m(%arg0: !u64i) -> !s32i {
%0 = cir.alloca !u64i, !cir.ptr<!u64i>, ["len", init] {alignment = 8 : i64}
%1 = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"] {alignment = 4 : i64}
%2 = cir.alloca !cir.ptr<!u8i>, !cir.ptr<!cir.ptr<!u8i>>, ["saved_stack"]
ast #cir.var.decl.ast {alignment = 8 : i64}
cir.store %arg0, %0 : !u64i, !cir.ptr<!u64i>
%3 = cir.load align(8) %0 : !cir.ptr<!u64i>, !u64i
%4 = cir.stack_save : !cir.ptr<!u8i>
cir.store align(8) %4, %2 : !cir.ptr<!u8i>, !cir.ptr<!cir.ptr<!u8i>>
%5 = cir.alloca !s32i, !cir.ptr<!s32i>, %3 : !u64i, ["vla"] ast
#cir.var.decl.ast {alignment = 16 : i64}
%6 = cir.const #cir.int<0> : !s32i
%7 = cir.ptr_stride %5, %6 : (!cir.ptr<!s32i>, !s32i) -> !cir.ptr<!s32i>
%8 = cir.load align(4) %7 : !cir.ptr<!s32i>, !s32i
cir.store %8, %1 : !s32i, !cir.ptr<!s32i>
cir.br ^bb2
^bb1: // pred: ^bb2
%9 = cir.load %1 : !cir.ptr<!s32i>, !s32i
cir.return %9 : !s32i
^bb2: // pred: ^bb0
%10 = cir.load align(8) %2 : !cir.ptr<!cir.ptr<!u8i>>, !cir.ptr<!u8i>
cir.stack_restore %10 : !cir.ptr<!u8i>
cir.br ^bb1
}
```
The alternative would look someting like this:
```
cir.func dso_local @_Z2f5m(%arg0: !u64i) -> !s32i {
%0 = cir.alloca !u64i, !cir.ptr<!u64i>, ["len", init] {alignment = 8 : i64}
%1 = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"] {alignment = 4 : i64}
%2 = cir.alloca !cir.ptr<!u8i>, !cir.ptr<!cir.ptr<!u8i>>, ["saved_stack"]
ast #cir.var.decl.ast {alignment = 8 : i64}
cir.store %arg0, %0 : !u64i, !cir.ptr<!u64i>
%3 = cir.load align(8) %0 : !cir.ptr<!u64i>, !u64i
%4 = cir.stack_save : !cir.ptr<!u8i>
cir.store align(8) %4, %2 : !cir.ptr<!u8i>, !cir.ptr<!cir.ptr<!u8i>>
cir.cleanup.scope CallStackRestore {
%5 = cir.alloca !s32i, !cir.ptr<!s32i>, %3 : !u64i, ["vla"] ast
#cir.var.decl.ast {alignment = 16 : i64}
%6 = cir.const #cir.int<0> : !s32i
%7 = cir.ptr_stride %5, %6 : (!cir.ptr<!s32i>, !s32i) -> !cir.ptr<!s32i>
%8 = cir.load align(4) %7 : !cir.ptr<!s32i>, !s32i
cir.store %8, %1 : !s32i, !cir.ptr<!s32i>
%9 = cir.load %1 : !cir.ptr<!s32i>, !s32i
cir.return %9 : !s32i
} cleanup {
%10 = cir.load align(8) %2 : !cir.ptr<!cir.ptr<!u8i>>, !cir.ptr<!u8i>
cir.stack_restore %10 : !cir.ptr<!u8i>
}
}
```
The FlattenCFG transform would need to recognize the the `cir.return` statement
was returning through a cleanup scope and apply the necessary branching. That
would simplify the EHScopeStck handling significantly in CIR, but the
complexity would just be moved to the FlattenCFG pass, and this would mean we
were diverging from the classic codegen code structure.
Thoughts?
https://github.com/llvm/llvm-project/pull/163849
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
