[PATCH] D76902: [PowerPC][AIX] ByVal formal argument support: multiple registers

[Chris Bowler via Phabricator via llvm-commits]

cebowleratibm added a comment.

I had intended to move the 5-8 byte byval callee cases out of aix64-cc-byval.ll and put them here.  I don't want aix64-cc-byval.ll to exist when we're done.  If the 5-8 byte tests aren't useful for the callee I'd rather delete them.  They were of interest to the caller handling to make sure that we implement the left justification correctly.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7073
 
-      const unsigned StackSize = alignTo(ByValSize, PtrByteSize);
+      const unsigned StackSize = alignTo(Flags.getByValSize(), PtrByteSize);
       const int FI = MF.getFrameInfo().CreateFixedObject(
----------------
We currently error on byval alignment > PtrByteSize.  I'd be inclined to assert it here as well to make it really hard to emit invalid code.

(Noting that we need to decide how to handle 8 byte aligned structs in 32-bit mode)


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-byval.ll:784
+; ASM-LABEL: .test_byval_32Byte:
 
+; ASM32:       stw 8, 44(1)
----------------
I'm ok with the test as written, though I would have been inclined to write the test like this:
```
; ASM32-DAG:   stw 3, 24(1)
; ASM32-DAG:   stw 4, 28(1)
; ASM32-DAG:   stw 5, 32(1)
; ASM32-DAG:   stw 6, 36(1)
; ASM32-DAG:   stw 7, 40(1)
; ASM32-DAG:   stw 8, 44(1)
; ASM32-DAG:   stw 9, 48(1)
; ASM32-DAG:   stw 10, 52(1)
; ASM32-DAG:   lbz 3, 45(1)
; ASM32-NEXT:  blr
```
Understanding that this permits the lbz to move above the stw 3.  It's easier to read, accepts more valid outputs at the expense of accepting some invalid outputs.  It's been my preferred approach to these tests so far.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76902