[PATCH] D20263: X86: Avoid using _chkstk when lowering WIN_ALLOCA instructions

[Michael Kuperstein via llvm-commits]

mkuper added inline comments.

================
Comment at: lib/Target/X86/X86.h:62
@@ -61,1 +61,3 @@
 
+/// XXX
+FunctionPass *createX86WinAllocaExpander();
----------------
I think this needs a description. :-)

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:16606
@@ -16609,1 +16605,3 @@
+    unsigned SizeReg = MRI.createVirtualRegister(getRegClassFor(SPTy));
+    Chain = DAG.getCopyToReg(Chain, dl, SizeReg, Size);
     SDVTList NodeTys = DAG.getVTList(MVT::Other, MVT::Glue);
----------------
Do we still need a CopyToReg? The previous code had it because it needed specifically a copy to EAX glued to the future-chkstk. Can you use Size directly?

================
Comment at: lib/Target/X86/X86WinAllocaExpander.cpp:120
@@ +119,3 @@
+  // Compute the reverse post-order.
+  SmallVector<MachineBasicBlock*, 16> RPO;
+  RPO.reserve(MF.size());
----------------
We have a ReversePostOrderTraversal<> in ADT. Can you use that, or is it inappropriate?

================
Comment at: lib/Target/X86/X86WinAllocaExpander.cpp:166
@@ +165,3 @@
+      // A stackrestore makes the offset unknown.
+      if (MI.isCopy() && MI.getOperand(0).isReg() &&
+          MI.getOperand(0).getReg() == StackPtr)
----------------
Are you sure this is conservative enough?
Perhaps blacklist anything that defs the stack pointer, except for a specific whitelist (calls, pushes, pops...) that touches the tip? Or would we get a huge whitelist?

================
Comment at: lib/Target/X86/X86WinAllocaExpander.cpp:263
@@ +262,3 @@
+  if (MF.getFunction()->hasFnAttribute("stack-probe-size")) {
+    MF.getFunction()
+        ->getFnAttribute("stack-probe-size")
----------------
StackProbeSize = MF.... ?
(Perhaps add a test for this?)


http://reviews.llvm.org/D20263