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

[Reid Kleckner]

================
Comment at: include/llvm/MC/MCAsmInfo.h:50
@@ -49,2 +49,3 @@
   ItaniumWinEH, /// Itanium EH built on Windows unwind info (.pdata and .xdata)
+  MSVC,         /// MSVC compatible exception handling
 };
----------------
rjmccall wrote:
> Let's not call this just "MSVC" when we know that MSVC actually uses several different schemes.
That's true, but this model will handle all of them. MSVC has two main EH personalites: C++ EH and SEH. Both use outlined cleanups, and this EH model will be used to trigger that outlining in the backend. The language-specific tables will key off the name of the personality function instead of this enum.

Maybe I could call this OutlinedWinEH or something to make it clearer that it's more of a family?

================
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);
----------------
andrew.w.kaylor wrote:
> The new exception handling implementation doesn't use Itanium encoding, does it?
It's confusing. There are two "Itanium" things here, neither of which is the ISA. There is the EH chapter of the Itanium C++ ABI, documented here (http://mentorembedded.github.io/cxx-abi/abi-eh.html), and there is the encoding of the UNWIND_INFO opcodes in .xdata. Saleem added this enum, so maybe he knows more. Presumably what happened was that Microsoft reused the opcodes they were going to use for Itanium for x86_64.

================
Comment at: lib/CodeGen/AsmPrinter/EHStreamer.h:68
@@ -65,2 +67,3 @@
 
     // The landing pad starts at PadLabel.
+    const LandingPadInfo *LPad;
----------------
andrew.w.kaylor wrote:
> This comment is now wrong.  Also, does a null pointer still mean there is no landing pad?
Woops. Yeah, null means "gap", which in the Itanium LSDA model requires emitting an entry in the call site table.

================
Comment at: lib/CodeGen/AsmPrinter/Win64Exception.cpp:113
@@ +112,3 @@
+      else
+        report_fatal_error("unexpected personality function");
+    } else {
----------------
andrew.w.kaylor wrote:
> Will there eventually be another call added here if the personality function is _CxxFrameHandler or will that be considered a different exception handling type?
Yes, other personality functions such as _CxxFrameHandler and _except_handler3 will be handled here under the same overall EH scheme. They have different tables, but a similar model that requires outlining.

================
Comment at: lib/CodeGen/AsmPrinter/Win64Exception.cpp:141
@@ +140,3 @@
+///       imagerel32 FilterOrFinally;  // Function pointer.
+///       imagerel32 LabelLPad;        // Zero for __finally.
+///     } Entries[NumEntries];
----------------
andrew.w.kaylor wrote:
> The comment "Zero for __finally" applies to the "FilterOrFinally" member, right?  That wasn't clear to me on first reading.
OK, I added some more text to the previous paragraph, as there isn't room inline here.

================
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.
----------------
andrew.w.kaylor wrote:
> Could you clarify what "as we would for DWARF" means?  This comment doesn't really tell me what's happening here.
I guess I meant "as we would for the Itanium LSDA" instead. This comment predated my attempt to separate DWARF CFI from the Itanium LSDA. Previously those concepts were conflated.

================
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");
+
----------------
andrew.w.kaylor wrote:
> 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."
How about "filter clauses in landing pads"?

================
Comment at: lib/CodeGen/AsmPrinter/Win64Exception.cpp:223
@@ +222,3 @@
+        assert(unsigned(TypeID - 1) < TypeInfos.size());
+        const GlobalValue *TI = TypeInfos[TypeID - 1];
+        if (TI)
----------------
andrew.w.kaylor wrote:
> Based on the comments above, this must be a function pointer, right?  I'm confused by the references to TypeID.
Yes, it's a function pointer, but the existing code consistently refers to this operand of the landing pad catch clause as the "type info" and it is assigned a "type id" returned from @llvm.eh.typeid.for(i8* @my_type_info). More comments...

================
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);
----------------
andrew.w.kaylor wrote:
> Are there two different ways to specify a __finally handler?
"catch i8* null" is a catch-all handler, not a `__finally` handler. The difference is that unwinding will continue through `__finally`, but catch-all handles the exception.

================
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
----------------
andrew.w.kaylor wrote:
> Can you explain how the results from the filter function get propagated to the landingpad return value?
This %ehptr is just zero for now. =/ It's probably in some live-in physical register (%eax?), though.

The result of the filter functions is reflected by the selector value, which indicates which filter function fired.

================
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**
----------------
andrew.w.kaylor wrote:
> I take it this function signature is dictated by the __C_specific_handler implementation.  Can you document that and describe the parameters?
I can do that here, but I was planning to do that in the not-yet implemented CodeGenPrep pass that needs to interact with the parameters.

http://reviews.llvm.org/D6300