[clang] db78c30 - [RISCV] Deduplicate RISCVISAInfo::toFeatures/toFeatureVector. NFC (#76942)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 9 00:33:56 PST 2024
Author: Luke Lau
Date: 2024-01-09T15:33:51+07:00
New Revision: db78c30ba772af1466bb0d0c1d376c8e642ee4a9
URL: https://github.com/llvm/llvm-project/commit/db78c30ba772af1466bb0d0c1d376c8e642ee4a9
DIFF: https://github.com/llvm/llvm-project/commit/db78c30ba772af1466bb0d0c1d376c8e642ee4a9.diff
LOG: [RISCV] Deduplicate RISCVISAInfo::toFeatures/toFeatureVector. NFC (#76942)
toFeatures and toFeatureVector both output a list of target feature
flags, just with a slightly different interface. toFeatures keeps any
unsupported extensions, and also provides a way to append negative
extensions (AddAllExtensions=true).
This patch combines them into one function, so that a later patch will
be be able to get a std::vector of features that includes all the
negative extensions, which was previously only possible through the
StrAlloc interface.
Added:
Modified:
clang/lib/Basic/Targets/RISCV.cpp
clang/lib/Driver/ToolChains/Arch/RISCV.cpp
llvm/include/llvm/Support/RISCVISAInfo.h
llvm/lib/Object/ELFObjectFile.cpp
llvm/lib/Support/RISCVISAInfo.cpp
llvm/unittests/Support/RISCVISAInfoTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Basic/Targets/RISCV.cpp b/clang/lib/Basic/Targets/RISCV.cpp
index 59ae12eed94014..daaa8639ae8358 100644
--- a/clang/lib/Basic/Targets/RISCV.cpp
+++ b/clang/lib/Basic/Targets/RISCV.cpp
@@ -296,7 +296,7 @@ bool RISCVTargetInfo::initFeatureMap(
}
// RISCVISAInfo makes implications for ISA features
- std::vector<std::string> ImpliedFeatures = (*ParseResult)->toFeatureVector();
+ std::vector<std::string> ImpliedFeatures = (*ParseResult)->toFeatures();
// parseFeatures normalizes the feature set by dropping any explicit
// negatives, and non-extension features. We need to preserve the later
@@ -413,7 +413,7 @@ static void handleFullArchString(StringRef FullArchStr,
// Forward the invalid FullArchStr.
Features.push_back("+" + FullArchStr.str());
} else {
- std::vector<std::string> FeatStrings = (*RII)->toFeatureVector();
+ std::vector<std::string> FeatStrings = (*RII)->toFeatures();
Features.insert(Features.end(), FeatStrings.begin(), FeatStrings.end());
}
}
diff --git a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
index 0717e3b813e1e2..16a8b3cc42bab4 100644
--- a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -42,9 +42,9 @@ static bool getArchFeatures(const Driver &D, StringRef Arch,
return false;
}
- (*ISAInfo)->toFeatures(
- Features, [&Args](const Twine &Str) { return Args.MakeArgString(Str); },
- /*AddAllExtensions=*/true);
+ for (const std::string &Str : (*ISAInfo)->toFeatures(/*AddAllExtension=*/true,
+ /*IgnoreUnknown=*/false))
+ Features.push_back(Args.MakeArgString(Str));
if (EnableExperimentalExtensions)
Features.push_back(Args.MakeArgString("+experimental"));
diff --git a/llvm/include/llvm/Support/RISCVISAInfo.h b/llvm/include/llvm/Support/RISCVISAInfo.h
index 09c4edd6df60e9..c539448683d368 100644
--- a/llvm/include/llvm/Support/RISCVISAInfo.h
+++ b/llvm/include/llvm/Support/RISCVISAInfo.h
@@ -68,9 +68,8 @@ class RISCVISAInfo {
parseFeatures(unsigned XLen, const std::vector<std::string> &Features);
/// Convert RISC-V ISA info to a feature vector.
- void toFeatures(std::vector<StringRef> &Features,
- llvm::function_ref<StringRef(const Twine &)> StrAlloc,
- bool AddAllExtensions) const;
+ std::vector<std::string> toFeatures(bool AddAllExtensions = false,
+ bool IgnoreUnknown = true) const;
const OrderedExtensionMap &getExtensions() const { return Exts; };
@@ -83,7 +82,6 @@ class RISCVISAInfo {
bool hasExtension(StringRef Ext) const;
std::string toString() const;
- std::vector<std::string> toFeatureVector() const;
StringRef computeDefaultABI() const;
static bool isSupportedExtensionFeature(StringRef Ext);
diff --git a/llvm/lib/Object/ELFObjectFile.cpp b/llvm/lib/Object/ELFObjectFile.cpp
index 95c4f9f8545db2..ae21b81c10c82a 100644
--- a/llvm/lib/Object/ELFObjectFile.cpp
+++ b/llvm/lib/Object/ELFObjectFile.cpp
@@ -315,7 +315,7 @@ Expected<SubtargetFeatures> ELFObjectFileBase::getRISCVFeatures() const {
else
llvm_unreachable("XLEN should be 32 or 64.");
- Features.addFeaturesVector(ISAInfo->toFeatureVector());
+ Features.addFeaturesVector(ISAInfo->toFeatures());
}
return Features;
diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp
index a9b7e209915a13..70f531e40b90e6 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -466,35 +466,38 @@ bool RISCVISAInfo::compareExtension(const std::string &LHS,
return LHS < RHS;
}
-void RISCVISAInfo::toFeatures(
- std::vector<StringRef> &Features,
- llvm::function_ref<StringRef(const Twine &)> StrAlloc,
- bool AddAllExtensions) const {
- for (auto const &Ext : Exts) {
- StringRef ExtName = Ext.first;
-
+std::vector<std::string> RISCVISAInfo::toFeatures(bool AddAllExtensions,
+ bool IgnoreUnknown) const {
+ std::vector<std::string> Features;
+ for (const auto &[ExtName, _] : Exts) {
+ // i is a base instruction set, not an extension (see
+ // https://github.com/riscv/riscv-isa-manual/blob/main/src/naming.adoc#base-integer-isa)
+ // and is not recognized in clang -cc1
if (ExtName == "i")
continue;
+ if (IgnoreUnknown && !isSupportedExtension(ExtName))
+ continue;
if (isExperimentalExtension(ExtName)) {
- Features.push_back(StrAlloc("+experimental-" + ExtName));
+ Features.push_back((llvm::Twine("+experimental-") + ExtName).str());
} else {
- Features.push_back(StrAlloc("+" + ExtName));
+ Features.push_back((llvm::Twine("+") + ExtName).str());
}
}
if (AddAllExtensions) {
for (const RISCVSupportedExtension &Ext : SupportedExtensions) {
if (Exts.count(Ext.Name))
continue;
- Features.push_back(StrAlloc(Twine("-") + Ext.Name));
+ Features.push_back((llvm::Twine("-") + Ext.Name).str());
}
for (const RISCVSupportedExtension &Ext : SupportedExperimentalExtensions) {
if (Exts.count(Ext.Name))
continue;
- Features.push_back(StrAlloc(Twine("-experimental-") + Ext.Name));
+ Features.push_back((llvm::Twine("-experimental-") + Ext.Name).str());
}
}
+ return Features;
}
// Extensions may have a version number, and may be separated by
@@ -1269,22 +1272,6 @@ std::string RISCVISAInfo::toString() const {
return Arch.str();
}
-std::vector<std::string> RISCVISAInfo::toFeatureVector() const {
- std::vector<std::string> FeatureVector;
- for (auto const &Ext : Exts) {
- std::string ExtName = Ext.first;
- if (ExtName == "i") // i is not recognized in clang -cc1
- continue;
- if (!isSupportedExtension(ExtName))
- continue;
- std::string Feature = isExperimentalExtension(ExtName)
- ? "+experimental-" + ExtName
- : "+" + ExtName;
- FeatureVector.push_back(Feature);
- }
- return FeatureVector;
-}
-
llvm::Expected<std::unique_ptr<RISCVISAInfo>>
RISCVISAInfo::postProcessAndChecking(std::unique_ptr<RISCVISAInfo> &&ISAInfo) {
ISAInfo->updateImplication();
diff --git a/llvm/unittests/Support/RISCVISAInfoTest.cpp b/llvm/unittests/Support/RISCVISAInfoTest.cpp
index 7463824b5b5248..42759f30fd1bc3 100644
--- a/llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -477,25 +477,45 @@ TEST(ParseArchString, RejectsConflictingExtensions) {
}
}
-TEST(ToFeatureVector, IIsDroppedAndExperimentalExtensionsArePrefixed) {
+TEST(ToFeatures, IIsDroppedAndExperimentalExtensionsArePrefixed) {
auto MaybeISAInfo1 =
RISCVISAInfo::parseArchString("rv64im_zicond", true, false);
ASSERT_THAT_EXPECTED(MaybeISAInfo1, Succeeded());
- EXPECT_THAT((*MaybeISAInfo1)->toFeatureVector(),
+ EXPECT_THAT((*MaybeISAInfo1)->toFeatures(),
ElementsAre("+m", "+experimental-zicond"));
auto MaybeISAInfo2 = RISCVISAInfo::parseArchString(
"rv32e_zicond_xventanacondops", true, false);
ASSERT_THAT_EXPECTED(MaybeISAInfo2, Succeeded());
- EXPECT_THAT((*MaybeISAInfo2)->toFeatureVector(),
+ EXPECT_THAT((*MaybeISAInfo2)->toFeatures(),
ElementsAre("+e", "+experimental-zicond", "+xventanacondops"));
}
-TEST(ToFeatureVector, UnsupportedExtensionsAreDropped) {
+TEST(ToFeatures, UnsupportedExtensionsAreDropped) {
auto MaybeISAInfo =
RISCVISAInfo::parseNormalizedArchString("rv64i2p0_m2p0_xmadeup1p0");
ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
- EXPECT_THAT((*MaybeISAInfo)->toFeatureVector(), ElementsAre("+m"));
+ EXPECT_THAT((*MaybeISAInfo)->toFeatures(), ElementsAre("+m"));
+}
+
+TEST(ToFeatures, UnsupportedExtensionsAreKeptIfIgnoreUnknownIsFalse) {
+ auto MaybeISAInfo =
+ RISCVISAInfo::parseNormalizedArchString("rv64i2p0_m2p0_xmadeup1p0");
+ ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
+ EXPECT_THAT((*MaybeISAInfo)->toFeatures(false, false),
+ ElementsAre("+m", "+xmadeup"));
+}
+
+TEST(ToFeatures, AddAllExtensionsAddsNegativeExtensions) {
+ auto MaybeISAInfo = RISCVISAInfo::parseNormalizedArchString("rv64i2p0_m2p0");
+ ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
+
+ auto Features = (*MaybeISAInfo)->toFeatures(true);
+ EXPECT_GT(Features.size(), 1UL);
+ EXPECT_EQ(Features.front(), "+m");
+ // Every feature after should be a negative feature
+ for (auto &NegativeExt : llvm::drop_begin(Features))
+ EXPECT_TRUE(NegativeExt.substr(0, 1) == "-");
}
TEST(OrderedExtensionMap, ExtensionsAreCorrectlyOrdered) {
More information about the cfe-commits
mailing list