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

Roger Ferrer Ibanez via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 28 03:13:41 PDT 2017

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`


More information about the cfe-commits mailing list