[PATCH] D73209: [AIX] Implement caller arguments passed in stack memory

[Zarko Todorovski via Phabricator via llvm-commits]

ZarkoCA added inline comments.
Herald added a subscriber: wuzish.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:6873
+    // Reserve and initialize GPRs or initialize the PSA as required.
     MVT RegVT = IsPPC64 ? MVT::i64 : MVT::i32;
     for (unsigned I = 0; I < StoreSize; I += PtrByteSize) {
----------------
mark this as const as well? 


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:6886-6891
+        // If there are insufficient GPRs, the PSA needs to be initialized.
+        // Initialization occurs even if an FPR was initialized for
+        // compatibility with the AIX XL compiler. The full memory for the
+        // argument will be initialized even if a prior word is saved in GPR.
+        State.addLoc(CCValAssign::getMem(ValNo, ValVT, Offset, LocVT, LocInfo));
+        break;
----------------
Can we add a test for this? Specifically checking if the full memory for the arg is initialized. 


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7091-7099
+    if (VA.isMemLoc()) {
+      SDValue PtrOff =
+          DAG.getConstant(VA.getLocMemOffset(), dl, StackPtr.getValueType());
+      PtrOff = DAG.getNode(ISD::ADD, dl, PtrVT, StackPtr, PtrOff);
+      MemOpChains.push_back(
+          DAG.getStore(Chain, dl, Arg, PtrOff, MachinePointerInfo()));
+
----------------
I would prefer to have the MemLoc code after the RegLoc. To me it's more intuitive that way since we use up registers first and then stack.  It's purely a personal preference though and have no issue with keeping it as is. 


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7147-7151
+      if (I != E) {
+        // If only 1 GPR was available, there will only be one custom GPR and the argument will also pass in memory.
+        CCValAssign &PeekArg = ArgLocs[I];
+        if (PeekArg.isRegLoc() && PeekArg.getValNo() == PeekArg.getValNo()) {          
+          assert(PeekArg.needsCustom() && "A second custom GPR is expected.");
----------------
Can we have a testcase for this? 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73209/new/

https://reviews.llvm.org/D73209