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
-----------------
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:
<result> = invoke <function>(<function args>)
to label <normal label> unwind label <exception label>
<catch info>
Here <catch info> 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:
<catch info> = [personality <ptr>] [cleanup] [catches <list of
catches and filters>]
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.
What about getting rid of invoke?
---------------------------------
If we want to get rid of invoke and allow any instruction in a basic block
to throw, that's not a problem. The unwind label and catch info is then
just attached to the basic block instead. This proposal is orthogonal to
the issue of allowing arbitrary instructions to throw.
The eh.selector intrinsic
-------------------------
The eh.selector intrinsic changes to take no arguments whatsoever, just like
eh.exception:
declare i32 @llvm.eh.selector() nounwind
In the same way as eh.exception gives you the value of the exception register
in a landing pad, eh.selector gives you the value of the selector register.
All the extra gumph currently in eh.selector is moved into invoke instructions
instead.
The meaning of the value of the eh.selector doesn't change, it is the same
as now: the id of the catch (or filter) that the exception matched, or zero
if it matched a cleanup.
It's actually possible to remove eh.selector altogether - see the
"variants"
section below.
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>
The codegenerators would lower this to a call to _Unwind_Resume (the second
selector argument would not be used - it only exists to simplify inlining,
see below - the first, exception, argument would be passed as the parameter of
_UnwindResume). The goal here is to simplify inlining and avoid explicitly
dragging in library functions like _Unwind_Resume. It is possible to do without
this new instruction, but I think it represents an improvement so I include it
here.
An example
----------
Here is a technique for auto-generating examples of the new scheme from
existing LLVM IR (in fact it should be possible to auto-upgrade existing
bitcode using this scheme; I've left out some details which are only needed
for rather crazy IR; I'm not claiming that auto-upgrade is always possible,
just very often possible):
(1) For every invoke instruction, look in its unwind block for the eh.selector
call. Remove the eh.selector arguments, and stick them in the invoke's
catch info instead.
(2) Replace calls to _Unwind_Resume (or _Unwind_Resume_Or_Rethrow which is what
we use right now) with a "rewind" instruction.
That's all folks!
Here's how the new scheme would look for Bill's example (I used the
above recipe
to generate this from the LLVM IR produced by dragonegg):
#include <cstdio>
extern void foo();
struct A { ~A(); };
struct B { ~B(); };
struct C { ~C(); };
void bar() {
try {
foo();
A a;
foo();
B b;
foo();
C c;
foo();
} catch (int i) {
printf("caught int: %d\n", i);
} catch (const char *c) {
printf("caught string: %s\n", c);
} catch (...) {
printf("catchall\n");
}
}
target datalayout =
"e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
target triple = "x86_64-unknown-linux-gnu"
%struct.A = type <{ i8 }>
%struct.__fundamental_type_info_pseudo = type { %struct.__type_info_pseudo }
%struct.__pointer_type_info_pseudo = type { %struct.__type_info_pseudo, i32,
%struct.type_info* }
%struct.__type_info_pseudo = type { i8*, i8* }
%struct.type_info = type opaque
@_ZTIi = external constant %struct.__fundamental_type_info_pseudo
@_ZTIPKc = external constant %struct.__pointer_type_info_pseudo
@.str = private constant [16 x i8] c"caught int: %d\0A\00", align 1
@.str1 = private constant [19 x i8] c"caught string: %s\0A\00", align
1
@.str2 = private constant [9 x i8] c"catchall\00", align 1
@llvm.eh.catch.all.value = linkonce constant i8* null, section
"llvm.metadata"
@llvm.used = appending global [1 x i8*] [i8* bitcast (i8**
@llvm.eh.catch.all.value to i8*)], section "llvm.metadata"
define void @_Z3barv() {
entry:
%memtmp = alloca %struct.A, align 8
%memtmp1 = alloca %struct.A, align 8
%memtmp2 = alloca %struct.A, align 8
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
"3": ; preds = %entry
invoke void @_Z3foov()
to label %"4" unwind label %lpad25 personality
@__gxx_personality_v0
catches %struct.__fundamental_type_info_pseudo* @_ZTIi,
%struct.__pointer_type_info_pseudo* @_ZTIPKc, i8* null
"4": ; preds =
%"3"
invoke void @_Z3foov()
to label %"5" unwind label %lpad26 personality
@__gxx_personality_v0
catches %struct.__fundamental_type_info_pseudo* @_ZTIi,
%struct.__pointer_type_info_pseudo* @_ZTIPKc, i8* null
"5": ; preds =
%"4"
invoke void @_Z3foov()
to label %"6" unwind label %"10" personality
@__gxx_personality_v0
catches %struct.__fundamental_type_info_pseudo*,
%struct.__pointer_type_info_pseudo* @_ZTIPKc, i8* null
"6": ; preds =
%"5"
invoke void @_ZN1CD1Ev(%struct.A* %memtmp)
to label %"7" unwind label %lpad26 personality
@__gxx_personality_v0
catches %struct.__fundamental_type_info_pseudo* @_ZTIi,
%struct.__pointer_type_info_pseudo* @_ZTIPKc, i8* null
"7": ; preds =
%"6"
invoke void @_ZN1BD1Ev(%struct.A* %memtmp1)
to label %"8" unwind label %lpad25 personality
@__gxx_personality_v0
catches %struct.__fundamental_type_info_pseudo* @_ZTIi,
%struct.__pointer_type_info_pseudo* @_ZTIPKc, i8* null
"8": ; preds =
%"7"
invoke void @_ZN1AD1Ev(%struct.A* %memtmp2)
to label %return unwind label %lpad personality @__gxx_personality_v0
catches %struct.__fundamental_type_info_pseudo* @_ZTIi,
%struct.__pointer_type_info_pseudo* @_ZTIPKc, i8* null
"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
lpad26: ; preds = %"6",
%"4"
%exc_ptr29 = call i8* @llvm.eh.exception()
%filter30 = call i32 @llvm.eh.selector()
br label %"11"
"11": ; preds =
%"10", %lpad26
%filt_tmp8.0 = phi i32 [ %filter30, %lpad26 ], [ %filter32, %"10" ]
%exc_tmp5.0 = phi i8* [ %exc_ptr29, %lpad26 ], [ %exc_ptr31, %"10"
]
invoke void @_ZN1BD1Ev(%struct.A* %memtmp1)
to label %"12" unwind label %fail41 personality
@__gxx_personality_v0
catches i32 1 ; <- this is an empty filter, i.e. one that catches everything
lpad25: ; preds = %"7",
%"3"
%exc_ptr27 = call i8* @llvm.eh.exception()
%filter28 = call i32 @llvm.eh.selector()
br label %"12"
"12": ; preds =
%"11", %lpad25
%filt_tmp12.0 = phi i32 [ %filter28, %lpad25 ], [ %filt_tmp8.0,
%"11" ]
%exc_tmp10.0 = phi i8* [ %exc_ptr27, %lpad25 ], [ %exc_tmp5.0,
%"11" ]
invoke void @_ZN1AD1Ev(%struct.A* %memtmp2)
to label %"16" unwind label %fail44 personality
@__gxx_personality_v0
catches i32 1 ; <- this is an empty filter, i.e. one that catches everything
lpad: ; preds = %"8",
%entry
%exc_ptr = call i8* @llvm.eh.exception()
%filter = call i32 @llvm.eh.selector()
br label %"16"
"16": ; preds =
%"12", %lpad
%exc_tmp14.0 = phi i8* [ %exc_ptr, %lpad ], [ %exc_tmp10.0, %"12" ]
%filt_tmp16.0 = phi i32 [ %filter, %lpad ], [ %filt_tmp12.0, %"12"
]
%typeid = call i32 @llvm.eh.typeid.for(i8* bitcast
(%struct.__fundamental_type_info_pseudo* @_ZTIi to i8*))
%0 = icmp eq i32 %filt_tmp16.0, %typeid
br i1 %0, label %"18", label %1
; <label>:1 ; preds =
%"16"
%typeid24 = call i32 @llvm.eh.typeid.for(i8* bitcast
(%struct.__pointer_type_info_pseudo* @_ZTIPKc to i8*))
%2 = icmp eq i32 %filt_tmp16.0, %typeid24
%3 = call i8* @__cxa_begin_catch(i8* %exc_tmp14.0) nounwind
br i1 %2, label %"20", label %"22"
"18": ; preds =
%"16"
%4 = call i8* @__cxa_begin_catch(i8* %exc_tmp14.0) nounwind
%5 = bitcast i8* %4 to i32*
%6 = load i32* %5, align 4
%7 = call i32 (i8*, ...)* @printf(i8* noalias getelementptr inbounds ([16 x
i8]* @.str, i64 0, i64 0), i32 %6)
call void @__cxa_end_catch() nounwind
ret void
"20": ; preds = %1
%8 = call i32 (i8*, ...)* @printf(i8* noalias getelementptr inbounds ([19 x
i8]* @.str1, i64 0, i64 0), i8* %3)
call void @__cxa_end_catch() nounwind
ret void
"22": ; preds = %1
%9 = call i32 @puts(i8* getelementptr inbounds ([9 x i8]* @.str2, i64 0, i64
0))
call void @__cxa_end_catch()
ret void
return: ; preds = %"8"
ret void
fail: ; preds = %"10"
%exc_ptr39 = call i8* @llvm.eh.exception()
%filter40 = call i32 @llvm.eh.selector()
call void @_ZSt9terminatev() noreturn nounwind
unreachable
fail41: ; preds = %"11"
%exc_ptr42 = call i8* @llvm.eh.exception()
%filter43 = call i32 @llvm.eh.selector()
call void @_ZSt9terminatev() noreturn nounwind
unreachable
fail44: ; preds = %"12"
%exc_ptr45 = call i8* @llvm.eh.exception()
%filter46 = call i32 @llvm.eh.selector()
call void @_ZSt9terminatev() noreturn nounwind
unreachable
}
declare void @_Z3foov()
declare void @_ZN1CD1Ev(%struct.A*)
declare void @_ZN1BD1Ev(%struct.A*)
declare void @_ZN1AD1Ev(%struct.A*)
declare void @__cxa_end_catch()
declare i32 @llvm.eh.typeid.for(i8*) nounwind
declare i8* @__cxa_begin_catch(i8*) nounwind
declare i32 @printf(i8* noalias nocapture, ...) nounwind
declare i32 @puts(i8* nocapture) nounwind
declare i8* @llvm.eh.exception() nounwind readonly
declare i32 @llvm.eh.selector() nounwind
declare i32 @__gxx_personality_v0(i32, i64, i8*, i8*)
declare void @_ZSt9terminatev() noreturn nounwind
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.
The eh.set.selector and eh.set.exception intrinsics
---------------------------------------------------
These new intrinsics can be used to set the value of the selector and
exception "registers", i.e. after a call eh.selector after a call to
eh.set.selector(%val) will return %val. Implementing this can be done
easily in DwarfEHPrepare (in fact I considered adding these some time
ago to slightly simplify llvm-gcc lowering of the corresponding gcc
construct).
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.
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 <foo catch info>
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 :) 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).
Ease of implementation
----------------------
Since this scheme is about as close as you can get to the existing scheme,
while still solving all problems the existing scheme has, the implementation
cost is actually quite small - mostly parsing the new invoke instruction and
storing the catch info inside the InvokeInst. A bit of work on the inliner,
a bit of work on the code generators.
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 :) All
of
this badness just goes away with this scheme.
Variants
--------
With this scheme eh.exception and eh.selector are used pretty much the same
everywhere: every time you call eh.exception you likely call eh.selector and
vice versa. We could get rid of eh.selector altogether by having eh.exception
return a pair of values (or a pointer to a stack slot in which codegen
sticks the two register values). This requires a little bit of mucking around
with eh.typeid.for but nothing complicated.
The catch info could be made generic by using a metadata rather than a
list of catches etc. So invoke would in essence have a metadata argument
in much the same way as intrinsics can have a metadata argument. Then
different kinds of metadata could be used for dwarf eh, SEH etc.
Bad things
----------
I hate the way dwarf typeinfos, catches and filters are being baked into the
IR. Maybe metadata (see above) helps with this.
What I didn't cover
-------------------
I should have provided more examples and explanations, but I'd rather go to
bed
instead :)
Ciao,
Duncan.
On Dec 1, 2010, at 1:37 PM, Duncan Sands wrote:> 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: > > <result> = invoke <function>(<function args>) > to label <normal label> unwind label <exception label> <catch info> > > Here <catch info> 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: > > <catch info> = [personality <ptr>] [cleanup] [catches <list of catches and filters>] > > 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* nullThe 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 <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. > > 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 <foo catch info> > > 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 :) 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 :) 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>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20101201/967b949a/attachment.html>
On Dec 1, 2010, at 1:37 PM, Duncan Sands wrote:> 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. 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 %lpadUsing 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 :) 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.
On 1 December 2010 21:37, Duncan Sands <baldrick at free.fr> wrote:> <catch info> = [personality <ptr>] [cleanup] [catches <list of catches and filters>]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,> On 1 December 2010 21:37, Duncan Sands<baldrick at free.fr> wrote: >> <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.
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,>> 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 :) 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.