[PATCH] D71698: [AArch64][SVE] Add intrinsic for non-faulting loads

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12460
 
-  if ((SignExtSrcVT != GLD1SrcMemVT) || !Src.hasOneUse())
+  unsigned OpNum = NewOpc == AArch64ISD::LDNF1S ? 3 : 4;
+  EVT LD1SrcMemVT = cast<VTSDNode>(Src->getOperand(OpNum))->getVT();
----------------
Move the assignment of `MemVTOpNum` to the switch statement above instead of special-casing it here?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12461
+  unsigned OpNum = NewOpc == AArch64ISD::LDNF1S ? 3 : 4;
+  EVT LD1SrcMemVT = cast<VTSDNode>(Src->getOperand(OpNum))->getVT();
+
----------------
nit: `s/LD1SrcMemVT/SrcMemVT/`


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12469
+
+  SmallVector<SDValue, 4> Ops = {Src->getOperand(0), Src->getOperand(1),
+                                 Src->getOperand(2), Src->getOperand(3)};
----------------
Better make the default '5' if there is a large likelihood of there being 5 default values.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12469
+
+  SmallVector<SDValue, 4> Ops = {Src->getOperand(0), Src->getOperand(1),
+                                 Src->getOperand(2), Src->getOperand(3)};
----------------
sdesmalen wrote:
> Better make the default '5' if there is a large likelihood of there being 5 default values.
Instead of special -casing LDNF1S below, you can write this as:

  SmallVector<SDValue, 5> Ops;
  for(unsigned I=0; I<Src->getNumOperands(); ++I)
    Ops.push_back(Src->getOperand(I));


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

https://reviews.llvm.org/D71698