llvm dev - May 2015 - [LLVMdev] New EH representation for MSVC compatibility

On Fri, May 15, 2015 at 5:27 PM, Kaylor, Andrew <andrew.kaylor at
intel.com>
wrote:
>  I like the way this sorts out with regard to funclet code generation.
> It feels very natural for Windows EH, though obviously not as natural for
> non-Windows targets and I think it is likely to block some optimizations
> that are currently possible with those targets.
>
Right, it will block some of today's optimizations by default. I'm OK
with
this because we can add those optimizations back by checking if the
personality is Itanium-family (sjlj, arm, or dwarf), and optimizing EH
codepaths is not usually performance critical.

> > If the unwind label is missing, then control leaves the function after
> the EH action is completed. If a function is inlined, EH blocks with
> missing unwind labels are wired up to the unwind label used by the inlined
> call site.
>
>
>
> Is this saying that a “missing” unwind label corresponds to telling the
> runtime to continue the search at the next frame?
>
Yep. For the C++ data structure it would simply be a missing or null
operand.

> Your example looks wrong in this regard, unless I’m misunderstanding it.
> It looks like any exceptions that aren’t caught in that function will lead
> to a terminate call.
>
Well, those are the intended semantics of noexcept, unless I'm mistaken.
And the inliner *should* wire up the unwind edge of the terminateblock to
the unwind edge of the inlined invoke instruction, because it's natural to
lower terminateblock to a catch-all plus termination call block. I wanted
to express that as data, though, so that in the common case that the
noexcept function is not inlined, we can simply flip the "noexcept"
bit in
the EH info. There's a similar optimization we can do for Itanium that we
miss today.
> Invokes that are reached after a catchblock without following any unwind
> edges must transitively unwind to the first catchend block that the
> catchblock unwinds to.
>
>
>
> I’m not sure I understand this correctly.  In particular, I’m confused
> about the roles of resume and catchend.
>
catchendblock is really there to support figuring out which calls were
inside the catch scope. resume has two roles: moving to the next EH action
after a cleanup, and transitioning from the catch block back to normal
control flow. Some of my coworkers said it should be split into two
instructions for each purpose, and I could go either way.

> > %val = cleanupblock <valty> unwind label %nextaction
>
>
>
> Why isn’t this a terminator?  It seems like it performs the same sort of
> role as catchblock, except presumably it is always entered.  I suppose
> that’s probably the answer to my question, but it strikes me as an
> ambiguity in the scheme.  The catchblock instruction is more or less a
> conditional branch whereas the cleanupblock is more like a label with a
> hint as to an unconditional branch that will happen later.  And I guess
> that’s another thing that bothers me -- a resume instruction at the end of
> a catch implementation means something subtly different than a resume
> instruction at the end of a cleanup implementation.
>
Yeah, reusing the resume instruction for both these things might not be
good. I liked not having to add more terminator instructions, though. I
think most optimizations will not care about the differences between the
two kinds of resume. For CFG formation purposes, it either has one
successor or none, and that's enough for most users.

I felt that cleanupblock should not be a terminator because it keeps the IR
more concise. The smaller an IR construct is, the more people seem to
understand it, so I tried to go with that.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150518/04fb58fd/attachment.html>

I hadn’t noticed the “noexcept” specifier in your example.  That clears up part
of my concerns, but I still have some problems.


With regard to the multiple meanings of ‘resume’ I am more concerning about
developers who are reading the IR understanding it than about passes operating
on it.  Apart from making it harder to debug problems related to control flow at
resume instructions I think this makes it more likely that code which mishandles
it will be introduced down the road.  If I’m reading things correctly, a resume
instruction in your proposal could mean:

a) We’re done handling this exception, continue normal execution at this label.
b) We’re done handling this exception, continue execution in an enclosing catch
handler at this label.
c) We’re done executing this termination handler, check the catch handler at
this label to see if it can handle the current exception.
d) We’re done executing this termination handler, now execute the termination
handler at this label.
e) We’re done executing this termination handler, continue handling the
exception in the runtime.

