[PATCH] Generate SEH unwinding info on Win64

[Reid Kleckner]

================
Comment at: lib/MC/MCAsmStreamer.cpp:1138
@@ -1137,3 +1138,1 @@
-  OS << "\t.seh_pushreg ";
-  EmitRegisterName(Register);
   EmitEOL();
----------------
Vadim Chugunov wrote:
> Reid Kleckner wrote:
> > This should be a separate revert.  The existing test case is incorrect, it was doing '.seh_savereg %rsi, 16' and disassembling that with %rbp.
> I have it as a separate commit in git, it just got squashed when posting diff to Phabricator.  Do you want me to actually submit it as a separate review?
No, I can commit it.

================
Comment at: lib/Target/X86/MCTargetDesc/X86MCAsmInfo.cpp:148
@@ +147,3 @@
+    PointerSize = 8;
+    ExceptionsType = ExceptionHandling::Win64;
+  }
----------------
Vadim Chugunov wrote:
> Reid Kleckner wrote:
> > I think this means we will start attempting to emit cleanups for x86_64-pc-windows-msvc, which we probably aren't ready for.  Can we hold off on this change, or are you reasonably sure it's safe?
> Not sure I understand...   Can you please elaborate?
Ignore this comment.  I worry this will break 'clang-cl -m64' because we emit 'invoke' instructions in the frontend to run C++ destructors.  Clang is currently relying on the fact that LLVM drops the landing pads on the floor and generates regular calls.  Clang shouldn't be relying on that, though.

================
Comment at: lib/Target/X86/X86FrameLowering.cpp:402-410
@@ +401,11 @@
+//
+// [if needs FP]
+//     .seh_setframe %rbp, KKK
+//
+// ; Currently only Win64 spills non-GPRs
+// [for all callee-saved XMM registers]
+//     [if needs FP]
+//         movaps  %xmmXX, -NNN(%rbp)
+//     [else]
+//         movaps  %xmmXX, NNN(%rsp)
+//
----------------
Vadim Chugunov wrote:
> Reid Kleckner wrote:
> > Are these comments accurate when we have an FP *and* the stack needs realignment?  It seems like we should move the win64-specific XMM CSR saves to happen before the stack realignment.  This should be safe since win64 also gives us 16-byte stack alignment.
> Stack realignment happens before stack pointer decrement, so XMM slots would be be in "invalid" zone at that point.  We'd have to swap re-alignment and SP decrement in order to do what you suggest.  I did not want to mess with existing frame setup order too much...
> But I don't see the problem with the way it is now, because (re-alignment + SP decrement) is guaranteed to allocate at least as much space as SP decrement alone.  And the xmm spill slots are accessed via rbp, which is set up before stack realignment, so no problem there either.
OK, that makes sense.  We may end up spilling xmm registers into the stack realignment gap, but that should be OK.  Can you add a comment something like:
  ; It's OK if the stack was realigned and xmm registers are spilled into the realignment gap.

================
Comment at: lib/Target/X86/X86FrameLowering.cpp:779
@@ +778,3 @@
+  if (NeedsWin64SEH) {
+    for (unsigned i = 0, e = CSI.size(); i != e; ++i) {
+      unsigned Reg = CSI[i].getReg();
----------------
This can also be range-based.

================
Comment at: lib/Target/X86/X86FrameLowering.cpp:1092
@@ -983,2 +1091,3 @@
 
+  std::vector<CalleeSavedInfo> &CSI = const_cast<std::vector<CalleeSavedInfo>&>(CSI_);
   DebugLoc DL = MBB.findDebugLoc(MI);
----------------
Vadim Chugunov wrote:
> Reid Kleckner wrote:
> > :(
> > 
> > Is TargetFrameLowering::getCalleeSavedSpillSlots() not powerful enough to express the offsets you need?  If it isn't, I think the right way forward is to add a more flexible interface to TFL, like TFL::appendCalleeSavedSpillSlot(std::vector<CSI> &CSIs), where the default implementation forwards on to check 'TFL->getCalleeSavedSpillSlots(NumFixedSpillSlots)' so existing targets don't require changes.
> The problem with getCalleeSavedSpillSlots() is that it would reserve spill slots regardless of whether the register actually needs to be spilled, which would increase stack usage unnecessarily.
> 
> I was thinking of using hasReservedSpillSlot() for this purpose, because, in principle, it could create spill slots on-the-fly, but then we'd be relying on the order of register enumeration. Also, it'd need to store state somewhere, and the code overall would be more convoluted.
> 
> How about changing getCalleeSavedSpillSlots() signature to take non-const vector<CalleeSavedInfo>?
> 
> If that's too hacky, I'd propose to create TargetFrameLowering::assignSpillSlots(vector<CSI>& ) callback, that would allow to override PEI::calculateCalleeSavedRegisters() in a target-specific manner.
> If that's too hacky, I'd propose to create TargetFrameLowering::assignSpillSlots(vector<CSI>& ) callback, that would allow to override PEI::calculateCalleeSavedRegisters() in a target-specific manner.

This is basically what I was trying to suggest.  The default implementation should do what calculateCalleeSavedRegisters() does now, so you don't have to change every target.

http://reviews.llvm.org/D4081