[PATCH] D15139: [IR] Reformulate LLVM's EH funclet IR

[Reid Kleckner via llvm-commits]

rnk added inline comments.

================
Comment at: lib/CodeGen/AsmPrinter/WinException.cpp:415-419
@@ -423,7 +414,7 @@
       const MachineInstr &MI = *MBBI;
-      if (!VisitingInvoke && LastStateChange.NewState != NullState &&
+      if (!VisitingInvoke && LastStateChange.NewState != BaseState &&
           MI.isCall() && !EHStreamer::callToNoUnwindFunction(&MI)) {
         // Indicate a change of state to the null state.  We don't have
         // start/end EH labels handy but the caller won't expect them for
         // null state regions.
         LastStateChange.PreviousEndLabel = CurrentEndLabel;
----------------
The way this is supposed to work is that we're not supposed to emit separate start/end labels for an invoke that happens to be in BaseState. This would occur in this kind of situation:
  void f() {
    HasDtor o;
    try {
      g(1);
    } catch(...) {
      g(2); // Also in BaseState
    }
  }

In this case, we don't need separate start/end labels for the g(2) call, we can just use the funclet start entry in BaseState. We can treat it just like a potentially throwing call.

================
Comment at: lib/CodeGen/AsmPrinter/WinException.cpp:443-447
@@ -451,8 +442,7 @@
       int NewState = StateAndEnd.first;
-      // Ignore EH labels explicitly annotated with the null state (which
-      // can happen for invokes that unwind to a chain of endpads the last
-      // of which unwinds to caller).  We'll see the subsequent invoke and
-      // report a transition to the null state same as we do for calls.
-      if (NewState == NullState)
+      // Ignore EH labels explicitly annotated with the null state.
+      // We'll see the subsequent invoke and report a transition to the null
+      // state same as we do for calls.
+      if (NewState == BaseState)
         continue;
       // Keep track of the fact that we're between EH start/end labels so
----------------
Yeah, we should be able to do that.

================
Comment at: lib/CodeGen/AsmPrinter/WinException.cpp:469-473
@@ -478,7 +468,7 @@
   // Iteration hit the end of the block range.
-  if (LastStateChange.NewState != NullState) {
+  if (LastStateChange.NewState != BaseState) {
     // Report the end of the last new state
     LastStateChange.PreviousEndLabel = CurrentEndLabel;
     LastStateChange.NewStartLabel = nullptr;
-    LastStateChange.NewState = NullState;
+    LastStateChange.NewState = BaseState;
     // Leave CurrentEndLabel non-null to distinguish this state from end.
----------------
I don't think I understand the question.

I think the idea is, when we get to the end of the funclet, if there was some previous state change out of the base state, we should emit label entries to transition back to the base state for the epilogue.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:180-184
@@ +179,7 @@
+    int BaseState = -1;
+    if (FuncletUnwindDest == InvokeUnwindDest) {
+      auto BaseStateI = FuncInfo.FuncletBaseStateMap.find(FuncletPad);
+      if (BaseStateI != FuncInfo.FuncletBaseStateMap.end())
+        BaseState = BaseStateI->second;
+    }
+
----------------
I would expect the CLR to have the same kinds of constraints as C++ here.

The base state is the state of the funclet prologues and epilogues, and it is the state that we previously assigned to the endpads. If there is a plain callsite at the beginning of a funclet before any new exceptional scopes are created, it should be in the base state. If that callsite throws, the runtime should take the appropriate "next action", which would follow the catchswitch unwind edge or the cleanupret unwind edge.

Now that we don't have endpads, we need to identify callsites that unwind out of the funclet. We do this by comparing the unwind destination of the funclet with the unwind destination of each call site, and if they agree, they must be in the base state.

If we don't do this, we would end up in a situation where a call inside a catch funclet gets a state outside of the CatchLow/CatchHigh interval. I'm pretty sure this would confuse CxxFrameHandler3.


http://reviews.llvm.org/D15139