I suppose (a) and (b) are more or less the same and it doesn’t entirely matter
whether the destination is in normal code or exception code.  In practical terms
(c) and (d) may be the same also, but logically, in terms of how the runtime
works, they are different.  I’m pretty sure there’s a gap in my understanding of
your proposal because I don’t understand how e() is represented at all.

As an exercise, I tried to work through the IR that would be produced in the
non-optimized case for the following code:

void test() {
  try {
    Obj o1;
    try {
      f();
    } catch (int) {}
    Obj o2;
    try {
      g();
    } catch (int) {}
    h();
  } catch (int) {}
}

Here’s what I came up with:

define void @foo() personality i32 (...)* @__CxxFrameHandler3 {
  %e.addr = alloca i32
  invoke void @f(i32 1)
    to label %cont1 unwind label %cleanup.Obj
cont1:
  invoke void @g(i32 2)
    to label %cont2 unwind label %cleanup.Obj.1
cont2:
  invoke void @h(i32 2)
    to label %cont3 unwind label %cleanup.Obj.2
cont3:
  call void @~Obj()
  call void @~Obj()
  br label %return
return:
  ret void

cleanup.Obj:
  cleanupblock unwind label %maycatch.int
  call void @~Obj()
  resume label %maycatch.int

maycatch.int:
  catchblock void [i8* @typeid.int, i32 7, i32* %e.addr]
    to label %catch.int unwind label %catchend
catch.int:
  resume label %cont1
catchend:
  resume

cleanup.Obj.1:
  cleanupblock unwind label %maycatch.int.1
  call void @~Obj()
  call void @~Obj()
  resume label %maycatch.int.1

maycatch.int.1:
  catchblock void [i8* @typeid.int, i32 7, i32* %e.addr]
    to label %catch.int.1 unwind label %catchend.1
catch.int.1:
  resume label %cont2
catchend.1:
  resume


cleanup.Obj.2:
  cleanupblock unwind label %maycatch.int.2
  call void @~Obj()
  call void @~Obj()
  resume label %maycatch.int.2

maycatch.int.2:
  catchblock void [i8* @typeid.int, i32 7, i32* %e.addr]
    to label %catch.int.2 unwind label %catchend.2
catch.int.2:
  resume label %return
catchend.2:
  resume
}

I don’t know if I got that right, but it seems to me that there are a couple of
problems with this.  Most obviously, there is a good bit of duplicated code here
(which the optimization passes will probably want to combine).

More significantly though is that it doesn’t correctly describe what happens if
a non-int exception is thrown in any of the called functions.  For instance, if
a non-int exception is thrown from g() that is caught somewhere further down the
stack, the runtime should call a terminate handler that destructs o1 and then
call a terminate handler that destructs o2.  However, my IR doesn’t describe a
terminate handler that destructs just o2 and I don’t know how I could get it to
do so within the scheme that you have proposed.

Do you have a way to handle this case that I haven’t perceived?


In a mostly unrelated matter, have you thought about what needs to be done to
prevent catchblock blocks from being combined?  For example, suppose you have
code that looks like this:

void test() {
  try {
    f();
  } catch (int) {
    x();
    y();
    z();
  }
  try {
    g();
  } catch (…) {
  }
  try {
    h();
  } catch (int) {
    x();
    y();
    z();
  }
}

I think it’s very likely that if we don’t do anything to prevent it the IR
generated for this will be indistinguishable from the IR generated for this:

void test() {
  try {
    f();
    try {
      g();
    } catch (…) {
    }
    h();
  } catch (int) {
    x();
    y();
    z();
  }
}

In this case that might be OK, but theoretically the calls to f() and h() should
get different states and there are almost certainly cases where failing to
recognize that will cause problems.  What’s more, the same basic pattern arises
for this case:

void test() {
  try {
    f();
  } catch (int) {
    x();
    y();
    z();
  }
  try {
    g();
  } catch (float) {
  }
  try {
    h();
  } catch (int) {
    x();
    y();
    z();
  }
}

But in this case, if we get the state numbering wrong an int-exception from g()
could end up being incorrectly caught by the xyz handler.

