Skip to content

Commit

Permalink
[NFC][AArch64] Use optional returns in target parser instead of 'inva…
Browse files Browse the repository at this point in the history
…lid' 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
  • Loading branch information
pratlucas committed Jan 27, 2023
1 parent 0a51bc7 commit 9ea00fc
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 158 deletions.
25 changes: 13 additions & 12 deletions clang/lib/Basic/Targets/AArch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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=")) {
Expand Down
17 changes: 10 additions & 7 deletions clang/lib/Driver/ToolChains/Arch/AArch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);

Expand Down
29 changes: 12 additions & 17 deletions llvm/include/llvm/TargetParser/AArch64TargetParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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;
}
Expand All @@ -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)};
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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);

Expand Down
19 changes: 9 additions & 10 deletions llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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());
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
41 changes: 20 additions & 21 deletions llvm/lib/TargetParser/AArch64TargetParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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())
Expand Down Expand Up @@ -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)
Expand All @@ -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);

Expand All @@ -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 {};
}
Loading

0 comments on commit 9ea00fc

Please sign in to comment.