[PATCH] D74225: [AIX] Implement formal arguments passed in stack memory

Fri Feb 28 07:44:21 PST 2020

ZarkoCA marked 2 inline comments as done.
ZarkoCA added a comment.

In D74225#1894378 <https://reviews.llvm.org/D74225#1894378>, @sfertile wrote:

> In D74225#1894003 <https://reviews.llvm.org/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 load.

> 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?

================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7014
+    // memory from their register image.
+    if ((LocSize > ValSize) && IsPPC64)
+      CurArgOffset += LocSize - ValSize;
----------------
ZarkoCA wrote:
> sfertile wrote:
> > 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 :)
> 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. 
I made it so we to the adjustment on 32BIT too and added a bool test case.

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

https://reviews.llvm.org/D74225