[PATCH] D76401: [PowerPC][AIX] ByVal formal argument support: single register.

[Chris Bowler via Phabricator via llvm-commits]

cebowleratibm added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:6981
 
+static unsigned roundToMultiple(unsigned Size, unsigned Multiple) {
+  // Round up.
----------------
LLVM has alignTo, which I believe you can use.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7066
+      // First create the FrameIndex for the object, and add it to the In vals.
+      const unsigned TrueSize = Flags.getByValSize();
+      const unsigned StackSize = roundToMultiple(TrueSize, PtrByteSize);
----------------
I don't object to the name TrueSize but I prefer the name ByValSize.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7067
+      const unsigned TrueSize = Flags.getByValSize();
+      const unsigned StackSize = roundToMultiple(TrueSize, PtrByteSize);
+
----------------
I understand that you're likely preparing for the next patch, but I think it's simple enough to:

if (ByValSize > PtrByteSize) report_fatal_error

then skip the rounding and write the single register case.  You know the StackSize is always PtrByteSize.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7094
+      SDValue Store =
+          DAG.getStore(CopyFrom.getValue(1), dl, CopyFrom, FrameIndex,
+                       MachinePointerInfo::getFixedStack(MF, FI, 0));
----------------
Does this store get elided by optimization when possible?

For example:
struct S {int i;};
int foo(S s) { return s.i; }

I assume we don't want to manifest the store to the stack.  It's odd to me because we don't manifest the store for the equivalent:

int foo(int i) { return i; }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76401