Support: correct AArch64 TargetParser implementation

The architecture enumeration is shared across ARM and AArch64.  However, the
data is not.  The code incorrectly would index into the array using the
architecture index which was offset by the ARMv7 architecture enumeration.  We
do not have a marker for indicating the architectural family to which the
enumeration belongs so we cannot be clever about offsetting the index (at least
it is not immediately apparent to me).  Instead, fall back to the tried-and-true
method of slowly iterating the array (its not a large array, so the impact of
this is not too high).

Because of the incorrect indexing, if we were lucky, we would crash, but usually
we would return an invalid StringRef.  We did not have any tests for the AArch64
target parser previously;.  Extend the previous tests I had added for ARM to
cover AArch64 for ensuring that we return expected StringRefs.

Take the opportunity to change some iterator types to references.

This work is needed to support parsing `.arch name` directives in the AArch64
target asm parser.

llvm-svn: 272145

lib/Support/TargetParser.cpp

@@ -462,50 +462,52 @@ bool llvm::AArch64::getArchFeatures(unsigned ArchKind,
 }
 
 StringRef llvm::AArch64::getArchName(unsigned ArchKind) {
-  if (ArchKind >= ARM::AK_LAST)
-    return StringRef();
-  return AArch64ARCHNames[ArchKind].getName();
+  for (const auto &AI : AArch64ARCHNames)
+    if (AI.ID == ArchKind)
+      return AI.getName();
+  return StringRef();
 }
 
 StringRef llvm::AArch64::getCPUAttr(unsigned ArchKind) {
-  if (ArchKind == ARM::AK_INVALID || ArchKind >= ARM::AK_LAST)
-    return StringRef();
-  return AArch64ARCHNames[ArchKind].getCPUAttr();
+  for (const auto &AI : AArch64ARCHNames)
+    if (AI.ID == ArchKind)
+      return AI.getCPUAttr();
+  return StringRef();
 }
 
 StringRef llvm::AArch64::getSubArch(unsigned ArchKind) {
-  if (ArchKind == ARM::AK_INVALID || ArchKind >= ARM::AK_LAST)
-    return StringRef();
-  return AArch64ARCHNames[ArchKind].getSubArch();
+  for (const auto &AI : AArch64ARCHNames)
+    if (AI.ID == ArchKind)
+      return AI.getSubArch();
+  return StringRef();
 }
 
 unsigned llvm::AArch64::getArchAttr(unsigned ArchKind) {
-  if (ArchKind >= ARM::AK_LAST)
-    return ARMBuildAttrs::CPUArch::v8_A;
-  return AArch64ARCHNames[ArchKind].ArchAttr;
+  for (const auto &AI : AArch64ARCHNames)
+    if (AI.ID == ArchKind)
+      return AI.ArchAttr;
+  return ARMBuildAttrs::CPUArch::v8_A;
 }
 
 StringRef llvm::AArch64::getArchExtName(unsigned AArchExtKind) {
-  for (const auto AE : AArch64ARCHExtNames) {
+  for (const auto &AE : AArch64ARCHExtNames)
     if (AArchExtKind == 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) {
+    for (const auto &AE : AArch64ARCHExtNames) {
       if (AE.NegFeature && ArchExtBase == AE.getName())
         return AE.NegFeature;
     }
   }
-  for (const auto AE : AArch64ARCHExtNames) {
+
+  for (const auto &AE : AArch64ARCHExtNames)
     if (AE.Feature && ArchExt == AE.getName())
       return AE.Feature;
-  }
-
   return nullptr;
 }
 
@@ -515,10 +517,9 @@ StringRef llvm::AArch64::getDefaultCPU(StringRef Arch) {
     return StringRef();
 
   // Look for multiple AKs to find the default for pair AK+Name.
-  for (const auto CPU : AArch64CPUNames) {
+  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
   return "generic";

unittests/Support/TargetParserTest.cpp

@@ -13,6 +13,20 @@
 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,
+#include "llvm/Support/AArch64TargetParser.def"
+#undef AARCH64_ARCH
+};
+
+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;
@@ -47,5 +61,32 @@ TEST(TargetParserTest, ARMFPUName) {
     EXPECT_TRUE(FK == ARM::FK_LAST ? ARM::getFPUName(FK).empty()
                                    : !ARM::getFPUName(FK).empty());
 }
+
+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());
+}
+
+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());
+}
+
+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());
+}
 }