[PATCH] D153432: [SDAGBuilder] Handle multi-part arguments in argument copy elision (PR63430)

[Phoebe Wang via Phabricator via llvm-commits]

pengfei accepted this revision.
pengfei added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:10928
+      for (EVT VT : ValueVTs)
+        NumParts += TLI->getNumRegistersForCallingConv(*CurDAG->getContext(),
+                                                       F.getCallingConv(), VT);
----------------
nikic wrote:
> pengfei wrote:
> > nikic wrote:
> > > arsenm wrote:
> > > > For some reason this is split into two hooks for the register type and the number of registers. Wouldn't you need to account for the split and promoted / demoted case?
> > > Not sure which case you have in mind here, but this is doing the same as the code below (line 10954).
> > IIUC, we cannot assume all `ValueVTs` are the same VT. So it doesn't look correct to sum them together.
> > How about iterate Ins[i] and check for `isSplitEnd`?
> This doesn't assume that the VTs are the same, it just sums of the parts for all the (potentially different) VTs. We don't care about the types, just how many there are.
Thinking twice, it doesn't matter if ValueVTs is the same or not. We can see them as a whole for conservative reason.


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

https://reviews.llvm.org/D153432