[NFC][AArch64] Use optional returns in target parser instead of 'invalid' objects

This updates the parsing methods in AArch64's Target Parser to make use
of optional returns instead of "invalid" enum values, making the API's
behaviour clearer.

Reviewed By: lenary, tmatheson

Differential Revision: https://reviews.llvm.org/D142539

Author: pratlucas

clang/lib/Basic/Targets/AArch64.cpp

@@ -19,6 +19,7 @@
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/AArch64TargetParser.h"
 #include "llvm/Support/ARMTargetParserCommon.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include <optional>
 
 using namespace clang;
@@ -223,8 +224,7 @@ bool AArch64TargetInfo::validateBranchProtection(StringRef Spec, StringRef,
 }
 
 bool AArch64TargetInfo::isValidCPUName(StringRef Name) const {
-  return Name == "generic" ||
-         llvm::AArch64::parseCpu(Name).Arch != llvm::AArch64::INVALID;
+  return Name == "generic" || llvm::AArch64::parseCpu(Name);
 }
 
 bool AArch64TargetInfo::setCPU(const std::string &Name) {
@@ -681,19 +681,19 @@ void AArch64TargetInfo::setFeatureEnabled(llvm::StringMap<bool> &Features,
   Features[Name] = Enabled;
   // If the feature is an architecture feature (like v8.2a), add all previous
   // architecture versions and any dependant target features.
-  const llvm::AArch64::ArchInfo &ArchInfo =
+  const std::optional<llvm::AArch64::ArchInfo> ArchInfo =
       llvm::AArch64::ArchInfo::findBySubArch(Name);
 
-  if (ArchInfo == llvm::AArch64::INVALID)
+  if (!ArchInfo)
     return; // Not an architecure, nothing more to do.
 
   for (const auto *OtherArch : llvm::AArch64::ArchInfos)
-    if (ArchInfo.implies(*OtherArch))
+    if (ArchInfo->implies(*OtherArch))
       Features[OtherArch->getSubArch()] = Enabled;
 
   // Set any features implied by the architecture
   uint64_t Extensions =
-      llvm::AArch64::getDefaultExtensions("generic", ArchInfo);
+      llvm::AArch64::getDefaultExtensions("generic", *ArchInfo);
   std::vector<StringRef> CPUFeats;
   if (llvm::AArch64::getExtensionFeatures(Extensions, CPUFeats)) {
     for (auto F : CPUFeats) {
@@ -949,9 +949,9 @@ bool AArch64TargetInfo::initFeatureMap(
     const std::vector<std::string> &FeaturesVec) const {
   std::vector<std::string> UpdatedFeaturesVec;
   // Parse the CPU and add any implied features.
-  const llvm::AArch64::ArchInfo &Arch = llvm::AArch64::parseCpu(CPU).Arch;
-  if (Arch != llvm::AArch64::INVALID) {
-    uint64_t Exts = llvm::AArch64::getDefaultExtensions(CPU, Arch);
+  std::optional<llvm::AArch64::CpuInfo> CpuInfo = llvm::AArch64::parseCpu(CPU);
+  if (CpuInfo) {
+    uint64_t Exts = llvm::AArch64::getDefaultExtensions(CPU, CpuInfo->Arch);
     std::vector<StringRef> CPUFeats;
     llvm::AArch64::getExtensionFeatures(Exts, CPUFeats);
     for (auto F : CPUFeats) {
@@ -1033,13 +1033,14 @@ ParsedTargetAttr AArch64TargetInfo::parseTargetAttr(StringRef Features) const {
       FoundArch = true;
       std::pair<StringRef, StringRef> Split =
           Feature.split("=").second.trim().split("+");
-      const llvm::AArch64::ArchInfo &AI = llvm::AArch64::parseArch(Split.first);
+      const std::optional<llvm::AArch64::ArchInfo> AI =
+          llvm::AArch64::parseArch(Split.first);
 
       // Parse the architecture version, adding the required features to
       // Ret.Features.
-      if (AI == llvm::AArch64::INVALID)
+      if (!AI)
         continue;
-      Ret.Features.push_back(AI.ArchFeature.str());
+      Ret.Features.push_back(AI->ArchFeature.str());
       // Add any extra features, after the +
       SplitAndAddFeatures(Split.second, Ret.Features);
     } else if (Feature.startswith("cpu=")) {

clang/lib/Driver/ToolChains/Arch/AArch64.cpp

@@ -123,18 +123,21 @@ static bool DecodeAArch64Features(const Driver &D, StringRef text,
 static bool DecodeAArch64Mcpu(const Driver &D, StringRef Mcpu, StringRef &CPU,
                               std::vector<StringRef> &Features) {
   std::pair<StringRef, StringRef> Split = Mcpu.split("+");
+  CPU = Split.first;
   const llvm::AArch64::ArchInfo *ArchInfo = &llvm::AArch64::ARMV8A;
-  CPU = llvm::AArch64::resolveCPUAlias(Split.first);
 
   if (CPU == "native")
     CPU = llvm::sys::getHostCPUName();
 
   if (CPU == "generic") {
     Features.push_back("+neon");
   } else {
-    ArchInfo = &llvm::AArch64::parseCpu(CPU).Arch;
-    if (*ArchInfo == llvm::AArch64::INVALID)
+    const std::optional<llvm::AArch64::CpuInfo> CpuInfo =
+        llvm::AArch64::parseCpu(CPU);
+    if (!CpuInfo)
       return false;
+    ArchInfo = &CpuInfo->Arch;
+
     Features.push_back(ArchInfo->ArchFeature);
 
     uint64_t Extension = llvm::AArch64::getDefaultExtensions(CPU, *ArchInfo);
@@ -156,11 +159,11 @@ getAArch64ArchFeaturesFromMarch(const Driver &D, StringRef March,
   std::string MarchLowerCase = March.lower();
   std::pair<StringRef, StringRef> Split = StringRef(MarchLowerCase).split("+");
 
-  const llvm::AArch64::ArchInfo *ArchInfo =
-      &llvm::AArch64::parseArch(Split.first);
+  std::optional <llvm::AArch64::ArchInfo> ArchInfo =
+      llvm::AArch64::parseArch(Split.first);
   if (Split.first == "native")
-    ArchInfo = &llvm::AArch64::getArchForCpu(llvm::sys::getHostCPUName().str());
-  if (*ArchInfo == llvm::AArch64::INVALID)
+    ArchInfo = llvm::AArch64::getArchForCpu(llvm::sys::getHostCPUName().str());
+  if (!ArchInfo)
     return false;
   Features.push_back(ArchInfo->ArchFeature);
 

llvm/include/llvm/TargetParser/AArch64TargetParser.h

@@ -91,7 +91,6 @@ enum CPUFeatures {
 // feature name (though the canonical reference for those is AArch64.td)
 // clang-format off
 enum ArchExtKind : uint64_t {
-  AEK_INVALID =     0,
   AEK_NONE =        1,
   AEK_CRC =         1 << 1,  // FEAT_CRC32
   AEK_CRYPTO =      1 << 2,
@@ -252,7 +251,6 @@ inline constexpr ExtensionInfo Extensions[] = {
     {"wfxt", AArch64::AEK_NONE, {}, {}, FEAT_WFXT, "+wfxt", 550},
     // Special cases
     {"none", AArch64::AEK_NONE, {}, {}, FEAT_MAX, "", ExtensionInfo::MaxFMVPriority},
-    {"invalid", AArch64::AEK_INVALID, {}, {}, FEAT_MAX, "", 0},
 };
 // clang-format on
 
@@ -280,12 +278,12 @@ struct ArchInfo {
   //       v       v       v       v       v
   //     v8.9a > v8.8a > v8.7a > v8.6a > v8.5a > v8.4a > ... > v8a;
   //
-  // v8r and INVALID have no relation to anything. This is used to
-  // determine which features to enable for a given architecture. See
+  // v8r has no relation to anything. This is used to determine which
+  // features to enable for a given architecture. See
   // AArch64TargetInfo::setFeatureEnabled.
   bool implies(const ArchInfo &Other) const {
     if (this->Profile != Other.Profile)
-      return false; // ARMV8R and INVALID
+      return false; // ARMV8R
     if (this->Version.getMajor() == Other.Version.getMajor()) {
       return this->Version > Other.Version;
     }
@@ -300,11 +298,10 @@ struct ArchInfo {
   StringRef getSubArch() const { return ArchFeature.substr(1); }
 
   // Search for ArchInfo by SubArch name
-  static const ArchInfo &findBySubArch(StringRef SubArch);
+  static std::optional<ArchInfo> findBySubArch(StringRef SubArch);
 };
 
 // clang-format off
-inline constexpr ArchInfo INVALID   = { VersionTuple{0, 0}, AProfile, "invalid", "+", (AArch64::AEK_NONE)};
 inline constexpr ArchInfo ARMV8A    = { VersionTuple{8, 0}, AProfile, "armv8-a", "+v8a", (AArch64::AEK_FP | AArch64::AEK_SIMD), };
 inline constexpr ArchInfo ARMV8_1A  = { VersionTuple{8, 1}, AProfile, "armv8.1-a", "+v8.1a", (ARMV8A.DefaultExts | AArch64::AEK_CRC | AArch64::AEK_LSE | AArch64::AEK_RDM)};
 inline constexpr ArchInfo ARMV8_2A  = { VersionTuple{8, 2}, AProfile, "armv8.2-a", "+v8.2a", (ARMV8_1A.DefaultExts | AArch64::AEK_RAS)};
@@ -325,10 +322,10 @@ inline constexpr ArchInfo ARMV8R    = { VersionTuple{8, 0}, RProfile, "armv8-r",
 // clang-format on
 
 // The set of all architectures
-static constexpr std::array<const ArchInfo *, 17> ArchInfos = {
-    &INVALID,  &ARMV8A,   &ARMV8_1A, &ARMV8_2A, &ARMV8_3A, &ARMV8_4A,
-    &ARMV8_5A, &ARMV8_6A, &ARMV8_7A, &ARMV8_8A, &ARMV8_9A, &ARMV9A,
-    &ARMV9_1A, &ARMV9_2A, &ARMV9_3A, &ARMV9_4A, &ARMV8R,
+static constexpr std::array<const ArchInfo *, 16> ArchInfos = {
+    &ARMV8A,   &ARMV8_1A, &ARMV8_2A, &ARMV8_3A, &ARMV8_4A, &ARMV8_5A,
+    &ARMV8_6A, &ARMV8_7A, &ARMV8_8A, &ARMV8_9A, &ARMV9A, &ARMV9_1A,
+    &ARMV9_2A, &ARMV9_3A, &ARMV9_4A, &ARMV8R,
 };
 
 // Details of a specific CPU.
@@ -495,8 +492,6 @@ inline constexpr CpuInfo CpuInfos[] = {
      (AArch64::AEK_FP16 | AArch64::AEK_RAND | AArch64::AEK_SM4 |
       AArch64::AEK_SHA3 | AArch64::AEK_SHA2 | AArch64::AEK_AES |
       AArch64::AEK_MTE | AArch64::AEK_SB | AArch64::AEK_SSBS)},
-    // Invalid CPU
-    {"invalid", INVALID, (AArch64::AEK_INVALID)},
 };
 
 // An alias for a CPU.
@@ -516,13 +511,13 @@ StringRef resolveCPUAlias(StringRef CPU);
 // Information by Name
 uint64_t getDefaultExtensions(StringRef CPU, const ArchInfo &AI);
 void getFeatureOption(StringRef Name, std::string &Feature);
-const ArchInfo &getArchForCpu(StringRef CPU);
+std::optional<ArchInfo> getArchForCpu(StringRef CPU);
 
 // Parser
-const ArchInfo &parseArch(StringRef Arch);
-ArchExtKind parseArchExt(StringRef ArchExt);
+std::optional<ArchInfo> parseArch(StringRef Arch);
+std::optional<ExtensionInfo> parseArchExtension(StringRef Extension);
 // Given the name of a CPU or alias, return the correponding CpuInfo.
-const CpuInfo &parseCpu(StringRef Name);
+std::optional<CpuInfo> parseCpu(StringRef Name);
 // Used by target parser tests
 void fillValidCPUArchList(SmallVectorImpl<StringRef> &Values);
 

llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

@@ -51,6 +51,7 @@
 #include "llvm/Support/AArch64TargetParser.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include <cassert>
 #include <cctype>
 #include <cstdint>
@@ -6880,18 +6881,18 @@ bool AArch64AsmParser::parseDirectiveArch(SMLoc L) {
   std::tie(Arch, ExtensionString) =
       getParser().parseStringToEndOfStatement().trim().split('+');
 
-  const AArch64::ArchInfo &ArchInfo = AArch64::parseArch(Arch);
-  if (ArchInfo == AArch64::INVALID)
+  std::optional<AArch64::ArchInfo> ArchInfo = AArch64::parseArch(Arch);
+  if (!ArchInfo)
     return Error(ArchLoc, "unknown arch name");
 
   if (parseToken(AsmToken::EndOfStatement))
     return true;
 
   // Get the architecture and extension features.
   std::vector<StringRef> AArch64Features;
-  AArch64Features.push_back(ArchInfo.ArchFeature);
+  AArch64Features.push_back(ArchInfo->ArchFeature);
   AArch64::getExtensionFeatures(
-      AArch64::getDefaultExtensions("generic", ArchInfo), AArch64Features);
+      AArch64::getDefaultExtensions("generic", *ArchInfo), AArch64Features);
 
   MCSubtargetInfo &STI = copySTI();
   std::vector<std::string> ArchFeatures(AArch64Features.begin(), AArch64Features.end());
@@ -6902,7 +6903,7 @@ bool AArch64AsmParser::parseDirectiveArch(SMLoc L) {
   if (!ExtensionString.empty())
     ExtensionString.split(RequestedExtensions, '+');
 
-  ExpandCryptoAEK(ArchInfo, RequestedExtensions);
+  ExpandCryptoAEK(*ArchInfo, RequestedExtensions);
 
   FeatureBitset Features = STI.getFeatureBits();
   for (auto Name : RequestedExtensions) {
@@ -6987,19 +6988,17 @@ bool AArch64AsmParser::parseDirectiveCPU(SMLoc L) {
   if (!ExtensionString.empty())
     ExtensionString.split(RequestedExtensions, '+');
 
-  // FIXME This is using tablegen data, but should be moved to ARMTargetParser
-  // once that is tablegen'ed
-  if (!getSTI().isCPUStringValid(CPU)) {
+  const std::optional<llvm::AArch64::ArchInfo> CpuArch = llvm::AArch64::getArchForCpu(CPU);
+  if (!CpuArch) {
     Error(CurLoc, "unknown CPU name");
     return false;
   }
+  ExpandCryptoAEK(*CpuArch, RequestedExtensions);
 
   MCSubtargetInfo &STI = copySTI();
   STI.setDefaultFeatures(CPU, /*TuneCPU*/ CPU, "");
   CurLoc = incrementLoc(CurLoc, CPU.size());
 
-  ExpandCryptoAEK(llvm::AArch64::getArchForCpu(CPU), RequestedExtensions);
-
   for (auto Name : RequestedExtensions) {
     // Advance source location past '+'.
     CurLoc = incrementLoc(CurLoc, 1);

llvm/lib/TargetParser/AArch64TargetParser.cpp

@@ -31,8 +31,11 @@ uint64_t AArch64::getDefaultExtensions(StringRef CPU,
     return AI.DefaultExts;
 
   // Note: this now takes cpu aliases into account
-  const CpuInfo &Cpu = parseCpu(CPU);
-  return Cpu.Arch.DefaultExts | Cpu.DefaultExtensions;
+  std::optional<CpuInfo> Cpu = parseCpu(CPU);
+  if (!Cpu)
+    return AI.DefaultExts;
+
+  return Cpu->Arch.DefaultExts | Cpu->DefaultExtensions;
 }
 
 void AArch64::getFeatureOption(StringRef Name, std::string &Feature) {
@@ -45,20 +48,22 @@ void AArch64::getFeatureOption(StringRef Name, std::string &Feature) {
   Feature = Name.str();
 }
 
-const AArch64::ArchInfo &AArch64::getArchForCpu(StringRef CPU) {
+std::optional<AArch64::ArchInfo> AArch64::getArchForCpu(StringRef CPU) {
   if (CPU == "generic")
     return ARMV8A;
 
   // Note: this now takes cpu aliases into account
-  const CpuInfo &Cpu = parseCpu(CPU);
-  return Cpu.Arch;
+  std::optional<CpuInfo> Cpu = parseCpu(CPU);
+  if (!Cpu)
+    return {};
+  return Cpu->Arch;
 }
 
-const AArch64::ArchInfo &AArch64::ArchInfo::findBySubArch(StringRef SubArch) {
+std::optional<AArch64::ArchInfo> AArch64::ArchInfo::findBySubArch(StringRef SubArch) {
   for (const auto *A : AArch64::ArchInfos)
     if (A->getSubArch() == SubArch)
       return *A;
-  return AArch64::INVALID;
+  return {};
 }
 
 uint64_t AArch64::getCpuSupportsMask(ArrayRef<StringRef> FeatureStrs) {
@@ -75,9 +80,6 @@ uint64_t AArch64::getCpuSupportsMask(ArrayRef<StringRef> FeatureStrs) {
 
 bool AArch64::getExtensionFeatures(uint64_t InputExts,
                                    std::vector<StringRef> &Features) {
-  if (InputExts == AArch64::AEK_INVALID)
-    return false;
-
   for (const auto &E : Extensions)
     /* INVALID and NONE have no feature name. */
     if ((InputExts & E.ID) && !E.Feature.empty())
@@ -110,7 +112,6 @@ StringRef AArch64::getArchExtFeature(StringRef ArchExt) {
 
 void AArch64::fillValidCPUArchList(SmallVectorImpl<StringRef> &Values) {
   for (const auto &C : CpuInfos)
-    if (C.Arch != INVALID)
       Values.push_back(C.Name);
 
   for (const auto &Alias : CpuAliases)
@@ -123,28 +124,28 @@ bool AArch64::isX18ReservedByDefault(const Triple &TT) {
 }
 
 // Allows partial match, ex. "v8a" matches "armv8a".
-const AArch64::ArchInfo &AArch64::parseArch(StringRef Arch) {
+std::optional<AArch64::ArchInfo> AArch64::parseArch(StringRef Arch) {
   Arch = llvm::ARM::getCanonicalArchName(Arch);
   if (checkArchVersion(Arch) < 8)
-    return AArch64::INVALID;
+    return {};
 
   StringRef Syn = llvm::ARM::getArchSynonym(Arch);
   for (const auto *A : ArchInfos) {
     if (A->Name.endswith(Syn))
       return *A;
   }
-  return AArch64::INVALID;
+  return {};
 }
 
-AArch64::ArchExtKind AArch64::parseArchExt(StringRef ArchExt) {
+std::optional<AArch64::ExtensionInfo> AArch64::parseArchExtension(StringRef ArchExt) {
   for (const auto &A : Extensions) {
     if (ArchExt == A.Name)
-      return static_cast<ArchExtKind>(A.ID);
+      return A;
   }
-  return AArch64::AEK_INVALID;
+  return {};
 }
 
-const AArch64::CpuInfo &AArch64::parseCpu(StringRef Name) {
+std::optional<AArch64::CpuInfo> AArch64::parseCpu(StringRef Name) {
   // Resolve aliases first.
   Name = resolveCPUAlias(Name);
 
@@ -153,7 +154,5 @@ const AArch64::CpuInfo &AArch64::parseCpu(StringRef Name) {
     if (Name == C.Name)
       return C;
 
-  // "generic" returns invalid.
-  assert(Name != "invalid" && "Unexpected recursion.");
-  return parseCpu("invalid");
+  return {};
 }