[RFC]Add unittests to {ARM | AArch64}TargetParser

[jojo]

Summary

• Add unittest to {ARM | AArch64}TargetParser
• Correct a incorrect indexing problem in AArch64TargetParser The architecture enumeration is shared across ARM and AArch64 in original implementation.But In the code,I just used the index which was offset by the ARM, and this would index into the array incorrectly. To make AArch64 has its own arch enum,or we will do a lot of slowly iterating.
• Correct a spelling error by the way. parameter of llvm::AArch64::getArchExtName
• Correct a writing mistake, in llvm::ARM::parseArchISA

[jojo]

Remove unnecessary whitespace.

[compnerd]

include/llvm/Support/TargetParser.h

152

I think that you should use the AK_ prefix as is the norm. That said, you could do something else which IMO is better: use enum class and drop the prefix.

unittests/Support/TargetParserTest.cpp

45

Please delete this. I had added it to avoid hardcoding the offset. Just iterate over the enumeration below. In fact, it is unused.

89

This set really feels weird to me.

[jojo]

Remove uncessary items according to inline comments.

include/llvm/Support/TargetParser.h

152

If we use enum class, we must do type casting when we perform operations between "unsigned int" and archkind. There are a lot of cases like this, in the TargetParser and places using it. And some generic structure definition shared between arm and aarch64, should be made a set for each.
Do we have to do this? Let AK_ point to ARM, and AAK_ point to AArch64 seems to make sense also.

unittests/Support/TargetParserTest.cpp

45

Indeed,remove it.

89

I removed two of these which are unnecessary.
The remaining two arrays are needed for some test items,with the help of def file can avoid writing a long array.And if there are changes happened on arch or cpu in def,we can do as small
changes as possible.
Hi,compnerd,If you still feel it can not be accepted,Could you say it further? Can't you accept the way they are defined? Or just don't want to have something like that? so it can get me more clear.
Thank you very much! :)

[jojo]

• Add unittest to ARMTargetParser
• Correct a writing mistake,in llvm::ARM::parseArchISA.
• Make some small changes on AArch64TargetParser

[rengolin]

include/llvm/Support/TargetParser.h

152

I think we should leave that discussion for when we change this to a class design, or when we move to table-generated tables. I think it should be fine as is, for now.

[rengolin]

Hi Jojo,

Thanks for the patch. I like how thorough were your tests, and it'll certainly cover a lot more than the current TripleTest. At some point, after we finish this, I think we should clean that one up, so that it stops redundant (and slow) tests.

Saleem (@compnerd), are you happy with the changes? I think we can discuss style in the class discussion (since it's going to change again, anyway), and just get all tools to use the parser for now.

cheers,
--renato

[compnerd]

I think that Id still prefer the AK_ prefix. However, the only thing that is preventing me from accepting the change is that I haven't really carefully looked through all the cases. If you have done so, and everything looks okay, fine by me :-).

[jojo]

Add test for ARM::parseArchISA and ARM::parseArchEndian.

[jojo]

Dear all,

I‘m not so sure about the changes.That would be appreciated if you could give any advice.
It will take some time to read through all the changes,Thank you very much for your review.

[rengolin]

In D21785#479075, @compnerd wrote:

I think that Id still prefer the AK_ prefix. However, the only thing that is preventing me from accepting the change is that I haven't really carefully looked through all the cases. If you have done so, and everything looks okay, fine by me :-).

I don't mind either way (AK, AAK), but if there is anything stopping us from using AK, we should move that discussion later.

If not, I agree we should just keep AK instead. AK means "ArchKind", not "ARMKind", so AArch64 would also be "ArchKind", thus AK.

That would also make this change a lot smaller, and clearer.

thanks,
--renato

[jojo]

Looks like that keep using prefix AK is more reasonable.Update diff to use AK instead.

[rengolin]

lib/Support/TargetParser.cpp

455

Now I see why you wanted a different enum.

@compnerd, I really don't mind either way, so I'll leave it up to you. :)

[compnerd]

There are a bunch of place that could use a range based for loop, please prefer them instead.

lib/Support/TargetParser.cpp

419

Isn't this the original bug that is being fixed rather than just adding tests for the target parser?

455

I don't think that this is necessarily bad. It does make it more obvious if you are targeting a newer architecture variant.

462

Given that you are doing explicit checks, why not simplify this to:

if (ArchKind == ...)
  ...
if (ArchKind == ...)
  ...
return ArchKind > AArch64::ArchKind::AK_INVALID && ArchKind < AArch64::ArchKind::AK_LAST;

467–469

Shouldn't this check for AK_INVALID as well?

485–487

Shouldn't this check for AK_INVALID as well?

unittests/Support/TargetParserTest.cpp

170

space before the <.

Why not use a range based loop?

