I agree, but I don't know the right solution for this :)

Some thoughts on exception handling intrinsics.

(1) eh.typeid.for
This has two problems that I see:
(a) the typeid depends on the personality function: if we have multiple
personalities in one function, each one is going to have its own list of
typeinfos (Ada typeinfos for the Ada personality, C++ typeinfos for the
C++ personality etc). The unwinder gives us the typeinfo index into the
typeinfo list for the personality function.
(b) the typeid depends on the function you are in. Maybe not a big deal,
but it's kind of weird that you can call eh.typeid.for with the same argument
in different functions and get different results.

Solutions:
(x) The radical solution: get rid of eh.typeid.for by modifying
eh.selector to return the typeinfo itself rather than the index of the
typeinfo. This has the advantage of reducing the number of concepts
that eh users need to know about, and turns the notion of "typeid"
into a codegen detail. It also would remove the dependence on the
personality function. At codegen time typeids would need to be
reintroduced. Code like this (pseudo code!):
%gv = eh.selector(...)
if (%gv == typeinfo1)
do_something;
else if (%gv == typeinfo2)
do_something else;
would need to be expanded into:
%id = eh.old_style_selector(...);
switch (%id) {
case 1:
%gv = typeinfo1;
case 2:
%gv = typeinfo2;
...
}
if (%gv == typeinfo1)
do_something;
else if (%gv == typeinfo2)
do_something else;

Hopefully this expanded code would be efficiently simplified by running a few
optimizer passes before codegen really begins. I don't know if this kind of
approach fits the way codegen is organized.

(y) Add the personality function as an additional eh.typeid.for argument.
This would solve (a) but not (b).

(2) eh.selector
This intrinsic has a sensible meaning: check if the exception belongs to
one of a list of typeinfos, and return which one it matches. However the
current implementation means it only works if placed in a landing pad, and
even if originally placed in one it can be moved around by the optimizers.
This makes the intrinsic fragile - I expect it to cause more trouble once
more than just llvm-gcc starts generating it.
So let me make a radical suggestion which would result in eh.selector working
no matter where it is placed, even if it is in another function! [This has
some runtime cost, however some simple analysis can eliminate the cost if
eh.selector is in a landing pad or not far from one - more on this later].
Here is the idea: at codegen time expand
%id = eh.selector(exception, personality, typeinfo1, ...)
into
invoke @​_Unwind_Resume(exception) unwind %classify <= place personality and
typeinfo list in exception table for this invoke
unreachable
classify:
%id = selector_register_value
i.e. rethrow the exception simply for the purpose of classifying it.
Since we synthesize the invoke at codegen time there is no problem
attaching the personality and typeinfos to it.
Now the obvious objection is that this is costly. So here's an optimization:
if the eh.selector is in the landing pad for an invoke, add the personality
and typeinfos to that invoke, and just inspect the selector_register_value,
i.e. do what we do right now. More generally, if we can work out which
invoke(s) the exception parameter came from, then we can avoid the rethrow
and add the typeinfos to those invokes.
Thus eh.selector would work anywhere, and the current implementation can be
thought of as an optimization. The hacky workaround
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070611/050446.html
that I suggested for this PR can then be seen as a poor man's version of the
suggested optimization method: trying to back assign eh.selector to a set of
invokes.

Additional problems with the current eh.selector lowering:
(1) if two eh.selector intrinsics are used in the same landing
pad, the result may well be wrong. What happens now is that the
typeinfos are concatenated (in reverse order). Suppose the second
eh.selector has, for example, a catch-all typeinfo argument, while
the first has a specific typeinfo, say T. If an instance of T is
thrown then the first selector will not return the typeid
of T. That is because the catch-all typeinfo of the other
selector will be encountered first by the unwind code, so
that is what will always be returned. This would be fixed
by my suggestion for eh.selector lowering in the previous
comment.
(2) The first argument to eh.selector is a pointer to an exception,
but this argument is ignored. The selector is always taken to be for
the current landing pad i.e. the current exception, even if some
previous exception is actually passed as the argument.

