Fix failure to invoke exception handler on Win64.

[vadimcn]

Summary

Fixes an obscure Win64 unwinding failure which I've stumbled upon. Please see comments for details.
MSVC does the same, btw.

[rnk]

lib/Target/X86/X86FrameLowering.cpp

838

This seems like a do { } while (...) loop.

844

Does this not return one-past-the-end?

That aside, I don't think this will work in general, because basic block placement runs *after* prologue-epilogue insertion.

For now I think we should always insert a nop if the epilogue is at the beginning of a basic block, just to be safe. The actual solution is probably to insert some kind of target-specific pseudo that we lower out after block placement to either a nop or nothing.

test/CodeGen/X86/win64_call_epi.ll

2

Does this fail without -O0? Just curious.

4

Can you come up with a test case that fails when block placement runs? Something like this maybe:

define void @bar(i1 zeroext %cond ) {
  br i1 %cond, label %a, label %b  ; <- Add metadata to indicate that %a is *highly* likely over %b.
a:
  call void @foo()
  br label %done
b:
  call void @baz()
  store i32 0, i32* @something
  br label %done
done:
  ret void
}

[vadimcn]

lib/Target/X86/X86FrameLowering.cpp

844

Does this not return one-past-the-end?

Hmm, yes it probably does.

That aside, I don't think this will work in general, because basic block placement runs *after* prologue-epilogue insertion.

I can make it do what you suggest, though I think that 'ret' starts out in a MBB of its' own quite often.

The actual solution is probably to insert some kind of target-specific pseudo that we lower out after block placement to either a nop or nothing.

I was considering creating 'SEH_Epilogue' pseudo, then checking for preceding CALLs when lowering it in X86AsmPrinter. Unfortunately, some pseudos, like EH_LABEL, are not lowered till the last moment, so it'd still have to have logic to skip those.
How does that sound?

test/CodeGen/X86/win64_call_epi.ll

2

No, it still works. Just something I copied from another test...

4

