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

Andy Kaylor andrew.kaylor at intel.com
Wed May 6 12:46:14 PDT 2015


REPOSITORY
  rL LLVM

================
Comment at: lib/CodeGen/AsmPrinter/Win64Exception.cpp:391
@@ -378,3 +390,3 @@
   OS.EmitIntValue(0x19930522, 4);                      // MagicNumber
-  OS.EmitIntValue(FuncInfo.UnwindMap.size(), 4);       // MaxState
+  OS.EmitIntValue(FuncInfo.MaxState, 4);               // MaxState
   OS.EmitValue(createImageRel32(UnwindMapXData), 4);   // UnwindMap
----------------
majnemer wrote:
> When is `MaxState` different from `FuncInfo.UnwindMap.size()`.
I'd have to go back to my test cases to see if I can isolate the conditions that cause this.  It was definitely happening.  It's quite possible that this is an indication that the Unwind table was missing an entry.

================
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);
----------------
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.

http://reviews.llvm.org/D9512

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






More information about the llvm-commits mailing list