https://github.com/andykaylor commented:
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`.
https://github.com/llvm/llvm-project/pull/190612
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