(1) if two eh.selector intrinsics are used in the same landing
pad, the result may well be wrong.

Likewise for two filters or mixes of filters and selectors. This
is going to happen, because catches can be nested inside filters
in C++, and the only way we have to represent this for the moment
is one or more eh.filter followed by one or more eh.selector intrinsics.
[All of the catche and filter info needs to be attached to each
invoke inside the inner scope]. Amusingly enough, llvm-convert
doesn't handle even the nested catch case right now - only the
inner catches are taken into account.

failing test

The test above fails with llvm-as < test | opt -std-compile-opts | llc -enable-eh
Seems to be the same basic problem, the loop optimizers have split a critical edge into a landing pad.

Split is done in LoopSimplify::SplitBlockPredecessors which does not know about landing pads, and needs to.

Split is done in LoopSimplify::SplitBlockPredecessors which does not know
about landing pads, and needs to.

It sounds like this problem needs to get a bug report of its own,
since (if I understand right) it is a bug in loop splitting, and
not really due to the design of exception handling (this bug).

I don't think so; the loop splitting is an issue only because of the underlying problem:
"Exception handling typeinfos and personality functions are currently
supplied by an intrinsic (eh.selector or eh.filter) that is placed in
the invoke landing pad. This is a flawed scheme: the optimizers can move
the intrinsic out of the landing pad."
It is true that the short-term fix I have in mind is to duplicate the landing pad somehow (the IR->machine level conversion looks more promising than the loop splitting at this point) rather than fixing the underlying design problem.

If I understand right, whats happening here is that before the Bar
call there is a store to %tmp31. This store has been moved after
the call to Bar, into both the unwind and normal code paths. This
results in

    invoke void @​Bar( i64 %tmp22, %struct.Range* %effectiveRange )
                    to label %invcont23 unwind label %unwind.loopexit

...
unwind.loopexit: ; preds = %cond_true
store i64 %tmp31.tmp.0, i64* %tmp31, align 4
br label %unwind

unwind: ; preds = %unwind.loopexit, %entry
%eh_ptr = call i8* @​llvm.eh.exception( )

in which the calls to the exception handling intrinsics have been
pushed by the store to a later basic block. Tricky :) I will
think about it.

That's not exactly how this arose. Originally there was a single landing pad, unwind, which could be reached from either Foo or Bar (the latter inside a loop). The loop optimizer decided to split the
(abnormal) Bar->unwind edge to put some code on it; the new block unwind.loopexit becomes the new landing pad for Bar, since it's reachable only by throwing. However unwind.loopexit does not have an eh.selector in it. Is it sufficient for the loop code that does this (see comment 7) to add one, do you think?

How about simply removing the assertion (SelectionDAGISel.cpp:4495)?
This routine, copyCatchInfo, is taking the selector call in unwind,
and applying the typeinfos in it to unwind.loopexit. This is exactly
what we want in this case! I'm pretty sure I only added the assertion
because it seemed like a funky case that might cause problems elsewhere.
For example, the selector register will be marked live-in on both
unwind and unwind.loopexit. Will the register value from unwind.loopexit
survive to be used in unwind, or will it get borked somehow because it is
also marked live-in on unwind? Since I had no testcase, and I couldn't
see how this could happen in practice (hah!) it seemed best to just abort.
I don't have time tonight to check that everything is fine if the assertion
is removed, maybe tomorrow.

I tried that and (after turning off various other checking that it broke) the EH tables did not look right, although I don't grok them well enough to be sure. The trouble is that the original 'unwind' is still a valid landing pad, reached from Foo.

The tables look fine to me, so I've committed this. It's not clear to
me if the exception and exception selector registers are being handled
properly because I don't understand ppc assembler.

Looks good. I thought there had to be a 1-1 correspondence between selectors and landing pads, I think, but there doesn't. Thanks for fixing this.

If you are happy, can you please check in the testcase.

Some additional thoughts on this: it may be possible to eliminate the
list of typeinfos from eh.selector altogether. Currently eh.selector
is used in three different way (that should probably be separated):

