Alternative exception handling proposal

Here is an alternative proposal to Bill's. It is closer to what we already
have (which can be seen as a good or a bad thing!), and is also closer to
what gcc does. It is more incremental than Bill's and introduces fewer
new concepts.

Executive summary

Executive summary

Remove the personality and list of catches out of eh.selector and stick them
directly on invoke instructions.

The invoke instruction

The invoke instruction is modified by adding extra catch info to it:

= invoke ()
to label unwind label

Here comprises all the stuff we currently bundle into eh.selector,
namely the personality function, a list of catch type infos and filters, and
a flag indicating a cleanup (in eh.selector the flag is the number 0). A
possible syntax:

= [personality ] [cleanup] [catches ]

Here’s an example where there is no cleanup and there are two handlers:

invoke void @_Z3foov()
to label %invcont unwind label %catch.handlers personality
@__gxx_personality_v0 catches %struct.__fundamental_type_info_pseudo* @_ZTIi,
%struct.__pointer_type_info_pseudo* @_ZTIPKc

Note that unlike in Bill’s proposal there isn’t a label for each catch
object, just one global label (the existing unwind label).

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.

I marked the personality function as being optional since in fact if there
is only a cleanup and no catches or filters then the personality is not needed
(recent gcc implements this optimization).

Note that there is no longer any need to append a catch-all (as llvm-gcc
sometimes has to) or do any other mucking around to get proper cleanup
semantics, the list of catches just corresponds directly to those in the
original function.

This is similar to my first proposal. But it also suffers from a major problem, which stopped that proposal dead in its tracks. Namely, you have information in one place which needs to be shared in two different, but possibly disjoint, places: the type, filters, and personality information. In order to generate the EH tables, you need to know this information at the throw site and at the place which makes the decision of which catch handler to invoke. There is no guarantee in your proposal that the invoke can be associated with the proper eh.selector call. And because of (C++) cleanups and inlining, it’s the rule not the exception.

Example, if you have this:

invoke void @foo()
to label %invcont unwind label %lpad

personality @__gxx_personality_v0
catches %struct.__fundamental_type_info_pseudo* @_ZTIi,
%struct.__pointer_type_info_pseudo* @_ZTIPKc

lpad:
call void @bar(%A* %a) ; a cleanup
br label %ppad

ppad:
%eh_ptr = call i8* llvm.eh.exception()
%eh_sel = call i32 llvm.eh.selector()
; code to clean up.

The call to @bar can insert an arbitrarily complex amount of code, including invokes, llvm.eh.selector calls, etc. Because there is no relationship between the invoke of @foo and %eh_sel in ppad, we lose that information at ppad, which is where we need it.

The code in DwarfEHPrepare::MoveExceptionValueCalls that moves the call to llvm.eh.exception into the landing pad, and which you want to do for llvm.eh.selector as well, will only complicate matters. It would introduce PHI nodes for llvm.eh.selector values like it currently does for llvm.eh.exception values.

invoke void @_Z3foov()
to label %“3” unwind label %lpad personality @__gxx_personality_v0
catches %struct.__fundamental_type_info_pseudo* @_ZTIi,
%struct.__pointer_type_info_pseudo* @_ZTIPKc, i8* null

The use of “i8* null” here is just as bad as it is for the current llvm.eh.selector call. There’s no way to determine from this list whether the last value is truly the catchall value or for a catch handler.

“10”: ; preds = %“5”
%exc_ptr31 = call i8* @llvm.eh.exception()
%filter32 = call i32 @llvm.eh.selector()
invoke void @_ZN1CD1Ev(%struct.A* %memtmp)
to label %“11” unwind label %fail personality @__gxx_personality_v0
catches i32 1 ; ← this is an empty filter, i.e. one that catches everything

Filter? What do you mean by this?

How is it codegened

Code generation is like now, only simpler. The DwarfEHPrepare pass, which
currently has to do crazy things about catch-all’s will still exist but much
simpler: it just has to ensure that the value of eh.selector makes sense no
matter where it is declared, like it does already for eh.exception, in fact
the same code could be used for both.

Currently when the code generators see an invoke, they rummage around in
the landing pad looking for an eh.selector call so they can extract the
catch info (and if it doesn’t find one, it tries to look in sucessor blocks
because loop passes like to move eh.selector there…). Now they don’t have
to rummage because the needed information is directly attached to the invoke.

