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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm accepted this revision.
arsenm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp:687
   const AArch64TargetLowering &TLI = *getTLI<AArch64TargetLowering>();
-  CCAssignFn *AssignFn =
-      TLI.CCAssignFnForCall(F.getCallingConv(), /*IsVarArg=*/false);
+  CCAssignFn *AssignFn = TLI.CCAssignFnForCall(F.getCallingConv(), IsWin64 && F.isVarArg());
 
----------------
efriedma wrote:
> dzhidzhoev wrote:
> > arsenm wrote:
> > > Parameter name suggest this should just be isVarArg
> > https://github.com/llvm/llvm-project/blob/9408164254d26d5305fe4e0267b668c41c1266ed/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp#L602
> > On the trunk we pass false even if the function is vararg. Are you sure I should change this behavior for non win64 calling conventions?
> The way AArch64TargetLowering::CCAssignFnForCall works is a little weird.  For Darwin calling conventions, "fixed" arguments to varargs functions are passed differently from "variadic" arguments.  Instead of recording that information in the arguments themselves, we call a different AssignFn for fixed vs. variadic arguments.
> 
> There are no such thing as variadic parameters, though; you can only access variadic arguments through va_arg.  So we don't need to handle variadic arguments here.
> 
> Maybe there's a better way to implement the whole thing (if we record whether an argument is fixed in ArgFlagsTy, we could unify the Darwin calling conventions), but this is consistent with the way it currently works for SelectionDAG ISel.
I vaguely remember encountering this mess before


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130903