BTW, finding cases like this is the primary reason that I’ve been trying to push
my current in-flight patch onto the sinking ship that is our current
implementation.  I mentioned to you before that the test suite I’m using passes
with my proposed patch, but that’s only true with optimizations disabled.  With
optimizations turned on I’m seeing all kinds of fun things like similar handlers
being combined and common instructions being hoisted above a shared(!)
eh_begincatch call in if-else paired handlers.  I don’t know if it will be worth
trying to fix these problems, but seeing them in action has been very
instructive.

-Andy


From: Reid Kleckner [mailto:rnk at google.com]
Sent: Monday, May 18, 2015 11:54 AM
To: Kaylor, Andrew
Cc: LLVM Developers Mailing List; Bill Wendling; Nick Lewycky
Subject: Re: New EH representation for MSVC compatibility

On Fri, May 15, 2015 at 5:27 PM, Kaylor, Andrew <andrew.kaylor at
intel.com<mailto:andrew.kaylor at intel.com>> wrote:
I like the way this sorts out with regard to funclet code generation.  It feels
very natural for Windows EH, though obviously not as natural for non-Windows
targets and I think it is likely to block some optimizations that are currently
possible with those targets.

Right, it will block some of today's optimizations by default. I'm OK
with this because we can add those optimizations back by checking if the
personality is Itanium-family (sjlj, arm, or dwarf), and optimizing EH codepaths
is not usually performance critical.
> If the unwind label is missing, then control leaves the function after the
EH action is completed. If a function is inlined, EH blocks with missing unwind
labels are wired up to the unwind label used by the inlined call site.
Is this saying that a “missing” unwind label corresponds to telling the runtime
to continue the search at the next frame?

Yep. For the C++ data structure it would simply be a missing or null operand.

Your example looks wrong in this regard, unless I’m misunderstanding it.  It
looks like any exceptions that aren’t caught in that function will lead to a
terminate call.

Well, those are the intended semantics of noexcept, unless I'm mistaken. And
the inliner *should* wire up the unwind edge of the terminateblock to the unwind
edge of the inlined invoke instruction, because it's natural to lower
terminateblock to a catch-all plus termination call block. I wanted to express
that as data, though, so that in the common case that the noexcept function is
not inlined, we can simply flip the "noexcept" bit in the EH info.
There's a similar optimization we can do for Itanium that we miss today.
> Invokes that are reached after a catchblock without following any unwind
edges must transitively unwind to the first catchend block that the catchblock
unwinds to.
I’m not sure I understand this correctly.  In particular, I’m confused about the
roles of resume and catchend.

catchendblock is really there to support figuring out which calls were inside
the catch scope. resume has two roles: moving to the next EH action after a
cleanup, and transitioning from the catch block back to normal control flow.
Some of my coworkers said it should be split into two instructions for each
purpose, and I could go either way.
> %val = cleanupblock <valty> unwind label %nextaction
Why isn’t this a terminator?  It seems like it performs the same sort of role as
catchblock, except presumably it is always entered.  I suppose that’s probably
the answer to my question, but it strikes me as an ambiguity in the scheme.  The
catchblock instruction is more or less a conditional branch whereas the
cleanupblock is more like a label with a hint as to an unconditional branch that
will happen later.  And I guess that’s another thing that bothers me -- a resume
instruction at the end of a catch implementation means something subtly
different than a resume instruction at the end of a cleanup implementation.

Yeah, reusing the resume instruction for both these things might not be good. I
liked not having to add more terminator instructions, though. I think most
optimizations will not care about the differences between the two kinds of
resume. For CFG formation purposes, it either has one successor or none, and
that's enough for most users.

I felt that cleanupblock should not be a terminator because it keeps the IR more
concise. The smaller an IR construct is, the more people seem to understand it,
so I tried to go with that.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150518/9d9f2bb7/attachment.html>

