[PATCH] D130903: [AArch64][GlobalISel] Lower formal arguments of AAPCS & ms_abi variadic functions.

Sat Aug 20 12:52:11 PDT 2022

mstorsjo added inline comments.

================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp:565
+      GPRIdx = MFI.CreateFixedObject(GPRSaveSize, -(int)GPRSaveSize, false);
+      if (GPRSaveSize & 15)
+        // The extra size here, if triggered, will always be 8.
----------------
dzhidzhoev wrote:
> mstorsjo wrote:
> > dzhidzhoev wrote:
> > > dzhidzhoev wrote:
> > > > paquette wrote:
> > > > > Can you explain why this is the case in a comment? Even referencing some win64 doc would be useful?
> > > > I'm not sure I have clean understanding of this line. It was introduced here https://reviews.llvm.org/D35720
> > > clear*
> > The reason why it's always 8, is that if we're reserving space for a number of 8 byte registers and align it to 16, we either will have a 8 byte gap, or no gap at all.
> > 
> > As for the exact reasons why we manually added a gap in the SelectionDAG implementation, I don't really remember anything more than what you can read in the old phabricator review unfortunately.
> It seems like these lines don't affect test output. Let's try to remove them.
I can't guarantee that there aren't gaps in the testcases for this combination though, so it'd be good if you'd manually try to see if there are other cases that can be triggered (with either more or less formal parameters to the function, and/or more or less regular stack allocation.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130903