> > What? Yet another EH proposal?! This one is different from the > others in that > I'm planning to start implementing this shortly. But I want your > feedback! I've > all ready gotten a lot of feedback from Chris, John, Jim, Eric, and > many others. > Now is your turn! > > Please read this proposal and send me your comments, suggestions, > and concerns. > > -bw >Bill, 1) it is good to see that the "exception regions" idea has been abandoned, it is mathematically inconsistent with modern optimization theory, and at best would require extra passes to translate into/outof that representation form. 2) it is good to see the prohibition of mixing by inlining of exception handing from different languages, it has always been my thought that this cannot be well defined in general, different languages not only have different control-flow semantics for exception handling, but the different type systems (what is a derived type and what constitutes "type match" for exceptions) are not always going to be compatible. 3) the other non-cosmetic portion of this proposal boils down to: a) every invoke must point to a landingpad-block b) every landingpad-block must start with a catch specification (LandingpadInst) c) catch specifications (LandingpadInst) must not occur anywhere else don't take "cosmetic" as a criticism, I think gcc suffers from bad cosmetics, and llvm benefits from good cosmetics. The logic I see is that we already have PHINode and TerminateInst that have explicit restrictions, so it makes sense that if catch- specifications have restrictions they too should be Instructions rather than Intrinsics. 3.b) I have been thinking about other possible control-flow-graph invariants of the landingpad blocks and the catch blocks that they lead to, but so far have not come up with very muchl, I wonder if anyone else is thinking about this...?... for example cleanups come before __cxa_begin_catch, but it isn't clear what is a cleanup and what isn't other than what comes before a __cxa_begin_catch and what comes after ? however, using that as the definition of cleanup, for C++ any InvokeInst that is so identified as cleanup then its only operand has to be terminate (I think, someone please correct me if I've made an incorrect conclusion here). 3.c) I have been thinking about whether the original source code structure of try-catch statements can be reconstructed from the IR, are two try-catches nested, either in the try or the catch part, or are they disjoint, and can the cleanups be identified as such at the IR level or have things potentially been mixmastered up too much after optimization. I wonder if anyone else is thinking about this also...?... 4) IIUC, llvm has inherited a bug from gcc where the debugger cannot let the user know an exception is going to be uncaught until after the stack has been unwound -- contrary to the design intentions of the unwind library that most exception implementations are based on (with a two phase unwind algorithm) -- which creates a problem for the debugger user. so, the question is will there be a specific recognizable "catch all types" type that can occur in the landingpad's catch list ? and will there be a __llvm_personality_v0 that is designed to do the right thing for this case. yes, I know this is a can-of-worms, it will break gcc compatibility, but then perhaps we can be the motivation for gnu folks to fix their implementation, be the leader rather than the follower.!. 4.b) it is not at all clear from your write up what the "cleanup" option for a landingpad is, and how this is used when both cleanups AND catches are necessary in a given try-catch source code statement, including if one of the user specified catches is a catch-all. 5) its not clear from your email what is done with the result value of the landingpad instruction, but I presume that your intent is that this does not change from the current scheme where the "llvm.eh.typeid.for()" is called and its result is compared with the landingpad instruction's result... ...and then a miracle happens in CodeGen, and most of the intrinsics are thrown away and the hard register contents at the resumption at a landingpad from an Unwind include the value that llvm.eh.typeid.for() would have returned... this is the sort of thing I'm talking about when I imply that the current scheme is poorly documented! Also, what is going to happen for the case of cleanup AND catches, currently the result of not only the llvm.eh.select() result is cached, but in fact the complete decoding of it relative to all the llvm.eh.typeid.for() calls is cached, then the cleanup code executed, THEN finally the already decoded value is used to "switch" from the landing pad to the correct catch-block. who is going to generate all that code, is it still going to be explicit in the IR, or is CodeGen going to now be responsible creating it. 6) it would be nice if the existing UnwindInst could be retained. I wince at naming an instruction "Resume" since in the English language it is so ambiguous (resume normal execution following the conclusion of handing an exception, verses resume throwing an exception). IE cosmetics do matter. 7) there are still lots of other intrinsics/routines involved: __cxa_allocate_exception __cxa_throw, cxa_rethrow __cxa_begin_catch(), __cxa_end_catch although these particular ones seem to be the easiest to document as they do seem to be translated verbatim (no CodeGen miracles). 8) I really like the idea of "terminate" being one of the options to the landingpad instruction, it makes identification of abnormal code more direct (otherwise control- flow analysis has to be done to see if __terminate() is reachable to conclude that something is abnormal code, and I really don't like that analysis, it seems too error- prone as __terminate() might be reachable for other reasons (not that I have come up with such a scenario yet, but I think I might be able to), and this conclusion would then be ambiguous). Even if support for the terminate option required a new __llvm_personality_v0 and a new Unwind library function, I am still in favor of having and using it. But I suspect that CodeGen can lower this into the same old MC branch to a block that only contains __terminate() that we currently see in IR, and a new personality and Unwind aren't necessary, but would still be a nice optimization. sincerely, Peter Lawrence. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20110727/e7a0f051/attachment.html>
On Jul 27, 2011, at 10:27 AM, Peter Lawrence wrote:> 3.b) I have been thinking about other possible control-flow-graph invariants of the > landingpad blocks and the catch blocks that they lead to, but so far have not come up > with very muchl, I wonder if anyone else is thinking about this...?... > > for example cleanups come before __cxa_begin_catch, but it isn't clear what is a cleanup > and what isn't other than what comes before a __cxa_begin_catch and what comes after ?The EH representation is independent of things like this.> however, using that as the definition of cleanup, for C++ any InvokeInst that is so > identified as cleanup then its only operand has to be terminate (I think, someone > please correct me if I've made an incorrect conclusion here).In C++, any destructor call executed as an EH cleanup would need to be an invoke whose unwind edge leads to a landing pad with a catch-all and a call to std::terminate(). However, after inlining etc., I don't know that this gives us any interesting invariants in the IR.> 3.c) I have been thinking about whether the original source code structure of try-catch > statements can be reconstructed from the IR, are two try-catches nested, either in the > try or the catch part, or are they disjoint, and can the cleanups be identified as such at > the IR level or have things potentially been mixmastered up too much after optimization. > I wonder if anyone else is thinking about this also...?...It would be difficult to reliably reconstruct try/catch statements from the IR even before optimization.> 4) IIUC, llvm has inherited a bug from gcc where the debugger cannot let the user know an exception is > going to be uncaught until after the stack has been unwound -- contrary to the design intentions of the > unwind library that most exception implementations are based on (with a two phase unwind algorithm) -- > which creates a problem for the debugger user.I don't see this as a compiler bug. I can't imagine any personality function design which would let debuggers interrupt or control unwinding without hooking libUnwind, short of requiring every single call to have an associated landing pad which the personality always lands at, even if there's nothing to do there. That will never, ever be acceptable.> and will there be a __llvm_personality_v0 that is designed to do the right thing for this case. > > yes, I know this is a can-of-worms, it will break gcc compatibility, but then perhaps we can be the > motivation for gnu folks to fix their implementation, be the leader rather than the follower.!.Using our own personality function would not necessarily break GCC compatibility; we'd just need to provide it in compiler-rt or something.> 4.b) it is not at all clear from your write up what the "cleanup" option for a landingpad is, and > how this is used when both cleanups AND catches are necessary in a given try-catch source > code statement, including if one of the user specified catches is a catch-all.The 'cleanup' bit says that the personality function needs to land there even if there's no handler. And yes, it's technically redundant with a catch-all handler.> 5) its not clear from your email what is done with the result value of the landingpad instruction, > but I presume that your intent is that this does not change from the current scheme where > the "llvm.eh.typeid.for()" is called and its result is compared with the landingpad instruction's > result... > > ...and then a miracle happens in CodeGen, and most of the intrinsics are thrown away and the > hard register contents at the resumption at a landingpad from an Unwind include the value that > llvm.eh.typeid.for() would have returned...The miracle is just that llvm.eh.typeid.for are replaced with constant values after all interprocedural optimizations are finished. Unfortunately, since the range of constants is global over the function, there is no other reasonable way to do this while maintaining correctness across inlining and dead code elimination.> Also, what is going to happen for the case of cleanup AND catches, currently the result of not > only the llvm.eh.select() result is cached, but in fact the complete decoding of it relative to > all the llvm.eh.typeid.for() calls is cached, then the cleanup code executed, THEN finally the > already decoded value is used to "switch" from the landing pad to the correct catch-block. > > who is going to generate all that code, is it still going to be explicit in the IR, or is CodeGen going > to now be responsible creating it.It will still be explicit in the IR.> 6) it would be nice if the existing UnwindInst could be retained. I wince at naming an instruction > "Resume" since in the English language it is so ambiguous (resume normal execution following > the conclusion of handing an exception, verses resume throwing an exception). IE cosmetics > do matter.I would be fine with still calling resume "unwind", but the new instruction does need to carry extra information.> 7) there are still lots of other intrinsics/routines involved: > __cxa_allocate_exception > __cxa_throw, cxa_rethrow > __cxa_begin_catch(), __cxa_end_catch > although these particular ones seem to be the easiest to document as they do seem to be > translated verbatim (no CodeGen miracles).These are not intrinsics, and it's not our responsibility to document them. If you're borrowing the Itanium C++ EH routines to implement exceptions in your own language, then you need to understand how Itanium C++ EH works, and you should read their documentation.> 8) I really like the idea of "terminate" being one of the options to the landingpad > instruction, it makes identification of abnormal code more direct (otherwise control- > flow analysis has to be done to see if __terminate() is reachable to conclude that > something is abnormal code, and I really don't like that analysis, it seems too error- > prone as __terminate() might be reachable for other reasons (not that I have come > up with such a scenario yet, but I think I might be able to), and this conclusion would > then be ambiguous)._gxx_personality_v0 can only do its special-case terminate encoding in the LSDA if that's the only possible handler. That means that, for correctness under inlining, front-ends targeting that personality will still always need their landing pads to contain explicit calls to std::terminate(). John.
On Jul 27, 2011, at 11:10 AM, John McCall wrote:>> >> 6) it would be nice if the existing UnwindInst could be retained. I wince at naming an instruction >> "Resume" since in the English language it is so ambiguous (resume normal execution following >> the conclusion of handing an exception, verses resume throwing an exception). IE cosmetics >> do matter. > > I would be fine with still calling resume "unwind", but the new instruction > does need to carry extra information.It should not be called "unwind" since it is different than the old thing. I would be supportive of "resume_unwind" or something like that though. -Chris
On Jul 27, 2011, at 10:27 AM, Peter Lawrence wrote:> 1) it is good to see that the "exception regions" idea has been abandoned, it is mathematically > inconsistent with modern optimization theory, and at best would require extra passes to translate > into/outof that representation form. >Yeah. I didn't want to obscure the main proposal by inappropriate nomenclature.> 3.b) I have been thinking about other possible control-flow-graph invariants of the > landingpad blocks and the catch blocks that they lead to, but so far have not come up > with very muchl, I wonder if anyone else is thinking about this...?... > > for example cleanups come before __cxa_begin_catch, but it isn't clear what is a cleanup > and what isn't other than what comes before a __cxa_begin_catch and what comes after ? >As John mentioned, the EH representation is independent (and indeed ignorant) of things like this. The front-ends need to generate the correct code.> 4.b) it is not at all clear from your write up what the "cleanup" option for a landingpad is, and > how this is used when both cleanups AND catches are necessary in a given try-catch source > code statement, including if one of the user specified catches is a catch-all. >It's something I noticed from GCC's exception handling tables. If there's a cleanup that's been inlined, then even if that cleanup has "catches", it still is marked as a cleanup. As John mentioned, it's so that the personality function knows to stop at that function to run the cleanup.> 5) its not clear from your email what is done with the result value of the landingpad instruction, > but I presume that your intent is that this does not change from the current scheme where > the "llvm.eh.typeid.for()" is called and its result is compared with the landingpad instruction's > result... >The values the landingpad returns are those that are set by the personality function upon re-entry into the function. On X86, it's the EAX and EDX registers. One of those values is a pointer to the exception handling object. The other is a "selector" value, that we can then use to determine which (if any) of the clauses should be run.> ...and then a miracle happens in CodeGen, and most of the intrinsics are thrown away and the > hard register contents at the resumption at a landingpad from an Unwind include the value that > llvm.eh.typeid.for() would have returned... >The llvm.eh.typeid.for is a hold-over from the old design. It's returns a constant value that can be compared against the "selector" the personality function returns. It remains because it gives an explicit representation of how the decision table of which catch to call is executed. It's similar to a series of if-then-elses.> this is the sort of thing I'm talking about when I imply that the current scheme is poorly documented! >Indeed! And one of the outcomes will be much better documentation.> Also, what is going to happen for the case of cleanup AND catches, currently the result of not > only the llvm.eh.select() result is cached, but in fact the complete decoding of it relative to > all the llvm.eh.typeid.for() calls is cached, then the cleanup code executed, THEN finally the > already decoded value is used to "switch" from the landing pad to the correct catch-block. > > who is going to generate all that code, is it still going to be explicit in the IR, or is CodeGen going > to now be responsible creating it. >It will be explicit in the IR, as it is now. :)> 6) it would be nice if the existing UnwindInst could be retained. I wince at naming an instruction > "Resume" since in the English language it is so ambiguous (resume normal execution following > the conclusion of handing an exception, verses resume throwing an exception). IE cosmetics > do matter. >The UnwindInst carries too much history with it to remain, and the new behavior is different than what the 'unwind' instruction. I agree with Chris that a mix of something like "resumeunwind" would make more sense.> 7) there are still lots of other intrinsics/routines involved: > __cxa_allocate_exception > __cxa_throw, cxa_rethrow > __cxa_begin_catch(), __cxa_end_catch > although these particular ones seem to be the easiest to document as they do seem to be > translated verbatim (no CodeGen miracles). >It would involve hard-coding language-specific calls and ABIs into LLVM. That's something we try to avoid.> 8) I really like the idea of "terminate" being one of the options to the landingpad > instruction, it makes identification of abnormal code more direct (otherwise control- > flow analysis has to be done to see if __terminate() is reachable to conclude that > something is abnormal code, and I really don't like that analysis, it seems too error- > prone as __terminate() might be reachable for other reasons (not that I have come > up with such a scenario yet, but I think I might be able to), and this conclusion would > then be ambiguous). > > Even if support for the terminate option required a new __llvm_personality_v0 and > a new Unwind library function, I am still in favor of having and using it. But I suspect > that CodeGen can lower this into the same old MC branch to a block that only contains > __terminate() that we currently see in IR, and a new personality and Unwind > aren't necessary, but would still be a nice optimization. >After we get the basic functionality down, we can discuss further changes like this. It's good to keep it in mind, though. As John mentioned, it may not be suitable in all cases, but for some it's a potential win. -bw
On Jul 27, 2011, at 11:10 AM, John McCall wrote:> >> 4) IIUC, llvm has inherited a bug from gcc where the debugger >> cannot let the user know an exception is >> going to be uncaught until after the stack has been unwound -- >> contrary to the design intentions of the >> unwind library that most exception implementations are based on >> (with a two phase unwind algorithm) -- >> which creates a problem for the debugger user. > > I don't see this as a compiler bug. I can't imagine any > personality function > design which would let debuggers interrupt or control unwinding > without > hooking libUnwind, short of requiring every single call to have an > associated landing pad which the personality always lands at, even if > there's nothing to do there. That will never, ever be acceptable.John, I'm not able to figure out what you're really trying to say here. I am suggesting that there be a unique function that libUnwind calls in the event it detects that an exception is going to go uncaught all the way out past main, and that the user be able to set a break-point on that function (it could be the existing function "terminate", or a new one created just for this one purpose), so that the stack can be examined before it gets unwound. I'm not sure what you mean by "hooking", "interrupting", or "controlling". I am just suggesting to be allowed to set a break-point on some unique function. I finally dug deeper into the issue and figured out this is actually a problem with DWARF encoding, or the way that type info is encoded by GCC into DWARF and decoded by __gcc_personality from DWARF. (there is a comment somewhere in the LLVM documentation that IIRC seems to imply the problem is with the Types parameters to llvm.eh.select, but that is incorrect, the problem is deeper than that, it is with the underlying DWARF tables). In short the problem is that there is an ambiguity between a cleanup handler having an Action Table entry that looks like .byte 1 ;; Type = 1 (ie #1 entry in Types Table) .byte 0 ;; Next = 0 (ie none, ie this is the list terminator for this try-statement) together with a corresponding Types Table entry #1 that looks like .long 0 ;; RTTI pointer == NULL and a user explicit try-catchall statement which also contains the exact same DWARF encoding. Instead a user explicit catch-all should have an explicit entry in the Types Table (perhaps "void" could be the "user explicit match anything" marker) rather than containing the NULL value. Right now "cleanups" look to __gcc_personality exactly like "user explicit catch-all", so there is no way for __gcc_personality to tell that something will not be caught (if an exception will only go through cleanups all the way out past main, it "looks" to __gcc_personality that it is actually being caught by "catch-all"s, so __gcc_personality currently cannot figure this out until after the stack is entirely unwound and the user is then SOL). Peter Lawrence. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20110728/ed81de44/attachment.html>
Bill,
something to laugh about.....
I had originally mis-read the llvm eh doc concerning llvm.eh.selector
and llvm.eh.typeid.for,
they are clearly documented as returning something like an index into
a type table (in our
case specifically a DWARF Types Table index),
but I had mis-read llvm.eh.selector to mean it returned the index /
ordinal of which parameter
in its parameter list (not which type in the Type Table) was a match.
this lead to substantial confusion on my part about what "magic" was
taking place during CodeGen.
-Peter Lawrence.
On Jul 28, 2011, at 1:41 AM, Bill Wendling wrote:
>
>> 5) its not clear from your email what is done with the result
>> value of the landingpad instruction,
>> but I presume that your intent is that this does not change from
>> the current scheme where
>> the "llvm.eh.typeid.for()" is called and its result is
compared
>> with the landingpad instruction's
>> result...
>>
> The values the landingpad returns are those that are set by the
> personality function upon re-entry into the function. On X86, it's
> the EAX and EDX registers. One of those values is a pointer to the
> exception handling object. The other is a "selector" value, that
we
> can then use to determine which (if any) of the clauses should be run.
>
>> ...and then a miracle happens in CodeGen, and most of the
>> intrinsics are thrown away and the
>> hard register contents at the resumption at a landingpad from an
>> Unwind include the value that
>> llvm.eh.typeid.for() would have returned...
>>
> The llvm.eh.typeid.for is a hold-over from the old design. It's
> returns a constant value that can be compared against the
> "selector" the personality function returns. It remains because
it
> gives an explicit representation of how the decision table of which
> catch to call is executed. It's similar to a series of if-then-elses.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20110728/3bab677e/attachment.html>