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

Tue Oct 4 09:08:18 PDT 2022

MattDevereau added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:6754
+    }
     bool CalleeInSVE = any_of(Ins, [](ISD::InputArg &In){
       return In.VT.isScalableVector();
----------------
paulwalker-arm wrote:
> peterwaller-arm wrote:
> > I can't think of a way that you can get a returned scalable type which doesn't go in a register, but it doesn't seem inconceivable it may be possible in the distant future. Since we're leaving a subtlety here it seems worth a brief comment of this form:
> > 
> >   // Check the types of the returned values. For the time being it is sufficient to check the VT as the RegVT is not known at this point.
> Can we handle this in the same way as we do for arguments. The necessary work is done within `LowerCallResult`. It looks like every target does something special with `LowerCall` our only caller so I think we should hoist the necessary logic to construct `RVLocs` into `LowerCall` and pass it into `LowerCallResult` instead of `Ins`.
> 
> Doing this means we can use the same lambda as  above to give us a bit of future proofness (I don't expect extra test coverage because I don't believe this is all that well defined given there's no way to write a suitable example in C). 
Is it possible to hoist this cleanly and eliminate all of the RVLocs assignments in `LowerCallResult`? The work done in `LowerCallResult` is dependent on CallConv which is set by the CalleeInSVE/CalleeOutSVE lambdas here. When 1:1 hoisting the RVLocs work in LowerCallResult to replace CalleeInSVE here I'm seeing test failures in `arm64_32-fastisel.ll`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133484