[PATCH] Generate SEH unwinding info on Win64

[Vadim Chugunov]

================
Comment at: include/llvm/CodeGen/MachineFrameInfo.h:486
@@ -486,1 +485,3 @@
+  int CreateFixedObject(uint64_t Size, int64_t SPOffset, bool Immutable,
+                        bool isSpillSlot = false);
 
----------------
Reid Kleckner wrote:
> Let's try to avoid contiguous boolean parameters, especially when some have default arguments.  They make it hard to do refactorings like removing the 'Immutable' parameter here.
> 
> Instead, CreateFixedSpillSlot(Size, SPOffset) sounds like the right interface to me.
Will do.

================
Comment at: lib/MC/MCAsmStreamer.cpp:1138
@@ -1137,3 +1138,1 @@
-  OS << "\t.seh_pushreg ";
-  EmitRegisterName(Register);
   EmitEOL();
----------------
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?

================
Comment at: lib/Target/X86/MCTargetDesc/X86MCAsmInfo.cpp:148
@@ +147,3 @@
+    PointerSize = 8;
+    ExceptionsType = ExceptionHandling::Win64;
+  }
----------------
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?

================
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)
+//
----------------
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.

================
Comment at: lib/Target/X86/X86FrameLowering.cpp:738-742
@@ +737,7 @@
+    if (HasFP) {
+        // We need to set frame base offset low enough such that all saved
+        // register offsets would be positive relative to it, but we can't
+        // just use NumBytes, because .seh_setframe offset must be <=240.
+        // So we pretend to have only allocated enough space to spill the
+        // non-volatile registers.
+
----------------
Reid Kleckner wrote:
> The comment below about "Don't care about the rest of the stack allocation" should get merged in here.  I spent a while trying to understand why this scary comment is OK, and the comment below explains it.
okay

================
Comment at: lib/Target/X86/X86FrameLowering.cpp:910
@@ -799,3 +909,3 @@
 
-    if (Opc != X86::POP32r && Opc != X86::POP64r && Opc != X86::DBG_VALUE &&
-        !PI->isTerminator())
+    if (Opc != X86::POP32r && Opc != X86::POP64r &&
+        Opc != X86::DBG_VALUE && !PI->isTerminator())
----------------
Reid Kleckner wrote:
> Seems like a superfluous formatting change
okay, will undo

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

http://reviews.llvm.org/D4081