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

Eric Christopher echristo at gmail.com
Thu Jan 30 17:49:26 PST 2014



================
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;
----------------
Might want to explain when we would expect it to be negative.

================
Comment at: lib/Target/X86/X86FrameLowering.cpp:1529
@@ -1528,1 +1528,3 @@
           .addReg(StackPtr).addImm(Amount);
+      } else if (Amount < 0) {
+        Amount = -Amount;
----------------
Ditto.

================
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,
----------------
Rename this in a separate patch?

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

================
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();
----------------
This looks weird. What's up?

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7096
@@ -7093,3 +7095,3 @@
         Flags.setSRet();
-      if (Args[i].isByVal) {
+      if (Args[i].isByVal)
         Flags.setByVal();
----------------
This looks weird. 

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5386
@@ -5385,1 +5385,3 @@
   if (!CanLowerReturn) {
+    assert(!CS.hasInAllocaArgument() &&
+           "sret demotion is incompatible with inalloca");
----------------
Should probably document this assertion somewhere?

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

================
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 {
----------------
This change seems weird. Even if it's documented to be the last one I'm not sure why...


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



More information about the llvm-commits mailing list