[PATCH] [ARM64] Implement NEON post-increment LD1 (lane) and post-increment LD1R

[James Molloy]

Hi Hao,

I have a couple of comments.

Cheers,

James

================
Comment at: lib/Target/ARM64/ARM64ISelDAGToDAG.cpp:1159
@@ -1155,9 +1158,3 @@
   SDValue SuperReg = SDValue(Ld, 1);
-  EVT WideVT = RegSeq.getOperand(1)->getValueType(0);
-  static unsigned QSubs[] = { ARM64::qsub0, ARM64::qsub1, ARM64::qsub2,
-                              ARM64::qsub3 };
-  for (unsigned i = 0; i < NumVecs; ++i) {
-    SDValue NV = CurDAG->getTargetExtractSubreg(QSubs[i], dl, WideVT, SuperReg);
-    if (Narrow)
-      NV = NarrowVector(NV, *CurDAG);
-    ReplaceUses(SDValue(N, i), NV);
+  if (NumVecs == 1)
+    ReplaceUses(SDValue(N, 0),
----------------
Please add braces around this if clause? either there should be no braces around if or else, or braces around both.

================
Comment at: lib/Target/ARM64/ARM64ISelDAGToDAG.cpp:2670
@@ +2669,3 @@
+  case ARM64ISD::LD1DUPpost: {
+    if (VT == MVT::v8i8)
+      return SelectPostLoad(Node, 1, ARM64::LD1Rv8b_POST, ARM64::dsub0);
----------------
Would a switch be clearer here?

================
Comment at: lib/Target/ARM64/ARM64ISelDAGToDAG.cpp:2746
@@ +2745,3 @@
+  case ARM64ISD::LD1LANEpost: {
+    if (VT == MVT::v16i8 || VT == MVT::v8i8)
+      return SelectPostLoadLane(Node, 1, ARM64::LD1i8_POST);
----------------
Having all these individual MVT:: cases makes it likely you'll miss one or someone will miss one on an update. Would it be easier to switch on the EVT::getVectorElementSizeInBits()?

================
Comment at: lib/Target/ARM64/ARM64ISelLowering.cpp:7098
@@ +7097,3 @@
+  // If it is not LOAD/EXTLOAD, can not do such combine.
+  if (LD->getOpcode() != ISD::LOAD && LD->getOpcode() != ISD::EXTLOAD)
+    return SDValue();
----------------
What about SEXTLOAD and ZEXTLOAD here?

================
Comment at: lib/Target/ARM64/ARM64ISelLowering.cpp:7129
@@ +7128,3 @@
+      uint32_t IncVal = CInc->getZExtValue();
+      unsigned NumBytes = (VT.getSizeInBits() / 8) / VT.getVectorNumElements();
+      if (IncVal != NumBytes)
----------------
GetScalarSizeInBits() may simplify this.

================
Comment at: lib/Target/ARM64/ARM64ISelLowering.cpp:7158
@@ +7157,3 @@
+    DCI.CombineTo(N, SDValue(UpdN.getNode(), 0));     // Dup/Inserted Result
+    DCI.CombineTo(User, SDValue(UpdN.getNode(), 1));  // Write backe register
+
----------------
Typo: "write back"

http://reviews.llvm.org/D3740