[AIX] Implement formal arguments passed in stack memory

[ZarkoCA]

Summary

This patch is the callee side for https://reviews.llvm.org/D73209. It removes the fatal error when we pass more formal arguments than available registers.

[cebowleratibm]

There's some trailing whitespace that emits warnings when I apply the patch. Probably a good idea to tidy it up. I'll continue to review the content of that patch.

$ git apply D74225.diff.txt
D74225.diff.txt:157: trailing whitespace.
; 32BIT: - { id: 3, type: default, offset: 80, size: 4
D74225.diff.txt:161: trailing whitespace.
; 32BIT: - { id: 7, type: default, offset: 64, size: 4
D74225.diff.txt:179: trailing whitespace.
; 64BIT: - { id: 0, type: default, offset: 168, size: 8
D74225.diff.txt:180: trailing whitespace.
; 64BIT: - { id: 1, type: default, offset: 160, size: 8
D74225.diff.txt:181: trailing whitespace.
; 64BIT: - { id: 2, type: default, offset: 152, size: 8
warning: squelched 10 whitespace errors
warning: 15 lines add whitespace errors.

[cebowleratibm]

Did a passthrough of the test changes with a number of comments. I expect you wanted to make some test updates, nonetheless I thought I'd document what I'd like to see in the test coverage. Will review code changes next.

llvm/test/CodeGen/PowerPC/aix-cc-abi.ll

1302

suggest using 32BIT-DAG for the liveins.

1311

I suggest 32BIT-DAG and reorder the memory locations in growing offset. IMO it makes it easier to read and understand the test.

1327

DAG.

1336

DAG and reorder.

1346

I would like to see the expected output for the assembly added. We should validate that the instructions emitted locate the arguments in the correct memory location. We should also strive for consistency with the other tests, which have expected assembly.

1374

suggest DAG.

1387

remove blank line or add blank line in the earlier test for the sake of consistency.

1400

I would also like to see the expected assembly here as well.

1401

Additional test requests:
-1 call that passes 1, 2, 4, 8 byte ints and 4, 8 byte floats in memory (mixed with all sizes represented.)
-burn the first 8 GPR on float args, then confirm integer args passed in memory.

ex: foo(double, double, double, double, char, short, int)

[ZarkoCA]

Added assembly tests and fixed white space errors in tests.

[cebowleratibm]

This concludes my initial review. Mostly tidy up comments. The logic seems correct so semantically I think it's ok (for the incremental support that it adds.) I'll see if I can break it on the next round.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

6957

I think it would be useful to add a !byVal assertion on the args just to make it clear the logic you've implemented is not intended to handle this case.

6959

whitespace. formatting.

6960

I think it's better if this def is placed closer to its use.

6979

I'd rather see this pushed into the respective conditional blocks. It's not used outside so it's cleaner to contain it locally.

6982

I think this should be an assertion. CC_AIX should never push a loc which is neither reg nor mem. You could switch this up a bit:

if (VA.isRegLoc()) { ...}
assume(VA.isMemLoc)
// no need for "if (VA.isMemLoc())"

7008

Since there doesn't appear to be any necessary handling to sign or zero extend on this code path, should there be an assertion?

7023

formatting.

llvm/test/CodeGen/PowerPC/aix-cc-abi.ll

1271

The convention we've used for cc tests so far is to do expected output for both callee and caller. I'd like to see the caller IR and expected output as well.

[ZarkoCA]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

6982

I can change that to an assert. I was only following the precedent in LowerCall_AIX which uses 'report_fatal_error'.

I can check the code to:

 assert((VA.isRegLoc() || VA.isMemLoc())

if (VA.isRegloc()){
  register code
  ...
}else {
assert(VA.isMemLoc())
  memory code
  ...
}

Does that work?

[ZarkoCA]

Added new testcases which include both callee and callee IR and assembly expected output.
Addressed comments:

1. Added ByVal assertion
2. Replaced no RegLoc/MemLoc fatal error with an assertion
3. Moved variable definitions closer to where they are used
4. Fixed formatting

[ZarkoCA]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

6957

Good idea, added.

7008

I added assert(ObjSize <= ArgSize); is this what you had in mind? Or is a different assert specifically related to sign or zero extension needed?

llvm/test/CodeGen/PowerPC/aix-cc-abi.ll

1271

Added both caller and callee for all tests.

1311

Agreed, it does look better once done that way.

[ZarkoCA]

llvm/test/CodeGen/PowerPC/aix-cc-abi.ll

1346

Added assembly expected output.

1401

Thanks, that's a really good testcase. I added it as mix_callee below along with the respective caller testcase.

[cebowleratibm]

Code semantics look ok. If you can post a revision to address the minor concerns I need to have one more pass-through of the test updates in detail.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

7045

suggest continue here, then get rid of the else { }

7049

you already latched VA.getLocVT and VA.getValVT into locals you can reuse.

7051

I find the use of name "ObjSize" and "ArgSize" confusing. Suggest sticking to terminology "ValueSize" and "LocSize".

7054

suggest:

// AIX objects are right justified because they are word written to stack memory from their register image.

llvm/test/CodeGen/PowerPC/aix-cc-abi.ll

1311

suggest 32BIT-LABEL to tag the expected sections.

1350

Suggest adding a LABEL check for the assembly output. Applies multiple.

1351

suggest reordering these lwz closer to the reg use for readability.

[ZarkoCA]

Addressed comments and fixed testcases.

[ZarkoCA]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

7051

Thanks, I agree it's better calling them something else.

[cebowleratibm]

All minor comments except for the concern on whether or not a truncation node is needed when ValSize < LocSize.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

6846

I agree with this change though I plan that it will be committed on another change to correct the caller stackarg patch I authored previously. I think it's best to do this as an independent change with new test coverage for what was missed.

6850

this comment is now in the wrong place. It belongs with line 6844.

7028–7029

MinReservedArea is now a misnomer. This is effectively the LSA + PSA size, whatever the PSA size was evaluated to. Min is always 56 in 32-bit and 112 in 64-bit.

7029

Do we need to add a truncate node when ValueSize is less than LocSize?

Suggest "ValSize".

7054

updated suggestion:

// Objects are right-justified because AIX is big-endian.

I tend to avoid the use of the term word because in Power ISA it means 4 bytes. To others it means PtrByteSize.

llvm/test/CodeGen/PowerPC/aix-cc-abi.ll

1301

32BIT-LABEL ?

1327

64BIT-LABEL probably better.

1337

64BIT-LABEL

1565

CHECK-LABEL

1580

CHECK-LABEL

1589

CHECK-LABEL

1817

LABEL

1823

LABEL

1832

LABEL

1846

LABEL

[ZarkoCA]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

6846

I agree, it wasn't meant to be included here initially. I added it when trying to figure out some of the offsets I was seeing. I will keep it in but let the follow up patch which includes this land first before I do any commits of this one.

7028–7029

I think the name is used in all PPC targets because of the call to FuncInfo->setMinReservedArea, I agree with the comment though.

I changed it to CallerReservedArea. I think if I do that, then we don't even need the comment.

7029

From what I've looked into, no other PPC targets add a truncate node in their respective LowerFormalArguments() functions. I did find it being done on x86 for i1s, however, and poked around to see why the difference is there.

As far as I understand, it looks like we don't need to do on PPC since PPCTargetLowering::LowerLOAD() will do that when we set the load node in LowerFormalArguments.

The relevant code snippet from PPCTargetLowering::LowerLOAD() is below:

SDValue NewLD =
      DAG.getExtLoad(ISD::EXTLOAD, dl, getPointerTy(DAG.getDataLayout()), Chain,
                     BasePtr, MVT::i8, MMO);
  SDValue Result = DAG.getNode(ISD::TRUNCATE, dl, MVT::i1, NewLD);

I'm fairly sure that's the reason why we don't see truncation to loads from memory in the various PPC LowerFormalArguments() since it's done in LowerLoad for certain types.

llvm/test/CodeGen/PowerPC/aix-cc-abi.ll

1301

Sorry, missed those in the initial update.

[ZarkoCA]

Added Check-Label to testcases.
Changed variable names.
Fixed testcases to improve readability.

[cebowleratibm]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

6844–6848

Braces are no longer necessary.

6853

trailing whitespace.

7025

I don't feel that this comment adds to understanding the code. I suggest removing it.

7027

I don't feel that this comment adds to understanding the code. I suggest removing it.

7035

formatting.

[ZarkoCA]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

7029

To add further, the special load handling is needed for i1s since we cant do i1 sized loads on Power. We extend to an i8 and then truncate.

Also, in 64Bit mode for example, the caller will store parameters sized 4 bytes or less with 8 byte loads and sign or zero extended to 8bytes. For the callee, we adjust the offset where the load happens to load only the 4bytes containing the argument and then do a 4byte load from there. There shouldn't be a need to truncate in that case.

[ZarkoCA]

Fixed trailing whitespace issues when applying the patch.
Fixed formatting and removed redundant comments.

[cebowleratibm]

I think we need some changes to ensure frame objects are created with their proper size and not word size. i64 is passed as two i32s but we should coalesce these into a single 8-byte object. Like-wise arguments smaller than word size are generating frame objects that are word size.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

7054

I don't see the updated comment reflected in the current patch. I still see:

// AIX objects are right justified because they are word written to stack
// memory from their register image.

and I would prefer this to say

// Objects are right-justified because AIX is big-endian.

llvm/test/CodeGen/PowerPC/aix-cc-abi.ll

1320

When i looked at Linux PPC32 I saw that they used 8 byte frame objects for the long long parameters. I think we should be consistent with the precedent.

1352

I got confused by why there's no load at 56 to account for the msw of the 9th parameter (i64) then realized the result is stored in i32, so the optimizer is probably getting clever. To keep the test straight forward and avoid confusion, I'd suggest accumulating the result in an i64 so that we'll see (and can validate) the load at 56.

1356

My preference is to see the:
add 3, 3, 5
add 3, 3, 6
add 3,3, 7
...
as a block and coalesce the stack loads next to where they're used.

1382

The ASM32PWR4-DAG expects the add operations but they're omitted in the ASM64PWR4-DAG expected output.

I don't strongly prefer either though I strongly prefer consistency.

1409

I usually name the toc load appending ADDR to indicate the register holds the address of the variable, then name the loads after to indicate they hold the value of the variable. Seems the naming you have here his backwards.

1438

Logically I like to see the parameter initializations in order. I find it easier to understand in the approach.

1499

would be a bit nicer to tidy the order of parameter reg inits.

1632

A little bit nicer if these are moved next to their use in the DAG block.

1693

why are all the other STW flagged "killed" and this one not?

1713

I don't think that it's useful to expect the constant pool loads given that the STW matches on SCRATCHREG. There's no validation of where each constant is written and we're not intending to test the constant loads anyways.

1781

REGDADDR

1784

lfd [[REGD:[0-9]+]], 0([[REGDADDR]])

1785

diddo on REGF1 and REGF2

1861

Should be size 2. Though the arg passes in 4 bytes, the object is 2 bytes at offset 58.

[sfertile]

In D74225#1894003, @cebowleratibm wrote:

I think we need some changes to ensure frame objects are created with their proper size and not word size.

We have 2 choices for the smaller then save slot sized objects:

1. Follow XL's behavior and perform a load of the entire save slot, and create frame objects of this size to match. If we chose this we would have to build and insert 'AssertSExt' and 'AssertZExt` nodes and truncates appropriately. The truncates should get cleaned up down stream due to the assert-extended nodes.
2. Perform the offset adjustment and load using the load instruction with the correct extending type, and have frame objects created at the correct offset matching the size of the load.

It seems we are mixing these 2 right now? but I have to play with some smaller tests to make sure my understanding is correct.

Having the caller and callee semantics in the same test is helpful, but it leads to having really large tests. Can we split the tests into 2. 1 for everything fits in regs and 1 for when we have to go to the stack?

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

6846

I don't see the LocInfo being used on the Memloc path for lower formal args, so this shouldn't semantically affect this patch.Is it included because it affects the lit tests in an observable way? If it doesn't pull it out, if it does then you can pull this patch into your branch and use just the diff of your changes for this review.

7031

Why don't we have to perform the adjustment for ppc32? IIUC ValSize is promoted to i32 for i16/i8, but what about i1s? They shouldn't be promoted by the generic selection dag code because they are a legal register type on PowerPC. At the very least we need a lit test that includes i1 zeroext passed in a stack slot. Sorry if there is one, I scanned through and don't see one but the test changes are 1000 lines long :)

[ZarkoCA]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

7054

I missed this, sorry about that.

llvm/test/CodeGen/PowerPC/aix-cc-abi.ll

1320

I spent a while investigating this and it looks like the PPC32 and 32BIT AIX do the same thing here.

I see that both create two 4yte sized fixed frame objects for one i64 int:
AIX:

fixedStack:
  - { id: 0, type: default, offset: 60, size: 4, alignment: 4, stack-id: default,
      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
  - { id: 1, type: default, offset: 56, size: 4, alignment: 8, stack-id: default,
      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }

PPC32

fixedStack:
  - { id: 0, type: default, offset: 12, size: 4, alignment: 4, stack-id: default,
      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
  - { id: 1, type: default, offset: 8, size: 4, alignment: 8, stack-id: default,
      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }

However, if we don't run the IR under opt I see we add it adds allocas and stores for each of the parameter and this is what I think creates local stack objects for all the parameters, including a size 8 for the i64 parameter in both AIX and PPC32.

On PPC32 and 32BIT AIX all of these local stack objects are at offset 0 and a negative local-offset, their size is :

- { id: 8, name: ll9.addr, type: default, offset: 0, size: 8, alignment: 8,
    stack-id: default, callee-saved-register: '', callee-saved-restored: true,
    local-offset: -40, debug-info-variable: '', debug-info-expression: '',
    debug-info-location: '' }

I'm not sure that what the use of these stack objects is since they are eliminated if I run the IR under opt which is what I've done to generate the IR for all test cases. I think they are related to lowering of the stack frame but I'm not entirely sure yet.

1352

My initial thought in doing it this was to show that on AIX the upper word of the i64 holds the most significant values. But I agree, it's confusing and it's better to accumulate in an i64.

1356

I think it's best to remove these as per your comment and my reply below.

1382

This is inconsistent, your right. I'm leaning toward only showing the loads and stores in these test cases to more clear about what we are testing. It's also the fastest change to make.

1409

Thanks, fixed now.

1438

Yes, that's better, I agree.

1693

I'm not sure...I found that if I change the value that's held in that register (the double 1.000000e-01) to anything else, the register is then flagged "killed" like the rest.

I will need to follow up with some more investigation to find what determines when a register will not be killed in a case like this.

1713

In this case here I'm using SCRATCHREG only to not hard code the registers in which we do the constant pool loads. Hypothetically, they can be any register and I don't want the test to break if any of those scratch registers change at any point in time. I could take them out altogether maybe?

1861

To me it looks like CC_AIX promotes i16 to i32s, and the ValVT passed to getLoad() is i32, so I think this is why it does a 4 byte load at offset 56.

// Promote integers if needed.
    if (ValVT.getSizeInBits() < RegVT.getSizeInBits())
      LocInfo = ArgFlags.isSExt() ? CCValAssign::LocInfo::SExt
                                  : CCValAssign::LocInfo::ZExt;

Is this not correct in 32BIT mode? I see PPC32 do the same thing and XLC on AIX also using a 4byte load (lwz) at offset 56 for an i16.

[ZarkoCA]

Updated comment.
Fixed testcases:

• Rearranged IR and assembly of parameters in which they are initialized.
• Renamed filecheck variables to follow convention already established in aix-cc-abi.ll

[ZarkoCA]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

6846

It's not needed for the lower formal args but this patch includes caller tests which are affected.

7031

Sorry, @sfertile I missed your comments and you're right. The i1 zeroext is a case which we'll need to do exactly that. I will remove the IsPPC64 and a test for it.

[ZarkoCA]

Change the offset for args smaller than loc size on 32BIT too and add a bool stack test case.

[ZarkoCA]

In D74225#1894378, @sfertile wrote:

In D74225#1894003, @cebowleratibm wrote:

I think we need some changes to ensure frame objects are created with their proper size and not word size.

We have 2 choices for the smaller then save slot sized objects:

1. Follow XL's behavior and perform a load of the entire save slot, and create frame objects of this size to match. If we chose this we would have to build and insert 'AssertSExt' and 'AssertZExt` nodes and truncates appropriately. The truncates should get cleaned up down stream due to the assert-extended nodes.
2. Perform the offset adjustment and load using the load instruction with the correct extending type, and have frame objects created at the correct offset matching the size of the load.

It seems we are mixing these 2 right now? but I have to play with some smaller tests to make sure my understanding is correct.

On the callee side, for i32s on 64BIT and i1s in 32/64BIT we create fixed objects the size of the value type and then use the appropriate load (lw or lb). Except for i8 and i16s which I think are extended to fit the reg size. I think this is consistent with LLVM PPC32.

On the caller side we always do a PtrByteSize store.

Having the caller and callee semantics in the same test is helpful, but it leads to having really large tests. Can we split the tests into 2. 1 for everything fits in regs and 1 for when we have to go to the stack?

Maybe we can do an NFC patch which splits the tests?

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

7031

I made it so we to the adjustment on 32BIT too and added a bool test case.

[ZarkoCA]

Fix whitespace errors, sorry for the noise.

[ZarkoCA]

Rebase off https://reviews.llvm.org/D75126 and fix expected output of caller side tests based on that patch.

[sfertile]

LGTM.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

static bool CC_AIX(unsigned ValNo, MVT ValVT, MVT LocVT,
   case MVT::i64:
     // i64 arguments should have been split to i32 for PPC32.
     assert(IsPPC64 && "PPC32 should have split i64 values.");
     LLVM_FALLTHROUGH;
   case MVT::i1:
   case MVT::i32: {
     const unsigned Offset = State.AllocateStack(PtrByteSize, PtrByteSize);
     const MVT RegVT = IsPPC64 ? MVT::i64 : MVT::i32;
     if (unsigned Reg = State.AllocateReg(IsPPC64 ? GPR_64 : GPR_32)) {
       // Promote integers if needed.
       if (ValVT.getSizeInBits() < RegVT.getSizeInBits())
         LocInfo = ArgFlags.isSExt() ? CCValAssign::LocInfo::SExt
                                     : CCValAssign::LocInfo::ZExt;
       State.addLoc(CCValAssign::getReg(ValNo, ValVT, Reg, RegVT, LocInfo));
     }
     else
       State.addLoc(CCValAssign::getMem(ValNo, ValVT, Offset, RegVT, LocInfo));
 
     return false;
   }
   case MVT::f32:
   case MVT::f64: {
     // Parameter save area (PSA) is reserved even if the float passes in fpr.
     const unsigned StoreSize = LocVT.getStoreSize();
     // Floats are always 4-byte aligned in the PSA on AIX.
     // This includes f64 in 64-bit mode for ABI compatibility.
if (getTargetMachine().Options.GuaranteedTailCallOpt)
     report_fatal_error("Tail call support is unimplemented on AIX.");
 
   if (useSoftFloat())
     report_fatal_error("Soft float support is unimplemented on AIX.");
 
   const PPCSubtarget &Subtarget =
       static_cast<const PPCSubtarget &>(DAG.getSubtarget());
   if (Subtarget.hasQPX())
     report_fatal_error("QPX support is not supported on AIX.");
 
+ // Potential tail calls could cause overwriting of argument stack slots.
+  const bool IsImmutable = !(getTargetMachine().Options.GuaranteedTailCallOpt &&
+                             (CallConv == CallingConv::Fast));
   const bool IsPPC64 = Subtarget.isPPC64();
   const unsigned PtrByteSize = IsPPC64 ? 8 : 4;
 
   // Assign locations to all of the incoming arguments.
   SmallVector<CCValAssign, 16> ArgLocs;
   MachineFunction &MF = DAG.getMachineFunction();
   CCState CCInfo(CallConv, isVarArg, MF, ArgLocs, *DAG.getContext());
 
+  const EVT PtrVT = getPointerTy(MF.getDataLayout());
   // Reserve space for the linkage area on the stack.
   const unsigned LinkageSize = Subtarget.getFrameLowering()->getLinkageSize();
-  // On AIX a minimum of 8 words is saved to the parameter save area.
-  const unsigned MinParameterSaveArea = 8 * PtrByteSize;
-  CCInfo.AllocateStack(LinkageSize + MinParameterSaveArea, PtrByteSize);
+  CCInfo.AllocateStack(LinkageSize, PtrByteSize);
   CCInfo.AnalyzeFormalArguments(Ins, CC_AIX);
 
   for (CCValAssign &VA : ArgLocs) {
-
-    if (VA.isMemLoc()) {
-      // For compatibility with the AIX XL compiler, the float args in the
-      // parameter save area are initialized even if the argument is available
-      // in register.  The caller is required to initialize both the register
-      // and memory, however, the callee can choose to expect it in either.  The
-      // memloc is dismissed here because the argument is retrieved from the
-      // register.
-      if (VA.needsCustom())
-        continue;
-      report_fatal_error(
-          "Handling of formal arguments on the stack is unimplemented!");
-    }
-
-    assert(VA.isRegLoc() && "Unexpected argument location.");
-
     EVT ValVT = VA.getValVT();
     MVT LocVT = VA.getLocVT();
+    SDValue ArgValue;
+
+    if (!VA.isRegLoc() && !VA.isMemLoc())
+      report_fatal_error("Unexpected location for function call argument.");
+
+    if (VA.isRegLoc()) {
       MVT::SimpleValueType SVT = ValVT.getSimpleVT().SimpleTy;
       unsigned VReg =
           MF.addLiveIn(VA.getLocReg(), getRegClassForSVT(SVT, IsPPC64));
-    SDValue ArgValue = DAG.getCopyFromReg(Chain, dl, VReg, LocVT);
+      ArgValue = DAG.getCopyFromReg(Chain, dl, VReg, LocVT);
       if (ValVT.isScalarInteger() &&
           (ValVT.getSizeInBits() < LocVT.getSizeInBits())) {
         ISD::ArgFlagsTy Flags = Ins[VA.getValNo()].Flags;
         ArgValue =
             truncateScalarIntegerArg(Flags, ValVT, DAG, ArgValue, LocVT, dl);
       }
       InVals.push_back(ArgValue);
     }
 
+    if (VA.isMemLoc()) {
+      // For compatibility with the AIX XL compiler, the float args in the
+      // parameter save area are initialized even if the argument is available
+      // in register.  The caller is required to initialize both the register
+      // and memory, however, the callee can choose to expect it in either.
+      // The memloc is dismissed here because the argument is retrieved from
+      // the register.
+      if (VA.needsCustom())
+        continue;
+
+      // Get the extended size of the argument type in stack
+      const unsigned ArgSize = VA.getLocVT().getStoreSize();
+      // Get the actual size of the argument type
+      const unsigned ObjSize = VA.getValVT().getStoreSize();
+      int CurArgOffset = VA.getLocMemOffset();
+      // Objects in AIX are right justified.
+      if (ArgSize < ObjSize)
+        CurArgOffset += ArgSize - ObjSize;
+      MachineFrameInfo &MFI = MF.getFrameInfo();
+      int FI = MFI.CreateFixedObject(ArgSize, CurArgOffset, IsImmutable);
+      SDValue FIN = DAG.getFrameIndex(FI, PtrVT);
+      ArgValue =
+          DAG.getLoad(VA.getLocVT(), dl, Chain, FIN, MachinePointerInfo());
+      InVals.push_back(ArgValue);
+    }
+    }
+
+  // On AIX a minimum of 8 words is saved to the parameter save area.
+  const unsigned MinParameterSaveArea = 8 * PtrByteSize;
   // Area that is at least reserved in the caller of this function.
-  unsigned MinReservedArea = CCInfo.getNextStackOffset();
+  unsigned MinReservedArea =
+      std::max(CCInfo.getNextStackOffset(), LinkageSize + MinParameterSaveArea);
 
   // Set the size that is at least reserved in caller of this function. Tail
   // call optimized function's reserved stack space needs to be aligned so
   // that taking the difference between two stack areas will result in an
   // aligned stack.
   MinReservedArea =
       EnsureStackAlignment(Subtarget.getFrameLowering(), MinReservedArea);
   PPCFunctionInfo *FuncInfo = MF.getInfo<PPCFunctionInfo>();
   FuncInfo->setMinReservedArea(MinReservedArea);
 
   return Chain;
 }
 
 SDValue PPCTargetLowering::LowerCall_AIX(
     SDValue Chain, SDValue Callee, CallFlags CFlags,
     const SmallVectorImpl<ISD::OutputArg> &Outs,
     const SmallVectorImpl<SDValue> &OutVals,
     const SmallVectorImpl<ISD::InputArg> &Ins, const SDLoc &dl,
     SelectionDAG &DAG, SmallVectorImpl<SDValue> &InVals,
     ImmutableCallSite CS) const {
 
   assert((CFlags.CallConv == CallingConv::C ||
           CFlags.CallConv == CallingConv::Cold ||
           CFlags.CallConv == CallingConv::Fast) &&
          "Unexpected calling convention!");
 
   if (CFlags.IsPatchPoint)
     report_fatal_error("This call type is unimplemented on AIX.");
 
   const PPCSubtarget& Subtarget =
       static_cast<const PPCSubtarget&>(DAG.getSubtarget());
   if (Subtarget.hasQPX())
     report_fatal_error("QPX is not supported on AIX.");

llvm/test/CodeGen/PowerPC/aix-cc-abi.ll

 ; ASM64PWR4-DAG:   li 7, 5
 ; ASM64PWR4-DAG:   li 8, 6
 ; ASM64PWR4-DAG:   li 9, 7
 ; ASM64PWR4-DAG:   stfd 1, 120(1)
 ; ASM64PWR4-DAG:   ld 10, 120(1)
 ; ASM64PWR4-NEXT:  bl .test_stackarg_float3
 ; ASM64PWR4-NEXT:  nop
 ; ASM64PWR4-NEXT:  addi 1, 1, 128
+
+
+define i32 @test_ints_stack(i32 %i1, i32 %i2, i32 %i3, i32 %i4, i32 %i5, i32 %i6, i32 %i7, i32 %i8, i64 %ll9, i16 signext %s10, i8 zeroext %c11, i32 %ui12, i32 %si13, i64 %ll14, i8 zeroext %uc15, i32 %i16) {
+entry:
+  %add = add nsw i32 %i1, %i2
+  %add1 = add nsw i32 %add, %i3
+  %add2 = add nsw i32 %add1, %i4
+  %add3 = add nsw i32 %add2, %i5
+  %add4 = add nsw i32 %add3, %i6
+  %add5 = add nsw i32 %add4, %i7
+  %add6 = add nsw i32 %add5, %i8
+  %conv = sext i32 %add6 to i64
+  %add7 = add nsw i64 %conv, %ll9
+  %conv8 = sext i16 %s10 to i64
+  %add9 = add nsw i64 %add7, %conv8
+  %conv10 = zext i8 %c11 to i64
+  %add11 = add nsw i64 %add9, %conv10
+  %conv12 = zext i32 %ui12 to i64
+  %add13 = add nsw i64 %add11, %conv12
+  %conv14 = sext i32 %si13 to i64
+  %add15 = add nsw i64 %add13, %conv14
+  %add16 = add nsw i64 %add15, %ll14
+  %conv17 = zext i8 %uc15 to i64
+  %add18 = add nsw i64 %add16, %conv17
+  %conv19 = sext i32 %i16 to i64
+  %add20 = add nsw i64 %add18, %conv19
+  %conv21 = trunc i64 %add20 to i32
+  ret i32 %conv21
+}
+
+; CHECK-LABEL: name: test_ints_stack
+
+; 32BIT:       liveins:
+; 32BIT-NEXT:  - { reg: '$r3', virtual-reg: '' }
+; 32BIT-NEXT:  - { reg: '$r4', virtual-reg: '' }
+; 32BIT-NEXT:  - { reg: '$r5', virtual-reg: '' }
+; 32BIT-NEXT:  - { reg: '$r6', virtual-reg: '' }
+; 32BIT-NEXT:  - { reg: '$r7', virtual-reg: '' }
+; 32BIT-NEXT:  - { reg: '$r8', virtual-reg: '' }
+; 32BIT-NEXT:  - { reg: '$r9', virtual-reg: '' }
+; 32BIT-NEXT:  - { reg: '$r10', virtual-reg: '' }
+; 32BIT:       fixedStack:
+; 32BIT:       - { id: 0, type: default, offset: 92, size: 4
+; 32BIT:       - { id: 1, type: default, offset: 88, size: 4
+; 32BIT:       - { id: 2, type: default, offset: 84, size: 4
+; 32BIT:       - { id: 3, type: default, offset: 80, size: 4 
+; 32BIT:       - { id: 4, type: default, offset: 76, size: 4
+; 32BIT:       - { id: 5, type: default, offset: 72, size: 4
+; 32BIT:       - { id: 6, type: default, offset: 68, size: 4
+; 32BIT:       - { id: 7, type: default, offset: 64, size: 4 
+; 32BIT:       - { id: 8, type: default, offset: 60, size: 4
+; 32BIT:       - { id: 9, type: default, offset: 56, size: 4
+; 32BIT:       body:             |
+; 32BIT-NEXT:    bb.0.entry:
+; 32BIT-NEXT:      liveins: $r3, $r4, $r5, $r6, $r7, $r8, $r9, $r10
+
+
+; 64BIT:       liveins:
+; 64BIT-NEXT:  - { reg: '$x3', virtual-reg: '' }
+; 64BIT-NEXT:  - { reg: '$x4', virtual-reg: '' }
+; 64BIT-NEXT:  - { reg: '$x5', virtual-reg: '' }
+; 64BIT-NEXT:  - { reg: '$x6', virtual-reg: '' }
+; 64BIT-NEXT:  - { reg: '$x7', virtual-reg: '' }
+; 64BIT-NEXT:  - { reg: '$x8', virtual-reg: '' }
+; 64BIT-NEXT:  - { reg: '$x9', virtual-reg: '' }
+; 64BIT-NEXT:  - { reg: '$x10', virtual-reg: '' }
+; 64BIT:       fixedStack:
+; 64BIT:       - { id: 0, type: default, offset: 168, size: 8 
+; 64BIT:       - { id: 1, type: default, offset: 160, size: 8 
+; 64BIT:       - { id: 2, type: default, offset: 152, size: 8 
+; 64BIT:       - { id: 3, type: default, offset: 144, size: 8 
+; 64BIT:       - { id: 4, type: default, offset: 136, size: 8 
+; 64BIT:       - { id: 5, type: default, offset: 128, size: 8 
+; 64BIT:       - { id: 6, type: default, offset: 120, size: 8 
+; 64BIT:       - { id: 7, type: default, offset: 112, size: 8 
+; 64BIT:       body:             |
+; 64BIT-NEXT:    bb.0.entry:
+; 64BIT-NEXT:      liveins: $x3, $x4, $x5, $x6, $x7, $x8, $x9, $x10
+
+define double @test_fpr_stack(double %d1, double %d2, double %d3, double %d4, double %d5, double %d6, double %d7, double %d8, double %d9, double %s10, double %l11, double %d12, double %d13, float %f14, double %d15, float %f16) {
+  entry:
+    %add = fadd double %d1, %d2
+    %add1 = fadd double %add, %d3
+    %add2 = fadd double %add1, %d4
+    %add3 = fadd double %add2, %d5
+    %add4 = fadd double %add3, %d6
+    %add5 = fadd double %add4, %d7
+    %add6 = fadd double %add5, %d8
+    %add7 = fadd double %add6, %d9
+    %add8 = fadd double %add7, %s10
+    %add9 = fadd double %add8, %l11
+    %add10 = fadd double %add9, %d12
+    %add11 = fadd double %add10, %d13
+    %add12 = fadd double %add11, %d13
+    %conv = fpext float %f14 to double
+    %add13 = fadd double %add12, %conv
+    %add14 = fadd double %add13, %d15
+    %conv15 = fpext float %f16 to double
+    %add16 = fadd double %add14, %conv15
+    ret double %add16
+  }
+
+; CHECK-LABEL: name: test_fpr_stack{{.*}}
+
+; CHECK:      liveins:
+; CHECK-NEXT: - { reg: '$f1', virtual-reg: '' }
+; CHECK-NEXT: - { reg: '$f2', virtual-reg: '' }
+; CHECK-NEXT: - { reg: '$f3', virtual-reg: '' }
+; CHECK-NEXT: - { reg: '$f4', virtual-reg: '' }
+; CHECK-NEXT: - { reg: '$f5', virtual-reg: '' }
+; CHECK-NEXT: - { reg: '$f6', virtual-reg: '' }
+; CHECK-NEXT: - { reg: '$f7', virtual-reg: '' }
+; CHECK-NEXT: - { reg: '$f8', virtual-reg: '' }
+; CHECK-NEXT: - { reg: '$f9', virtual-reg: '' }
+; CHECK-NEXT: - { reg: '$f10', virtual-reg: '' }
+; CHECK-NEXT: - { reg: '$f11', virtual-reg: '' }
+; CHECK-NEXT: - { reg: '$f12', virtual-reg: '' }
+; CHECK-NEXT: - { reg: '$f13', virtual-reg: '' }
+
+; CHECK:      fixedStack:
+; 32BIT:      - { id: 0, type: default, offset: 140, size: 4 
+; 32BIT:      - { id: 1, type: default, offset: 132, size: 8 
+; 32BIT:      - { id: 2, type: default, offset: 128, size: 4 
+
+; 64BIT:      - { id: 0, type: default, offset: 168, size: 4
+; 64BIT:      - { id: 1, type: default, offset: 160, size: 8 
+; 64BIT:      - { id: 2, type: default, offset: 152, size: 4
+
+; CHECK:      body:             |
+; CHECK-NEXT:   bb.0.entry:
+; CHECK-NEXT:     liveins: $f1, $f2, $f3, $f4, $f5, $f6, $f7, $f8, $f9, $f10, $f11, $f12, $f13
+
+