On 05/18/2015 11:53 AM, Reid Kleckner wrote:
> On Fri, May 15, 2015 at 5:27 PM, Kaylor, Andrew 
> <andrew.kaylor at intel.com <mailto:andrew.kaylor at
intel.com>> wrote:
>
>     I like the way this sorts out with regard to funclet code
>     generation.  It feels very natural for Windows EH, though
>     obviously not as natural for non-Windows targets and I think it is
>     likely to block some optimizations that are currently possible
>     with those targets.
>
>
> Right, it will block some of today's optimizations by default. I'm
OK
> with this because we can add those optimizations back by checking if 
> the personality is Itanium-family (sjlj, arm, or dwarf), and
> optimizing EH codepaths is not usually performance critical.
Leaving aside the rest of the thread, I feel the need to refute this 
point in isolation.  I've found that optimizing (usually simplifying and 
eliminating) exception paths ends up being *extremely* important for my 
workloads.  Failing to optimize exception paths sufficiently tends to 
indirectly hurt things like inlining for example.  Any design which 
starts with the assumption that optimizing exception paths isn't 
important is going to be extremely problematic for me.
>
>     > If the unwind label is missing, then control leaves the function
>     after the EH action is completed. If a function is inlined, EH
>     blocks with missing unwind labels are wired up to the unwind label
>     used by the inlined call site.
>
>     Is this saying that a “missing” unwind label corresponds to
>     telling the runtime to continue the search at the next frame?
>
>
> Yep. For the C++ data structure it would simply be a missing or null 
> operand.
>
>     Your example looks wrong in this regard, unless I’m
>     misunderstanding it.  It looks like any exceptions that aren’t
>     caught in that function will lead to a terminate call.
>
>
> Well, those are the intended semantics of noexcept, unless I'm 
> mistaken. And the inliner *should* wire up the unwind edge of the 
> terminateblock to the unwind edge of the inlined invoke instruction, 
> because it's natural to lower terminateblock to a catch-all plus 
> termination call block. I wanted to express that as data, though, so 
> that in the common case that the noexcept function is not inlined, we 
> can simply flip the "noexcept" bit in the EH info. There's a
similar
> optimization we can do for Itanium that we miss today.
>
>     > Invokes that are reached after a catchblock without following any
>     unwind edges must transitively unwind to the first catchend block
>     that the catchblock unwinds to.
>
>     I’m not sure I understand this correctly.  In particular, I’m
>     confused about the roles of resume and catchend.
>
>
> catchendblock is really there to support figuring out which calls were 
> inside the catch scope. resume has two roles: moving to the next EH 
> action after a cleanup, and transitioning from the catch block back to 
> normal control flow. Some of my coworkers said it should be split into 
> two instructions for each purpose, and I could go either way.
>
>     > %val = cleanupblock <valty> unwind label %nextaction
>
>     Why isn’t this a terminator?  It seems like it performs the same
>     sort of role as catchblock, except presumably it is always
>     entered.  I suppose that’s probably the answer to my question, but
>     it strikes me as an ambiguity in the scheme.  The catchblock
>     instruction is more or less a conditional branch whereas the
>     cleanupblock is more like a label with a hint as to an
>     unconditional branch that will happen later.  And I guess that’s
>     another thing that bothers me -- a resume instruction at the end
>     of a catch implementation means something subtly different than a
>     resume instruction at the end of a cleanup implementation.
>
>
> Yeah, reusing the resume instruction for both these things might not 
> be good. I liked not having to add more terminator instructions, 
> though. I think most optimizations will not care about the differences 
> between the two kinds of resume. For CFG formation purposes, it either 
> has one successor or none, and that's enough for most users.
>
> I felt that cleanupblock should not be a terminator because it keeps 
> the IR more concise. The smaller an IR construct is, the more people 
> seem to understand it, so I tried to go with that.
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150518/10827bc6/attachment.html>

> optimizing EH codepaths is not usually performance critical.
>> Leaving aside the rest of the thread, I feel the need to refute this
point in isolation.  I've found that optimizing (usually simplifying and
eliminating) exception paths ends up being *extremely* important for my
workloads.  Failing to optimize exception paths sufficiently tends to indirectly
hurt things like inlining for example.  Any design which starts with the
assumption that optimizing exception paths isn't important is going to be
extremely problematic for me.
That's interesting.

