[AIX] Extend integer arguments to register width when passed in stack memory.

[cebowleratibm]

Summary

This is a follow up to the previous patch: [AIX] Implement caller arguments passed in stack memory. https://reviews.llvm.org/rGb373ec8ce76a00cd079f70d108b13129c3a4a7b9

This corrects a defect in AIX 64-bit where an i32 is written to the stack with stw (4 bytes) rather than the expected std (8 bytes.) Integer arguments pass on the stack as images of their register representation.

I also took the opportunity to tidy up some of the calling convention AIX tests I added in my last commit. This patch adds the missed assembly expected output for the stack arg int case, which would have caught this problem. I also took the opportunity to reorder some of the calling convention stack arg tests into a more logical order to make them more readable.

[ZarkoCA]

It would be better if the test cleanup was done in a separate patch to make it clearer what the expected output of the patch. But that said, it's not difficult to find added testcase and it would be ok with me to leave the modified test cases.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

6812

Suggestion: Move this error to line 6818, just after all the variable declarations required, or make this the last error (i.e. move to line 6832 and have the declarations just above it).

[cebowleratibm]

In D75126#1891669, @ZarkoCA wrote:

It would be better if the test cleanup was done in a separate patch to make it clearer what the expected output of the patch. But that said, it's not difficult to find added testcase and it would be ok with me to leave the modified test cases.

I agree many of the test updates could be a separate NFC patch.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

6812

I preferred to keep the errors as a block. I needed to move some basic variable setup above the error block. I can move this error later in the sequence but I wouldn't move the variable setup into the error block, so this wouldn't buy anything. I prefer to keep the current approach.

[cebowleratibm]

Some of the prior test changes have been committed as NFC patches. I've updated the diff and readers can now see the instruction changes for widening arguments to register width before writing to the stack.

[ZarkoCA]

Looks good to me, thanks for updating the tests in a separate patch.

[sfertile]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

6812

For 32-bit codegen we legalize i64s by breaking them into 2 i32s. Is i128 handled similarly, or is it a legal type which this error is trying to catch?

6813

Minor nit: word size --> register size, and maybe 'argument' --> 'integer argument'.

[cebowleratibm]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

6812

i128 is handled similarly to i64. In PPC64 it will take 2 registers and in PPC32 it will take 4 registers.

The code was not designed to handle this case so I put it in as a defensive measure.

[cebowleratibm]

One comment update. Should be ready to commit.

[sfertile]

LGTM.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

6812

Thanks for checking. In that case I would say make it an assert with a message that reads along the lines: "Illegal integer argument exceeds register size."

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

