[PATCH] Implement inalloca codegen for x86 with the new inalloca design

[Reid Kleckner]

  I'll split some stuff and reupload.


================
Comment at: lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp:78
@@ +77,3 @@
+    if (const AllocaInst *AI = dyn_cast<AllocaInst>(I)) {
+      if (!AI->isStaticAlloca())
+        continue;
----------------
Eric Christopher wrote:
> Eh? I think I see it but it could use a comment.
sure

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5386
@@ -5385,1 +5385,3 @@
   if (!CanLowerReturn) {
+    assert(!CS.hasInAllocaArgument() &&
+           "sret demotion is incompatible with inalloca");
----------------
Eric Christopher wrote:
> Should probably document this assertion somewhere?
Added some docs.  I'm not really sure it's worth it though.  This feature was added in r86876 by Kenneth Uildricks (remember him?) and never documented, so far as I know.

================
Comment at: lib/Target/X86/X86FastISel.cpp:2490
@@ -2485,2 +2489,3 @@
     return 0;
+  assert(!C->isUsedWithInAlloca());
 
----------------
Eric Christopher wrote:
> Comment?
sure

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:2933
@@ -2915,3 +2932,3 @@
   // Create the CALLSEQ_END node.
-  unsigned NumBytesForCalleeToPush;
+  unsigned NumBytesForCalleeToPop;
   if (X86::isCalleePop(CallConv, Is64Bit, isVarArg,
----------------
Eric Christopher wrote:
> Rename this in a separate patch?
Sure.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7096
@@ -7093,3 +7095,3 @@
         Flags.setSRet();
-      if (Args[i].isByVal) {
+      if (Args[i].isByVal)
         Flags.setByVal();
----------------
Eric Christopher wrote:
> This looks weird. 
I'll try to expand this comment here.

================
Comment at: include/llvm/Support/CallSite.h:271
@@ -270,3 +270,3 @@
 
-  /// @brief Determine if there are any inalloca arguments.
+  /// @brief Determine if the last argument is an inalloca argument.
   bool hasInAllocaArgument() const {
----------------
Eric Christopher wrote:
> This change seems weird. Even if it's documented to be the last one I'm not sure why...
It has to be last because it holds all the arguments passed in memory, and memory arguments are always the last parameters in all supported calling conventions.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7323
@@ -7313,3 +7322,3 @@
         Flags.setSRet();
-      if (F.getAttributes().hasAttribute(Idx, Attribute::ByVal)) {
+      if (F.getAttributes().hasAttribute(Idx, Attribute::ByVal))
         Flags.setByVal();
----------------
Eric Christopher wrote:
> This looks weird. What's up?
ditto

================
Comment at: lib/Target/X86/X86FrameLowering.cpp:1495
@@ -1494,3 +1494,3 @@
   DebugLoc DL = I->getDebugLoc();
-  uint64_t Amount = !reseveCallFrame ? I->getOperand(0).getImm() : 0;
-  uint64_t CalleeAmt = isDestroy ? I->getOperand(1).getImm() : 0;
+  int64_t Amount = !reseveCallFrame ? I->getOperand(0).getImm() : 0;
+  int64_t CalleeAmt = isDestroy ? I->getOperand(1).getImm() : 0;
----------------
Eric Christopher wrote:
> Might want to explain when we would expect it to be negative.
I think this only happened in an intermediate state when I had bugs, because CalleeAmt was greater than Amount.  I'm going to commit this separately as an assertion that Amount is positive.  addImm takes a signed int64_t, so the signed type seems correct here.


http://llvm-reviews.chandlerc.com/D2637