Yeah, this breaks it. :(
Okay, that probably means I am gonna have to go with the SEH_Epilogue approach...

[rnk]

lib/Target/X86/X86FrameLowering.cpp

844

The SEH_Epilogue pseudo sounds good, even with exciting logic to skip non-instruction pseudos.

[vadimcn]

Implemented 'SEH_Epilogue' approach.

[rnk]

lgtm

I guess I'll apply this with the suggested tweaks, to save a round trip. Thanks for the patch!

lib/Target/X86/X86MCInstLower.cpp

982–983

Usually when you declare locals in a case label like this you need to wrap it in braces. There are no subsequent cases today, but you should go ahead and wrap it now.

986

I think some pseudos emit real code, so I would soften this to say that by assuming they aren't real, we will conservatively emit a nop in more cases than is absolutely necessary.

[rnk]

Grumble grumble auto close why doesn't this work.

lib/Target/X86/X86FrameLowering.cpp

 }
 
 void X86FrameLowering::emitEpilogue(MachineFunction &MF,
                                     MachineBasicBlock &MBB) const {
   const MachineFrameInfo *MFI = MF.getFrameInfo();
   X86MachineFunctionInfo *X86FI = MF.getInfo<X86MachineFunctionInfo>();
   const X86RegisterInfo *RegInfo =
       static_cast<const X86RegisterInfo *>(MF.getTarget().getRegisterInfo());
   const TargetInstrInfo &TII = *MF.getTarget().getInstrInfo();
   MachineBasicBlock::iterator MBBI = MBB.getLastNonDebugInstr();
   assert(MBBI != MBB.end() && "Returning block has no instructions");
   unsigned RetOpcode = MBBI->getOpcode();
   DebugLoc DL = MBBI->getDebugLoc();
   const X86Subtarget &STI = MF.getTarget().getSubtarget<X86Subtarget>();
   bool Is64Bit = STI.is64Bit();
   bool IsLP64 = STI.isTarget64BitLP64();
   bool UseLEA = STI.useLeaForSP();
   unsigned StackAlign = getStackAlignment();
   unsigned SlotSize = RegInfo->getSlotSize();
   unsigned FramePtr = RegInfo->getFrameRegister(MF);
   unsigned StackPtr = RegInfo->getStackRegister();
 
+  bool IsWinEH =
+      MF.getTarget().getMCAsmInfo()->getExceptionHandlingType() ==
+      ExceptionHandling::WinEH;
+  bool NeedsWinEH = IsWinEH && MF.getFunction()->needsUnwindTableEntry();
+
   switch (RetOpcode) {
   default:
     llvm_unreachable("Can only insert epilog into returning blocks");
   case X86::RETQ:
   case X86::RETL:
   case X86::RETIL:
   case X86::RETIQ:
   case X86::TCRETURNdi:
void X86FrameLowering::emitEpilogue(MachineFunction &MF,
   // realigned.
   if (RegInfo->needsStackRealignment(MF) || MFI->hasVarSizedObjects()) {
     if (RegInfo->needsStackRealignment(MF))
       MBBI = FirstCSPop;
     if (CSSize != 0) {
       unsigned Opc = getLEArOpcode(IsLP64);
       addRegOffset(BuildMI(MBB, MBBI, DL, TII.get(Opc), StackPtr),
                    FramePtr, false, -CSSize);
+      --MBBI;
     } else {
       unsigned Opc = (Is64Bit ? X86::MOV64rr : X86::MOV32rr);
       BuildMI(MBB, MBBI, DL, TII.get(Opc), StackPtr)
         .addReg(FramePtr);
+      --MBBI;
     }
   } else if (NumBytes) {
     // Adjust stack pointer back: ESP += numbytes.
     emitSPUpdate(MBB, MBBI, StackPtr, NumBytes, Is64Bit, IsLP64, UseLEA,
                  TII, *RegInfo);
+    --MBBI;
+  }
+
+  // Windows unwinder will not invoke function's exception handler if IP
+  // is either in prologue or in epilogue.  This behavior causes a problem
+  // when a call immediately precedes an epilogue, because the return
+  // address points into the epilogue.  To cope with that, we insert
+  // an epilogue marker here, then replace it with a 'nop' if it ends up
+  // immediately after a CALL in the final emitted code.
+  if (NeedsWinEH) {
+    BuildMI(MBB, MBBI, DL, TII.get(X86::SEH_Epilogue));
   }
 
   // We're returning from function via eh_return.
   if (RetOpcode == X86::EH_RETURN || RetOpcode == X86::EH_RETURN64) {
     MBBI = MBB.getLastNonDebugInstr();
     MachineOperand &DestAddr  = MBBI->getOperand(0);
     assert(DestAddr.isReg() && "Offset should be in register!");
     BuildMI(MBB, MBBI, DL,

lib/Target/X86/X86InstrCompiler.td

let isPseudo = 1 in {
   def SEH_StackAlloc : I<0, Pseudo, (outs), (ins i32imm:$size),
                             "#SEH_StackAlloc $size", []>;
   def SEH_SetFrame : I<0, Pseudo, (outs), (ins i32imm:$reg, i32imm:$offset),
                             "#SEH_SetFrame $reg, $offset", []>;
   def SEH_PushFrame : I<0, Pseudo, (outs), (ins i1imm:$mode),
                             "#SEH_PushFrame $mode", []>;
   def SEH_EndPrologue : I<0, Pseudo, (outs), (ins),
                             "#SEH_EndPrologue", []>;
+  def SEH_Epilogue : I<0, Pseudo, (outs), (ins),
+                            "#SEH_Epilogue", []>;
 }
 
 //===----------------------------------------------------------------------===//
 // Pseudo instructions used by segmented stacks.
 //
 
 // This is lowered into a RET instruction by MCInstLower.  We need
 // this so that we don't have to have a MachineBasicBlock which ends

lib/Target/X86/X86MCInstLower.cpp

void X86AsmPrinter::LowerPATCHPOINT(const MachineInstr &MI) {
   unsigned NumBytes = opers.getMetaOper(PatchPointOpers::NBytesPos).getImm();
   assert(NumBytes >= EncodedBytes &&
          "Patchpoint can't request size less than the length of a call.");
 
   EmitNops(OutStreamer, NumBytes - EncodedBytes, Subtarget->is64Bit(),
            getSubtargetInfo());
 }
 
+// Returns instruction preceding MBBI in MachineFunction.
+// If MBBI is the first instruction of the first basic block, returns null.
+static MachineBasicBlock::const_iterator prev_inst(
+  MachineBasicBlock::const_iterator MBBI) {
+
+  const MachineBasicBlock* MBB = MBBI->getParent();
+  while (MBBI == MBB->begin()) {
+    if (MBB == MBB->getParent()->begin())
+      return nullptr;
+    MBB = MBB->getPrevNode();
+    MBBI = MBB->end();
+  }
+  return --MBBI;
+}
+
 void X86AsmPrinter::EmitInstruction(const MachineInstr *MI) {
   X86MCInstLower MCInstLowering(*MF, *this);
   const X86RegisterInfo *RI =
       static_cast<const X86RegisterInfo *>(TM.getRegisterInfo());
 
   switch (MI->getOpcode()) {
   case TargetOpcode::DBG_VALUE:
     llvm_unreachable("Should be handled target independently");
void X86AsmPrinter::EmitInstruction(const MachineInstr *MI) {
 
   case X86::SEH_PushFrame:
     OutStreamer.EmitWinCFIPushFrame(MI->getOperand(0).getImm());
     return;
 
   case X86::SEH_EndPrologue:
     OutStreamer.EmitWinCFIEndProlog();
     return;
+
+  case X86::SEH_Epilogue:
+    MachineBasicBlock::const_iterator MBBI(MI);
+    // Check if preceded by a call and emit nop if so.
+    for (MBBI = prev_inst(MBBI); MBBI; MBBI = prev_inst(MBBI)) {
+      // Pseudo's aren't real instructions, so they don't count.
+      if (!MBBI->isPseudo()) {
+        if (MBBI->isCall())
+          EmitAndCountInstruction(MCInstBuilder(X86::NOOP));
+        break;
+      }
+    }
+    return;
   }
 
   MCInst TmpInst;
   MCInstLowering.Lower(MI, TmpInst);
   EmitAndCountInstruction(TmpInst);
 }

test/CodeGen/X86/win64_call_epi.ll

+; RUN: llc < %s -mtriple=x86_64-pc-mingw32 | FileCheck %s -check-prefix=WIN64
+
+declare void @bar()
+declare void @baz()
+declare i32 @personality(...)
+
+; Check for 'nop' between the last call and the epilogue.
+define void @foo1() {
+
+    invoke void @bar()
+        to label %normal
+        unwind label %catch
+
+normal:
+    ret void
+
+catch:
+    %1 = landingpad { i8*, i32 } personality i32 (...)* @personality cleanup
+    resume { i8*, i32 } %1
+}
+; WIN64-LABEL: foo1:
+; WIN64: .seh_proc foo1
+; WIN64: callq bar
+; WIN64: nop
+; WIN64: addq ${{[0-9]+}}, %rsp
+; WIN64: retq
+; Check for 'ud2' after noreturn call
+; WIN64: callq _Unwind_Resume
+; WIN64-NEXT: ud2
+; WIN64: .seh_endproc
+
+
+; Check it still works when blocks are reordered.
+@something = global i32 0
+define void @foo2(i1 zeroext %cond ) {
+    br i1 %cond, label %a, label %b, !prof !0
+a:
+    call void @bar()
+    br label %done
+b:
+    call void @baz()
+    store i32 0, i32* @something
+    br label %done
+done:
+    ret void
+}
+!0 = metadata !{metadata !"branch_weights", i32 100, i32 0}
+; WIN64-LABEL: foo2:
+; WIN64: callq bar
+; WIN64: nop
+; WIN64: addq ${{[0-9]+}}, %rsp
+; WIN64: retq
+
+
+; Check nop is not emitted when call is not adjacent to epilogue.
+define i32 @foo3() {
+    call void @bar()
+    ret i32 0
+}
+; WIN64-LABEL: foo3:
+; WIN64: callq bar
+; WIN64: xorl
+; WIN64-NOT: nop
+; WIN64: addq ${{[0-9]+}}, %rsp
+; WIN64: retq