[PATCH] D34686: [AArch64] Add hasFP16VectorArithmetic helper function. NFCI

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

I agree with the spirit of the change but I think you want to use `AArch64::` enumerators instead of those in `ARM::`



================
Comment at: lib/Basic/Targets.cpp:6190
   unsigned HasFullFP16;
+  unsigned ArchKind;
 
----------------
There is an `AArch64::ArchKind` enum in TargetParser.h, not sure if it would be useable here. 


================
Comment at: lib/Basic/Targets.cpp:6332
 
-    if (V8_1A)
+    if (ArchKind == llvm::ARM::AK_ARMV8_1A)
       Builder.defineMacro("__ARM_FEATURE_QRDMX", "1");
----------------
I wonder if this is wrong, if so please fix it in a further change otherwise this one wouldn't be a NFCI.


================
Comment at: lib/Basic/Targets.cpp:6364
     HasFullFP16 = 0;
+    ArchKind = llvm::ARM::AK_INVALID;
 
----------------
Would it make sense to set this to `AArch64::AK_ARMV8A` instead?


================
Comment at: lib/Basic/Targets.cpp:6376
       if (Feature == "+v8.1a")
-        V8_1A = 1;
+        ArchKind = llvm::ARM::AK_ARMV8_1A;
       if (Feature == "+v8.2a")
----------------
I think you want to use `llvm::AArch64::AK_ARMV8_1A` instead here.


================
Comment at: lib/Basic/Targets.cpp:6378
       if (Feature == "+v8.2a")
-        V8_2A = 1;
+        ArchKind = llvm::ARM::AK_ARMV8_2A;
       if (Feature == "+fullfp16")
----------------
Ditto with `llvm::AArch64::AK_ARMV8_2A`


https://reviews.llvm.org/D34686