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

[Tim Northover]

  Hi Jiangning,

  I've got a couple of comments applying to particular bits of code, and could we have some LLVM-side CodeGen tests too?

  Cheers.

  Tim.


================
Comment at: lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:635
@@ -624,1 +634,3 @@
 
+SDNode *AArch64DAGToDAGISel::Combine2D(SDLoc dl, SDValue D0, SDValue D1) {
+  SDValue idef = SDValue(CurDAG->getMachineNode(TargetOpcode::IMPLICIT_DEF,
----------------
Functions should start with a lower-case letter and variables upper-case.

No need to be over-zealous in reworking the patch, but seeing "Combine2D" next to "createQPairNode" later on is a bit jarring.

================
Comment at: lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:717-721
@@ +716,7 @@
+  case 1: {
+    if (is64BitTbl) {
+      SDValue idef = SDValue(CurDAG->getMachineNode(TargetOpcode::IMPLICIT_DEF,
+                                                    dl, MVT::v2i64), 0);
+      TblReg = CurDAG->getTargetInsertSubreg(AArch64::sub_64, dl,
+                                             MVT::v2i64, idef, V0);
+    } else
----------------
Could most of this logic be combined into a "packVectorList" (or some other appropriately named) function that uses appropriate Combines, INSERT_SUBREGS and createQXYZ functions?

It could take a SmallVector of the values that need concatenating. In fact, it looks like it might be able to *replace* all the createQXYZ functions, maybe in less space (though possibly not the createDXYZ cleanly, it's a bit difficult to say).

I think it would greatly simplify this function and probably remove the need for the large switch statement.


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