See my point above about the eh.selector call.

Inlining

Many a plausible seeming exception handling scheme has fallen by the way-side
because it interacts poorly with inlining.

Here is how inlining would work with this scheme. It’s pretty close to how
it works right now. Suppose you have

invoke void @foo()
to label %invcont unwind label %lpad

and you want to inline foo. Suppose foo contains an invoke:

invoke void @bar()
to label %invcont2 unwind label %lpad2

Then after inlining you have an invoke of bar in which foo’s catch info
has been appended to bar’s:

invoke void @bar()
to label %invcont2 unwind label %lpad2

What does appending to mean? If the
personality functions are different then you are in trouble and need to
disallow the inlining! The cleanup flag is the “or” of the foo and bar
cleanup flags. The catches are the bar catches followed by the foo
catches.

Now suppose foo contains a call:

call void @baz()

Then after inlining you have an invoke of baz with a copy of foo’s
catch info:

invoke void @baz()
to label %continue unwind label %lpad

In short inlining is exactly as before, except that you have to append foo’s
catch info to everything you inline.

Now suppose foo has an instance of the rewind instruction:

rewind i8* %exception, i32 %selector

Then after inlining this becomes:

eh.set.exception(%exception)
eh.set.selector(%selector)
br label %lpad

The calls to eh.set.exception and eh.set.selector ensure that in %lpad the
calls to eh.exception and eh.selector return the right values.

Will everything work?

I am confident that it will work fine, for a very simple reason: this is exactly
what gcc does! Of course it is in disguise, a wolf in sheep’s clothing some
might say :slight_smile: In fact moving closer to gcc like this is probably the best way
to be sure that exception handling works properly, since gcc is what everyone
tests against whether we like it or not (for example libstdc++ exploits some
details of how gcc implements exception handling that are not specified by the
standard, i.e. are implementation defined, and this has caused trouble for LLVM
in the past).

I would suspect that GCC has proper EH table generation mostly because it keeps tables on the side; whereas we do not and cannot. Our current EH tables are pretty poor. I would love to be able to generate tables similar to theirs.

What does it solve?

It solves the problem of eh.selector calls being moved far away from landing
pads by optimizers (or being placed far away from landing pads by front end
authors who don’t know that they need to be careful). It solves the problem
that LLVM inlining doesn’t interact well with cleanups which is the reason why
llvm-gcc sticks catch-alls in funny places and has to stand on its head to get
things working. This was essentially due to (1) invoke semantics (invoke
always unwinds to the landing pad), and (2) inlining an _Unwind_Resume through
an invoke resulting in catch info being placed on the _Unwind_Resume and far
away from the call that actually throws the exception. People who’ve worked
in the guts of LLVM exception handling know what I’m talking about :slight_smile: All of
this badness just goes away with this scheme.

Bad things


I hate the way dwarf typeinfos, catches and filters are being baked into the
IR. Maybe metadata (see above) helps with this.

Metadata cannot be counted on to remain.

How will your implementation allow us to remove the Horrible Hack from DwarfEHPrepare.cpp? Right now we catch and throw at almost every level that the exception can propagate up. How will your proposal solve this?

-bw

You are greatly underestimating the amount of work the inliner has to do under this proposal. In fact, the only thing that your proposal simplifies is DwarfEHPrepare.

The inliner would still need to do two extra things:

1. It would need to adjust landing pads so that they forward selectors they don't understand to the enclosing landing pad. Consider the following code:

void a();
void b() {
   try { a(); } catch (int i) {}
}
void c() {
   try { b(); } catch (float f) {}
}

The landing pad in b() only has one case to worry about, so it's naturally going to immediately enter the catch handler for 'int'. This is obviously semantically wrong if the combined invoke unwinds there.

2. It would need to rewrite calls to _Unwind_Resume on cleanup-only paths if the enclosing invoke has a handler. The EH machinery does not expect a landing pad which claims to handle an exception to just call _Unwind_Resume before handling it; that's why we currently have to use hacks to call _Unwind_Resume_or_Rethrow instead.

Also, some platforms provide an alternative to _Unwind_Resume that doesn't require the compiler to pass the exception pointer; we'd like to be able to use those.

