On Dec 2, 2010, at 1:40 AM, Duncan Sands wrote:> Two amendments: > >> The semantics of the invoke instruction are slightly modified: if an exception >> unwinds and it doesn't match anything in the list of catches and filters, >> and there is no cleanup, then control doesn't branch to the unwind label, >> unwinding simply continues out of the function. > > in fact the new semantics would be that if an exception doesn't match then > it is unspecified whether it just unwinds straight through the invoke or > branches to the landing pad. This forces front-ends to output code to > continue unwinding ("rewind" instruction, see below) if the selector does > not match any of the catches on the invoke. This in turn ensures that inlining > works correctly.Okay, this is at least workable, and it has the advantage of requiring less invasive changes to IR structure. It has the disadvantage of requiring us to emit a cleanup path for every handling scope, which is a code size pessimization, but catches are rare relative to cleanups, so that's probably not a problem in practice — and it should be easy to optimize if we really care, since by definition we're talking about a landing pad which doesn't do anything interesting before it tests the selector value. I haven't really thought about how to actually make the data flow correctly, though, so I might let you and Bill fight out the best representation.>> The rewind instruction >> ---------------------- >> >> For extra goodness, I propose introducing a new instruction "rewind" that takes >> an exception pointer and selector value as arguments: >> rewind<ptr>,<i32> > > Actually the existing "unwind" instruction can be repurposed for this, as there > was no real need for rewind to take any arguments. All that is needed is that > unwind be lowered to a call of eh.exception, which is then passed as the > argument to _Unwind_Resume, In fact codegen already does this!I'm unhappy about how this bakes _Unwind_Resume into the backend, particularly since that prevents us from using better alternatives when they're available (e.g. the ARM EH ABI's _cxa_end_cleanup(), which saves code size by not requiring the exception pointer, but which we can only use if we're linking in the C++ standard library). One idea that comes to mind is that we could make Yet Another call-like instruction, a terminator like 'invoke' but with no successors and with the special replaced-with-a-branch behavior when inlined through an invoke. So the front-end could call whatever function it pleases, taking responsibility for passing in the right information. That is, instead of: rewind i8* %saved_exception_ptr you'd have: rewind void @_Unwind_Resume(i8* %saved_exception_ptr) or: rewind void @_cxa_end_cleanup() or, for targets that want special code sequences for resuming, something like: rewind void @llvm.crazytarget.end_cleanup() This also handily eliminates the problem of EHPrepare having to calculate data flow itself to the rewind instruction. Since Gabor already made most of the world use CallSite, adding a third call-like wouldn't be the end of the world, although I think most operations on calls (inlining, etc.) wouldn't apply to rewinds anyway. John.
Hi John,>>> For extra goodness, I propose introducing a new instruction "rewind" that takes >>> an exception pointer and selector value as arguments: >>> rewind<ptr>,<i32> >> >> Actually the existing "unwind" instruction can be repurposed for this, as there >> was no real need for rewind to take any arguments. All that is needed is that >> unwind be lowered to a call of eh.exception, which is then passed as the >> argument to _Unwind_Resume, In fact codegen already does this! > > I'm unhappy about how this bakes _Unwind_Resume into the backend, particularly > since that prevents us from using better alternatives when they're available > (e.g. the ARM EH ABI's _cxa_end_cleanup(), which saves code size by not requiring > the exception pointer, but which we can only use if we're linking in the C++ standard > library).the code generators can lower "unwind" to a call to _cxa_end_cleanup on that platform.> One idea that comes to mind is that we could make Yet Another call-like instruction, > a terminator like 'invoke' but with no successors and with the special > replaced-with-a-branch behavior when inlined through an invoke. So the front-end > could call whatever function it pleases, taking responsibility for passing in the right > information. > > That is, instead of: > rewind i8* %saved_exception_ptr > you'd have: > rewind void @_Unwind_Resume(i8* %saved_exception_ptr) > or: > rewind void @_cxa_end_cleanup() > or, for targets that want special code sequences for resuming, something like: > rewind void @llvm.crazytarget.end_cleanup() > > This also handily eliminates the problem of EHPrepare having to calculate data flow > itself to the rewind instruction. > > Since Gabor already made most of the world use CallSite, adding a third > call-like wouldn't be the end of the world, although I think most operations on calls > (inlining, etc.) wouldn't apply to rewinds anyway.I think this is way too complicated. Ciao, Duncan.
Hi John,>> Two amendments: >> >>> The semantics of the invoke instruction are slightly modified: if an exception >>> unwinds and it doesn't match anything in the list of catches and filters, >>> and there is no cleanup, then control doesn't branch to the unwind label, >>> unwinding simply continues out of the function. >> >> in fact the new semantics would be that if an exception doesn't match then >> it is unspecified whether it just unwinds straight through the invoke or >> branches to the landing pad. This forces front-ends to output code to >> continue unwinding ("rewind" instruction, see below) if the selector does >> not match any of the catches on the invoke. This in turn ensures that inlining >> works correctly. > > Okay, this is at least workable, and it has the advantage of requiring less invasive > changes to IR structure. It has the disadvantage of requiring us to emit a cleanup > path for every handling scope, which is a code size pessimization, but catches are > rare relative to cleanups, so that's probably not a problem in practice — and > it should be easy to optimize if we really care, since by definition we're talking > about a landing pad which doesn't do anything interesting before it tests the selector > value.in Ada cleanups come after handlers, so it's a bit trickier, but yes in theory the code generators could try to remove pointless code of this kind. It needs to be a codegen pass because only then do you know that the inliner will not be run later.> I haven't really thought about how to actually make the data flow correctly, though, > so I might let you and Bill fight out the best representation.I don't much like eh.exception myself - the exception value should really be an argument for the landing pad somehow. That would make it impossible to use wrong, unlike now where front-ends have to be careful to call eh.exception in each landing pad and stash the value in a variable. In practice that's not a problem (for example unlike with the catch info the optimizers will never create problems) but it shows that eh.exception is not a good abstraction. But to some extent this is orthogonal to how catch info is handled. Ciao, Duncan.
On Dec 2, 2010, at 4:45 AM, Duncan Sands wrote:>> I'm unhappy about how this bakes _Unwind_Resume into the backend, particularly >> since that prevents us from using better alternatives when they're available >> (e.g. the ARM EH ABI's _cxa_end_cleanup(), which saves code size by not requiring >> the exception pointer, but which we can only use if we're linking in the C++ standard >> library). > > the code generators can lower "unwind" to a call to _cxa_end_cleanup on that > platform.No, they can't, it's language-library-specific. Only the frontend knows whether it's safe to introduce that dependency. Also, _Unwind_Resume has a slightly different name in ARM sjlj EH; it would be great if codegen didn't have to hard-code all this again.>> One idea that comes to mind is that we could make Yet Another call-like instruction, >> a terminator like 'invoke' but with no successors and with the special >> replaced-with-a-branch behavior when inlined through an invoke. So the front-end >> could call whatever function it pleases, taking responsibility for passing in the right >> information. > > I think this is way too complicated.Then instead of using a call, it can be a special kind of unconditional branch which the inliner rewrites into a normal branch; that would look exactly like your "rewind" instruction except for having a successor. John. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20101202/c2f3d11c/attachment.html>
On Dec 2, 2010, at 4:45 AM, Duncan Sands wrote:>> I'm unhappy about how this bakes _Unwind_Resume into the backend, particularly >> since that prevents us from using better alternatives when they're available >> (e.g. the ARM EH ABI's _cxa_end_cleanup(), which saves code size by not requiring >> the exception pointer, but which we can only use if we're linking in the C++ standard >> library). > > the code generators can lower "unwind" to a call to _cxa_end_cleanup on that > platform.No, they can't, it's language-library-specific. Only the frontend knows whether it's safe to introduce that dependency. Also, _Unwind_Resume has a slightly different name in ARM sjlj EH; it would be great if codegen didn't have to hard-code all this again.>> One idea that comes to mind is that we could make Yet Another call-like instruction, >> a terminator like 'invoke' but with no successors and with the special >> replaced-with-a-branch behavior when inlined through an invoke. So the front-end >> could call whatever function it pleases, taking responsibility for passing in the right >> information. > > I think this is way too complicated.Then instead of using a call, it can be a special kind of unconditional branch which the inliner rewrites into a normal branch; that would look exactly like your "rewind" instruction except for having a successor. John. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20101202/afbb90cc/attachment.html>
On Dec 2, 2010, at 4:59 AM, Duncan Sands wrote:>>> Two amendments: >>> >>>> The semantics of the invoke instruction are slightly modified: if an exception >>>> unwinds and it doesn't match anything in the list of catches and filters, >>>> and there is no cleanup, then control doesn't branch to the unwind label, >>>> unwinding simply continues out of the function. >>> >>> in fact the new semantics would be that if an exception doesn't match then >>> it is unspecified whether it just unwinds straight through the invoke or >>> branches to the landing pad. This forces front-ends to output code to >>> continue unwinding ("rewind" instruction, see below) if the selector does >>> not match any of the catches on the invoke. This in turn ensures that inlining >>> works correctly. >> >> Okay, this is at least workable, and it has the advantage of requiring less invasive >> changes to IR structure. It has the disadvantage of requiring us to emit a cleanup >> path for every handling scope, which is a code size pessimization, but catches are >> rare relative to cleanups, so that's probably not a problem in practice — and >> it should be easy to optimize if we really care, since by definition we're talking >> about a landing pad which doesn't do anything interesting before it tests the selector >> value. > > in Ada cleanups come after handlers, so it's a bit trickier, but yes in theory > the code generators could try to remove pointless code of this kind.Well, remember, this optimization is only actually possible if there aren't any cleanups. :)> It needs to be a codegen pass because only then do you know that the inliner > will not be run later.Right, this is what I had in mind.>> I haven't really thought about how to actually make the data flow correctly, though, >> so I might let you and Bill fight out the best representation. > > I don't much like eh.exception myself - the exception value should really be an > argument for the landing pad somehow.I agree — I personally prefer functional-style IRs to SSA, in part because they make it relatively painless to introduce opaque values along special edges like this. But I don't think we'll be able to sneak that change into this redesign. :) John.