SDValue PPCTargetLowering::LowerCall_Darwin(
   return FinishCall(CFlags, dl, DAG, RegsToPass, InFlag, Chain, CallSeqStart,
                     Callee, SPDiff, NumBytes, Ins, InVals, CS);
 }
 
 static bool CC_AIX(unsigned ValNo, MVT ValVT, MVT LocVT,
                    CCValAssign::LocInfo LocInfo, ISD::ArgFlagsTy ArgFlags,
                    CCState &State) {
 
+  const PPCSubtarget &Subtarget = static_cast<const PPCSubtarget &>(
+      State.getMachineFunction().getSubtarget());
+  const bool IsPPC64 = Subtarget.isPPC64();
+  const unsigned PtrByteSize = IsPPC64 ? 8 : 4;
+  const MVT RegVT = IsPPC64 ? MVT::i64 : MVT::i32;
+
+  assert((!ValVT.isInteger() ||
+          (ValVT.getSizeInBits() <= RegVT.getSizeInBits())) &&
+         "Integer argument exceeds register size: should have been legalized");
+
   if (ValVT == MVT::f128)
     report_fatal_error("f128 is unimplemented on AIX.");
 
   if (ArgFlags.isByVal())
     report_fatal_error("Passing structure by value is unimplemented.");
 
   if (ArgFlags.isNest())
     report_fatal_error("Nest arguments are unimplemented.");
 
   if (ValVT.isVector() || LocVT.isVector())
     report_fatal_error("Vector arguments are unimplemented on AIX.");
 
-  const PPCSubtarget &Subtarget = static_cast<const PPCSubtarget &>(
-      State.getMachineFunction().getSubtarget());
-  const bool IsPPC64 = Subtarget.isPPC64();
-  const unsigned PtrByteSize = IsPPC64 ? 8 : 4;
-
   static const MCPhysReg GPR_32[] = {// 32-bit registers.
                                      PPC::R3, PPC::R4, PPC::R5, PPC::R6,
                                      PPC::R7, PPC::R8, PPC::R9, PPC::R10};
   static const MCPhysReg GPR_64[] = {// 64-bit registers.
                                      PPC::X3, PPC::X4, PPC::X5, PPC::X6,
                                      PPC::X7, PPC::X8, PPC::X9, PPC::X10};
 
   // Arguments always reserve parameter save area.
   switch (ValVT.SimpleTy) {
   default:
     report_fatal_error("Unhandled value type for argument.");
   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;
+    // AIX integer arguments are always passed in register width.
+    if (unsigned Reg = State.AllocateReg(IsPPC64 ? GPR_64 : GPR_32))
       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.
     const unsigned Offset = State.AllocateStack(IsPPC64 ? 8 : StoreSize, 4);
     unsigned FReg = State.AllocateReg(FPR);
     if (FReg)
       State.addLoc(CCValAssign::getReg(ValNo, ValVT, FReg, LocVT, LocInfo));
 
     // Reserve and initialize GPRs or initialize the PSA as required.
-    const MVT RegVT = IsPPC64 ? MVT::i64 : MVT::i32;
     for (unsigned I = 0; I < StoreSize; I += PtrByteSize) {
       if (unsigned Reg = State.AllocateReg(IsPPC64 ? GPR_64 : GPR_32)) {
         assert(FReg && "An FPR should be available when a GPR is reserved.");
         if (State.isVarArg()) {
           // Successfully reserved GPRs are only initialized for vararg calls.
           // Custom handling is required for:
           //   f64 in PPC32 needs to be split into 2 GPRs.
           //   f32 in PPC64 needs to occupy only lower 32 bits of 64-bit GPR.
SDValue PPCTargetLowering::LowerCall_AIX(
   for (unsigned I = 0, E = ArgLocs.size(); I != E;) {
     CCValAssign &VA = ArgLocs[I++];
 
     SDValue Arg = OutVals[VA.getValNo()];
 
     if (!VA.isRegLoc() && !VA.isMemLoc())
       report_fatal_error("Unexpected location for function call argument.");
 
-    if (VA.isRegLoc() && !VA.needsCustom()) {
-      switch (VA.getLocInfo()) {
-      default:
-        report_fatal_error("Unexpected argument extension type.");
-      case CCValAssign::Full:
-        break;
-      case CCValAssign::ZExt:
-        Arg = DAG.getNode(ISD::ZERO_EXTEND, dl, VA.getLocVT(), Arg);
-        break;
-      case CCValAssign::SExt:
-        Arg = DAG.getNode(ISD::SIGN_EXTEND, dl, VA.getLocVT(), Arg);
-        break;
-      }
-      RegsToPass.push_back(std::make_pair(VA.getLocReg(), Arg));
 
+    if (VA.isRegLoc() && !VA.needsCustom()) {
+      RegsToPass.push_back(std::make_pair(VA.getLocReg(), Arg));
       continue;
     }
 
     if (VA.isMemLoc()) {
       SDValue PtrOff =
           DAG.getConstant(VA.getLocMemOffset(), dl, StackPtr.getValueType());
       PtrOff = DAG.getNode(ISD::ADD, dl, PtrVT, StackPtr, PtrOff);
       MemOpChains.push_back(

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

 ; 64BIT-DAG:   $x4 = LI8 2
 ; 64BIT-DAG:   $x5 = LI8 3
 ; 64BIT-DAG:   $x6 = LI8 4
 ; 64BIT-DAG:   $x7 = LI8 5
 ; 64BIT-DAG:   $x8 = LI8 6
 ; 64BIT-DAG:   $x9 = LI8 7
 ; 64BIT-DAG:   $x10 = LI8 8
 ; 64BIT-DAG:   renamable $x[[REGCADDR:[0-9]+]] = LDtoc @c, $x2 :: (load 8 from got)
-; 64BIT-DAG:   renamable $r[[REGC:[0-9]+]] = LBZ 0, killed renamable $x[[REGCADDR]] :: (dereferenceable load 1 from @c)
-; 64BIT-DAG:   STW killed renamable $r[[REGC]], 112, $x1 :: (store 4)
+; 64BIT-DAG:   renamable $x[[REGC:[0-9]+]] = LBZ8 0, killed renamable $x[[REGCADDR]] :: (dereferenceable load 1 from @c)
+; 64BIT-DAG:   STD killed renamable $x[[REGC]], 112, $x1 :: (store 8)
 ; 64BIT-DAG:   renamable $x[[REGSIADDR:[0-9]+]] = LDtoc @si, $x2 :: (load 8 from got)
-; 64BIT-DAG:   renamable $r[[REGSI:[0-9]+]] = LHA 0, killed renamable $x[[REGSIADDR]] :: (dereferenceable load 2 from @si)
-; 64BIT-DAG:   STW killed renamable $r[[REGSI]], 120, $x1 :: (store 4)
+; 64BIT-DAG:   renamable $x[[REGSI:[0-9]+]] = LHA8 0, killed renamable $x[[REGSIADDR]] :: (dereferenceable load 2 from @si)
+; 64BIT-DAG:   STD killed renamable $x[[REGSI]], 120, $x1 :: (store 8)
 ; 64BIT-DAG:   renamable $x[[REGIADDR:[0-9]+]] = LDtoc @i, $x2 :: (load 8 from got)
-; 64BIT-DAG:   renamable $r[[REGI:[0-9]+]] = LWZ 0, killed renamable $x[[REGIADDR]] :: (dereferenceable load 4 from @i)
-; 64BIT-DAG:   STW killed renamable $r[[REGI]], 128, $x1 :: (store 4)
+; 64BIT-DAG:   renamable $x[[REGI:[0-9]+]] = LWZ8 0, killed renamable $x[[REGIADDR]] :: (dereferenceable load 4 from @i)
+; 64BIT-DAG:   STD killed renamable $x[[REGI]], 128, $x1 :: (store 8)
 ; 64BIT-DAG:   renamable $x[[REGLLIADDR:[0-9]+]] = LDtoc @lli, $x2 :: (load 8 from got)
 ; 64BIT-DAG:   renamable $x[[REGLLI:[0-9]+]] = LD 0, killed renamable $x[[REGLLIADDR]] :: (dereferenceable load 8 from @lli)
 ; 64BIT-DAG:   STD killed renamable $x[[REGLLI]], 136, $x1 :: (store 8)
-; 64BIT-DAG:   STW renamable $r[[REGI]], 144, $x1 :: (store 4)
+; 64BIT-DAG:   STD renamable $x[[REGI]], 144, $x1 :: (store 8)
 ; 64BIT-NEXT:  BL8_NOP <mcsymbol .test_stackarg_int>, csr_aix64, implicit-def dead $lr8, implicit $rm, implicit $x3, implicit $x4, implicit $x5, implicit $x6, implicit $x7, implicit $x8, implicit $x9, implicit $x10, implicit $x2, implicit-def $r1
 ; 64BIT-NEXT:  ADJCALLSTACKUP 152, 0, implicit-def dead $r1, implicit $r1
 
 ; The DAG block permits some invalid inputs for the benefit of allowing more valid orderings.
 ; ASM64PWR4-DAG:   stdu 1, -160(1)
 ; ASM64PWR4-DAG:   li 3, 1
 ; ASM64PWR4-DAG:   li 4, 2
 ; ASM64PWR4-DAG:   li 5, 3
 ; ASM64PWR4-DAG:   li 6, 4
 ; ASM64PWR4-DAG:   li 7, 5
 ; ASM64PWR4-DAG:   li 8, 6
 ; ASM64PWR4-DAG:   li 9, 7
 ; ASM64PWR4-DAG:   li 10, 8
 ; ASM64PWR4-DAG:   ld [[REGCADDR:[0-9]+]], LC5(2)
 ; ASM64PWR4-DAG:   lbz [[REGC:[0-9]+]], 0([[REGCADDR]])
-; ASM64PWR4-DAG:   stw [[REGC]], 112(1)
+; ASM64PWR4-DAG:   std [[REGC]], 112(1)
 ; ASM64PWR4-DAG:   ld [[REGSIADDR:[0-9]+]], LC3(2)
 ; ASM64PWR4-DAG:   lha [[REGSI:[0-9]+]], 0([[REGSIADDR]])
-; ASM64PWR4-DAG:   stw [[REGSI]], 120(1)
+; ASM64PWR4-DAG:   std [[REGSI]], 120(1)
 ; ASM64PWR4-DAG:   ld [[REGIADDR:[0-9]+]], LC4(2)
 ; ASM64PWR4-DAG:   lwz [[REGI:[0-9]+]], 0([[REGIADDR]])
-; ASM64PWR4-DAG:   stw [[REGI]], 128(1)
+; ASM64PWR4-DAG:   std [[REGI]], 128(1)
 ; ASM64PWR4-DAG:   ld [[REGLLIADDR:[0-9]+]], LC6(2)
 ; ASM64PWR4-DAG:   ld [[REGLLI:[0-9]+]], 0([[REGLLIADDR]])
 ; ASM64PWR4-DAG:   std [[REGLLI]], 136(1)
-; ASM64PWR4-DAG:   stw [[REGI]], 144(1)
+; ASM64PWR4-DAG:   std [[REGI]], 144(1)
 ; ASM64PWR4-NEXT:  bl .test_stackarg_int
 ; ASM64PWR4-NEXT:  nop
 
 ; Basic saving of floating point type arguments to the parameter save area.
 ; The float and double arguments will pass in both fpr as well as parameter save area.
 define void @call_test_stackarg_float() {
 entry:
   %0 = load float, float* @f, align 4