The 'dispatch' instruction simplifies this by presenting an obvious place to rewrite (which doesn't actually require rewriting — you just add/redirect the 'resume' edge to point to the enclosing landing pad).

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.

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!

Then after inlining this becomes:

    eh.set.exception(%exception)
    eh.set.selector(%selector)
    br label %lpad

Using unwind means that the calls to eh.set.exception and eh.set.selector
are no longer needed here. In fact the existing inliner logic for dealing
with "unwind" can be kept unaltered.

Ciao,

Duncan.

Hi Bill,

This is similar to my first proposal.

yup, I still consider your first proposal to have been basically sound.

But it also suffers from a major problem,

which stopped that proposal dead in its tracks. Namely, you have information in
one place which needs to be shared in two different, but possibly disjoint,
places: the type, filters, and personality information. In order to generate the
EH tables, you need to know this information at the throw site and at the place
which makes the decision of which catch handler to invoke. There is no guarantee
in your proposal that the invoke can be associated with the proper eh.selector
call. And because of (C++) cleanups and inlining, it's the rule not the exception.

I disagree that this information is needed anywhere except the invoke. If it
was needed arbitrarily far downstream then of course my proposal would be dead.
But it isn't! Got an example where it is?

Example, if you have this:

invoke void @foo()
to label %invcont unwind label %lpad
personality @__gxx_personality_v0
catches %struct.__fundamental_type_info_pseudo* @_ZTIi,
%struct.__pointer_type_info_pseudo* @_ZTIPKc

lpad:
call void @bar(%A* %a) ; a cleanup
br label %ppad

ppad:
%eh_ptr = call i8* llvm.eh.exception()
%eh_sel = call i32 llvm.eh.selector()
; code to clean up.

The call to @bar can insert an arbitrarily complex amount of code, including
invokes, llvm.eh.selector calls, etc. Because there is no relationship between
the invoke of @foo and %eh_sel in ppad, we lose that information at ppad, which
is where we need it.

It would of course be wrong to expect eh.exception to return the original value
in ppad if you inlined an invoke via the call to @bar, and reached %ppad via the
unwind branch of that invoke because a new exception was thrown. This is not a
problem. Here's how gcc does it. In fact llvm-gcc does exactly the same thing!
In lpad gcc grabs the exception and selector using the equivalent of
eh.exception and eh.selector and stashes the values in local variables. It
then uses those stashed variables everywhere, for example in ppad to do the
comparisons with eh.typeid.for etc. It doesn't try to get the value via
eh.exception in ppad. Since presumably you know this (since llvm-gcc does it)
maybe you were talking about something else?

The code in DwarfEHPrepare::MoveExceptionValueCalls that moves the call to
llvm.eh.exception into the landing pad, and which you want to do for
llvm.eh.selector as well, will only complicate matters. It would introduce PHI
nodes for llvm.eh.selector values like it currently does for llvm.eh.exception
values.

Sure, but that's not a problem because the catch info is only needed at invokes,
there is no need to go searching for it downstream, and I'm not sure why you
think there is such a need.

invoke void @_Z3foov()
to label %"3" unwind label %lpad personality @__gxx_personality_v0
catches %struct.__fundamental_type_info_pseudo* @_ZTIi,
%struct.__pointer_type_info_pseudo* @_ZTIPKc, i8* null

The use of "i8* null" here is just as bad as it is for the current
llvm.eh.selector call. There's no way to determine from this list whether the
last value is truly the catchall value or for a catch handler.

There is a catch-all here only because there is a catch-all in the original
code:

    } catch (...) {
      printf("catchall\n");
    }

In my proposal you don't need to know about catch-all, add special catch-alls
etc. If there was a catch-all in the original code then there is one on the
invoke, otherwise there is not. There is no special treatment of catch-all.

"10": ; preds = %"5"
%exc_ptr31 = call i8* @llvm.eh.exception()
%filter32 = call i32 @llvm.eh.selector()
invoke void @_ZN1CD1Ev(%struct.A* %memtmp)
to label %"11" unwind label %fail personality @__gxx_personality_v0
catches i32 1 ; <- this is an empty filter, i.e. one that catches everything

Filter? What do you mean by this?

http://llvm.org/docs/ExceptionHandling.html#throw_filters

Will everything work?
---------------------

