[PATCH] D11041: New EH representation for MSVC compatibility

[Joseph Tremoulet]

JosephTremoulet added inline comments.

================
Comment at: docs/LangRef.rst:5123-5125
@@ +5122,5 @@
+
+The ``catchblock`` must be provided a ``normal`` label to transfer control
+to if the ``catchblock`` matches the exception and an ``exception``
+label to transfer control to if it doesn't.
+
----------------
rnk wrote:
> JosephTremoulet wrote:
> > majnemer wrote:
> > > JosephTremoulet wrote:
> > > > What should the exception label look like for a catch that is unconditionally entered (like catch(...) in C++)?  Or do front-ends need to translate such things to cleanups?
> > > The catchblock should unwind to a catchendblock in that case.
> > That seems problematic.  Doesn't that leave us without a way to indicate in the CFG that exceptions don't escape a catch-all?  We'll definitely want to e.g. figure out when a handler has no incoming exceptions and can be removed (which could happen because there is an inner catch-all), or determine whether exceptions escape a function (to propagate that info up the call graph).  Maybe EH-specific analyses like that could be expected to know about this and check whether a catchblock's arguments indicate catch-all for the given personality, but I expect we'll also want the information be visible to more generic analyses like reachability and dominance.
> > I'd like to offer a suggested change, but I'm not sure I fully understand what problem catchendblock is solving, so my first question would be what it's needed for and whether there's another way to solve that problem (the RFC just said "It is merely a placeholder to help reconstruct which invokes were part of the catch blocks of a try"; could that be computed by checking whether catchret is reachable from the invokes?).  Barring that, it seems we'd want something like a `catchallblock` that isn't a terminator, doesn't have an unwind edge, and would have whatever the appropriate arguments are for a catchblock that catches everything; front-ends or an EH-aware transform could recognize when a catchblock catches everything and the catchendblock would have no other predecessors, and write the last catchblock as catchallblock  (dropping the catchendblock) instead.
> It turns out that you need this edge in order to generate the right __CxxFrameHandler3 tables when catch-all is involved. Consider:
>   try { throw 1; }
>   catch (...) {
>     try { throw; }
>     catch (...) { throw; }
>     }
>   }
> 
> The inner catch-all block unwinds to the catchendblock of the outer catch. This affects our planned EH state numbering algorithm.
> 
> Basically, I think that only EH-aware transforms should be allowed to do this kind of CFG simplification.
1) Regarding your specific example:  Since the inner catch(...) re-throws, there's still unwind to out of the function (or at least the snippet); the inner catchendblock would have two predecessors, and I'm more concerned about cases where the artificial edge from the catchblock is the only predecessor of the catchendblock.  Or in terms of the example, something more like this:
```
try { throw 1; }
catch (...) {
  try { throw; }
  catch (...) { /* nothing throws here */ }
  }
}
```
I'd like to be able to record that a function with that body doesn't unwind out of it.

2) Maybe I'd need more details on your planned EH state numbering algorithm, but the idea would be that you'd identify anywhere that you clone a catch-all into a handler, and consider the catch-all a child of that handler instead of top-level.  Would that be doable?

3) I agree with this:
> Basically, I think that only EH-aware transforms should be allowed to do this kind of CFG simplification.

So let's say I'm writing an EH-aware transform that knows my target has flexible EH reporting.  What is the output of the CFG simplification that it performs?  It seems to me that it doesn't have any valid way to simplify the CFG, that's the part that's concerning.


Repository:
  rL LLVM

http://reviews.llvm.org/D11041