[PATCH] CodeGen support for x86_64 SEH catch handlers in LLVM

[Andy Kaylor]

I've made some specific comments on things that I found confusing.

Maybe this doesn't need to be entirely settled now, but it isn't clear to me what you imagine the IR to look like before outlining and how the transition from that to the outlined filter functions will happen.  In particular, could you describe what would happen if the filter condition made reference to local variables in the function?

For instance, suppose I wanted a filter in the safe_div example that looked like this:

__except(GetExceptionCode() == EXCEPTION_INT_DIVIDE_BY_ZERO && *n == 0)

what would that look like?

================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:248
@@ -246,3 +247,3 @@
     default: llvm_unreachable("unsupported unwinding information encoding");
     case WinEH::EncodingType::Itanium:
       ES = new Win64Exception(this);
----------------
The new exception handling implementation doesn't use Itanium encoding, does it?

================
Comment at: lib/CodeGen/AsmPrinter/EHStreamer.h:68
@@ -65,2 +67,3 @@
 
     // The landing pad starts at PadLabel.
+    const LandingPadInfo *LPad;
----------------
This comment is now wrong.  Also, does a null pointer still mean there is no landing pad?

================
Comment at: lib/CodeGen/AsmPrinter/Win64Exception.cpp:113
@@ +112,3 @@
+      else
+        report_fatal_error("unexpected personality function");
+    } else {
----------------
Will there eventually be another call added here if the personality function is _CxxFrameHandler or will that be considered a different exception handling type?

================
Comment at: lib/CodeGen/AsmPrinter/Win64Exception.cpp:141
@@ +140,3 @@
+///       imagerel32 FilterOrFinally;  // Function pointer.
+///       imagerel32 LabelLPad;        // Zero for __finally.
+///     } Entries[NumEntries];
----------------
The comment "Zero for __finally" applies to the "FilterOrFinally" member, right?  That wasn't clear to me on first reading.

================
Comment at: lib/CodeGen/AsmPrinter/Win64Exception.cpp:159
@@ +158,3 @@
+
+  // Compute call site entries as we would for DWARF, but with a fake actions
+  // table.
----------------
Could you clarify what "as we would for DWARF" means?  This comment doesn't really tell me what's happening here.

================
Comment at: lib/CodeGen/AsmPrinter/Win64Exception.cpp:215
@@ +214,3 @@
+      MCSymbol *ClauseLabel = LPad->ClauseLabels[I];
+      assert(TypeID >= 0 && "SEH personality functions don't support filters");
+
----------------
The existing naming is a bit unfortunate.  I take it this is referring to landingpad type filters, not SEH filters.  At least in the Windows-specific code perhaps we can use a term like "type filters."

================
Comment at: lib/CodeGen/AsmPrinter/Win64Exception.cpp:223
@@ +222,3 @@
+        assert(unsigned(TypeID - 1) < TypeInfos.size());
+        const GlobalValue *TI = TypeInfos[TypeID - 1];
+        if (TI)
----------------
Based on the comments above, this must be a function pointer, right?  I'm confused by the references to TypeID.

================
Comment at: lib/CodeGen/AsmPrinter/Win64Exception.cpp:226
@@ +225,3 @@
+          Asm->OutStreamer.EmitValue(createImageRel32(Asm->getSymbol(TI)), 4);
+        else // catch i8* null
+          Asm->OutStreamer.EmitIntValue(0, 4);
----------------
Are there two different ways to specify a __finally handler?

================
Comment at: test/CodeGen/X86/seh-safe-div.ll:36
@@ +35,3 @@
+          catch i8* bitcast (i32 (i8*, i8*)* @safe_div_filt1 to i8*)
+  %ehptr = extractvalue { i8*, i32 } %vals, 0
+  %sel = extractvalue { i8*, i32 } %vals, 1
----------------
Can you explain how the results from the filter function get propagated to the landingpad return value?

================
Comment at: test/CodeGen/X86/seh-safe-div.ll:126
@@ +125,3 @@
+
+define i32 @safe_div_filt0(i8* %eh_info, i8* %rsp) {
+  %eh_info_c = bitcast i8* %eh_info to i32**
----------------
I take it this function signature is dictated by the __C_specific_handler implementation.  Can you document that and describe the parameters?

http://reviews.llvm.org/D6300