On Dec 9, 2007, at 1:01 PM, Duncan Sands wrote:> Hi Dale, > >> #include <cstdio> >> class A { >> public: >> A() {} >> ~A() {} >> }; >> void f() { >> A a; >> throw 5.0; >> } >> main() { >> try { >> f(); >> } catch(...) { printf("caught\n"); } >> } > > this example indeed shows the problem. Let me explain to see if we > agree on what > the problem is. Suppose we don't artificially add catch-alls to > selectors. Then > the above example compiles to: > > define void @_Z1fv() { > ... > invoke void @__cxa_throw( something ) noreturn > to label %somewhere unwind label %lpad > ... > lpad: > %eh_ptr = tail call i8* @llvm.eh.exception( ) > %eh_select8 = tail call i32 (i8*, i8*, ...)* > @llvm.eh.selector.i32( i8* %eh_ptr, i8* bitcast (i32 (...)* > @__gxx_personality_v0 to i8*))I wasn't advocating this; agree it is wrong.> tail call i32 (...)* @_Unwind_Resume( i8* %eh_ptr ) > unreachable > ... > } > > define i32 @main() { > entry: > invoke void @_Z1fv( ) > to label %somewhere2 unwind label %lpad2 > ... > lpad2: ; preds = %entry > %eh_ptr = tail call i8* @llvm.eh.exception( ) > %eh_select14 = tail call i32 (i8*, i8*, ...)* > @llvm.eh.selector.i32( i8* %eh_ptr, i8* bitcast (i32 (...)* > @__gxx_personality_v0 to i8*), i8* null ) > print_a_message_and_exit > ... > } > > And this works fine: main calls _Z1fv which throws an exception. > Execution branches to lpad where > (empty) cleanup code is run, then unwinding is resumed. The > unwinder unwinds into main, and branches > to lpad2 (because the selector has a catch-all, the null) which > prints a message and exits. > > If the inliner is run, then we get: > > define i32 @main() { > ... > invoke void @__cxa_throw( something ) noreturn > to label %somewhere unwind label %lpad.i > ... > lpad.i: > %eh_ptr.i = tail call i8* @llvm.eh.exception( ) ; > <i8*> [#uses=2] > %eh_select8.i = tail call i32 (i8*, i8*, ...)* > @llvm.eh.selector.i32( i8* %eh_ptr.i, i8* bitcast (i32 (...)* > @__gxx_personality_v0 to i8*)) > invoke i32 (...)* @_Unwind_Resume( i8* %eh_ptr.i ) > to label %somewhere2 unwind label %lpad2 > ... > lpad2: ; preds = %lpad.i > %eh_ptr = tail call i8* @llvm.eh.exception( ) ; > <i8*> [#uses=2] > %eh_select14 = tail call i32 (i8*, i8*, ...)* > @llvm.eh.selector.i32( i8* %eh_ptr, i8* bitcast (i32 (...)* > @__gxx_personality_v0 to i8*), i8* null ) > print_a_message_and_exit > ... > } > > This is perfectly correct given LLVM invoke semantics. > Unfortunately the unwinder doesn't > know about those :) When run, the exception is thrown but the > unwinder doesn't branch > to lpad.i because the selector doesn't state that that (or any) > exception should be caught. Thus > the program is terminated.OK.> 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. 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.> However if you add a catch-all to the selector instead: > %eh_select8.i = tail call i32 (i8*, i8*, ...)* > @llvm.eh.selector.i32( i8* %eh_ptr.i, i8* bitcast (i32 (...)* > @__gxx_personality_v0 to i8*), i8* null) > then the unwinder does branch to lpad.i. Then the _Unwind_Resume > call causes a branch to > lpad2 and all works perfectly. > > This is why I forcably push a catch-all at the end of each selector > call: because then if an > exception unwinds through an invoke, control always branches to the > landing pad, which is what > LLVM invoke semantics require and the inliner has exploited. > > Unfortunately it seems this breaks the Darwin unwinder.Yes.> 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.) I guess an easy but undesirable fallback position is to tell the inliner not to inline anything that invokes Unwind_Resume. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20071210/1bed530c/attachment.html>
Hi Dale,> > ... Suppose we don't artificially add catch-alls to > > selectors. Then > > the above example compiles to: > > ... > I wasn't advocating this; agree it is wrong.it's maybe not as wrong as it seems since having an empty selector is equivalent to having a cleanup (IIRC) :)> > ... 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. In other words, you want to model LLVM's EH on the way the gcc unwinder works. 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;*/ Ciao, Duncan.
On Dec 10, 2007, at 11:04 AM, Dale Johannesen wrote:> I guess an easy but undesirable fallback position is to tell the > inliner not to inline anything that invokes Unwind_Resume.Indeed this makes all the EH tests work on Darwin (llc and llc-beta; cbe and jit are still broken).
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.