On Nov 24, 2010, at 5:36 AM, Bill Wendling wrote:> On Nov 24, 2010, at 4:58 AM, John McCall wrote: >> >>> The object 'f' is in a different cleanup area than 'b' which, in turn >>> is in a different area than 'z'. These three regions should point to >>> three different landing pads (or different offsets in the same landing >>> pad), which (I believe) are encoded in IR by being declared after >>> different dispatch instructions, all of which within the same region. >> >> Nope. Three regions, three landing pads, three dispatch instructions. >> (actually four if Foo::Foo() can throw). The Baz-destructing region >> chains to the Bar-destructing region which chains to the Foo-destructing >> region which chains to the catching region; the first three are >> cleanup-only. >> > Ah ha! I think I had a different mental model than you did. Or at least I remembered things differently from the discussion. :-) For me, there is one dispatch per region, which is why I had the region number associated with the invokes as well as the "unwind to" edge coming from them. (See my response to Renato for a description.) I'll think more about your model...Hmm. The difference between our models is actually in what we're calling a region. In your model, adding a cleanup doesn't require a new region; you just create a new landing pad which does the cleanup and then branches to (I guess) the landing pad of its containing cleanup. So landing pads are many-to-one with regions and dispatch instructions. In my model, every independent landing pad is necessarily a region. So in my model, there is also one dispatch per region, but there are more regions. So, in this example: A { A(); ~A() throw(); }; void foo() throw() { A x; b(); } Your model has this as follows, modulo syntax: entry: %x = alloca %A invoke void @A::A(%A* %x) to label %succ0 unwind label %lp0 region 0 succ0: invoke void @b() to label %succ1 unwind label %lp1 region 0 succ1: call void @A::~A(%A* %x) nounwind ret void lp0: call void @A::~A(%A* %x) nounwind br label %lp1 lp1: dispatch region 0 [filter i8* null] # no resume edge Whereas my model has this as follows: entry: %x = alloca %A invoke void @A::A(%A* %x) to label %succ0 unwind label %lp0 region 0 succ0: invoke void @b() to label %succ1 unwind label %lp1 region 1 succ1: call void @A::~A(%A* %x) nounwind ret void lp0: call void @A::~A(%A* %x) nounwind dispatch region 0 [], resume label %lp1 lp1: dispatch region 1 [filter i8* null] # no resume edge I think my model has some nice conceptual advantages; for one, it gives you the constraint that only EH edges and dispatch instructions can lead to landing pads, which I think will simplify what EH preparation has to do. But I could be convinced. John.
On Nov 24, 2010, at 11:18 AM, John McCall wrote:> On Nov 24, 2010, at 5:36 AM, Bill Wendling wrote: >> Ah ha! I think I had a different mental model than you did. Or at least I remembered things differently from the discussion. :-) For me, there is one dispatch per region, which is why I had the region number associated with the invokes as well as the "unwind to" edge coming from them. (See my response to Renato for a description.) I'll think more about your model... > > Hmm. The difference between our models is actually in what we're calling a region. In your model, adding a cleanup doesn't require a new region; you just create a new landing pad which does the cleanup and then branches to (I guess) the landing pad of its containing cleanup. So landing pads are many-to-one with regions and dispatch instructions. In my model, every independent landing pad is necessarily a region. So in my model, there is also one dispatch per region, but there are more regions. > > So, in this example: > A { A(); ~A() throw(); }; > void foo() throw() { > A x; > b(); > } > > Your model has this as follows, modulo syntax: > entry: > %x = alloca %A > invoke void @A::A(%A* %x) to label %succ0 unwind label %lp0 region 0 > succ0: > invoke void @b() to label %succ1 unwind label %lp1 region 0 > succ1: > call void @A::~A(%A* %x) nounwind > ret void > lp0: > call void @A::~A(%A* %x) nounwind > br label %lp1 > lp1: > dispatch region 0 [filter i8* null] # no resume edge > > Whereas my model has this as follows: > entry: > %x = alloca %A > invoke void @A::A(%A* %x) to label %succ0 unwind label %lp0 region 0 > succ0: > invoke void @b() to label %succ1 unwind label %lp1 region 1 > succ1: > call void @A::~A(%A* %x) nounwind > ret void > lp0: > call void @A::~A(%A* %x) nounwind > dispatch region 0 [], resume label %lp1 > lp1: > dispatch region 1 [filter i8* null] # no resume edge > > I think my model has some nice conceptual advantages; for one, it gives you the constraint that only EH edges and dispatch instructions can lead to landing pads, which I think will simplify what EH preparation has to do. But I could be convinced. >Notice though that we would need to keep both the EH edge from the invoke and the region numbers, which you said was redundant (in your first email). :-) Consider if there were several cleanup landing pads leading in a chain, through successive dispatches, down to the last dispatch that decides which catch to execute: lp0: call void @A::~A(%A* %x) dispatch region 0 [], resume label %lp1 lp1: call void @B::~B(%B* %y) dispatch region 1 [], resume label %lp2 lp2: call void @C::~C(%C* %z) dispatch region 2 [], resume label %lp3 lp3: dispatch region 3 [filter i8* null] # no resume edge If we have inlining of any of those d'tor calls, we may lose the fact that the dispatch in, say, lp1 is a cleanup that lands onto the region 2 dispatch in lp2. We know from experience that once this information is lost, it's *really* hard to get it back again. That's what DwarfEHPrepare.cpp is all about, and I want to get rid of that pass because it's a series of hacks to get around our poor EH support. On a personal level, I'm not a big fan of forcing constraints on code when they aren't needed. We had problems in the past (with inlining into a cleanup part of an EH region) with enforcing the invariant that only unwind edges may jump to a landing pad. If we go back to the example above, if @C::~C() were inlined, it could come to pass that the dispatch is placed into a separate basic block and that the inlined code branches into that new basic block thus violating the constraint. -bw
On Nov 24, 2010, at 5:07 PM, Bill Wendling wrote:> On Nov 24, 2010, at 11:18 AM, John McCall wrote: > >> On Nov 24, 2010, at 5:36 AM, Bill Wendling wrote: >>> Ah ha! I think I had a different mental model than you did. Or at least I remembered things differently from the discussion. :-) For me, there is one dispatch per region, which is why I had the region number associated with the invokes as well as the "unwind to" edge coming from them. (See my response to Renato for a description.) I'll think more about your model... >> >> Hmm. The difference between our models is actually in what we're calling a region. In your model, adding a cleanup doesn't require a new region; you just create a new landing pad which does the cleanup and then branches to (I guess) the landing pad of its containing cleanup. So landing pads are many-to-one with regions and dispatch instructions. In my model, every independent landing pad is necessarily a region. So in my model, there is also one dispatch per region, but there are more regions. >> >> So, in this example: >> A { A(); ~A() throw(); }; >> void foo() throw() { >> A x; >> b(); >> } >> >> Your model has this as follows, modulo syntax: >> entry: >> %x = alloca %A >> invoke void @A::A(%A* %x) to label %succ0 unwind label %lp0 region 0 >> succ0: >> invoke void @b() to label %succ1 unwind label %lp1 region 0 >> succ1: >> call void @A::~A(%A* %x) nounwind >> ret void >> lp0: >> call void @A::~A(%A* %x) nounwind >> br label %lp1 >> lp1: >> dispatch region 0 [filter i8* null] # no resume edge >> >> Whereas my model has this as follows: >> entry: >> %x = alloca %A >> invoke void @A::A(%A* %x) to label %succ0 unwind label %lp0 region 0 >> succ0: >> invoke void @b() to label %succ1 unwind label %lp1 region 1 >> succ1: >> call void @A::~A(%A* %x) nounwind >> ret void >> lp0: >> call void @A::~A(%A* %x) nounwind >> dispatch region 0 [], resume label %lp1 >> lp1: >> dispatch region 1 [filter i8* null] # no resume edge >> >> I think my model has some nice conceptual advantages; for one, it gives you the constraint that only EH edges and dispatch instructions can lead to landing pads, which I think will simplify what EH preparation has to do. But I could be convinced. > > Notice though that we would need to keep both the EH edge from the invoke and the region numbers, which you said was redundant (in your first email). :-) Consider if there were several cleanup landing pads leading in a chain, through successive dispatches, down to the last dispatch that decides which catch to execute:> lp0: > call void @A::~A(%A* %x) > dispatch region 0 [], resume label %lp1 > lp1: > call void @B::~B(%B* %y) > dispatch region 1 [], resume label %lp2 > lp2: > call void @C::~C(%C* %z) > dispatch region 2 [], resume label %lp3 > lp3: > dispatch region 3 [filter i8* null] # no resume edge > > If we have inlining of any of those d'tor calls, we may lose the fact that the dispatch in, say, lp1 is a cleanup that lands onto the region 2 dispatch in lp2.What you mean is that, given a resume or invoke edge, we need to be able to find the dispatch for the target region. There are ways to make that happen without tagged edges; for example, you could make the landing pad a special subclass of BasicBlock with a pointer to the dispatch, although that'd be a fairly invasive change. Tagging the edges solves the problem for clients with a handle on an edge; clients that want to go from (say) a dispatch to its landing pad(s) will still have trouble.> We know from experience that once this information is lost, it's *really* hard to get it back again. That's what DwarfEHPrepare.cpp is all about, and I want to get rid of that pass because it's a series of hacks to get around our poor EH support.While I agree that the hacks need to go, there is always going to be some amount of custom codegen for EH just to get the special data to flow properly from landing pads to the eh.exception intrinsic and the dispatch instruction. My design would give you some very powerful assumptions to work with to implement that: both the intrinsic calls and the dispatch would always be dominated by the region's landing pad, which would in turn only be reachable via specific edges. I don't know how you're planning on implementing this without those assumptions, but if you say you don't need them, that certainly diminishes the appeal of my proposal.> On a personal level, I'm not a big fan of forcing constraints on code when they aren't needed. We had problems in the past (with inlining into a cleanup part of an EH region) with enforcing the invariant that only unwind edges may jump to a landing pad. If we go back to the example above, if @C::~C() were inlined, it could come to pass that the dispatch is placed into a separate basic block and that the inlined code branches into that new basic block thus violating the constraint.I'm not suggesting that the landing pad has to be the same block as the block with the dispatch instruction. That's an obviously unreasonable constraint; in fact, it wouldn't hold for the vast majority of C++ cleanups, since we generally have to invoke destructors so we can terminate if they throw. Since — unlike present-day branches between cleanups — dispatch edges will be initially opaque to the optimizer, I don't see much danger of it producing invalid code from otherwise-valid transformations. John.