diff options
author | Luke Lau <luke@igalia.com> | 2024-01-09 15:33:51 +0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-01-09 15:33:51 +0700 |
commit | db78c30ba772af1466bb0d0c1d376c8e642ee4a9 (patch) | |
tree | d4f1e8720eec2c5cc0e892ba1bfc5991ff2b2899 | |
parent | 2357e899cb11e05312c54b689ebd0355487be6bc (diff) | |
download | llvm-db78c30ba772af1466bb0d0c1d376c8e642ee4a9.zip llvm-db78c30ba772af1466bb0d0c1d376c8e642ee4a9.tar.gz llvm-db78c30ba772af1466bb0d0c1d376c8e642ee4a9.tar.bz2 |
[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.
-rw-r--r-- | clang/lib/Basic/Targets/RISCV.cpp | 4 | ||||
-rw-r--r-- | clang/lib/Driver/ToolChains/Arch/RISCV.cpp | 6 | ||||
-rw-r--r-- | llvm/include/llvm/Support/RISCVISAInfo.h | 6 | ||||
-rw-r--r-- | llvm/lib/Object/ELFObjectFile.cpp | 2 | ||||
-rw-r--r-- | llvm/lib/Support/RISCVISAInfo.cpp | 41 | ||||
-rw-r--r-- | llvm/unittests/Support/RISCVISAInfoTest.cpp | 30 |
6 files changed, 47 insertions, 42 deletions
diff --git a/clang/lib/Basic/Targets/RISCV.cpp b/clang/lib/Basic/Targets/RISCV.cpp index 59ae12e..daaa863 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 0717e3b..16a8b3c 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 09c4edd..c539448 100644 --- a/llvm/include/llvm/Support/RISCVISAInfo.h +++ b/llvm/include/llvm/Support/RISCVISAInfo.h @@ -68,9 +68,8 @@ public: 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 @@ public: 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 95c4f9f..ae21b81 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 a9b7e20..70f531e 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 7463824..42759f3 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) { |