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

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

Thanks, this looks pretty good, but of course I dug into some edge cases.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:5908
 
+void AArch64TargetLowering::analyzeCallOperands(
+    const CallLoweringInfo &CLI, CallingConv::ID CalleeCC, bool isVarArg,
----------------
Does this need to be a class method? Can it be a static helper with a TLI parameter?


================
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 {
+
----------------
CLI contains isVararg, CalleeCC, and Outs, I believe


================
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
----------------
We should skip this check for musttaill calls `(CLI.CB && CLI.CB->isMustTailCall())`


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:6063
+      if (!ArgLoc.isRegLoc())
+        return false;
+  }
----------------
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, ...);
};
```


================
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);
 
----------------
If you pass CLI here, the remaining parameters are redundant. All the parameters Outs, OutVals, Ins, CallConv, IsVarArg are members of CLI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120622