I wasn't thinking about performance so much as code size in my original
comment.  I've been looking at IR recently where code from multiple
exception handlers was combined while still maintaining the basic control flow
of the EH code.  This kind of optimization is wreaking havoc for our current
MSVC compatible EH implementation (hence the redesign), but I guess the Itanium
ABI scheme doesn't have a problem with it.

I suppose that is closely related to your concerns about inlining, I just
hadn't made the connection.

In theory the funclets should be able to share code blocks without any problem. 
The entry and exit points are the critical parts that make them funclets. 
I'm just not sure how we can get the optimization passes to recognize this
fact while still meeting the MSVC runtime constraints.  Reid's proposal of
separate catch blocks should help with that, but I'm still not sure
we'll want to use this representation for targets that don't need it.

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150518/f9a8cf6f/attachment.html>

On Mon, May 18, 2015 at 3:28 PM, Kaylor, Andrew <andrew.kaylor at
intel.com>
wrote:
>  I hadn’t noticed the “noexcept” specifier in your example.  That clears
> up part of my concerns, but I still have some problems.
>
>
>
>
>
> With regard to the multiple meanings of ‘resume’ I am more concerning
> about developers who are reading the IR understanding it than about passes
> operating on it.  Apart from making it harder to debug problems related to
> control flow at resume instructions I think this makes it more likely that
> code which mishandles it will be introduced down the road.  If I’m reading
> things correctly, a resume instruction in your proposal could mean:
>
>
>
> a) We’re done handling this exception, continue normal execution at this
> label.
>
> b) We’re done handling this exception, continue execution in an enclosing
> catch handler at this label.
>
> c) We’re done executing this termination handler, check the catch handler
> at this label to see if it can handle the current exception.
>
> d) We’re done executing this termination handler, now execute the
> termination handler at this label.
>
> e) We’re done executing this termination handler, continue handling the
> exception in the runtime.
>
>
>
> I suppose (a) and (b) are more or less the same and it doesn’t entirely
> matter whether the destination is in normal code or exception code.  In
> practical terms (c) and (d) may be the same also, but logically, in terms
> of how the runtime works, they are different.  I’m pretty sure there’s a
> gap in my understanding of your proposal because I don’t understand how e()
> is represented at all.
>
Yeah, Joseph agreed this was overloading resume too much. The new idea is
that a-b would be handled by 'endcatch' (I previously called this
'recover', but endcatch will be more readable) and c-e are represented
with
an 'endcleanup' instruction. The endcleanup will have an unwind label
operand indicating which EH action runs next, which handles both c and d,
depending on the nature of the block. If the unwind label is missing, then
execution continues in the calling function's actions.

> As an exercise, I tried to work through the IR that would be produced in
> the non-optimized case for the following code:
>
>
>
> void test() {
>
>   try {
>
>     Obj o1;
>
>     try {
>
>       f();
>
>     } catch (int) {}
>
>     Obj o2;
>
>     try {
>
>       g();
>
>     } catch (int) {}
>
>     h();
>
>   } catch (int) {}
>
> }
>
>
>
> Here’s what I came up with:
>
>
>
> define void @foo() personality i32 (...)* @__CxxFrameHandler3 {
>
>   %e.addr = alloca i32
>
>   invoke void @f(i32 1)
>
>     to label %cont1 unwind label %cleanup.Obj
>
> cont1:
>
>   invoke void @g(i32 2)
>
>     to label %cont2 unwind label %cleanup.Obj.1
>
> cont2:
>
>   invoke void @h(i32 2)
>
>     to label %cont3 unwind label %cleanup.Obj.2
>
> cont3:
>
>   call void @~Obj()
>
>   call void @~Obj()
>
>   br label %return
>
> return:
>
>   ret void
>
>
>
> cleanup.Obj:
>
>   cleanupblock unwind label %maycatch.int
>
>   call void @~Obj()
>
>   resume label %maycatch.int
>
>
>
> maycatch.int:
>
>   catchblock void [i8* @typeid.int, i32 7, i32* %e.addr]
>
>     to label %catch.int unwind label %catchend
>
> catch.int:
>
>   resume label %cont1
>
> catchend:
>
>   resume
>
>
>
> cleanup.Obj.1:
>
>   cleanupblock unwind label %maycatch.int.1
>
>   call void @~Obj()
>
>   call void @~Obj()
>
>   resume label %maycatch.int.1
>
>
>
> maycatch.int.1:
>
>   catchblock void [i8* @typeid.int, i32 7, i32* %e.addr]
>
>     to label %catch.int.1 unwind label %catchend.1
>
> catch.int.1:
>
>   resume label %cont2
>
> catchend.1:
>
>   resume
>
>
>
>
>
> cleanup.Obj.2:
>
>   cleanupblock unwind label %maycatch.int.2
>
>   call void @~Obj()
>
>   call void @~Obj()
>
>   resume label %maycatch.int.2
>
>
>
> maycatch.int.2:
>
>   catchblock void [i8* @typeid.int, i32 7, i32* %e.addr]
>
>     to label %catch.int.2 unwind label %catchend.2
>
> catch.int.2:
>
>   resume label %return
>
> catchend.2:
>
>   resume
>
> }
>
>
>
> I don’t know if I got that right, but it seems to me that there are a
> couple of problems with this.  Most obviously, there is a good bit of
> duplicated code here (which the optimization passes will probably want to
> combine).
>
First, each destructor gets it's own cleanupblock, and ultimately ends up
in it's own outlined funclet. This cuts down on ~Obj duplication.

