[PATCH] D76380: [PowerPC][AIX] Implement by-val caller arguments in multiple registers

[Chris Bowler via Phabricator via llvm-commits]

cebowleratibm marked 2 inline comments as done.
cebowleratibm added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-byval.ll:208
+
+%struct.S5 = type { [5 x i8] }
+
----------------
cebowleratibm wrote:
> sfertile wrote:
> > I don't see a lot of utility in testing each of 1 through 8 bytes with nearly exactly the same test. The are each so similar that we end up with a long verbose test with not a lot of practical coverage. Instead how about we add 
> > 
> > 1) one of 5,6,7,8 bytes but with other arguments before/after the by val.
> > 2) A test with a couple byvals mixed with other arguments.
> > 3) One test with a 'large' by val like  { [32 x i8] }.
> > 
> > Also use a struct type that isn't just containing an array. Ex if you choose 8 bytes as the size for the test in case 1 use:  { i16, i16, i16, i16} as the struct type.
> 1 and 2. I agree. 
> 3. I already have 31, 32 and 64 byte tests.
> 
> I'll change the struct layout in some of the tests.
1. is addressed in https://reviews.llvm.org/D76875
2. Was already addressed in https://reviews.llvm.org/D76401 (thanks Sean)
3. This patch has tests for 31 and 32 byte byvals.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76380