[PATCH] D90219: [SVE] Deal with SVE tuple call arguments correctly when running out of registers

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64CallingConvention.cpp:62
+        TLI->CCAssignFnForCall(State.getCallingConv(), /*IsVarArg=*/false);
+    bool Res = AssignFn(It.getValNo(), It.getValVT(), It.getValVT(),
+                        CCValAssign::Full, ArgFlags, State);
----------------
sdesmalen wrote:
> nit: To avoid `(void) Res;`, you can write:
>   if (AssignFn(....))
>     llvm_unreachable("Call operand has unhandled type");
OK sure, I thought this was the preferred way in LLVM as I copied this code from AArch64ISelLowering.cpp::LowerCALL/LowerFormalArguments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:4307
+
+      ArgValue = DAG.getLoad(PartLoad, DL, Chain, Ptr, MachinePointerInfo());
+      InVals.push_back(ArgValue);
----------------
sdesmalen wrote:
> Can you rewrite this into a single loop, in a way that it only adds the increment by VScale if NumParts > 1  (and same for increment of `ExtraArgLocs`). That makes the loop a bit more readable.
> 
> Also, a one-line comment describing what this code does would be useful too.
Sure I can take a look. However, I did write this originally as a single loop and thought it looked messy due to the fact we have 4 loads and 3 pointer increments, so the loop body needs additional control flow to avoid the final pointer addition. I think other places in SelectionDAG do it this way too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90219