I am confident that it will work fine, for a very simple reason: this is exactly
what gcc does! Of course it is in disguise, a wolf in sheep's clothing some
might say :slight_smile: In fact moving closer to gcc like this is probably the best way
to be sure that exception handling works properly, since gcc is what everyone
tests against whether we like it or not (for example libstdc++ exploits some
details of how gcc implements exception handling that are not specified by the
standard, i.e. are implementation defined, and this has caused trouble for LLVM
in the past).

I would suspect that GCC has proper EH table generation mostly because it keeps
tables on the side; whereas we do not and cannot. Our current EH tables are
pretty poor. I would love to be able to generate tables similar to theirs.

I think you are reading more into the gcc tables than actually exists. The
tables hold a set of nested regions. Each region consists of a set of basic
blocks. There are various types of regions, corresponding to handlers, filters,
cleanups etc. Given a basic block, what happens when an exception is thrown?
You wind up through the enclosing regions, from inner-most to outer-most looking
for what to do. If nothing matches then the exception unwinds out of the
function, otherwise the action specified by the region is taken.

Here is an equivalent way of storing regions, by attaching them to basic blocks:
given a basic block BB, consider all regions that contain BB, and attach their
info to BB in order of inner-most region to outer-most region. That's what
my "catch info" does - and I think it contains all relevant info from the gcc
regions. If so, we have all the same info gcc has, so if gcc can do something
then so can we. You might object that regions can contain multiple basic
blocks, and by attaching info to basic blocks (currently this means to invokes)
you no longer can tell if two basic blocks are in the same region or not. This
is true to some extent (you can reconstruct maximal regions by comparing catch
info on basic blocks) but I don't think it matters for anything, in gcc it is
just an optimization to reduce memory usage and duplicated effort.

I hate the way dwarf typeinfos, catches and filters are being baked into the
IR. Maybe metadata (see above) helps with this.

Metadata cannot be counted on to remain.

Is that also true for global metadata used as an argument to an intrinsic?
Do you have an idea for how to keep catches etc out of the definition of the
IR? I'm worried that if one day we add support for, say, SEH then we will
have to change how the IR is defined again, and that's better avoided.

How will your implementation allow us to remove the Horrible Hack from
DwarfEHPrepare.cpp? Right now we catch and throw at almost every level that the
exception can propagate up. How will your proposal solve this?

The horrible hack is not needed at all, pushing extra catch-alls is not needed
at all - it all goes away. Why were these needed? They were needed to handle
the effects of inlining, in particular that right now when you inline through
an invoke the catch info (contained in the selector) gets attached to the
inlined _Unwind_Resume which is far away from the place you really want it: you
want it on the inlined invoke that the _Unwind_Resume is downstream of. But
notice how inlining works with my scheme (described in my original proposal):
when inlining through an invoke, the catch info for that invoke gets appended
to everything you inline, including the invoke you inline. Thus it occurs in
the right place automatically! It also gets attached to the _Unwind_Resume,
which is also correct. If _Unwind_Resume is replaced with unwind (rewind in
my original proposal, since amended) then you can just replace unwind with a
branch and everything comes out in the wash (it is not obvious that everything
comes out in the wash, but nonetheless it does!).

Ciao,

Duncan.

Hi Duncan,

That would allow you to choose a different personality routine for
every invoke inside the same function (ie. same EH table), which
doesn't make sense to me...

It'd also disassociate the personality routine with the landing pads
(which are used to build the EH table).

cheers,
--renato

Hi Renato,

<catch info> = [personality<ptr>] [cleanup] [catches<list of catches and filters>]

That would allow you to choose a different personality routine for
every invoke inside the same function (ie. same EH table), which
doesn't make sense to me...

if you inline Ada code into C++ code you might get this. That said, I
keep oscillating between having personalities be per-function (and not
allowing inlining if there would be a personality clash) or per invoke.

It'd also disassociate the personality routine with the landing pads
(which are used to build the EH table).

In theory it is possible to have several personality functions per
function, but I'm not sure it is worth the effort of supporting.

In any case this is a minor point, but does show the problem of baking
personality functions, catches etc into the IR: changing your mind about
how you want to do things then involves a big cost...

Ciao,

Duncan.

Indeed.

Not to mention that there is no way to enforce that on a per-call
basis in any language I know. Also, I can't think of a reason for the
front-end to decide that per function call...

One personality definition per function should be enough...

