AmrDeveloper wrote:
> This doesn't actually make progress towards the goal of making BeginCatchOp
> target-independent. What we need is to move the `emitBeginCatch` call out of
> `CIRGenCXXABI` and `CIRGenItainumCXXABI` and instead have it in
> `CIRGenFunction` with an implementation that has the necessary level of
> abstraction to move all of the target-specific details into the
> `EHABILowering` pass.
>
> I think we can achieve that with the current definition of BeginCatchOp.
> Consider the following example:
>
> ```
> int catchCustom() {
> try {
> may_throw();
> } catch (CustomError e) {
> return e.error_code;
> }
> return 0;
> }
> ```
>
> That currently generates this CIR (assuming CustomError has a trivial copy
> constructor):
>
> ```
> %catch_token, %exn_ptr = cir.begin_catch %arg0 : !cir.eh_token ->
> (!cir.catch_token, !cir.ptr<!rec_CustomError>)
> cir.cleanup.scope {
> cir.copy %exn_ptr to %3 : !cir.ptr<!rec_CustomError>
> %4 = cir.get_member %3[0] {name = "error_code"} :
> !cir.ptr<!rec_CustomError> -> !cir.ptr<!s32i>
> %5 = cir.load align(4) %4 : !cir.ptr<!s32i>, !s32i
> cir.store %5, %0 : !s32i, !cir.ptr<!s32i>
> %6 = cir.load %0 : !cir.ptr<!s32i>, !s32i
> cir.return %6 : !s32i
> ^bb1: // no predecessors
> cir.yield
> } cleanup all {
> cir.end_catch %catch_token : !cir.catch_token
> cir.yield
> }
> ```
>
> The `cir.copy` is the only part of this that's target-specific, and I think
> generating that was a misstep on my part. We should instead just use the
> exception pointer that was returned from `cir.begin_catch` like this:
>
> ```
> %catch_token, %exn_ptr = cir.begin_catch %arg0 : !cir.eh_token ->
> (!cir.catch_token, !cir.ptr<!rec_CustomError>)
> cir.cleanup.scope {
> %4 = cir.get_member %exn_ptr[0] {name = "error_code"} :
> !cir.ptr<!rec_CustomError> -> !cir.ptr<!s32i>
> %5 = cir.load align(4) %4 : !cir.ptr<!s32i>, !s32i
> cir.store %5, %0 : !s32i, !cir.ptr<!s32i>
> %6 = cir.load %0 : !cir.ptr<!s32i>, !s32i
> cir.return %6 : !s32i
> ^bb1: // no predecessors
> cir.yield
> } cleanup all {
> cir.end_catch %catch_token : !cir.catch_token
> cir.yield
> }
> ```
>
> And we can do that whether `CustomError` has a trivial copy constructor or
> not. The target specific part will get inserted during `EHABILowering`. For
> the example above, before `EHABILowering` the catch would look like this:
>
> ```
> ^bb6(%6: !cir.eh_token): // pred: ^bb5
> %catch_token, %exn_ptr = cir.begin_catch %6 : !cir.eh_token ->
> (!cir.catch_t
> oken, !cir.ptr<!rec_CustomError>)
> cir.br ^bb7
> ^bb7: // pred: ^bb6
> %7 = cir.get_member %0[0] {name = "error_code"} :
> !cir.ptr<!rec_CustomError>
> -> !cir.ptr<!s32i>
> %8 = cir.load align(4) %7 : !cir.ptr<!s32i>, !s32i
> cir.store align(4) %8, %2 : !s32i, !cir.ptr<!s32i>
> cir.br ^bb8
> ^bb8: // pred: ^bb7
> cir.end_catch %catch_token : !cir.catch_token
> cir.br ^bb9
> ```
>
> And after, if `CustomError` has a trivial copy constructor, it would look
> like this:
>
> ```
> ^bb7(%10: !cir.ptr<!void>, %11: !u32i): // pred: ^bb6
> %12 = cir.call @__cxa_begin_catch(%10) : (!cir.ptr<!void>) ->
> !cir.ptr<!u8i>
> %13 = cir.cast bitcast %12 : !cir.ptr<!u8i> -> !cir.ptr<!rec_CustomError>
> cir.br ^bb8
> ^bb8: // pred: ^bb7
> cir.copy %13 to %0 : !cir.ptr<!rec_CustomError>
> %14 = cir.get_member %0[0] {name = "error_code"} :
> !cir.ptr<!rec_CustomError> -> !cir.ptr<!s32i>
> %15 = cir.load align(4) %14 : !cir.ptr<!s32i>, !s32i
> ...
> cir.call @__cxa_end_catch() : () -> ()
> ```
>
> But if `CustomError does not have a trivial copy constructor, `EHABILowering`
> would lower the catch handler to this:
>
> ```
> ^bb7(%10: !cir.ptr<!void>, %11: !u32i): // pred: ^bb6
> %12 = cir.load align(8) %10 : !cir.ptr<!rec_CustomError>,
> !cir.ptr<!cir.ptr<!!rec_CustomError>>
> %13 = cir.call @__cxa_get_exception_ptr(%12 : !cir.ptr<!rec_CustomError>)
> cir.call @_ZN11CustomErrorC2ERKS_(%13 : !cir.ptr<!rec_CustomError>)
> %14 = cir.call @__cxa_begin_catch(%10) : (!cir.ptr<!void>) ->
> !cir.ptr<!u8i>
> %15 = cir.cast bitcast %12 : !cir.ptr<!u8i> -> !cir.ptr<!rec_CustomError>
> cir.br ^bb8
> ^bb8: // pred: ^bb7
> %16 = cir.get_member %13[0] {name = "error_code"} :
> !cir.ptr<!rec_CustomError> -> !cir.ptr<!s32i>
> %17 = cir.load align(4) %16 : !cir.ptr<!s32i>, !s32i
> ...
> cir.call @__cxa_end_catch() : () -> ()
> ```
>
> That's a bit sloppy, but hopefully it's enough for you to see what I'm
> getting at.
>
> The key point is that we just need to move the logic of `initCatchParam` out
> of `CIRGenItaniumCXXABI` and into `EHABILowering`.
That makes sense. I already moved the code out of CXXABI classes to just CG. I
am not sure if, with the current definition of BeginCatchOp, we can handle all
cases (I am thinking of the Objc cases too).
My two concerns are:
- We have the exn_ptr from the begin_catch, but we don't know the destination
(alloca that we should store the exn in by using copy, copy construct, store
...etc).
- If we can know what exact approach we need to emit (memcopy, copy construct,
store, load store, dummy alloca, store, store, ...etc) without adding another
flag to the definition.
- If we handle the store part in the op lowering, do we actually need to return
exn_ptr 🤔?
I will investigate those cases and try to do them with minimal update to the
definition or without any update if it is possible
https://github.com/llvm/llvm-project/pull/190612
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