for (const auto &ARMCPUName : kARMCPUNames)
  EXPECT_EQ(ARMCPUName.DefaultFPU, ARM::getDefaultFPU(ARMCPUName.Name, 0));

179

Similar.

215

clang-format this

[jojo]

lib/Support/TargetParser.cpp

419

Yes.this is a bug fixing. For AArch64,it's unnecessary to OR the ArchBaseExtensions,as the tables for ARM and AArch64 have not exactly the same design.
The original bug is fixed by correcting the incorrect indexing problem and removing the ORing here.All the changes are included in this revision.

467–469

There is a record for "invalid" in ARCHNames table.
It will return what we want for ArchName and ArchAttr, no need to do additional check.

485–487

the same as above

unittests/Support/TargetParserTest.cpp

170

Yes,indeed.
Replace all similar cases.

215

Do you mean like this?

{ "crc", "+crc"},
... ...
{ "nocrc", "-crc"},
... ...

[rengolin]

unittests/Support/TargetParserTest.cpp

215

He probably just mean running "clang-format" over the file.

$ clang-format unittests/Support/TargetParserTest.cpp

should do.

[jojo]

clang-format unittests/Support/TargetParserTest.cpp

[jojo]

unittests/Support/TargetParserTest.cpp

215

Oh, No brainer.

[rengolin]

LGTM. Thanks!

include/llvm/Support/AArch64TargetParser.def

 //
 //===----------------------------------------------------------------------===//
 
 // NOTE: NO INCLUDE GUARD DESIRED!
 
 #ifndef AARCH64_ARCH
 #define AARCH64_ARCH(NAME, ID, CPU_ATTR, SUB_ARCH, ARCH_ATTR, ARCH_FPU, ARCH_BASE_EXT)
 #endif