Not to mention that there is no way to enforce that on a per-call
basis in any language I know. Also, I can't think of a reason for the
front-end to decide that per function call...

One personality definition per function should be enough...

It occurs when doing LTO and inlining functions written in one language
into functions written in another, thus my Ada inlined into C++ example.
Mixed language programming is quite common, so it may be worth supporting
this, but it's not clear.

Ciao, Duncan.

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.

Good point.

Still, mixing personalities in the same EH table is scary... :wink: Is
there any compiler that do this?

Apparently the gcc people are working on this as part of their LTO support.

Ciao,

Duncan.

Hi John,

Inlining
--------

Many a plausible seeming exception handling scheme has fallen by the way-side
because it interacts poorly with inlining.

Here is how inlining would work with this scheme. It's pretty close to how
it works right now. Suppose you have

   invoke void @foo()
           to label %invcont unwind label %lpad<foo catch info>

and you want to inline foo. Suppose foo contains an invoke:

   invoke void @bar()
           to label %invcont2 unwind label %lpad2<bar catch info>

Then after inlining you have an invoke of bar in which foo's catch info
has been appended to bar's:

   invoke void @bar()
           to label %invcont2 unwind label %lpad2<joined catch info>

What does appending<foo catch info> to<bar catch info> mean? If the
personality functions are different then you are in trouble and need to
disallow the inlining! The cleanup flag is the "or" of the foo and bar
cleanup flags. The catches are the bar catches followed by the foo
catches.

You are greatly underestimating the amount of work the inliner has to do under this proposal. In fact, the only thing that your proposal simplifies is DwarfEHPrepare.

needless to say, I disagree :slight_smile: See below.

The inliner would still need to do two extra things:

1. It would need to adjust landing pads so that they forward selectors they don't understand to the enclosing landing pad. Consider the following code:

  void a();
  void b() {
    try { a(); } catch (int i) {}
  }
  void c() {
    try { b(); } catch (float f) {}
  }

The landing pad in b() only has one case to worry about, so it's naturally going to immediately enter the catch handler for 'int'. This is obviously semantically wrong if the combined invoke unwinds there.

Here is the LLVM IR for functions "b" and "c".

define void @_Z1bv() {
entry:
   invoke void @_Z1av()
           to label %return unwind label %"<L1>" personality @__gxx_personality_v0 catches %struct.__fundamental_type_info_pseudo* @_ZTIi

"<L1>": ; preds = %entry
   %exc_ptr = tail call i8* @llvm.eh.exception()
   %filter = tail call i32 @llvm.eh.selector()
   %typeid = tail call i32 @llvm.eh.typeid.for(i8* bitcast (%struct.__fundamental_type_info_pseudo* @_ZTIi to i8*))
   %0 = icmp eq i32 %filter, %typeid
   br i1 %0, label %"<L2>", label %"<bb 5>"

"<bb 5>": ; preds = %"<L1>"
   unwind

"<L2>": ; preds = %"<L1>"
   %D.2112_2 = tail call i8* @__cxa_begin_catch(i8* %exc_ptr) nounwind
   tail call void @__cxa_end_catch() nounwind
   ret void

return: ; preds = %entry
   ret void
}

define void @_Z1cv() {
entry:
   invoke void @_Z1bv()
           to label %return unwind label %"<L1>" personality @__gxx_personality_v0 catches %struct.__fundamental_type_info_pseudo* @_ZTIf

"<L1>": ; preds = %entry
   %exc_ptr = tail call i8* @llvm.eh.exception()
   %filter = tail call i32 @llvm.eh.selector()
   %typeid = tail call i32 @llvm.eh.typeid.for(i8* bitcast (%struct.__fundamental_type_info_pseudo* @_ZTIf to i8*))
   %0 = icmp eq i32 %filter, %typeid
   br i1 %0, label %"<L2>", label %"<bb 5>"

"<bb 5>": ; preds = %"<L1>"
   unwind

"<L2>": ; preds = %"<L1>"
   %D.2106_2 = tail call i8* @__cxa_begin_catch(i8* %exc_ptr) nounwind
   tail call void @__cxa_end_catch() nounwind
   ret void

return: ; preds = %entry
   ret void
}

Here is the LLVM IR when you inline "b" into "c" according to the rules I
stated:

