[PATCH] Implement aarch64 neon instruction class SIMD Table lookup - LLVM

[Tim Northover]

  Hi Jiangning,

  This is looking a lot neater, thanks for cleaning it up. There's a few very minor things, but I've also spotted what looks like a larger issue with the implementation in some edge-cases.


================
Comment at: lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:737-738
@@ +736,4 @@
+    SDValue IDef =
+        SDValue(CurDAG->getMachineNode(TargetOpcode::IMPLICIT_DEF, Ops[PairPos],
+                                       Ops[PairPos].getValueType()),
+                0);
----------------
Reading the descriptions, I think this probably has to be a zero-vector rather than IMPLICIT_DEF to match the AArch64 VTBL semantics (and can't match VTBX semantics no matter what you do).

The issue is:
   vtbx.8 d0, {d1}, d2

If d2[0] = 8 then the AArch32 instruction will leave d0[0] unchanged. However:
    tbx v0.16b, {v1.16b}, v2.16b

will set it to v1.16b[8]. I don't think these can be reconciled without real pre or post-processing.

================
Comment at: lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:699
@@ -684,1 +698,3 @@
 
+SDNode *AArch64DAGToDAGISel::combine2D(SDLoc dl, SDValue D0, SDValue D1) {
+  SDValue idef = SDValue(CurDAG->getMachineNode(TargetOpcode::IMPLICIT_DEF,
----------------
Longer term, it would be nice if this could just be a CONCAT_VECTORS operation. It doesn't look like we've got the patterns for that yet though, so this isn't too bad in the meantime.

================
Comment at: lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:799-803
@@ +798,7 @@
+  EVT ResVT = N->getValueType(0);
+  bool is64BitRes;
+  switch (ResVT.getSimpleVT().SimpleTy) {
+  default: llvm_unreachable("unhandled table look up result type");
+  case MVT::v8i8  : is64BitRes = true; break;
+  case MVT::v16i8 : is64BitRes = false; break;
+  }
----------------
bool Is64BitRes = ResVT.is64BitVector();
?

================
Comment at: lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:973
@@ -837,3 +972,3 @@
     };
-    return SelectVLD(Node, 1, true, Opcodes);
+    return selectVLD(Node, 1, true, Opcodes);
   }
----------------
Good idea, but probably a separate patch. Could you go ahead and commit it as a refactor?


http://llvm-reviews.chandlerc.com/D2071