From a159e58c008435ba1992617b5ebfe8eb2d3fccb5 Mon Sep 17 00:00:00 2001 From: Alex Bradbury Date: Mon, 13 Mar 2023 15:14:43 +0000 Subject: [PATCH] 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 --- llvm/lib/Support/RISCVISAInfo.cpp | 3 +++ llvm/unittests/Support/RISCVISAInfoTest.cpp | 18 ++++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp index 8a12744..9bd9313 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 32b0976..64c557b 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) { -- 2.7.4