[PATCH] [WinEH] Update C++ exception state numbering code

[Reid Kleckner]

REPOSITORY
  rL LLVM

================
Comment at: lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp:588
@@ +587,3 @@
+// with each action before we start assigning state numbers.
+void WinEHNumbering::findActionRootLPads(const Function &F) {
+  auto I = VisitedHandlers.insert(&F);
----------------
andrew.w.kaylor wrote:
> rnk wrote:
> > majnemer wrote:
> > > `calculateStateNumbers` does the same basic traversal, how does stashing the first landing pad associated with the action help things?  Is it just to reenable the optimization in `processCallSite` ?
> > Yeah, if I read this right, all the RootLPad related changes are separable and are for reducing the number of TryBlockMapEntries.
> > 
> > What do you think about separating the ip2state handler begin label from the rootlpad change?
> The purpose of the root lpad change is to allow us to recognize which handlers belong to the same try block.  I wouldn't really just consider this an optimization.  The numbering is logically wrong without it.  That said, I'm not sure it's causing problems.
> 
> I've tried a lot of variations of this code over the past week, and it's hard to keep straight which changes were necessary to fix issues with what's in trunk and which fixed problems I saw after making other changes.  I know at some point things just didn't work at all without this change, but I just tested with it effectively disabled and it looks like it might be OK.
> 
> I can pull this out of the current change set if you like.
Like David, I expect there's a simpler way to figure out the state number of the first landingpad action list that mentioned any given handler, so I'd rather split it up and then we can bring it back with a more targetted test.

The current numbering is consistent with rewriting trys with multiple catches like this:
  try { }
  catch (A) { }
  catch (B) { }
  catch (C) { }
Right now we number this like:
  try {
    try {
      try {
      } catch (A) { /* except this is magically outside the catch B scope */ }
    } catch (B) { /* ditto for C */ }
  } catch (C) { }

I think this numbering discrepancy doesn't result in a correctness problem, it just means our tables are weird. A TryBlockMap entry always has exactly one handler, instead of having three as MSVC would in this case.

http://reviews.llvm.org/D9512

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/