define void @_Z1cv() {
entry:
   invoke void @_Z1av()
           to label %return.i unwind label %"<L1>.i" personality @__gxx_personality_v0 catches %struct.__fundamental_type_info_pseudo* @_ZTIi, %struct.__fundamental_type_info_pseudo* @_ZTIf

"<L1>.i": ; preds = %entry
   %exc_ptr.i = call i8* @llvm.eh.exception()
   %filter.i = call i32 @llvm.eh.selector()
   %typeid.i = call i32 @llvm.eh.typeid.for(i8* bitcast (%struct.__fundamental_type_info_pseudo* @_ZTIi to i8*))
   %0 = icmp eq i32 %filter.i, %typeid.i
   br i1 %0, label %"<L2>.i", label %"<bb 5>.i"

"<bb 5>.i": ; preds = %"<L1>.i"
   br label %"<L1>"

"<L2>.i": ; preds = %"<L1>.i"
   %D.2112_2.i = call i8* @__cxa_begin_catch(i8* %exc_ptr.i) nounwind
   call void @__cxa_end_catch() nounwind
   br label %_Z1bv.exit

return.i: ; preds = %entry
   br label %_Z1bv.exit

_Z1bv.exit: ; preds = %return.i, %"<L2>.i"
   br label %return

"<L1>": ; preds = %"<bb 5>.i"
   %exc_ptr = tail call i8* @llvm.eh.exception()
   %filter = tail call i32 @llvm.eh.selector()
   %typeid = tail call i32 @llvm.eh.typeid.for(i8* bitcast (%struct.__fundamental_type_info_pseudo* @_ZTIf to i8*))
   %1 = icmp eq i32 %filter, %typeid
   br i1 %1, label %"<L2>", label %"<bb 5>"

"<bb 5>": ; preds = %"<L1>"
   unwind

"<L2>": ; preds = %"<L1>"
   %D.2106_2 = tail call i8* @__cxa_begin_catch(i8* %exc_ptr) nounwind
   tail call void @__cxa_end_catch() nounwind
   ret void

return: ; preds = %_Z1bv.exit
   ret void
}

Looks good, right? It is true that I forgot to say that front-ends have to
output code to continue unwinding an exception if they don't recognise the
selector. I amended my proposal to say that it is unspecified whether a
non-matching exception results in a branch to the unwind label or not - this
forces front-ends to rewind any exceptions that don't match any of their
catches.

2. It would need to rewrite calls to _Unwind_Resume on cleanup-only paths if the enclosing invoke has a handler. The EH machinery does not expect a landing pad which claims to handle an exception to just call _Unwind_Resume before handling it; that's why we currently have to use hacks to call _Unwind_Resume_or_Rethrow instead.

Nope, because when inlining through an invoke the inliner appends the invoke's
catch info to everything it inlines. This is the key point that makes the
cleanup problem go away. As an optimization it is possible to convert an invoke
of _Unwind_Resume into a branch, but I think it would be better for front-ends
to output an "unwind" instruction instead of _Unwind_Resume and do this
optimization on "unwind".

Also, some platforms provide an alternative to _Unwind_Resume that doesn't require the compiler to pass the exception pointer; we'd like to be able to use those.

The code generators can lower the "unwind" instruction to whatever the platform
prefers to use to continue unwinding the exception. For example on ARM with
EABI you wouldn't want unwind to be lowered to _Unwind_Resume.

The 'dispatch' instruction simplifies this by presenting an obvious place to rewrite (which doesn't actually require rewriting — you just add/redirect the 'resume' edge to point to the enclosing landing pad).

As explained above no rewriting is needed with this scheme.

Best wishes,

Duncan.

Well, I was talking about the proposal you actually made, as opposed to your second proposal made ten hours after my response. :slight_smile:

I agree that your second proposal solves the obvious inlining problems with your first proposal.

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,

Well, I was talking about the proposal you actually made, as opposed to your second proposal made ten hours after my response. :slight_smile:

I agree that your second proposal solves the obvious inlining problems with your first proposal.

sorry for the mistake in my original proposal - that's what comes from doing
things at high speed :slight_smile: I actually realized I had made this mistake soon
after I had sent it, while trying to get to sleep, but waited until the next
morning to send the amendment. It is of course essential in order to have
inlining work.

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.

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.

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.

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. :slight_smile:

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. :slight_smile:

John.