[PATCH] D76929: [AArch64][SVE] Add SVE intrinsic for LD1RQ

Kerry McLaughlin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 14 08:33:24 PDT 2020


kmclaughlin marked 2 inline comments as done.
kmclaughlin added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:11592
 
+static SDValue performLD1RQCombine(SDNode *N, SelectionDAG &DAG) {
+  SDLoc DL(N);
----------------
andwar wrote:
> [Nit] I think that this method could be simplified and made more explicit:
> 
> ```
> static SDValue performLD1RQCombine(SDNode *N, SelectionDAG &DAG) {
>   SDLoc DL(N);
>   EVT VT = N->getValueType(0);
> 
>   EVT LoadVT = VT;
>   if (VT.isFloatingPoint())
>     LoadVT = VT.changeTypeToInteger();
> 
>   SDValue Ops[] = {N->getOperand(0), N->getOperand(2), N->getOperand(3)};
>   SDValue Load = DAG.getNode(AArch64ISD::LD1RQ, DL, {LoadVT, MVT::Other}, Ops);
> 
>   if (VT.isFloatingPoint()) {
>     SDValue LoadChain = SDValue(Load.getNode(), 1);
>     Load = DAG.getMergeValues(
>         {DAG.getNode(ISD::BITCAST, DL, VT, Load), LoadChain}, DL);
>   }
> 
>   return Load;
> }
> ```
> 
> This way:
>  * there's only 1 `return` statement
>  * you are being explicit about preserving the chain when generating the bit cast
>  * `Load` replaces a bit non-descriptive `L`
> 
> This is a matter of style, so please use what feels best. 
Done, these changes do make it neater :)


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

https://reviews.llvm.org/D76929





More information about the llvm-commits mailing list