[llvm] a159e58 - Reland [RISCV] Fix gaps in IgnoreUnknown=true for RISCVISAInfo::parseArchString
Alex Bradbury via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 13 08:20:18 PDT 2023
Author: Alex Bradbury
Date: 2023-03-13T15:19:15Z
New Revision: a159e58c008435ba1992617b5ebfe8eb2d3fccb5
URL: https://github.com/llvm/llvm-project/commit/a159e58c008435ba1992617b5ebfe8eb2d3fccb5
DIFF: https://github.com/llvm/llvm-project/commit/a159e58c008435ba1992617b5ebfe8eb2d3fccb5.diff
LOG: Reland [RISCV] Fix gaps in IgnoreUnknown=true for RISCVISAInfo::parseArchString
Prior to this patch, unrecognised z/s/sx/x prefixed extensions were not
ignored when IgnoreUnknown=true.
The first version of this patch, a7313f83b9ca9, incorrectly used
`!isSupportedExtension(Ext)` rather than `!isSupportedExtension(Name)`.
i.e. checked the full substring rather than the split out name, causing
incorrect behaviour when a version is specified. This was fixed and a
new test case addded.
Differential Revision: https://reviews.llvm.org/D145882
Added:
Modified:
llvm/lib/Support/RISCVISAInfo.cpp
llvm/unittests/Support/RISCVISAInfoTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp
index 8a12744d4b193..9bd9313350ce6 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -804,6 +804,9 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
Desc.str().c_str(), Name.str().c_str());
}
+ if (IgnoreUnknown && !isSupportedExtension(Name))
+ continue;
+
ISAInfo->addExtension(Name, Major, Minor);
// Extension format is correct, keep parsing the extensions.
// TODO: Save Type, Name, Major, Minor to avoid parsing them later.
diff --git a/llvm/unittests/Support/RISCVISAInfoTest.cpp b/llvm/unittests/Support/RISCVISAInfoTest.cpp
index 32b097627d9ec..64c557be33fef 100644
--- a/llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -230,7 +230,8 @@ TEST(ParseArchString, RejectsUnrecognizedExtensionNamesByDefault) {
}
TEST(ParseArchString, IgnoresUnrecognizedExtensionNamesWithIgnoreUnknown) {
- for (StringRef Input : {"rv32ib"}) {
+ for (StringRef Input : {"rv32ib", "rv32i_zmadeup", "rv64i_smadeup",
+ "rv32i_sxmadeup", "rv64i_xmadeup"}) {
auto MaybeISAInfo = RISCVISAInfo::parseArchString(Input, true, false, true);
ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
RISCVISAInfo &Info = **MaybeISAInfo;
@@ -238,13 +239,14 @@ TEST(ParseArchString, IgnoresUnrecognizedExtensionNamesWithIgnoreUnknown) {
EXPECT_EQ(Exts.size(), 1UL);
EXPECT_TRUE(Exts.at("i") == (RISCVExtensionInfo{"i", 2, 0}));
}
- // FIXME: These unrecognized extensions should be ignored just as in the
- // case above. The below captures the current (incorrect) behaviour.
- for (StringRef Input :
- {"rv32i_zmadeup", "rv64i_smadeup", "rv32i_sxmadeup", "rv64i_xmadeup"}) {
- auto MaybeISAInfo = RISCVISAInfo::parseArchString(Input, true, false, true);
- EXPECT_THAT_EXPECTED(MaybeISAInfo, Failed());
- }
+
+ // Checks that supported extensions aren't incorrectly ignored when a
+ // version is present (an early version of the patch had this mistake).
+ auto MaybeISAInfo =
+ RISCVISAInfo::parseArchString("rv32i_zbc1p0_xmadeup", true, false, true);
+ ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
+ RISCVISAInfo::OrderedExtensionMap Exts = (*MaybeISAInfo)->getExtensions();
+ EXPECT_TRUE(Exts.at("zbc") == (RISCVExtensionInfo{"zbc", 1, 0}));
}
TEST(ParseArchString, AcceptsVersionInLongOrShortForm) {
More information about the llvm-commits
mailing list