-AARCH64_ARCH("armv8-a", AK_ARMV8A, "8-A", "v8", ARMBuildAttrs::CPUArch::v8_A,
+AARCH64_ARCH("invalid", AAK_INVALID, nullptr, nullptr,
+             ARMBuildAttrs::CPUArch::v8_A, FK_NONE, AArch64::AEK_NONE)
+AARCH64_ARCH("armv8-a", AAK_ARMV8A, "8-A", "v8", ARMBuildAttrs::CPUArch::v8_A,
              FK_CRYPTO_NEON_FP_ARMV8,
              (AArch64::AEK_CRC | AArch64::AEK_CRYPTO | AArch64::AEK_FP |
               AArch64::AEK_SIMD | AArch64::AEK_FP16 | AArch64::AEK_PROFILE))
-AARCH64_ARCH("armv8.1-a", AK_ARMV8_1A, "8.1-A", "v8.1a",
+AARCH64_ARCH("armv8.1-a", AAK_ARMV8_1A, "8.1-A", "v8.1a",
              ARMBuildAttrs::CPUArch::v8_A, FK_CRYPTO_NEON_FP_ARMV8,
              (AArch64::AEK_CRC | AArch64::AEK_CRYPTO | AArch64::AEK_FP |
               AArch64::AEK_SIMD | AArch64::AEK_FP16 | AArch64::AEK_PROFILE))
-AARCH64_ARCH("armv8.2-a", AK_ARMV8_2A, "8.2-A", "v8.2a",
+AARCH64_ARCH("armv8.2-a", AAK_ARMV8_2A, "8.2-A", "v8.2a",
              ARMBuildAttrs::CPUArch::v8_A, FK_CRYPTO_NEON_FP_ARMV8,
              (AArch64::AEK_CRC | AArch64::AEK_CRYPTO | AArch64::AEK_FP |
               AArch64::AEK_SIMD | AArch64::AEK_FP16 | AArch64::AEK_PROFILE |
               AArch64::AEK_RAS))
 #undef AARCH64_ARCH
 
 #ifndef AARCH64_ARCH_EXT_NAME
 #define AARCH64_ARCH_EXT_NAME(NAME, ID, FEATURE, NEGFEATURE)
 #endif
 // FIXME: This would be nicer were it tablegen
 AARCH64_ARCH_EXT_NAME("invalid",  AArch64::AEK_INVALID,  nullptr,  nullptr)
 AARCH64_ARCH_EXT_NAME("none",     AArch64::AEK_NONE,     nullptr,  nullptr)
 AARCH64_ARCH_EXT_NAME("crc",      AArch64::AEK_CRC,      "+crc",   "-crc")
 AARCH64_ARCH_EXT_NAME("crypto",   AArch64::AEK_CRYPTO,   "+crypto","-crypto")
 AARCH64_ARCH_EXT_NAME("fp",       AArch64::AEK_FP,       "+fp-armv8",  "-fp-armv8")
 AARCH64_ARCH_EXT_NAME("simd",     AArch64::AEK_SIMD,     "+neon",  "-neon")
 AARCH64_ARCH_EXT_NAME("fp16",     AArch64::AEK_FP16,     "+fullfp16",  "-fullfp16")
 AARCH64_ARCH_EXT_NAME("profile",     AArch64::AEK_PROFILE,     "+spe",  "-spe")
 AARCH64_ARCH_EXT_NAME("ras",     AArch64::AEK_RAS,     "+ras",  "-ras")
 #undef AARCH64_ARCH_EXT_NAME
 
 #ifndef AARCH64_CPU_NAME
 #define AARCH64_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT)
 #endif
-AARCH64_CPU_NAME("cortex-a35", AK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
+AARCH64_CPU_NAME("cortex-a35", AAK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
                 (AArch64::AEK_SIMD | AArch64::AEK_CRC | AArch64::AEK_CRYPTO))
-AARCH64_CPU_NAME("cortex-a53", AK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, true,
+AARCH64_CPU_NAME("cortex-a53", AAK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, true,
                 ( AArch64::AEK_SIMD | AArch64::AEK_CRC | AArch64::AEK_CRYPTO))
-AARCH64_CPU_NAME("cortex-a57", AK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
+AARCH64_CPU_NAME("cortex-a57", AAK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
                 (AArch64::AEK_SIMD | AArch64::AEK_CRC | AArch64::AEK_CRYPTO))
-AARCH64_CPU_NAME("cortex-a72", AK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
+AARCH64_CPU_NAME("cortex-a72", AAK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
                 (AArch64::AEK_SIMD | AArch64::AEK_CRC | AArch64::AEK_CRYPTO))
-AARCH64_CPU_NAME("cortex-a73", AK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
+AARCH64_CPU_NAME("cortex-a73", AAK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
                 (AArch64::AEK_SIMD | AArch64::AEK_CRC | AArch64::AEK_CRYPTO))
-AARCH64_CPU_NAME("cyclone", AK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
+AARCH64_CPU_NAME("cyclone", AAK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
                 (AArch64::AEK_SIMD | AArch64::AEK_CRYPTO))
-AARCH64_CPU_NAME("exynos-m1", AK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
+AARCH64_CPU_NAME("exynos-m1", AAK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
                 (AArch64::AEK_SIMD | AArch64::AEK_CRC | AArch64::AEK_CRYPTO))
-AARCH64_CPU_NAME("kryo", AK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
+AARCH64_CPU_NAME("kryo", AAK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
                 (AArch64::AEK_SIMD | AArch64::AEK_CRC | AArch64::AEK_CRYPTO))
-AARCH64_CPU_NAME("vulcan", AK_ARMV8_1A, FK_CRYPTO_NEON_FP_ARMV8, false,
+AARCH64_CPU_NAME("vulcan", AAK_ARMV8_1A, FK_CRYPTO_NEON_FP_ARMV8, false,
                 (AArch64::AEK_SIMD | AArch64::AEK_CRC | AArch64::AEK_CRYPTO))
 // Invalid CPU
-AARCH64_CPU_NAME("invalid", AK_INVALID, FK_INVALID, true, AArch64::AEK_INVALID)
+AARCH64_CPU_NAME("invalid", AAK_INVALID, FK_INVALID, true, AArch64::AEK_INVALID)
 #undef AARCH64_CPU_NAME

include/llvm/Support/TargetParser.h

 unsigned parseArchProfile(StringRef Arch);
 unsigned parseArchVersion(StringRef Arch);
 
 } // namespace ARM
 
 // FIXME:This should be made into class design,to avoid dupplication. 
 namespace AArch64 {
 
+// Arch names.
+enum ArchKind {
+#define AARCH64_ARCH(NAME, ID, CPU_ATTR, SUB_ARCH, ARCH_ATTR, ARCH_FPU, ARCH_BASE_EXT) ID,
+#include "AArch64TargetParser.def"
+  AAK_LAST
+};
+
 // Arch extension modifiers for CPUs.
 enum ArchExtKind : unsigned {
   AEK_INVALID = 0x0,
   AEK_NONE = 0x1,
   AEK_CRC = 0x2,
   AEK_CRYPTO = 0x4,
   AEK_FP = 0x8,
   AEK_SIMD = 0x10,

lib/Support/TargetParser.cpp

static const struct {
   const char *NameCStr;
   size_t NameLength;
   const char *CPUAttrCStr;
   size_t CPUAttrLength;
   const char *SubArchCStr;
   size_t SubArchLength;
   unsigned DefaultFPU;
   unsigned ArchBaseExtensions;
-  ARM::ArchKind ID;
+  unsigned ID;
   ARMBuildAttrs::CPUArch ArchAttr; // Arch ID in build attributes.
 
   StringRef getName() const { return StringRef(NameCStr, NameLength); }
 
   // CPU class in build attributes.
   StringRef getCPUAttr() const { return StringRef(CPUAttrCStr, CPUAttrLength); }
 
   // Sub-Arch name.

 // List of CPU names and their arches.
 // The same CPU can have multiple arches and can be default on multiple arches.
 // When finding the Arch for a CPU, first-found prevails. Sort them accordingly.
 // When this becomes table-generated, we'd probably need two tables.
 // FIXME: TableGen this.
 static const struct {
   const char *NameCStr;
   size_t NameLength;
-  ARM::ArchKind ArchID;
+  unsigned ArchID;
   bool Default; // is $Name the default CPU for $ArchID ?
   unsigned DefaultExtensions;
 
   StringRef getName() const { return StringRef(NameCStr, NameLength); }
 } CPUNames[] = {
 #define ARM_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
   { NAME, sizeof(NAME) - 1, ID, IS_DEFAULT, DEFAULT_EXT },
 #include "llvm/Support/ARMTargetParser.def"

 }
 
 unsigned llvm::AArch64::getDefaultExtensions(StringRef CPU, unsigned ArchKind) {
   if (CPU == "generic")
     return AArch64ARCHNames[ArchKind].ArchBaseExtensions;
 
   return StringSwitch<unsigned>(CPU)
 #define AARCH64_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
-    .Case(NAME, AArch64ARCHNames[ID].ArchBaseExtensions | DEFAULT_EXT)
+    .Case(NAME, DEFAULT_EXT)
 #include "llvm/Support/AArch64TargetParser.def"
     .Default(AArch64::AEK_INVALID);
 }
 
 bool llvm::AArch64::getExtensionFeatures(unsigned Extensions,
                                      std::vector<const char *> &Features) {
 
   if (Extensions == AArch64::AEK_INVALID)

 
 bool llvm::AArch64::getFPUFeatures(unsigned FPUKind,
                                std::vector<const char *> &Features) {
   return ARM::getFPUFeatures(FPUKind, Features);
 }
 
 bool llvm::AArch64::getArchFeatures(unsigned ArchKind,
                                      std::vector<const char *> &Features) {
-  if (ArchKind == ARM::AK_INVALID || ArchKind >= ARM::AK_LAST)
+  if (ArchKind == AArch64::AAK_INVALID || ArchKind >= AArch64::AAK_LAST)
     return false;
 
-  if (ArchKind == ARM::AK_ARMV8_1A)
+  if (ArchKind == AArch64::AAK_ARMV8_1A)
     Features.push_back("+v8.1a");
-  if (ArchKind == ARM::AK_ARMV8_2A)
+  if (ArchKind == AArch64::AAK_ARMV8_2A)
     Features.push_back("+v8.2a");
 
   return true;
 }
 
 StringRef llvm::AArch64::getArchName(unsigned ArchKind) {
-  for (const auto &AI : AArch64ARCHNames)
-    if (AI.ID == ArchKind)
-      return AI.getName();
+  if (ArchKind >= AArch64::AAK_LAST)
     return StringRef();
+  return AArch64ARCHNames[ArchKind].getName();
 }
 
 StringRef llvm::AArch64::getCPUAttr(unsigned ArchKind) {
-  for (const auto &AI : AArch64ARCHNames)
-    if (AI.ID == ArchKind)
-      return AI.getCPUAttr();
+  if (ArchKind == AArch64::AAK_INVALID || ArchKind >= AArch64::AAK_LAST)
     return StringRef();
+  return AArch64ARCHNames[ArchKind].getCPUAttr();
 }
 
 StringRef llvm::AArch64::getSubArch(unsigned ArchKind) {
-  for (const auto &AI : AArch64ARCHNames)
-    if (AI.ID == ArchKind)
-      return AI.getSubArch();
+  if (ArchKind == AArch64::AAK_INVALID || ArchKind >= AArch64::AAK_LAST)
     return StringRef();
+  return AArch64ARCHNames[ArchKind].getSubArch();
 }
 
 unsigned llvm::AArch64::getArchAttr(unsigned ArchKind) {
-  for (const auto &AI : AArch64ARCHNames)
-    if (AI.ID == ArchKind)
-      return AI.ArchAttr;
+  if (ArchKind >= AArch64::AAK_LAST)
     return ARMBuildAttrs::CPUArch::v8_A;
+  return AArch64ARCHNames[ArchKind].ArchAttr;
 }
 
-StringRef llvm::AArch64::getArchExtName(unsigned AArchExtKind) {
+StringRef llvm::AArch64::getArchExtName(unsigned ArchExtKind) {
   for (const auto &AE : AArch64ARCHExtNames)
-    if (AArchExtKind == AE.ID)
+    if (ArchExtKind == AE.ID)
       return AE.getName();
   return StringRef();
 }
 
 const char *llvm::AArch64::getArchExtFeature(StringRef ArchExt) {
   if (ArchExt.startswith("no")) {
     StringRef ArchExtBase(ArchExt.substr(2));
     for (const auto &AE : AArch64ARCHExtNames) {
       if (AE.NegFeature && ArchExtBase == AE.getName())
         return AE.NegFeature;
     }
   }
 
   for (const auto &AE : AArch64ARCHExtNames)
     if (AE.Feature && ArchExt == AE.getName())
       return AE.Feature;
   return nullptr;
 }
 
 StringRef llvm::AArch64::getDefaultCPU(StringRef Arch) {
   unsigned AK = parseArch(Arch);
-  if (AK == ARM::AK_INVALID)
+  if (AK == AArch64::AAK_INVALID)
     return StringRef();
 
   // Look for multiple AKs to find the default for pair AK+Name.
   for (const auto &CPU : AArch64CPUNames)
     if (CPU.ArchID == AK && CPU.Default)
       return CPU.getName();
 
   // If we can't find a default then target the architecture instead

 unsigned llvm::AArch64::parseFPU(StringRef FPU) {
   return ARM::parseFPU(FPU);
 }
 
 // Allows partial match, ex. "v8a" matches "armv8a".
 unsigned llvm::AArch64::parseArch(StringRef Arch) {
   Arch = getCanonicalArchName(Arch);
   if (checkArchVersion(Arch) < 8)
-    return ARM::AK_INVALID;
+    return AArch64::AAK_INVALID;
 
   StringRef Syn = getArchSynonym(Arch);
   for (const auto A : AArch64ARCHNames) {
     if (A.getName().endswith(Syn))
       return A.ID;
   }
-  return ARM::AK_INVALID;
+  return AArch64::AAK_INVALID;
 }
 
 unsigned llvm::AArch64::parseArchExt(StringRef ArchExt) {
   for (const auto A : AArch64ARCHExtNames) {
     if (ArchExt == A.getName())
       return A.ID;
   }
   return AArch64::AEK_INVALID;
 }
 
 unsigned llvm::AArch64::parseCPUArch(StringRef CPU) {
   for (const auto C : AArch64CPUNames) {
     if (CPU == C.getName())
       return C.ArchID;
   }
-  return ARM::AK_INVALID;
+  return AArch64::AAK_INVALID;
 }
 
 // ARM, Thumb, AArch64
 unsigned llvm::AArch64::parseArchISA(StringRef Arch) {
   return ARM::parseArchISA(Arch);
 }
 
 // Little/Big endian

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

 bool AArch64AsmParser::parseDirectiveArch(SMLoc L) {
   SMLoc ArchLoc = getLoc();
 
   StringRef Arch, ExtensionString;
   std::tie(Arch, ExtensionString) =
       getParser().parseStringToEndOfStatement().trim().split('+');
 
   unsigned ID = AArch64::parseArch(Arch);
-  if (ID == ARM::AK_INVALID) {
+  if (ID == AArch64::AAK_INVALID) {
     Error(ArchLoc, "unknown arch name");
     return false;
   }
 
   MCSubtargetInfo &STI = copySTI();
   STI.setDefaultFeatures("", "");
   if (!ExtensionString.empty())
     STI.setDefaultFeatures("", ("+" + ExtensionString).str());

unittests/Support/TargetParserTest.cpp

 //===----------- TargetParser.cpp - Target Parser -------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "gtest/gtest.h"
 #include "llvm/Support/TargetParser.h"
+#include "llvm/Support/ARMBuildAttributes.h"
+#include "llvm/ADT/STLExtras.h"
+
 
 using namespace llvm;
 
 namespace {
 static const unsigned kAArch64ArchKinds[] = {
 #define AARCH64_ARCH(NAME, ID, CPU_ATTR, SUB_ARCH, ARCH_ATTR, ARCH_FPU,        \
                      ARCH_BASE_EXT)                                            \
-  llvm::ARM::ID,
+  llvm::AArch64::ID,
+#include "llvm/Support/AArch64TargetParser.def"
+#undef AARCH64_ARCH
+};
+
+static const unsigned kAArch64ArchExtKinds[] = {
+#define AARCH64_ARCH_EXT_NAME(NAME, ID, FEATURE, NEGFEATURE) \
+  ID,
+#include "llvm/Support/AArch64TargetParser.def"
+#undef AARCH64_ARCH_EXT_NAME
+};
+
+static const struct {
+  const char *Name;
+  unsigned DefaultFPU;
+  unsigned ArchBaseExtensions;
+  unsigned ID;
+  ARMBuildAttrs::CPUArch ArchAttr;
+} kAArch64ARCHNames[] = {
+#define AARCH64_ARCH(NAME, ID, CPU_ATTR, SUB_ARCH, ARCH_ATTR, ARCH_FPU, ARCH_BASE_EXT)       \
+  {NAME, llvm::ARM::ARCH_FPU, ARCH_BASE_EXT, llvm::AArch64::ID, ARCH_ATTR},
 #include "llvm/Support/AArch64TargetParser.def"
 #undef AARCH64_ARCH
 };
 
+static const struct {
+  const char *Name;
+  unsigned ID;
+  const char *Feature;
+  const char *NegFeature;
+} kAArch64ARCHExtNames[] = {
+#define AARCH64_ARCH_EXT_NAME(NAME, ID, FEATURE, NEGFEATURE) \
+  { NAME, ID, FEATURE, NEGFEATURE },
+#include "llvm/Support/AArch64TargetParser.def"
+#undef AARCH64_ARCH_EXT_NAME
+};
+
+static const struct {
+  const char *Name;
+  unsigned ID;
+  unsigned DefaultFPU;
+  unsigned DefaultExt;
+} kAArch64CPUNames[] = {
+#define AARCH64_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT) \
+  { NAME, llvm::AArch64::ID, llvm::ARM::DEFAULT_FPU, DEFAULT_EXT },
+#include "llvm/Support/AArch64TargetParser.def"
+#undef AARCH64_CPU_NAME
+};
+
 template <typename T, size_t N>
 bool contains(const T (&array)[N], const T element) {
   return std::find(std::begin(array), std::end(array), element) !=
          std::end(array);
 }
 
 TEST(TargetParserTest, ARMArchName) {
   for (ARM::ArchKind AK = static_cast<ARM::ArchKind>(0);
        AK <= ARM::ArchKind::AK_LAST;
        AK = static_cast<ARM::ArchKind>(static_cast<unsigned>(AK) + 1))
     EXPECT_TRUE(AK == ARM::AK_LAST ? ARM::getArchName(AK).empty()
                                    : !ARM::getArchName(AK).empty());
 }
 
 TEST(TargetParserTest, ARMCPUAttr) {
   for (ARM::ArchKind AK = static_cast<ARM::ArchKind>(0);
        AK <= ARM::ArchKind::AK_LAST;
        AK = static_cast<ARM::ArchKind>(static_cast<unsigned>(AK) + 1))
     EXPECT_TRUE((AK == ARM::AK_INVALID || AK == ARM::AK_LAST)
                     ? ARM::getCPUAttr(AK).empty()
                     : !ARM::getCPUAttr(AK).empty());
 }
 
 TEST(TargetParserTest, ARMSubArch) {
   for (ARM::ArchKind AK = static_cast<ARM::ArchKind>(0);
        AK <= ARM::ArchKind::AK_LAST;
        AK = static_cast<ARM::ArchKind>(static_cast<unsigned>(AK) + 1))
     EXPECT_TRUE((AK == ARM::AK_INVALID || AK == ARM::AK_IWMMXT ||
                  AK == ARM::AK_IWMMXT2 || AK == ARM::AK_LAST)
                     ? ARM::getSubArch(AK).empty()
                     : !ARM::getSubArch(AK).empty());
 }
 
 TEST(TargetParserTest, ARMFPUName) {
   for (ARM::FPUKind FK = static_cast<ARM::FPUKind>(0);
        FK <= ARM::FPUKind::FK_LAST;
        FK = static_cast<ARM::FPUKind>(static_cast<unsigned>(FK) + 1))
     EXPECT_TRUE(FK == ARM::FK_LAST ? ARM::getFPUName(FK).empty()
                                    : !ARM::getFPUName(FK).empty());
 }
 
+TEST(TargetParserTest, AArch64DefaultFPU) {
+  for (AArch64::ArchKind AK = static_cast<AArch64::ArchKind>(0);
+       AK < AArch64::ArchKind::AAK_LAST;
+       AK = static_cast<AArch64::ArchKind>(static_cast<unsigned>(AK) + 1))
+    EXPECT_EQ(kAArch64ARCHNames[AK].DefaultFPU,
+              AArch64::getDefaultFPU(StringRef("generic"), AK));
+
+  for (unsigned i = 0; i< array_lengthof(kAArch64CPUNames); i++)
+    EXPECT_EQ(kAArch64CPUNames[i].DefaultFPU,
+              AArch64::getDefaultFPU(kAArch64CPUNames[i].Name,
+                                     kAArch64CPUNames[i].ID));
+}
+
+TEST(TargetParserTest, AArch64DefaultExt) {
+  for (AArch64::ArchKind AK = static_cast<AArch64::ArchKind>(0);
+       AK < AArch64::ArchKind::AAK_LAST;
+       AK = static_cast<AArch64::ArchKind>(static_cast<unsigned>(AK) + 1))
+    EXPECT_EQ(kAArch64ARCHNames[AK].ArchBaseExtensions,
+              AArch64::getDefaultExtensions(StringRef("generic"), AK));
+
+  for (unsigned i = 0; i < array_lengthof(kAArch64CPUNames); i++)
+    EXPECT_EQ(kAArch64CPUNames[i].DefaultExt,
+              AArch64::getDefaultExtensions(kAArch64CPUNames[i].Name,
+                                            kAArch64CPUNames[i].ID));
+}
+
+TEST(TargetParserTest, AArch64ExtensionFeatures) {
+  std::vector<const char*> Features;
+  unsigned Extensions = AArch64::AEK_CRC |
+                        AArch64::AEK_CRYPTO | 
+                        AArch64::AEK_FP |
+                        AArch64::AEK_SIMD |
+                        AArch64::AEK_FP16 |
+                        AArch64::AEK_PROFILE |
+                        AArch64::AEK_RAS;
+
+  for (unsigned i = 0; i <= Extensions; i++)
+    EXPECT_TRUE(i == 0 ? !AArch64::getExtensionFeatures(i, Features)
+                       : AArch64::getExtensionFeatures(i, Features));
+}
+
+TEST(TargetParserTest, AArch64ArchFeatures) {
+  std::vector<const char*> Features;
+
+  for (AArch64::ArchKind AK = static_cast<AArch64::ArchKind>(0);
+       AK < AArch64::ArchKind::AAK_LAST;
+       AK = static_cast<AArch64::ArchKind>(static_cast<unsigned>(AK) + 1))
+    EXPECT_TRUE((AK == AArch64::AAK_INVALID || AK == AArch64::AAK_LAST)
+                       ? !AArch64::getArchFeatures(AK, Features)
+                       : AArch64::getArchFeatures(AK, Features));
+}
+
 TEST(TargetParserTest, AArch64ArchName) {
-  for (ARM::ArchKind AK = static_cast<ARM::ArchKind>(0);
-       AK <= ARM::ArchKind::AK_LAST;
-       AK = static_cast<ARM::ArchKind>(static_cast<unsigned>(AK) + 1))
-    EXPECT_TRUE(contains(kAArch64ArchKinds, static_cast<unsigned>(AK))
-                    ? !AArch64::getArchName(AK).empty()
-                    : AArch64::getArchName(AK).empty());
+  for (AArch64::ArchKind AK = static_cast<AArch64::ArchKind>(0);
+       AK <= AArch64::ArchKind::AAK_LAST;
+       AK = static_cast<AArch64::ArchKind>(static_cast<unsigned>(AK) + 1))
+    EXPECT_TRUE(AK == AArch64::AAK_LAST
+                    ? AArch64::getArchName(AK).empty()
+                    : !AArch64::getArchName(AK).empty());
 }
 
 TEST(TargetParserTest, AArch64CPUAttr) {
-  for (ARM::ArchKind AK = static_cast<ARM::ArchKind>(0);
-       AK <= ARM::ArchKind::AK_LAST;
-       AK = static_cast<ARM::ArchKind>(static_cast<unsigned>(AK) + 1))
-    EXPECT_TRUE(contains(kAArch64ArchKinds, static_cast<unsigned>(AK))
-                    ? !AArch64::getCPUAttr(AK).empty()
-                    : AArch64::getCPUAttr(AK).empty());
+  for (AArch64::ArchKind AK = static_cast<AArch64::ArchKind>(0);
+       AK <= AArch64::ArchKind::AAK_LAST;
+       AK = static_cast<AArch64::ArchKind>(static_cast<unsigned>(AK) + 1))
+    EXPECT_TRUE((AK == AArch64::AAK_INVALID || AK == AArch64::AAK_LAST)
+                    ? AArch64::getCPUAttr(AK).empty()
+                    : !AArch64::getCPUAttr(AK).empty());
 }
 
 TEST(TargetParserTest, AArch64SubArch) {
-  for (ARM::ArchKind AK = static_cast<ARM::ArchKind>(0);
-       AK <= ARM::ArchKind::AK_LAST;
-       AK = static_cast<ARM::ArchKind>(static_cast<unsigned>(AK) + 1))
-    EXPECT_TRUE(contains(kAArch64ArchKinds, static_cast<unsigned>(AK))
-                    ? !AArch64::getSubArch(AK).empty()
-                    : AArch64::getSubArch(AK).empty());
+  for (AArch64::ArchKind AK = static_cast<AArch64::ArchKind>(0);
+       AK <= AArch64::ArchKind::AAK_LAST;
+       AK = static_cast<AArch64::ArchKind>(static_cast<unsigned>(AK) + 1))
+    EXPECT_TRUE((AK == AArch64::AAK_INVALID || AK == AArch64::AAK_LAST)
+                    ? AArch64::getSubArch(AK).empty()
+                    : !AArch64::getSubArch(AK).empty());
+}
+
+TEST(TargetParserTest, AArch64ArchAttr) {
+  for (AArch64::ArchKind AK = static_cast<AArch64::ArchKind>(0);
+       AK <= AArch64::ArchKind::AAK_LAST;
+       AK = static_cast<AArch64::ArchKind>(static_cast<unsigned>(AK) + 1))
+    EXPECT_TRUE(AK == AArch64::AAK_LAST
+                ? (ARMBuildAttrs::CPUArch::v8_A == AArch64::getArchAttr(AK))
+                : (kAArch64ARCHNames[AK].ArchAttr == AArch64::getArchAttr(AK)));
+}
+
+TEST(TargetParserTest, AArch64ArchExtName) {
+  for (AArch64::ArchExtKind AEK = static_cast<AArch64::ArchExtKind>(0);
+       AEK <= AArch64::ArchExtKind::AEK_RAS;
+       AEK = static_cast<AArch64::ArchExtKind>(static_cast<unsigned>(AEK) + 1))
+    EXPECT_TRUE(contains(kAArch64ArchExtKinds, static_cast<unsigned>(AEK))
+                    ? !AArch64::getArchExtName(AEK).empty()
+                    : AArch64::getArchExtName(AEK).empty());
+}
+
+TEST(TargetParserTest, AArch64ArchExtFeature) {
+  const char *ArchExt[][4] = {
+    { "crc", "nocrc", "+crc", "-crc"}, 
+    { "crypto", "nocrypto", "+crypto", "-crypto"},
+    { "fp", "nofp", "+fp-armv8", "-fp-armv8"},
+    { "simd", "nosimd", "+neon", "-neon"},
+    { "fp16", "nofp16", "+fullfp16", "-fullfp16"},
+    { "profile", "noprofile", "+spe", "-spe"},
+    { "ras", "noras", "+ras", "-ras"}
+  };
+
+  for (unsigned i = 0; i < array_lengthof(ArchExt); i++) {
+    EXPECT_EQ(ArchExt[i][2], AArch64::getArchExtFeature(ArchExt[i][0]));
+    EXPECT_EQ(ArchExt[i][3], AArch64::getArchExtFeature(ArchExt[i][1]));
+  }
+}
+
+TEST(TargetParserTest, AArch64DefaultCPU) {
+  const char *Arch[] = {"armv8a", "armv8-a", "armv8",
+                        "armv8.1a", "armv8.1-a",
+                        "armv8.2a", "armv8.2-a"};
+
+  for (unsigned i = 0; i < array_lengthof(Arch); i++)
+    EXPECT_FALSE(AArch64::getDefaultCPU(Arch[i]).empty());
+}
+
+TEST(TargetParserTest, AArch64parseArch) {
+  const char *Arch[] = {"armv8", "armv8a", "armv8-a",
+                        "armv8.1a", "armv8.1-a",
+                        "armv8.2a", "armv8.2-a"
+                       };
+
+  for (unsigned i = 0; i < array_lengthof(Arch); i++)
+    EXPECT_NE(AArch64::AAK_INVALID, AArch64::parseArch(Arch[i]));
+  EXPECT_EQ(AArch64::AAK_INVALID, AArch64::parseArch("aarch64"));
+  EXPECT_EQ(AArch64::AAK_INVALID, AArch64::parseArch("arm64"));
+}
+
+TEST(TargetParserTest, AArch64parseArchExt) { 
+  const char *const ArchExt[] = { "none", "crc", "crypto", "fp",
+                                  "simd", "fp16", "profile", "ras"
+                                };
+
+  for (unsigned i = 0; i < array_lengthof(kAArch64ARCHExtNames); i++)
+    EXPECT_TRUE(contains(ArchExt, kAArch64ARCHExtNames[i].Name) 
+                ? (AArch64::AEK_INVALID !=
+                   AArch64::parseArchExt(kAArch64ARCHExtNames[i].Name))
+                : (AArch64::AEK_INVALID ==
+                   AArch64::parseArchExt(kAArch64ARCHExtNames[i].Name)));
+}
+
+TEST(TargetParserTest, AArch64parseCPUArch) { 
+  const char *const CPU[] = { "cortex-a35", "cortex-a53", "cortex-a57",
+                              "cortex-a72", "cortex-a73", "cyclone",
+                              "exynos-m1", "kryo", "vulcan"
+                            };
+
+  for (unsigned i = 0; i < array_lengthof(kAArch64CPUNames); i++)
+    EXPECT_TRUE(contains(CPU, kAArch64CPUNames[i].Name) 
+                ? (AArch64::AAK_INVALID !=
+                   AArch64::parseCPUArch(kAArch64CPUNames[i].Name))
+                : (AArch64::AAK_INVALID ==
+                   AArch64::parseCPUArch(kAArch64CPUNames[i].Name)));
 }
 }