[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/lib/Target/PowerPC/PPCISelLowering.cpp:7167
+          SDNodeFlags AddFlags;
+          AddFlags.setNoUnsignedWrap(true);
+          LoadAddr =
----------------
sfertile wrote:
> If we are setting the no-wrap flags maybe we should set both.
Are you referring to setNoSignedWrap?

Digging a bit more, I think we can use SelectionDAG::getObjectPtrOffset


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-byval.ll:208
+
+%struct.S5 = type { [5 x i8] }
+
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76380