[PATCH] D133484: [AArch64][SVE] Fix AArch64_SVE_VectorCall calling convention

[Matt Devereau via Phabricator via llvm-commits]

MattDevereau added a comment.

@paulwalker-arm I'm referring to this comment <https://reviews.llvm.org/D133484#inline-1302472>, particularly the sentence "I think we should hoist the necessary logic to construct RVLocs into LowerCall and pass it into LowerCallResult instead of Ins" which I assumed was referring to this whole block at the start of `LowerCallResult` here

  SDValue AArch64TargetLowering::LowerCallResult(
      SDValue Chain, SDValue InFlag, CallingConv::ID CallConv, bool isVarArg,
      const SmallVectorImpl<ISD::InputArg> &Ins, const SDLoc &DL,
      SelectionDAG &DAG, SmallVectorImpl<SDValue> &InVals, bool isThisReturn,
      SDValue ThisVal) const {
    CCAssignFn *RetCC = CCAssignFnForReturn(CallConv);
    // Assign locations to each value returned by this call.
    SmallVector<CCValAssign, 16> RVLocs;
    DenseMap<unsigned, SDValue> CopiedRegs;
    CCState CCInfo(CallConv, isVarArg, DAG.getMachineFunction(), RVLocs,
                   *DAG.getContext());
    CCInfo.AnalyzeCallResult(Ins, RetCC);

Is it not the case that `CCInfo(CallConv...` here is part of the necessary logic to construct RVLocs here and it is dependent on the work done in `LowerCall`, which can set CallConv to `CallingConv::AArch64_SVE_VectorCall` and affect this block of code? I do not see all tests passing when I move this entire block. It sounds like we're not on the same page on the word hoist which I assumed meant cut and paste everything related to RVLocs in `LowerCallResult` to `LowerCall`. Did you mean to keep the lines containing CCInfo in LowerCallResult but pass RVLocs by a non-const reference?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133484