[PATCH] D120622: [AArch64] Use correct calling convention for each vararg

[Le Philousophe via Phabricator via llvm-commits]

lephilousophe added a comment.

@rnk I think I addressed all your comments.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:5909-5910
+void AArch64TargetLowering::analyzeCallOperands(
+    const CallLoweringInfo &CLI, CallingConv::ID CalleeCC, bool isVarArg,
+    const SmallVectorImpl<ISD::OutputArg> &Outs, CCState &CCInfo) const {
+
----------------
rnk wrote:
> CLI contains isVararg, CalleeCC, and Outs, I believe
Indeed.
This is now using CLI.
Subtarget is private in TLI so I pass it as an argument.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:6054
+
+  if (isVarArg) {
+    // At least two cases here: if caller is fastcc then we can't have any
----------------
rnk wrote:
> We should skip this check for musttaill calls `(CLI.CB && CLI.CB->isMustTailCall())`
Done.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:6063
+      if (!ArgLoc.isRegLoc())
+        return false;
+  }
----------------
rnk wrote:
> I think we'll still come here and have the same bug for variadic functions with 9 parameters when one of the fixed params is passed in memory, consider:
> https://gcc.godbolt.org/z/Pb5EoK1d1
> ```
> struct A { virtual void f(int x0, int x1, int x2, int x3, int x4, int x5, int x6, int x7, int mem, ...); };
> struct B { virtual void f(int x0, int x1, int x2, int x3, int x4, int x5, int x6, int x7, int mem, ...); };
> struct C : A, B {
>     virtual void key();
>     virtual void f(int x0, int x1, int x2, int x3, int x4, int x5, int x6, int x7, int mem, ...);
> };
> ```
I added your test case to the regression tests.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:6182-6183
     // Check if it's really possible to do a tail call.
-    IsTailCall = isEligibleForTailCallOptimization(
-        Callee, CallConv, IsVarArg, Outs, OutVals, Ins, DAG);
+    IsTailCall = isEligibleForTailCallOptimization(Callee, CallConv, IsVarArg,
+                                                   Outs, OutVals, Ins, CLI);
 
----------------
rnk wrote:
> If you pass CLI here, the remaining parameters are redundant. All the parameters Outs, OutVals, Ins, CallConv, IsVarArg are members of CLI.
Indeed. This is fixed.
Except that CallConnv was modified in the function and CLI didn't get updated.
I used a reference instead and CLI gets updated.
Tests are passing so I expect this change doesn't break anything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120622