[PATCH] [ARMv8] Add NEON intrinsics

[Michael Gottesman]

  Some nits + substantive questions.


================
Comment at: utils/TableGen/NeonEmitter.cpp:2662
@@ -2656,3 +2661,3 @@
                            bool isShift, bool isHiddenLOp,
-                           ClassKind ck, const std::string &InstName,
+                           ClassKind ck, const std::string &InstName, const std::string ArchVersion,
 						   bool isA64,
----------------
const std::string ArchVersion => const std::string &ArchVersion

================
Comment at: utils/TableGen/NeonEmitter.cpp:970-973
@@ -969,2 +969,6 @@
     case 'n':
+      if ((NameRef.equals("vmul_n") || NameRef.equals("vmls_n")) &&
+          Proto[i] == 's' && OutTypeCode == "f32")
+        NormedProto += 'a';
+      else
       NormedProto += IsQuad? 'q' : 'd';
----------------
Is it possible to move this into the special cases section at the end of the function. The general design of the function is to handle the generic case defined by the input boolean flags and then afterwards perform any fix ups at the end in the special cases section.

Also do any of those special cases work/apply here or could be jerry rigged to do so?

================
Comment at: utils/TableGen/NeonEmitter.cpp:989-990
@@ -984,3 +988,4 @@
     case 'a':
-      if (HasLanePostfix) {
+      if (HasLanePostfix ||
+          (NameRef.equals("vmla_n") && OutTypeCode == "f32")) {
         NormedProto += 'a';
----------------
Same as comment on lines 970-973.

================
Comment at: utils/TableGen/NeonEmitter.cpp:1262
@@ -1256,3 +1261,3 @@
   // before the output type.
-  if (Prefix == "vcvt") {
+  if (StringRef(Prefix).startswith("vcvt")) {
     const std::string inTypeCode = NameRef.substr(NameRef.find_last_of("_")+1);
----------------
Can you add a comment here explaining that we want to make sure to match instructions like vcvt{a,n,p,m} which are safe to treat as a normal vcvt (I am assuming it is so since that is what you did here, no?)

================
Comment at: utils/TableGen/NeonEmitter.cpp:2745
@@ -2731,3 +2744,3 @@
   }
-  s += ");\n}\n\n";
+  s += ");\n}\n"+TestSuffix+"\n\n";
   return s;
----------------
Add spaces around the + to match the rest of the places you used a +.

================
Comment at: utils/TableGen/NeonEmitter.cpp:2831
@@ -2814,3 +2830,3 @@
         "\n"
-        "#include <arm_neon.h>\n"
+       "#include <arm_neon.h>\n"
         "\n";
----------------
Can you make this whitespace line up?

================
Comment at: utils/TableGen/NeonEmitter.cpp:2825-2826
@@ -2824,4 +2840,2 @@
   genTargetTest(OS, EmittedMap, false);
-
-  genTargetTest(OS, EmittedMap, true);
 }
----------------
>From what I can tell this makes the third argument to genTargetTest redundant.

Is this true? And if it is true, can you remove said argument from the function?


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