On Dec 10, 2007, at 11:38 AM, Duncan Sands wrote:>>> ... If you force a "cleanup" by changing the selector call to: >>> %eh_select8.i = tail call i32 (i8*, i8*, ...)* >>> @llvm.eh.selector.i32( i8* %eh_ptr.i, i8* bitcast (i32 (...)* >>> @__gxx_personality_v0 to i8*), i32 0) >>> then it doesn't work either: the unwinder observes that there is >>> only a cleanup, and >>> using some special logic (bogus in this case) deduces that the >>> exception will be rethrown >>> after running the cleanup code (and thus the program terminated), so >>> doesn't bother running >>> the cleanup code and directly terminates the program (apparently >>> terminating programs quickly >>> was important to whoever wrote the unwinder, I don't know why; the >>> Ada unwinder doesn't do >>> this for example :) ). >> >> OTOH, claiming that everything has a cleanup seems to me a correct >> description of what the IR code does: control reenters the throwing >> function to execute a possibly null cleanup, then resumes. > > I agree, and this is why I originally tried to get the desired effect > using cleanups. > >> The trouble is you can't simply copy that IR while inlining and >> expect >> things to still work, because the operation of Unwind_Resume depends >> on what stack frame it's in. I don't agree that the inlined version >> is correct IR. The 'invoke semantics' you're talking about are >> inextricably intertwined with _Unwind_Resume's semantics. > > The semantics of invoke are described in the LangRef. I took the > approach of artificially obtaining these semantics, which have an > impedance mismatch with the gcc unwinder, by playing with the way we > set up the exception table. If you manage to obtain invoke semantics > then you can expect inlining to work. And I did manage to obtain them > on linux - so this was a simple solution that also meant I didn't have > to audit all the optimizers to check how they used invoke (though most > likely the inliner is the only one that matters). > > You are suggesting changing the meaning of invoke, a bolder approach > which certainly has some advantages. If I understand right you want > to allow some exceptions to unwind through an invoke without branching > to the unwind label (instead they just keep on unwinding); which > exceptions are caught is to be determined by the eh.selector call.No, I don't want to change the semantics of invoke, at least I don't think so. When inlining, I want the inlined throw to reach cleanup code as it does. But I want the Unwind_Resume call that ends the cleanup code to be replaced with a control transfer to the handler (or cleanup) in the calling function, i.e. the inliner needs to know the semantics of Unwind_Resume.> In other words, you want to model LLVM's EH on the way the gcc > unwinder > works.Invoke's reason for existence is supporting EH, and we can't change the unwinder, so yes, we have to do that. So do you, the difference is our unwinders don't work quite the same.> With this model indeed the inliner needs to copy the selector > to the appropriate places when inlining. Perhaps some other > optimizers > need to be tweaked too. Unfortunately this approach immediately hits > PR1508 (http://llvm.org/bugs/show_bug.cgi?id=1508), i.e. the problem > of actually finding the selector for a landing pad. Personally I > would > like to see the selector somehow be attached to the unwind edge - > perhaps > it could be done as part of PR1269. > >>> ... It is true that you can imagine a solution in which the inliner >>> knows about selector calls >>> and shuffles them around. I will think about this. That said, >>> invoke is defined to have >>> certain semantics, and I don't much like the idea of trying to do an >>> end-run around them >>> and around optimizers that exploit them... >> >> I don't see much choice. I guess I'll look at getting the inliner to >> do what I think it should do. (I'll make it Darwin-specific at >> first, but it should work on Linux, and let me point out that you'll >> get more efficient code this way.) > > Yes, you certainly will get more efficient unwinding. This probably > doesn't matter much for Ada or C++, but I guess some languages out > there like to throw exceptions a lot. > >> I guess an easy but undesirable fallback position is to tell the >> inliner not to inline anything that invokes Unwind_Resume. > > If you push a "cleanup" instead of a catch-all on Darwin, does > everything work? This is exactly what you get if you apply your > patch > > - lang_eh_catch_all = return_null_tree; > +/* lang_eh_catch_all = return_null_tree;*/No, things are much better than there were, but inlining functions that call Unwind_Resume still causes the problem we've been talking about. Disabling that as well makes everything work.
Hi Dale,> No, I don't want to change the semantics of invoke, at least I don't > think so. > When inlining, I want the inlined throw to reach cleanup code as it > does. > But I want the Unwind_Resume call that ends the cleanup code to be > replaced with a control transfer to the handler (or cleanup) in the > calling > function, i.e. the inliner needs to know the semantics of Unwind_Resume.it seems to me that this is extremely tricky to do in general, though it is simpler if you suppose the IR was produced by gcc. Consider this example: class A {}; class B {}; int i; extern void f(); void g() { try { f(); } catch(A) { i = 1; } } void h() { try { g(); } catch(B) { i = 2; } } Without catch-alls this compiles to something like: define void @_Z1gv() { entry: invoke void @_Z1fv( ) to label %UnifiedReturnBlock unwind label %lpad ... lpad: ; preds = %entry %eh_ptr = tail call i8* @llvm.eh.exception( ) %eh_select = tail call i32 (i8*, i8*, ...)* @llvm.eh.selector.i32( i8* %eh_ptr, i32 (...)* @__gxx_personality_v0, i8* A) %eh_typeid = tail call i32 @llvm.eh.typeid.for.i32( i8* A ) %tmp15 = icmp eq i32 %eh_select, %eh_typeid br i1 %tmp15, label %bb, label %Unwind ... Unwind: ; preds = %lpad tail call i32 (...)* @_Unwind_Resume( i8* %eh_ptr ) unreachable ... } define void @_Z1hv() { entry: invoke void @_Z1gv( ) to label %UnifiedReturnBlock unwind label %lpad ... lpad: ; preds = %entry %eh_ptr = tail call i8* @llvm.eh.exception( ) %eh_select = tail call i32 (i8*, i8*, ...)* @llvm.eh.selector.i32( i8* %eh_ptr, i32 (...)* @__gxx_personality_v0, i8* B ) %eh_typeid = tail call i32 @llvm.eh.typeid.for.i32( i8* B ) %tmp15 = icmp eq i32 %eh_select, %eh_typeid br i1 %tmp15, label %bb, label %Unwind ... Unwind: ; preds = %lpad tail call i32 (...)* @_Unwind_Resume( i8* %eh_ptr ) unreachable ... } Currently when you inline you get something like: define void @_Z1hv() { entry: invoke void @_Z1fv( ) to label %UnifiedReturnBlock2 unwind label %lpad.i ... lpad.i: ; preds = %entry %eh_ptr.i = tail call i8* @llvm.eh.exception( ) %eh_select.i = tail call i32 (i8*, i8*, ...)* @llvm.eh.selector.i32( i8* %eh_ptr.i, i32 (...)* @__gxx_personality_v0 , i8* A ) %eh_typeid.i = tail call i32 @llvm.eh.typeid.for.i32( i8* A ) %tmp15.i = icmp eq i32 %eh_select.i, %eh_typeid.i br i1 %tmp15.i, label %bb.i, label %Unwind.i ... Unwind.i: ; preds = %lpad.i invoke i32 (...)* @_Unwind_Resume( i8* %eh_ptr.i ) to label %UnifiedUnreachableBlock unwind label %lpad ; <i32>:0 [#uses=0] ... lpad: ; preds = %Unwind.i %eh_ptr = tail call i8* @llvm.eh.exception( ) %eh_select = tail call i32 (i8*, i8*, ...)* @llvm.eh.selector.i32( i32 (...)* @__gxx_personality_v0 , i8* B ) %eh_typeid = tail call i32 @llvm.eh.typeid.for.i32( i8* B ) %tmp15 = icmp eq i32 %eh_select, %eh_typeid br i1 %tmp15, label %bb, label %Unwind ... Unwind: ; preds = %lpad tail call i32 (...)* @_Unwind_Resume( i8* %eh_ptr ) ; <i32>:1 [#uses=0] unreachable ... } However to get correct functioning the following adjustments have to be made: (1) B has to be appended to the selector call for A: %eh_select.i = tail call i32 (i8*, i8*, ...)* @llvm.eh.selector.i32( i8* %eh_ptr.i, i32 (...)* @__gxx_personality_v0 , i8* A ) -> %eh_select.i = tail call i32 (i8*, i8*, ...)* @llvm.eh.selector.i32( i8* %eh_ptr.i, i32 (...)* @__gxx_personality_v0 , i8* A , i8* B ) Otherwise if a B is thrown in f then the program will terminate. Here the main difficulty is finding the selector for the A landing pad. (2) The Unwind_Resume call needs to be turned into a jump to the handler code for the B case (this is half-way through lpad), something like: Unwind.i: ; preds = %lpad.i ; was an invoke of @_Unwind_Resume here ; was a call to @llvm.eh.exception here ; was a call to @llvm.eh.selector.i32 here %eh_typeid = tail call i32 @llvm.eh.typeid.for.i32( i8* B ) %tmp15 = icmp eq i32 %eh_select, %eh_typeid br i1 %tmp15, label %bb, label %Unwind This can all go wrong in several ways: (a) if the A landing pad already (for some reason) tested for B then step (1) will cause strangeness. However I think we can say that the code was relying on undefined behaviour and not worry about this. (b) there needs to be some analysis to find @_Unwind_Resume calls reachable from the A landing pad. They may also be reachable from other landing pads, so code duplication may be required. This could get complicated. (c) the selector call for the Unwind_Resume invoke needs to be determined and the result of the (modified) A selector call needs to be used instead. Since the selector may be shared by several landing pads this could get tricky too. To my mind a perfect solution would be: (i) Find a trick (like my catch-all trick) so that invokes always branch to the unwind label when an exception unwinds through it. In other words, preserve the traditional semantics of invoke. This makes life much simpler at the level of the IR optimizers. (ii) Add a pass that knows about Unwind_Resume and does the kind of transform described above when it isn't too hard. If it is too hard then it can give up because of (i). (iii) At codegen time, completely abandon invoke semantics (invoke doesn't exist there anyway) and exploit the way the unwinder works as much as possible. I've applied the darwin CFA unwinder change and will see if I can find a way of getting (i). Ciao, Duncan.
How about this?
Index: gcc-4.2.llvm/gcc/except.c
==================================================================---
gcc-4.2.llvm.orig/gcc/except.c 2007-12-12 20:55:30.000000000 +0100
+++ gcc-4.2.llvm/gcc/except.c 2007-12-12 20:56:19.000000000 +0100
@@ -4053,9 +4053,9 @@
{
/* The default c++ routines aren't actually c++ specific, so use those.
*/
/* LLVM local begin */
- llvm_unwind_resume_libfunc = llvm_init_one_libfunc ( USING_SJLJ_EXCEPTIONS ?
-
"_Unwind_SjLj_Resume"
- :
"_Unwind_Resume");
+ llvm_unwind_resume_libfunc + llvm_init_one_libfunc(USING_SJLJ_EXCEPTIONS ?
+ "_Unwind_SjLj_Resume" :
"_Unwind_Resume_or_Rethrow");
/* LLVM local end */
}
On Dec 12, 2007, at 11:01 AM, Duncan Sands wrote:> Hi Dale, > >> No, I don't want to change the semantics of invoke, at least I don't >> think so. >> When inlining, I want the inlined throw to reach cleanup code as it >> does. >> But I want the Unwind_Resume call that ends the cleanup code to be >> replaced with a control transfer to the handler (or cleanup) in the >> calling >> function, i.e. the inliner needs to know the semantics of >> Unwind_Resume. > > it seems to me that this is extremely tricky to do in general, > though it > is simpler if you suppose the IR was produced by gcc. Consider this > example:Yes, working my way through the tests I've hit several optimizer issues. Your example is even worse though:) In general the eh.exception stuff needs special handling beyond what I said.> To my mind a perfect solution would be: > (i) Find a trick (like my catch-all trick) so that invokes always > branch to the unwind > label when an exception unwinds through it. In other words, > preserve the traditional > semantics of invoke. This makes life much simpler at the level of > the IR optimizers. > (ii) Add a pass that knows about Unwind_Resume and does the kind of > transform described > above when it isn't too hard. If it is too hard then it can give up > because of (i). > (iii) At codegen time, completely abandon invoke semantics (invoke > doesn't exist there > anyway) and exploit the way the unwinder works as much as possible. > > I've applied the darwin CFA unwinder change and will see if I can > find a way of getting (i).Thanks. I did find a sledgehammer that works: replace the catch-all with a cleanup, and don't inline anything that calls Unwind_Resume. I'm trying to come up with something less stupid.> Index: gcc-4.2.llvm/gcc/except.c > ==================================================================> --- gcc-4.2.llvm.orig/gcc/except.c 2007-12-12 20:55:30.000000000 +0100 > +++ gcc-4.2.llvm/gcc/except.c 2007-12-12 20:56:19.000000000 +0100 > @@ -4053,9 +4053,9 @@ > { > /* The default c++ routines aren't actually c++ specific, so use > those. */ > /* LLVM local begin */ > - llvm_unwind_resume_libfunc = llvm_init_one_libfunc > ( USING_SJLJ_EXCEPTIONS ? > - > "_Unwind_SjLj_Resume" > - : > "_Unwind_Resume"); > + llvm_unwind_resume_libfunc > + llvm_init_one_libfunc(USING_SJLJ_EXCEPTIONS ? > + "_Unwind_SjLj_Resume" : > "_Unwind_Resume_or_Rethrow"); > /* LLVM local end */ > }Seems unlikely, but I'll try it.