(1) as a way of getting hold of the value in the selector register
(set by the runtime before it branches to a landing pad)
(2) as a way of specifying the personality function
(3) as a way of giving a list of typeids the exception should be
compared against. This includes the "catch-all" typeid (language
dependent) without which we don't know how to codegen invoke properly.
There may also be filter expressions.

As far as (1) is concerned there is no real problem if the selector
call occurs far away from the landing pad, because we could do the
following implementation: at codegen time, create a new variable
to hold the selector value, copy the register into it at the start
of each landing pad, and have eh.selector just read the variable
and return its values. Now that codegen has dominator info, it
shouldn't be hard to introduce a new variable like this at codegen
time. The same can be done for eh.exception.

As for (2), since currently we only support one personality per
function, it doesn't matter where in the function it is; the same
goes for specifying the "catch-all" typeid: we could have a special
intrinsic for saying what the personality and catch-all are, and
it would just need to be placed somewhere in the function. I don't
want to discuss this further here.

So now for (3). I'm not going to discuss filters here - I plan
to create a special PR for discussion of how filters could be
eliminated altogether from LLVM exception handling.

What is the list of typeids in the selector call for?
The eh handler code generated by llvm-gcc looks like this:

compare the exception with typeinfo T1
if it matches, branch to B1
otherwise, compare the exception with typeinfo T2
if it matches, branch to B2
...
otherwise, compare the exception with typeinfo TN
if it matches, branch to BN

The list of eh.selector typeinfos is exactly B1, ..., BN.
The point I want to make is that we could auto-generate
this list by analysing the code.

Why does eh.selector want to assemble this list at all?
After all, it is possible to query the runtime to find
out whether the exception matches a particular typeinfo,
so this query could simply be performed N times for each
of B1, ..., BN. (I explain below how this querying can
be done). The reason is that two queries is much more
expensive than one, so it is cheaper to perforrm batch queries,
if possible only performing one. In fact the current
scheme only allows for one (batch) query, the query being
specified by the list of typeinfos in eh.selector.

Thus we could do the following scheme: remove the list
of typeinfos from eh. In fact remove eh.selector altogether
and instead have an exception comparison intrinsic:
llvm.eh.compare(exception_pointer, typeinfo). [This also
means we can remove eh.typeid]. At codegen time, starting
from each landing pad, follow the cfg looking for eh.compare
calls (don't follow exception edges, since they give a new
exception). If it can be determined that they occur in a
certain order, generate the corresponding list of typeinfos
and perform one query with that [this should always be the
case when the code was generated by llvm-gcc], otherwise
there will need to be multiple queries. Now I come to think
about it, maybe the llvm.eh.compare call should not take
the exception_pointer as an argument, and just use the
current exception. Otherwise you may have some hassles
if you lose track of the exception pointer (though it's
not fatal since you can always just generate more queries).

So how do you query the runtime to find out whether the
exception belongs to a given list of typeinfos? Easy:
you generate an invoke of a reraise statement, attaching
the list of typeinfos to it, and arrange for it to unwind
to the next instruction. The selector register holds the
result of the query. As an optimization, when downstream
from an identifiable landing pad, the typeinfo list can be
attached to the original invoke [the common case, which
corresponds to what we try to do now].

I plan to create a special PR for discussion of how filters could be
eliminated altogether from LLVM exception handling.

llvm/llvm-bugzilla-archive#2157 .

Note that the llvm.eh.compare intrinsic would be readonly (and even
readnone in the typeinfo parameter) making it easy for standard
optimizations (correlated expressions...) to eliminate pointless
comparisons, giving automatic exception handling simplification.

Also note that the required analysis at the codegen level is
analogous to the problem of synthesizing switch statements out
of a series of comparisons and conditional branches.

Duncan, This is done, no? :-)

Bill fixed this.

mentioned in issue llvm/llvm-bugzilla-archive#1682

mentioned in issue llvm/llvm-bugzilla-archive#1914