Second, you seem to always have the invokes unwinding to cleanups which
then unwind to catch handlers, even if the try is more closely nested than
the cleanup. If f() throws int, we don't actually run cleanup.Obj, we run
the catch and rejoin normal control flow, which will call ~Obj for o1.

Here's how I'd translate that to IR, assuming Obj is empty and has no
ctor:

define void @foo() personality i32 (...)* @__CxxFrameHandler3 {
entry:
  %o1 = alloca i8
  %o2 = alloca i8
  invoke void @f()
    to label %cont1 unwind label %maycatch.int2
cont1:
  invoke void @g()
    to label %cont2 unwind label %maycatch.int3
cont2:
  invoke void @h()
    to label %cont3 unwind label %cleanup.Obj1
cont3:
  call void @~Obj(i8* %o2)
  call void @~Obj(i8* %o1)
  br label %return
return:
  ret void

maycatch.int2:
  catchblock [i8* @typeid.int]
    to label %catch.int2 unwind label %catchendblock2
catch.int2:
  endcatch label %cont2
catchendblock2:
  catchendblock unwind label %cleanup.Obj1

maycatch.int3:
  catchblock [i8* @typeid.int]
    to label %catch.int3 unwind label %catchendblock3
catch.int3:
  endcatch label %cont3
catchendblock3:
  catchendblock unwind label %cleanup.Obj2

cleanup.Obj2:
  cleanupblock unwind label %cleanup.Obj1
  call void @~Obj(i8* %o2)
  endcleanup from label %cleanup.Obj2 unwind label %cleanup.Obj1

cleanup.Obj1:
  cleanupblock unwind label %maycatch.int1
  call void @~Obj(i8* %o1)
  endcleanup from label %cleanup.Obj1 unwind label %maycatch.int1

maycatch.int1:
  catchblock [i8* @typeid.int]
    to label %catch.int1 unwind label %catchendblock1
catch.int1:
  endcatch label %return
catchendblock1:
  catchendblock ; no unwind label, uncaught exceptions leave the function
}

This has a total of 4 calls to ~Obj, so each cleanup is emitted exactly
twice: once for normal flow and once for exceptional flow.

> More significantly though is that it doesn’t correctly describe what
> happens if a non-int exception is thrown in any of the called functions.
> For instance, if a non-int exception is thrown from g() that is caught
> somewhere further down the stack, the runtime should call a terminate
> handler that destructs o1 and then call a terminate handler that destructs
> o2.  However, my IR doesn’t describe a terminate handler that destructs
> just o2 and I don’t know how I could get it to do so within the scheme that
> you have proposed.
>
The idea is that the program notionally "executes" the EH blocks and
each
block describes the intended successor. In the Itanium landingpad model,
that's pretty close to what actually happens, with an optimization to skip
execution of frames that have no cleanups and have no matching catches.

