[RISCV] Deduplicate version struct in RISCVISAInfo. NFC #77645
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We have two structs for representing the version of an extension in
RISCVISAInfo, with the exact same fields. This patch deduplicates them.
> [!NOTE]
> When renaming the struct, I also dropped the struct's name from the {}
> initializers in the supported extensions lists, rather than adding in the new
> name. I have no strong opinion on this though.
llvmbot
added
clang
|
@llvm/pr-subscribers-lld-elf @llvm/pr-subscribers-clang Author: Luke Lau (lukel97) ChangesWe have two structs for representing the version of an extension in > [!NOTE] Patch is 29.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/77645.diff 4 Files Affected:
diff --git a/clang/lib/Basic/Targets/RISCV.cpp b/clang/lib/Basic/Targets/RISCV.cpp
index daaa8639ae8358..abd947700a69ab 100644
--- a/clang/lib/Basic/Targets/RISCV.cpp
+++ b/clang/lib/Basic/Targets/RISCV.cpp
@@ -165,7 +165,7 @@ void RISCVTargetInfo::getTargetDefines(const LangOptions &Opts,
Builder.defineMacro(
Twine("__riscv_", ExtName),
- Twine(getVersionValue(ExtInfo.MajorVersion, ExtInfo.MinorVersion)));
+ Twine(getVersionValue(ExtInfo.Major, ExtInfo.Minor)));
}
if (ISAInfo->hasExtension("m") || ISAInfo->hasExtension("zmmul"))
diff --git a/llvm/include/llvm/Support/RISCVISAInfo.h b/llvm/include/llvm/Support/RISCVISAInfo.h
index 97f1051b0540a7..46df93d7522602 100644
--- a/llvm/include/llvm/Support/RISCVISAInfo.h
+++ b/llvm/include/llvm/Support/RISCVISAInfo.h
@@ -18,11 +18,6 @@
#include <vector>
namespace llvm {
-struct RISCVExtensionInfo {
- unsigned MajorVersion;
- unsigned MinorVersion;
-};
-
void riscvExtensionsHelp(StringMap<StringRef> DescMap);
class RISCVISAInfo {
@@ -30,6 +25,12 @@ class RISCVISAInfo {
RISCVISAInfo(const RISCVISAInfo &) = delete;
RISCVISAInfo &operator=(const RISCVISAInfo &) = delete;
+ /// Represents the major and version number components of a RISC-V extension.
+ struct ExtensionVersion {
+ unsigned Major;
+ unsigned Minor;
+ };
+
static bool compareExtension(const std::string &LHS, const std::string &RHS);
/// Helper class for OrderedExtensionMap.
@@ -41,7 +42,7 @@ class RISCVISAInfo {
/// OrderedExtensionMap is std::map, it's specialized to keep entries
/// in canonical order of extension.
- typedef std::map<std::string, RISCVExtensionInfo, ExtensionComparator>
+ typedef std::map<std::string, ExtensionVersion, ExtensionComparator>
OrderedExtensionMap;
RISCVISAInfo(unsigned XLen, OrderedExtensionMap &Exts)
@@ -104,8 +105,7 @@ class RISCVISAInfo {
OrderedExtensionMap Exts;
- void addExtension(StringRef ExtName, unsigned MajorVersion,
- unsigned MinorVersion);
+ void addExtension(StringRef ExtName, ExtensionVersion Version);
Error checkDependency();
diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp
index 70f531e40b90e6..86be440addc0e9 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -24,16 +24,11 @@
using namespace llvm;
namespace {
-/// Represents the major and version number components of a RISC-V extension
-struct RISCVExtensionVersion {
- unsigned Major;
- unsigned Minor;
-};
struct RISCVSupportedExtension {
const char *Name;
/// Supported version.
- RISCVExtensionVersion Version;
+ RISCVISAInfo::ExtensionVersion Version;
bool operator<(const RISCVSupportedExtension &RHS) const {
return StringRef(Name) < StringRef(RHS.Name);
@@ -50,161 +45,161 @@ static const char *RISCVGImplications[] = {
// NOTE: This table should be sorted alphabetically by extension name.
static const RISCVSupportedExtension SupportedExtensions[] = {
- {"a", RISCVExtensionVersion{2, 1}},
- {"c", RISCVExtensionVersion{2, 0}},
- {"d", RISCVExtensionVersion{2, 2}},
- {"e", RISCVExtensionVersion{2, 0}},
- {"f", RISCVExtensionVersion{2, 2}},
- {"h", RISCVExtensionVersion{1, 0}},
- {"i", RISCVExtensionVersion{2, 1}},
- {"m", RISCVExtensionVersion{2, 0}},
-
- {"smaia", RISCVExtensionVersion{1, 0}},
- {"ssaia", RISCVExtensionVersion{1, 0}},
- {"svinval", RISCVExtensionVersion{1, 0}},
- {"svnapot", RISCVExtensionVersion{1, 0}},
- {"svpbmt", RISCVExtensionVersion{1, 0}},
-
- {"v", RISCVExtensionVersion{1, 0}},
+ {"a", {2, 1}},
+ {"c", {2, 0}},
+ {"d", {2, 2}},
+ {"e", {2, 0}},
+ {"f", {2, 2}},
+ {"h", {1, 0}},
+ {"i", {2, 1}},
+ {"m", {2, 0}},
+
+ {"smaia", {1, 0}},
+ {"ssaia", {1, 0}},
+ {"svinval", {1, 0}},
+ {"svnapot", {1, 0}},
+ {"svpbmt", {1, 0}},
+
+ {"v", {1, 0}},
// vendor-defined ('X') extensions
- {"xcvalu", RISCVExtensionVersion{1, 0}},
- {"xcvbi", RISCVExtensionVersion{1, 0}},
- {"xcvbitmanip", RISCVExtensionVersion{1, 0}},
- {"xcvelw", RISCVExtensionVersion{1, 0}},
- {"xcvmac", RISCVExtensionVersion{1, 0}},
- {"xcvmem", RISCVExtensionVersion{1, 0}},
- {"xcvsimd", RISCVExtensionVersion{1, 0}},
- {"xsfvcp", RISCVExtensionVersion{1, 0}},
- {"xsfvfnrclipxfqf", RISCVExtensionVersion{1, 0}},
- {"xsfvfwmaccqqq", RISCVExtensionVersion{1, 0}},
- {"xsfvqmaccdod", RISCVExtensionVersion{1, 0}},
- {"xsfvqmaccqoq", RISCVExtensionVersion{1, 0}},
- {"xtheadba", RISCVExtensionVersion{1, 0}},
- {"xtheadbb", RISCVExtensionVersion{1, 0}},
- {"xtheadbs", RISCVExtensionVersion{1, 0}},
- {"xtheadcmo", RISCVExtensionVersion{1, 0}},
- {"xtheadcondmov", RISCVExtensionVersion{1, 0}},
- {"xtheadfmemidx", RISCVExtensionVersion{1, 0}},
- {"xtheadmac", RISCVExtensionVersion{1, 0}},
- {"xtheadmemidx", RISCVExtensionVersion{1, 0}},
- {"xtheadmempair", RISCVExtensionVersion{1, 0}},
- {"xtheadsync", RISCVExtensionVersion{1, 0}},
- {"xtheadvdot", RISCVExtensionVersion{1, 0}},
- {"xventanacondops", RISCVExtensionVersion{1, 0}},
-
- {"zawrs", RISCVExtensionVersion{1, 0}},
-
- {"zba", RISCVExtensionVersion{1, 0}},
- {"zbb", RISCVExtensionVersion{1, 0}},
- {"zbc", RISCVExtensionVersion{1, 0}},
- {"zbkb", RISCVExtensionVersion{1, 0}},
- {"zbkc", RISCVExtensionVersion{1, 0}},
- {"zbkx", RISCVExtensionVersion{1, 0}},
- {"zbs", RISCVExtensionVersion{1, 0}},
-
- {"zca", RISCVExtensionVersion{1, 0}},
- {"zcb", RISCVExtensionVersion{1, 0}},
- {"zcd", RISCVExtensionVersion{1, 0}},
- {"zce", RISCVExtensionVersion{1, 0}},
- {"zcf", RISCVExtensionVersion{1, 0}},
- {"zcmp", RISCVExtensionVersion{1, 0}},
- {"zcmt", RISCVExtensionVersion{1, 0}},
-
- {"zdinx", RISCVExtensionVersion{1, 0}},
-
- {"zfa", RISCVExtensionVersion{1, 0}},
- {"zfh", RISCVExtensionVersion{1, 0}},
- {"zfhmin", RISCVExtensionVersion{1, 0}},
- {"zfinx", RISCVExtensionVersion{1, 0}},
-
- {"zhinx", RISCVExtensionVersion{1, 0}},
- {"zhinxmin", RISCVExtensionVersion{1, 0}},
-
- {"zicbom", RISCVExtensionVersion{1, 0}},
- {"zicbop", RISCVExtensionVersion{1, 0}},
- {"zicboz", RISCVExtensionVersion{1, 0}},
- {"zicntr", RISCVExtensionVersion{2, 0}},
- {"zicsr", RISCVExtensionVersion{2, 0}},
- {"zifencei", RISCVExtensionVersion{2, 0}},
- {"zihintntl", RISCVExtensionVersion{1, 0}},
- {"zihintpause", RISCVExtensionVersion{2, 0}},
- {"zihpm", RISCVExtensionVersion{2, 0}},
-
- {"zk", RISCVExtensionVersion{1, 0}},
- {"zkn", RISCVExtensionVersion{1, 0}},
- {"zknd", RISCVExtensionVersion{1, 0}},
- {"zkne", RISCVExtensionVersion{1, 0}},
- {"zknh", RISCVExtensionVersion{1, 0}},
- {"zkr", RISCVExtensionVersion{1, 0}},
- {"zks", RISCVExtensionVersion{1, 0}},
- {"zksed", RISCVExtensionVersion{1, 0}},
- {"zksh", RISCVExtensionVersion{1, 0}},
- {"zkt", RISCVExtensionVersion{1, 0}},
-
- {"zmmul", RISCVExtensionVersion{1, 0}},
-
- {"zvbb", RISCVExtensionVersion{1, 0}},
- {"zvbc", RISCVExtensionVersion{1, 0}},
-
- {"zve32f", RISCVExtensionVersion{1, 0}},
- {"zve32x", RISCVExtensionVersion{1, 0}},
- {"zve64d", RISCVExtensionVersion{1, 0}},
- {"zve64f", RISCVExtensionVersion{1, 0}},
- {"zve64x", RISCVExtensionVersion{1, 0}},
-
- {"zvfh", RISCVExtensionVersion{1, 0}},
- {"zvfhmin", RISCVExtensionVersion{1, 0}},
+ {"xcvalu", {1, 0}},
+ {"xcvbi", {1, 0}},
+ {"xcvbitmanip", {1, 0}},
+ {"xcvelw", {1, 0}},
+ {"xcvmac", {1, 0}},
+ {"xcvmem", {1, 0}},
+ {"xcvsimd", {1, 0}},
+ {"xsfvcp", {1, 0}},
+ {"xsfvfnrclipxfqf", {1, 0}},
+ {"xsfvfwmaccqqq", {1, 0}},
+ {"xsfvqmaccdod", {1, 0}},
+ {"xsfvqmaccqoq", {1, 0}},
+ {"xtheadba", {1, 0}},
+ {"xtheadbb", {1, 0}},
+ {"xtheadbs", {1, 0}},
+ {"xtheadcmo", {1, 0}},
+ {"xtheadcondmov", {1, 0}},
+ {"xtheadfmemidx", {1, 0}},
+ {"xtheadmac", {1, 0}},
+ {"xtheadmemidx", {1, 0}},
+ {"xtheadmempair", {1, 0}},
+ {"xtheadsync", {1, 0}},
+ {"xtheadvdot", {1, 0}},
+ {"xventanacondops", {1, 0}},
+
+ {"zawrs", {1, 0}},
+
+ {"zba", {1, 0}},
+ {"zbb", {1, 0}},
+ {"zbc", {1, 0}},
+ {"zbkb", {1, 0}},
+ {"zbkc", {1, 0}},
+ {"zbkx", {1, 0}},
+ {"zbs", {1, 0}},
+
+ {"zca", {1, 0}},
+ {"zcb", {1, 0}},
+ {"zcd", {1, 0}},
+ {"zce", {1, 0}},
+ {"zcf", {1, 0}},
+ {"zcmp", {1, 0}},
+ {"zcmt", {1, 0}},
+
+ {"zdinx", {1, 0}},
+
+ {"zfa", {1, 0}},
+ {"zfh", {1, 0}},
+ {"zfhmin", {1, 0}},
+ {"zfinx", {1, 0}},
+
+ {"zhinx", {1, 0}},
+ {"zhinxmin", {1, 0}},
+
+ {"zicbom", {1, 0}},
+ {"zicbop", {1, 0}},
+ {"zicboz", {1, 0}},
+ {"zicntr", {2, 0}},
+ {"zicsr", {2, 0}},
+ {"zifencei", {2, 0}},
+ {"zihintntl", {1, 0}},
+ {"zihintpause", {2, 0}},
+ {"zihpm", {2, 0}},
+
+ {"zk", {1, 0}},
+ {"zkn", {1, 0}},
+ {"zknd", {1, 0}},
+ {"zkne", {1, 0}},
+ {"zknh", {1, 0}},
+ {"zkr", {1, 0}},
+ {"zks", {1, 0}},
+ {"zksed", {1, 0}},
+ {"zksh", {1, 0}},
+ {"zkt", {1, 0}},
+
+ {"zmmul", {1, 0}},
+
+ {"zvbb", {1, 0}},
+ {"zvbc", {1, 0}},
+
+ {"zve32f", {1, 0}},
+ {"zve32x", {1, 0}},
+ {"zve64d", {1, 0}},
+ {"zve64f", {1, 0}},
+ {"zve64x", {1, 0}},
+
+ {"zvfh", {1, 0}},
+ {"zvfhmin", {1, 0}},
// vector crypto
- {"zvkb", RISCVExtensionVersion{1, 0}},
- {"zvkg", RISCVExtensionVersion{1, 0}},
- {"zvkn", RISCVExtensionVersion{1, 0}},
- {"zvknc", RISCVExtensionVersion{1, 0}},
- {"zvkned", RISCVExtensionVersion{1, 0}},
- {"zvkng", RISCVExtensionVersion{1, 0}},
- {"zvknha", RISCVExtensionVersion{1, 0}},
- {"zvknhb", RISCVExtensionVersion{1, 0}},
- {"zvks", RISCVExtensionVersion{1, 0}},
- {"zvksc", RISCVExtensionVersion{1, 0}},
- {"zvksed", RISCVExtensionVersion{1, 0}},
- {"zvksg", RISCVExtensionVersion{1, 0}},
- {"zvksh", RISCVExtensionVersion{1, 0}},
- {"zvkt", RISCVExtensionVersion{1, 0}},
-
- {"zvl1024b", RISCVExtensionVersion{1, 0}},
- {"zvl128b", RISCVExtensionVersion{1, 0}},
- {"zvl16384b", RISCVExtensionVersion{1, 0}},
- {"zvl2048b", RISCVExtensionVersion{1, 0}},
- {"zvl256b", RISCVExtensionVersion{1, 0}},
- {"zvl32768b", RISCVExtensionVersion{1, 0}},
- {"zvl32b", RISCVExtensionVersion{1, 0}},
- {"zvl4096b", RISCVExtensionVersion{1, 0}},
- {"zvl512b", RISCVExtensionVersion{1, 0}},
- {"zvl64b", RISCVExtensionVersion{1, 0}},
- {"zvl65536b", RISCVExtensionVersion{1, 0}},
- {"zvl8192b", RISCVExtensionVersion{1, 0}},
+ {"zvkb", {1, 0}},
+ {"zvkg", {1, 0}},
+ {"zvkn", {1, 0}},
+ {"zvknc", {1, 0}},
+ {"zvkned", {1, 0}},
+ {"zvkng", {1, 0}},
+ {"zvknha", {1, 0}},
+ {"zvknhb", {1, 0}},
+ {"zvks", {1, 0}},
+ {"zvksc", {1, 0}},
+ {"zvksed", {1, 0}},
+ {"zvksg", {1, 0}},
+ {"zvksh", {1, 0}},
+ {"zvkt", {1, 0}},
+
+ {"zvl1024b", {1, 0}},
+ {"zvl128b", {1, 0}},
+ {"zvl16384b", {1, 0}},
+ {"zvl2048b", {1, 0}},
+ {"zvl256b", {1, 0}},
+ {"zvl32768b", {1, 0}},
+ {"zvl32b", {1, 0}},
+ {"zvl4096b", {1, 0}},
+ {"zvl512b", {1, 0}},
+ {"zvl64b", {1, 0}},
+ {"zvl65536b", {1, 0}},
+ {"zvl8192b", {1, 0}},
};
// NOTE: This table should be sorted alphabetically by extension name.
static const RISCVSupportedExtension SupportedExperimentalExtensions[] = {
- {"zacas", RISCVExtensionVersion{1, 0}},
+ {"zacas", {1, 0}},
- {"zcmop", RISCVExtensionVersion{0, 2}},
+ {"zcmop", {0, 2}},
- {"zfbfmin", RISCVExtensionVersion{0, 8}},
+ {"zfbfmin", {0, 8}},
- {"zicfilp", RISCVExtensionVersion{0, 4}},
- {"zicfiss", RISCVExtensionVersion{0, 4}},
+ {"zicfilp", {0, 4}},
+ {"zicfiss", {0, 4}},
- {"zicond", RISCVExtensionVersion{1, 0}},
+ {"zicond", {1, 0}},
- {"zimop", RISCVExtensionVersion{0, 1}},
+ {"zimop", {0, 1}},
- {"ztso", RISCVExtensionVersion{0, 1}},
+ {"ztso", {0, 1}},
- {"zvfbfmin", RISCVExtensionVersion{0, 8}},
- {"zvfbfwma", RISCVExtensionVersion{0, 8}},
+ {"zvfbfmin", {0, 8}},
+ {"zvfbfwma", {0, 8}},
};
static void verifyTables() {
@@ -237,8 +232,8 @@ void llvm::riscvExtensionsHelp(StringMap<StringRef> DescMap) {
for (const auto &E : SupportedExtensions)
ExtMap[E.Name] = {E.Version.Major, E.Version.Minor};
for (const auto &E : ExtMap) {
- std::string Version = std::to_string(E.second.MajorVersion) + "." +
- std::to_string(E.second.MinorVersion);
+ std::string Version = std::to_string(E.second.Major) + "." +
+ std::to_string(E.second.Minor);
PrintExtension(E.first, Version, DescMap[E.first]);
}
@@ -247,8 +242,8 @@ void llvm::riscvExtensionsHelp(StringMap<StringRef> DescMap) {
for (const auto &E : SupportedExperimentalExtensions)
ExtMap[E.Name] = {E.Version.Major, E.Version.Minor};
for (const auto &E : ExtMap) {
- std::string Version = std::to_string(E.second.MajorVersion) + "." +
- std::to_string(E.second.MinorVersion);
+ std::string Version = std::to_string(E.second.Major) + "." +
+ std::to_string(E.second.Minor);
PrintExtension(E.first, Version, DescMap["experimental-" + E.first]);
}
@@ -293,7 +288,7 @@ struct LessExtName {
};
} // namespace
-static std::optional<RISCVExtensionVersion>
+static std::optional<RISCVISAInfo::ExtensionVersion>
findDefaultVersion(StringRef ExtName) {
// Find default version of an extension.
// TODO: We might set default version based on profile or ISA spec.
@@ -309,12 +304,8 @@ findDefaultVersion(StringRef ExtName) {
return std::nullopt;
}
-void RISCVISAInfo::addExtension(StringRef ExtName, unsigned MajorVersion,
- unsigned MinorVersion) {
- RISCVExtensionInfo Ext;
- Ext.MajorVersion = MajorVersion;
- Ext.MinorVersion = MinorVersion;
- Exts[ExtName.str()] = Ext;
+void RISCVISAInfo::addExtension(StringRef ExtName, RISCVISAInfo::ExtensionVersion Version) {
+ Exts[ExtName.str()] = Version;
}
static StringRef getExtensionTypeDesc(StringRef Ext) {
@@ -337,7 +328,7 @@ static StringRef getExtensionType(StringRef Ext) {
return StringRef();
}
-static std::optional<RISCVExtensionVersion>
+static std::optional<RISCVISAInfo::ExtensionVersion>
isExperimentalExtension(StringRef Ext) {
auto I =
llvm::lower_bound(SupportedExperimentalExtensions, Ext, LessExtName());
@@ -634,8 +625,7 @@ RISCVISAInfo::parseFeatures(unsigned XLen,
continue;
if (Add)
- ISAInfo->addExtension(ExtName, ExtensionInfoIterator->Version.Major,
- ExtensionInfoIterator->Version.Minor);
+ ISAInfo->addExtension(ExtName, ExtensionInfoIterator->Version);
else
ISAInfo->Exts.erase(ExtName.str());
}
@@ -696,7 +686,7 @@ RISCVISAInfo::parseNormalizedArchString(StringRef Arch) {
if (MajorVersionStr.getAsInteger(10, MajorVersion))
return createStringError(errc::invalid_argument,
"failed to parse major version number");
- ISAInfo->addExtension(ExtName, MajorVersion, MinorVersion);
+ ISAInfo->addExtension(ExtName, {MajorVersion, MinorVersion});
}
ISAInfo->updateFLen();
ISAInfo->updateMinVLen();
@@ -775,7 +765,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
// ISA spec.
for (const auto *Ext : RISCVGImplications) {
if (auto Version = findDefaultVersion(Ext))
- ISAInfo->addExtension(Ext, Version->Major, Version->Minor);
+ ISAInfo->addExtension(Ext, *Version);
else
llvm_unreachable("Default extension version not found?");
}
@@ -794,7 +784,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
Minor = Version->Minor;
}
- ISAInfo->addExtension(StringRef(&Baseline, 1), Major, Minor);
+ ISAInfo->addExtension(StringRef(&Baseline, 1), {Major, Minor});
}
// Consume the base ISA version number and any '_' between rvxxx and the
@@ -860,7 +850,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
"unsupported standard user-level extension '%c'",
C);
}
- ISAInfo->addExtension(StringRef(&C, 1), Major, Minor);
+ ISAInfo->addExtension(StringRef(&C, 1), {Major, Minor});
// Consume full extension name and version, including any optional '_'
// between this extension and the next
@@ -928,7 +918,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
if (IgnoreUnknown && !isSupportedExtension(Name))
continue;
- ISAInfo->addExtension(Name, Major, Minor);
+ ISAInfo->addExtension(Name, {Major, Minor});
// Extension format is correct, keep parsing the extensions.
// TODO: Save Type, Name, Major, Minor to avoid parsing them later.
AllExts.push_back(Name);
@@ -1143,7 +1133,7 @@ void RISCVISAInfo::updateImplication() {
// implied
if (!HasE && !HasI) {
auto Version = findDefaultVersion("i");
- addExtension("i", Version->Major, Version->Minor);
+ addExtension("i", Version.value());
}
assert(llvm::is_sorted(ImpliedExts) && "Table not sorted by Name");
@@ -1164,7 +1154,7 @@ void RISCVISAInfo::updateImplication() {
if (Exts.count(ImpliedExt))
continue;
auto Version = findDefaultVersion(ImpliedExt);
- addExtension(ImpliedExt, Version->Major, Version->Minor);
+ addExtension(ImpliedExt, Version.value());
WorkList.insert(ImpliedExt);
}
}
@@ -1174,7 +1164,7 @@ void RISCVISAInfo::updateImplication() {
if (XLen == 32 && Exts.count("zce") && Exts.count("f") &&
!Exts.count("zcf")) {
auto Version = findDefaultVersion("zcf");
- addExtension("zcf", Version->Major, Version->Minor);
+ addExtension("zcf", Version.value());
}
}
@@ -1209,7 +1199,7 @@ void RISCVISAInfo::updateCombination() {
IsAllRequiredFeatureExist &= hasExtension(Ext);
if (IsAllRequiredFeatureExist) {
auto Version = findDefaultVersion(CombineExt);
- addExtension(CombineExt, Version->Major, Version->Minor);
+ addExtension(CombineExt, Version.value());
IsNewCombine = true;
}
}
@@ -1266,7 +1256,7 @@ std::string RISCVISAInfo::toString() const {
StringRef ExtName = Ext.first;
auto ExtInfo = Ext.second;
Arch << LS << ExtName;
- Arch << ExtInfo.MajorVersion << "p" << ExtInfo.MinorVersion;
+ Arch << ExtInfo.Major << "p" << ExtInfo.Minor;
}
return Arch.str();
diff --git a/llvm/unittests/Support/RISCVISAInfoTest.cpp b/llvm/unittests/Support/RISCVISAInfoTest.cpp
index 42759f30fd1bc3..236b513b692fb0 100644
--- a/llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -15,9 +15,9 @@ using ::testing::ElementsAre;
using namespace llvm;
-bool operator==(const llvm::RISCVExtensionInfo &A,
- const llvm::RISCVExtensionInfo &B) {
- return A.MajorVersion == B.MajorVersion && A.MinorVersion == B.MinorVersion;
+bool operator==(const RISCVISAInfo::ExtensionVersion &A,
+ const RISCVISAInfo::ExtensionVersion &B) {
+ return A.Major == B.Major && A.Minor == B.Minor;
}
TEST(ParseNormalizedArchString, RejectsUpperCase) {
@@ -50,28 +50,28 @@ TEST(ParseNormalizedArchString, AcceptsValidBaseISAsAndSetsXLen) {
ASSERT_THAT_EXPECTED(MaybeRV32I, Succeeded());
RISCVISAInfo &InfoRV32I = **MaybeRV32I;
EXPECT_EQ(InfoRV32I.getExtensions().size(), 1UL);
- EXPECT_TRUE(InfoRV32I.getExtensions().at("i") == (RISCVExtensionInfo{2, 0}));
+ EXPECT_TRUE(InfoRV32I.getExtensions().at("i") == (RISCVISAInfo::ExtensionVersion{2, 0}));
EXPECT_EQ(InfoRV32I.getXLen(), 32U);
auto MaybeRV32E = RISCVISAInfo::parseNormalizedArchString("rv32e2p0");
ASSERT_THAT_EXPECTED(MaybeRV32E, Succeeded());
RISCVISAInfo &InfoRV32E = **MaybeRV32E;
EXPECT_EQ(InfoRV32E.getExtensions().size(), 1UL);
- EXPECT_TRUE(InfoRV32E.getExtensions().at("e") == (RISCVExtensionInfo{2, 0}));
+ EXPECT_TRUE(InfoRV32E.getExtensions().at("e") == (RISCVISAInfo::ExtensionVersion{2, 0}));
EXPECT_EQ(InfoRV32E.getXLen(), 32U);
auto MaybeRV64I = RISCVISAInfo::parseNormalizedArchString("rv64i2p0");
ASSERT_THAT_EXPECTED(MaybeRV64I, Succeeded());
RISCV...
[truncated]
|
You can test this locally with the following command:git-clang-format --diff 14e291000f96c20e35ef494bd407f459b4617fca 10b636e878c770411ba068c2aca685c93a211efc -- clang/lib/Basic/Targets/RISCV.cpp lld/ELF/Arch/RISCV.cpp llvm/include/llvm/Support/RISCVISAInfo.h llvm/lib/Support/RISCVISAInfo.cpp llvm/unittests/Support/RISCVISAInfoTest.cppView the diff from clang-format here.diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp
index 390d950486..2d21d371dc 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -189,8 +189,7 @@ static const RISCVSupportedExtension SupportedExperimentalExtensions[] = {
{"zfbfmin", {0, 8}},
- {"zicfilp", {0, 4}},
- {"zicfiss", {0, 4}},
+ {"zicfilp", {0, 4}}, {"zicfiss", {0, 4}},
{"zicond", {1, 0}},
@@ -198,8 +197,7 @@ static const RISCVSupportedExtension SupportedExperimentalExtensions[] = {
{"ztso", {0, 1}},
- {"zvfbfmin", {0, 8}},
- {"zvfbfwma", {0, 8}},
+ {"zvfbfmin", {0, 8}}, {"zvfbfwma", {0, 8}},
};
static void verifyTables() {
|
…in the supported extensions list on the same line however. I've left those formatting changes out, so I expect the format CI to still fail after this.
justinfargnoli
pushed a commit
to justinfargnoli/llvm-project
that referenced
this pull request
Jan 28, 2024
We have two structs for representing the version of an extension in RISCVISAInfo, RISCVExtensionInfo and RISCVExtensionVersion, both with the exact same fields. This patch deduplicates them.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have two structs for representing the version of an extension in RISCVISAInfo,
RISCVExtensionInfoandRISCVExtensionVersion, both with the exact same fields. This patch deduplicates them.Note
When renaming the struct, I also dropped the struct's name from the {} initializers in the supported extensions lists, rather than adding in the new name. I have no strong opinion on this though.