[X86] Fix stack probe issue on windows32.

[LuoYuanke]

Summary

On windows if the frame size exceed 4096 bytes, compiler need to generate a call to _alloca_probe. X86CallFrameOptimization pass changes the reserved stack size and cause of stack probe function not be inserted. This patch fix the issue by detecting the call frame size, if the size exceed 4096 bytes, drop X86CallFrameOptimization.

[rnk]

Seems reasonable, but I have some suggestions.

llvm/lib/Target/X86/X86CallFrameOptimization.cpp

247–254 ↗

(On Diff #214056)

I see this pattern for calculating the stack probe size is already duplicated twice. Please factor it out into a method of X86ISelLowering, perhaps next to getStackProbeSymbolName and update the two callers along with this one.

258–261 ↗

(On Diff #214056)

I see that TargetInstrInfo uses the phrase "frame size" to refer to the size of the arguments used to pass call arguments, but I think that's easily confused with the size of the current function's stack frame. Maybe this comment would be clearer:

If any call allocates more argument stack memory than the stack probe size, don't do this optimization. Otherwise, this pass would need to synthesize additional stack probe calls to allocate memory for arguments.

263 ↗

(On Diff #214056)

Please move this check into the isLegal method where other preconditions are checked.

[LuoYuanke]

Update the patch corresponding to Reid's comments.

[rnk]

lgtm, thanks!

llvm/trunk/lib/Target/X86/X86CallFrameOptimization.cpp

bool X86CallFrameOptimization::isLegal(MachineFunction &MF) {
 
   // You would expect straight-line code between call-frame setup and
   // call-frame destroy. You would be wrong. There are circumstances (e.g.
   // CMOV_GR8 expansion of a select that feeds a function call!) where we can
   // end up with the setup and the destroy in different basic blocks.
   // This is bad, and breaks SP adjustment.
   // So, check that all of the frames in the function are closed inside
   // the same block, and, for good measure, that there are no nested frames.
+  //
+  // If any call allocates more argument stack memory than the stack
+  // probe size, don't do this optimization. Otherwise, this pass
+  // would need to synthesize additional stack probe calls to allocate
+  // memory for arguments.
   unsigned FrameSetupOpcode = TII->getCallFrameSetupOpcode();
   unsigned FrameDestroyOpcode = TII->getCallFrameDestroyOpcode();
+  bool UseStackProbe =
+      !STI->getTargetLowering()->getStackProbeSymbolName(MF).empty();
+  unsigned StackProbeSize = STI->getTargetLowering()->getStackProbeSize(MF);
   for (MachineBasicBlock &BB : MF) {
     bool InsideFrameSequence = false;
     for (MachineInstr &MI : BB) {
       if (MI.getOpcode() == FrameSetupOpcode) {
+        if (TII->getFrameSize(MI) >= StackProbeSize && UseStackProbe)
+          return false;
         if (InsideFrameSequence)
           return false;
         InsideFrameSequence = true;
       } else if (MI.getOpcode() == FrameDestroyOpcode) {
         if (!InsideFrameSequence)
           return false;
         InsideFrameSequence = false;
       }

llvm/trunk/lib/Target/X86/X86FrameLowering.cpp

void X86FrameLowering::emitPrologue(MachineFunction &MF,
   if (TailCallReturnAddrDelta && IsWin64Prologue)
     report_fatal_error("Can't handle guaranteed tail call under win64 yet");
 
   if (TailCallReturnAddrDelta < 0)
     X86FI->setCalleeSavedFrameSize(
       X86FI->getCalleeSavedFrameSize() - TailCallReturnAddrDelta);
 
   bool UseStackProbe = !STI.getTargetLowering()->getStackProbeSymbolName(MF).empty();
-
-  // The default stack probe size is 4096 if the function has no stackprobesize
-  // attribute.
-  unsigned StackProbeSize = 4096;
-  if (Fn.hasFnAttribute("stack-probe-size"))
-    Fn.getFnAttribute("stack-probe-size")
-        .getValueAsString()
-        .getAsInteger(0, StackProbeSize);
+  unsigned StackProbeSize = STI.getTargetLowering()->getStackProbeSize(MF);
 
   // Re-align the stack on 64-bit if the x86-interrupt calling convention is
   // used and an error code was pushed, since the x86-64 ABI requires a 16-byte
   // stack alignment.
   if (Fn.getCallingConv() == CallingConv::X86_INTR && Is64Bit &&
       Fn.arg_size() == 2) {
     StackSize += 8;
     MFI.setStackSize(StackSize);

llvm/trunk/lib/Target/X86/X86ISelLowering.h

unsigned getNumRegistersForCallingConv(LLVMContext &Context,
                                            EVT VT) const override;
 
     bool isIntDivCheap(EVT VT, AttributeList Attr) const override;
 
     bool supportSwiftError() const override;
 
     StringRef getStackProbeSymbolName(MachineFunction &MF) const override;
 
+    unsigned getStackProbeSize(MachineFunction &MF) const;
+
     bool hasVectorBlend() const override { return true; }
 
     unsigned getMaxSupportedInterleaveFactor() const override { return 4; }
 
     /// Lower interleaved load(s) into target specific
     /// instructions/intrinsics.
     bool lowerInterleavedLoad(LoadInst *LI,
                               ArrayRef<ShuffleVectorInst *> Shuffles,

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

if (!Subtarget.isOSWindows() || Subtarget.isTargetMachO() ||
     return "";
 
   // We need a stack probe to conform to the Windows ABI. Choose the right
   // symbol.
   if (Subtarget.is64Bit())
     return Subtarget.isTargetCygMing() ? "___chkstk_ms" : "__chkstk";
   return Subtarget.isTargetCygMing() ? "_alloca" : "_chkstk";
 }
+
+unsigned
+X86TargetLowering::getStackProbeSize(MachineFunction &MF) const {
+  // The default stack probe size is 4096 if the function has no stackprobesize
+  // attribute.
+  unsigned StackProbeSize = 4096;
+  const Function &Fn = MF.getFunction();
+  if (Fn.hasFnAttribute("stack-probe-size"))
+    Fn.getFnAttribute("stack-probe-size")
+        .getValueAsString()
+        .getAsInteger(0, StackProbeSize);
+  return StackProbeSize;
+}

llvm/trunk/test/CodeGen/X86/nomovtopush.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=i386-pc-windows-msvc | FileCheck %s
+
+target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
+target triple = "i386-pc-windows-msvc"
+
+%struct._param_str = type { i32, i32, [4096 x i32], i32 }
+
+@g_d = common dso_local local_unnamed_addr global i32 0, align 4
+@g_c = common dso_local local_unnamed_addr global i32 0, align 4
+@g_b = common dso_local local_unnamed_addr global i32 0, align 4
+@g_a = common dso_local local_unnamed_addr global i32 0, align 4
+@g_param = common dso_local global %struct._param_str zeroinitializer, align 4
+
+; Function Attrs: nounwind
+define dso_local i32 @test() local_unnamed_addr {
+; CHECK-LABEL: test:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    pushl %edi
+; CHECK-NEXT:    pushl %esi
+; CHECK-NEXT:    movl $16396, %eax # imm = 0x400C
+; CHECK-NEXT:    calll __chkstk
+; CHECK-NEXT:    movl _g_d, %eax
+; CHECK-NEXT:    movl _g_c, %ecx
+; CHECK-NEXT:    movl _g_b, %edx
+; CHECK-NEXT:    movl _g_a, %esi
+; CHECK-NEXT:    movl %eax, {{[0-9]+}}(%esp)
+; CHECK-NEXT:    movl %ecx, {{[0-9]+}}(%esp)
+; CHECK-NEXT:    movl %edx, {{[0-9]+}}(%esp)
+; CHECK-NEXT:    movl %esi, (%esp)
+; CHECK-NEXT:    calll _bar
+; CHECK-NEXT:    movl $4099, %ecx # imm = 0x1003
+; CHECK-NEXT:    movl %esp, %edi
+; CHECK-NEXT:    movl $_g_param, %esi
+; CHECK-NEXT:    rep;movsl (%esi), %es:(%edi)
+; CHECK-NEXT:    calll _foo
+; CHECK-NEXT:    xorl %eax, %eax
+; CHECK-NEXT:    addl $16396, %esp # imm = 0x400C
+; CHECK-NEXT:    popl %esi
+; CHECK-NEXT:    popl %edi
+; CHECK-NEXT:    retl
+entry:
+  %0 = load i32, i32* @g_d, align 4, !tbaa !3
+  %1 = load i32, i32* @g_c, align 4, !tbaa !3
+  %2 = load i32, i32* @g_b, align 4, !tbaa !3
+  %3 = load i32, i32* @g_a, align 4, !tbaa !3
+  %call = tail call i32 @bar(i32 %3, i32 %2, i32 %1, i32 %0) #2
+  tail call void @foo(%struct._param_str* byval nonnull align 4 @g_param) #2
+  ret i32 0
+}
+
+declare dso_local i32 @bar(i32, i32, i32, i32) local_unnamed_addr
+
+declare dso_local void @foo(%struct._param_str* byval align 4) local_unnamed_addr
+
+!3 = !{!4, !4, i64 0}
+!4 = !{!"int", !5, i64 0}
+!5 = !{!"omnipotent char", !6, i64 0}
+!6 = !{!"Simple C/C++ TBAA"}