If h throws a float exception, we should run, in order, maycatch.int3,
catchendblock3, cleanup.Obj2, cleanup.Obj1, maycatch.int1, catchendblock4,
and then unwind to the parent. Make sense?

> In a mostly unrelated matter, have you thought about what needs to be done
> to prevent catchblock blocks from being combined?  For example, suppose you
> have code that looks like this:
>
>
>
> void test() {
>
>   try {
>
>     f();
>
>   } catch (int) {
>
>     x();
>
>     y();
>
>     z();
>
>   }
>
>   try {
>
>     g();
>
>   } catch (…) {
>
>   }
>
>   try {
>
>     h();
>
>   } catch (int) {
>
>     x();
>
>     y();
>
>     z();
>
>   }
>
> }
>
>
>
> I think it’s very likely that if we don’t do anything to prevent it the IR
> generated for this will be indistinguishable from the IR generated for
this:
>
>
>
> void test() {
>
>   try {
>
>     f();
>
>     try {
>
>       g();
>
>     } catch (…) {
>
>     }
>
>     h();
>
>   } catch (int) {
>
>     x();
>
>     y();
>
>     z();
>
>   }
>
> }
>
In the original source, the catch-all block unwinds to the parent frame. In
the transformed example, it doesn't. Mid-level optimization passes should
keep the catch-all in the first example unwinding out to the parent,
otherwise things will break down.

Practically, the current middle-end will be unable to merge the catchblocks
because of catchendblock and this rule: "Invokes that are reached after a
catchblock without following any unwind edges must transitively unwind to
the first catchend block that the catchblock unwinds to."

The catchblocks will look like:

maycatch.int1:
  catchblock [i8* @typeid.int]
    to label %catch.int1 ; unwind to parent
catch.int1:
  invoke void @x() to label %cont1 unwind label %catchendblock1
cont1:
  invoke void @y() to label %cont2 unwind label %catchendblock1
cont2:
  invoke void @z() to label %cont3 unwind label %catchendblock1
cont4:
  endcatch label %...
catchendblock1:
  catchendblock ; unwind to parent

Any code merging optimization would have to ensure that those xyz calls
unwind to the catchendblock referenced by the catchblock. Practically, this
will require merging the entire catchblock, which *is* a legal
transformation, and we can compensate for it in the ip2state tables.
Imagine assigning these states:

call f() // state 0
call g() // state 2
catchall // state 3
call h() // state 0
catchint // state 1

So, f and h can have the same states even though they are split across the
g call in source order.

I need to write more about the state numbering algorithm we came up with,
it's the primary constraint on the design.

> In this case that might be OK, but theoretically the calls to f() and h()
> should get different states and there are almost certainly cases where
> failing to recognize that will cause problems.  What’s more, the same basic
> pattern arises for this case:
>
>
>
> void test() {
>
>   try {
>
>     f();
>
>   } catch (int) {
>
>     x();
>
>     y();
>
>     z();
>
>   }
>
>   try {
>
>     g();
>
>   } catch (float) {
>
>   }
>
>   try {
>
>     h();
>
>   } catch (int) {
>
>     x();
>
>     y();
>
>     z();
>
>   }
>
> }
>
>
>
> But in this case, if we get the state numbering wrong an int-exception
> from g() could end up being incorrectly caught by the xyz handler.
>
>
>
> BTW, finding cases like this is the primary reason that I’ve been trying
> to push my current in-flight patch onto the sinking ship that is our
> current implementation.  I mentioned to you before that the test suite I’m
> using passes with my proposed patch, but that’s only true with
> optimizations disabled.  With optimizations turned on I’m seeing all kinds
> of fun things like similar handlers being combined and common instructions
> being hoisted above a shared(!) eh_begincatch call in if-else paired
> handlers.  I don’t know if it will be worth trying to fix these problems,
> but seeing them in action has been very instructive.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150520/ed6fe30d/attachment.html>